Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add move detection and support to ASLayoutTransition #1006

Merged
merged 15 commits into from
Jul 13, 2018
Merged

Add move detection and support to ASLayoutTransition #1006

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b0c715d
Add move detection and support to ASLayoutTransition
wiseoldduck Jul 3, 2018
f189d1b
Update CHANGELOG.md
wiseoldduck Jul 3, 2018
09d43ae
Update CHANGELOG.md
wiseoldduck Jul 3, 2018
baba616
Update ASLayout+IGListKit.h
wiseoldduck Jul 3, 2018
beb204e
Update ASLayout+IGListKit.mm
wiseoldduck Jul 3, 2018
d4ce1bc
Use std collections to avoid NSNumber boxing
wiseoldduck Jul 6, 2018
18bf81a
Update ASLayoutTransition.mm
wiseoldduck Jul 7, 2018
22d24b5
Code review updates.
wiseoldduck Jul 10, 2018
9898f0e
Code review updates.
wiseoldduck Jul 10, 2018
a8cf065
Add commentary to NSArray+Diffing.h; make multimap elements unowned
wiseoldduck Jul 10, 2018
14150e1
Use std::make_pair, optimize ASLayout+IGListKit
wiseoldduck Jul 10, 2018
b400cac
Oops I thought I had added these headers but nope
wiseoldduck Jul 11, 2018
0c949c4
Simplify simplify
wiseoldduck Jul 11, 2018
067aa85
Diff subnodes instead of sublayouts
wiseoldduck Jul 12, 2018
d2ca43e
Another randomized test with actual ASLayouts
wiseoldduck Jul 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions AsyncDisplayKit.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
058D0A40195D057000B7D73C /* ASTextNodeTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 058D0A36195D057000B7D73C /* ASTextNodeTests.m */; };
058D0A41195D057000B7D73C /* ASTextNodeWordKernerTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 058D0A37195D057000B7D73C /* ASTextNodeWordKernerTests.mm */; };
05EA6FE71AC0966E00E35788 /* ASSnapshotTestCase.m in Sources */ = {isa = PBXBuildFile; fileRef = 05EA6FE61AC0966E00E35788 /* ASSnapshotTestCase.m */; };
0FAFDF7520EC1C90003A51C0 /* ASLayout+IGListKit.h in Headers */ = {isa = PBXBuildFile; fileRef = 0FAFDF7320EC1C8F003A51C0 /* ASLayout+IGListKit.h */; settings = {ATTRIBUTES = (Public, ); }; };
0FAFDF7620EC1C90003A51C0 /* ASLayout+IGListKit.mm in Sources */ = {isa = PBXBuildFile; fileRef = 0FAFDF7420EC1C90003A51C0 /* ASLayout+IGListKit.mm */; };
18C2ED7F1B9B7DE800F627B3 /* ASCollectionNode.h in Headers */ = {isa = PBXBuildFile; fileRef = 18C2ED7C1B9B7DE800F627B3 /* ASCollectionNode.h */; settings = {ATTRIBUTES = (Public, ); }; };
18C2ED831B9B7DE800F627B3 /* ASCollectionNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = 18C2ED7D1B9B7DE800F627B3 /* ASCollectionNode.mm */; };
1A6C000D1FAB4E2100D05926 /* ASCornerLayoutSpec.h in Headers */ = {isa = PBXBuildFile; fileRef = 1A6C000B1FAB4E2000D05926 /* ASCornerLayoutSpec.h */; settings = {ATTRIBUTES = (Public, ); }; };
Expand Down Expand Up @@ -108,7 +110,7 @@
509E68641B3AEDB7009B9150 /* ASCollectionViewLayoutController.m in Sources */ = {isa = PBXBuildFile; fileRef = 205F0E1C1B373A2C007741D0 /* ASCollectionViewLayoutController.m */; };
509E68651B3AEDC5009B9150 /* CoreGraphics+ASConvenience.h in Headers */ = {isa = PBXBuildFile; fileRef = 205F0E1F1B376416007741D0 /* CoreGraphics+ASConvenience.h */; settings = {ATTRIBUTES = (Public, ); }; };
509E68661B3AEDD7009B9150 /* CoreGraphics+ASConvenience.m in Sources */ = {isa = PBXBuildFile; fileRef = 205F0E201B376416007741D0 /* CoreGraphics+ASConvenience.m */; };
636EA1A41C7FF4EC00EE152F /* NSArray+Diffing.m in Sources */ = {isa = PBXBuildFile; fileRef = DBC452DA1C5BF64600B16017 /* NSArray+Diffing.m */; };
636EA1A41C7FF4EC00EE152F /* NSArray+Diffing.mm in Sources */ = {isa = PBXBuildFile; fileRef = DBC452DA1C5BF64600B16017 /* NSArray+Diffing.mm */; };
636EA1A51C7FF4EF00EE152F /* ASDefaultPlayButton.m in Sources */ = {isa = PBXBuildFile; fileRef = AEB7B0191C5962EA00662EF4 /* ASDefaultPlayButton.m */; };
680346941CE4052A0009FEB4 /* ASNavigationController.h in Headers */ = {isa = PBXBuildFile; fileRef = 68FC85DC1CE29AB700EDD713 /* ASNavigationController.h */; settings = {ATTRIBUTES = (Public, ); }; };
68355B341CB579B9001D4E68 /* ASImageNode+AnimatedImage.mm in Sources */ = {isa = PBXBuildFile; fileRef = 68355B2E1CB5799E001D4E68 /* ASImageNode+AnimatedImage.mm */; };
Expand Down Expand Up @@ -601,6 +603,8 @@
058D0A44195D058D00B7D73C /* ASBaseDefines.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASBaseDefines.h; sourceTree = "<group>"; };
05EA6FE61AC0966E00E35788 /* ASSnapshotTestCase.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASSnapshotTestCase.m; sourceTree = "<group>"; };
05F20AA31A15733C00DCA68A /* ASImageProtocols.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASImageProtocols.h; sourceTree = "<group>"; };
0FAFDF7320EC1C8F003A51C0 /* ASLayout+IGListKit.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "ASLayout+IGListKit.h"; sourceTree = "<group>"; };
0FAFDF7420EC1C90003A51C0 /* ASLayout+IGListKit.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = "ASLayout+IGListKit.mm"; sourceTree = "<group>"; };
18C2ED7C1B9B7DE800F627B3 /* ASCollectionNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCollectionNode.h; sourceTree = "<group>"; };
18C2ED7D1B9B7DE800F627B3 /* ASCollectionNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASCollectionNode.mm; sourceTree = "<group>"; };
1950C4481A3BB5C1005C8279 /* ASEqualityHelpers.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASEqualityHelpers.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -964,7 +968,7 @@
DB55C2601C6408D6004EDCF5 /* _ASTransitionContext.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = _ASTransitionContext.m; path = ../_ASTransitionContext.m; sourceTree = "<group>"; };
DB55C2651C641AE4004EDCF5 /* ASContextTransitioning.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASContextTransitioning.h; sourceTree = "<group>"; };
DBC452D91C5BF64600B16017 /* NSArray+Diffing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = "NSArray+Diffing.h"; sourceTree = "<group>"; };
DBC452DA1C5BF64600B16017 /* NSArray+Diffing.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = "NSArray+Diffing.m"; sourceTree = "<group>"; };
DBC452DA1C5BF64600B16017 /* NSArray+Diffing.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = "NSArray+Diffing.mm"; sourceTree = "<group>"; };
DBC452DD1C5C6A6A00B16017 /* ArrayDiffingTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; lineEnding = 0; path = ArrayDiffingTests.m; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objc; };
DBC453211C5FD97200B16017 /* ASDisplayNodeImplicitHierarchyTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; lineEnding = 0; path = ASDisplayNodeImplicitHierarchyTests.m; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objc; };
DBDB83921C6E879900D0098C /* ASPagerFlowLayout.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPagerFlowLayout.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1416,7 +1420,7 @@
205F0E1F1B376416007741D0 /* CoreGraphics+ASConvenience.h */,
205F0E201B376416007741D0 /* CoreGraphics+ASConvenience.m */,
DBC452D91C5BF64600B16017 /* NSArray+Diffing.h */,
DBC452DA1C5BF64600B16017 /* NSArray+Diffing.m */,
DBC452DA1C5BF64600B16017 /* NSArray+Diffing.mm */,
CC4981BA1D1C7F65004E13CC /* NSIndexSet+ASHelpers.h */,
CC4981BB1D1C7F65004E13CC /* NSIndexSet+ASHelpers.m */,
058D09F5195D050800B7D73C /* NSMutableAttributedString+TextKitAdditions.h */,
Expand Down Expand Up @@ -1635,6 +1639,8 @@
ACF6ED0A1B17843500DA7C62 /* ASInsetLayoutSpec.mm */,
ACF6ED0B1B17843500DA7C62 /* ASLayout.h */,
ACF6ED0C1B17843500DA7C62 /* ASLayout.mm */,
0FAFDF7320EC1C8F003A51C0 /* ASLayout+IGListKit.h */,
0FAFDF7420EC1C90003A51C0 /* ASLayout+IGListKit.mm */,
ACF6ED111B17843500DA7C62 /* ASLayoutElement.h */,
E55D86311CA8A14000A0C26F /* ASLayoutElement.mm */,
698C8B601CAB49FC0052DC3F /* ASLayoutElementExtensibility.h */,
Expand Down Expand Up @@ -1850,6 +1856,7 @@
69E0E8A71D356C9400627613 /* ASEqualityHelpers.h in Headers */,
698C8B621CAB49FC0052DC3F /* ASLayoutElementExtensibility.h in Headers */,
69F10C871C84C35D0026140C /* ASRangeControllerUpdateRangeProtocol+Beta.h in Headers */,
0FAFDF7520EC1C90003A51C0 /* ASLayout+IGListKit.h in Headers */,
B350623C1B010EFD0018CF92 /* _ASAsyncTransaction.h in Headers */,
68355B411CB57A6C001D4E68 /* ASImageContainerProtocolCategories.h in Headers */,
7630FFA81C9E267E007A7C0E /* ASVideoNode.h in Headers */,
Expand Down Expand Up @@ -2368,6 +2375,7 @@
E5B078001E69F4EB00C24B5B /* ASElementMap.m in Sources */,
9C8898BC1C738BA800D6B02E /* ASTextKitFontSizeAdjuster.mm in Sources */,
690ED59B1E36D118000627C0 /* ASImageNode+tvOS.m in Sources */,
0FAFDF7620EC1C90003A51C0 /* ASLayout+IGListKit.mm in Sources */,
CCDC9B4E200991D10063C1F8 /* ASGraphicsContext.m in Sources */,
CCCCCCD81EC3EF060087FE10 /* ASTextInput.m in Sources */,
34EFC7621B701CA400AD841F /* ASBackgroundLayoutSpec.mm in Sources */,
Expand Down Expand Up @@ -2402,7 +2410,7 @@
B350624E1B010EFD0018CF92 /* ASDisplayNode+AsyncDisplay.mm in Sources */,
E5667E8E1F33872700FA6FC0 /* _ASCollectionGalleryLayoutInfo.m in Sources */,
25E327591C16819500A2170C /* ASPagerNode.m in Sources */,
636EA1A41C7FF4EC00EE152F /* NSArray+Diffing.m in Sources */,
636EA1A41C7FF4EC00EE152F /* NSArray+Diffing.mm in Sources */,
B35062501B010EFD0018CF92 /* ASDisplayNode+DebugTiming.mm in Sources */,
DEC146B91C37A16A004A0EE7 /* ASCollectionInternal.m in Sources */,
254C6B891BF94F8A003EC431 /* ASTextKitRenderer+Positioning.mm in Sources */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master
* Add your own contributions to the next release on the line below this with your name.
- [ASLayoutTransition] Add support for preserving order after node moves during transitions. (This order defines the z-order as well.) [Kevin Smith](https://github.com/wiseoldduck) [#1006]
- [ASDisplayNode] Adds support for multiple interface state delegates. [Garrett Moon](https://github.com/garrettmoon) [#979](https://github.com/TextureGroup/Texture/pull/979)
- [ASDataController] Add capability to renew supplementary views (update map) when size change from zero to non-zero.[Max Wang](https://github.com/wsdwsd0829) [#842](https://github.com/TextureGroup/Texture/pull/842)
- Make `ASPerformMainThreadDeallocation` visible in C. [Adlai Holler](https://github.com/Adlai-Holler)
Expand Down
2 changes: 1 addition & 1 deletion Source/ASDisplayNode+Layout.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize
}

// Apply the subnode insertion immediately to be able to animate the nodes
[pendingLayoutTransition applySubnodeInsertions];
[pendingLayoutTransition applySubnodeInsertionsAndMoves];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside the scope of this change - but I've heard that CALayer .zPosition is animatable. If we wanted to, it might be possible to animate the move operations (not sure exactly what it looks like though).

I suppose doing the reordering at the beginning is better than at the end (usually this is less noticeable than a pop at the end), so I think this implementation is ideal for current needs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's great, maybe it is a cross-fade animation or something. But yes being able to animate that is a good win from this.
The second point is very related, as setting up animations that will run simultaneously also allows us to do the setup in whatever order makes life easiest for ourselves, not necessarily tied to the order in which anything plays out on the screen. But at present (where we are not animating z-order) I agree it looks nicer to have the z-order swap happen before the animations rather than at the end.


// Kick off animating the layout transition
{
Expand Down
1 change: 1 addition & 0 deletions Source/AsyncDisplayKit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,3 +137,4 @@

#import <AsyncDisplayKit/IGListAdapter+AsyncDisplayKit.h>
#import <AsyncDisplayKit/AsyncDisplayKit+IGListKitMethods.h>
#import <AsyncDisplayKit/ASLayout+IGListKit.h>
62 changes: 62 additions & 0 deletions Source/Details/NSArray+Diffing.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,60 @@

#import <Foundation/Foundation.h>

/**
* These changes can be used to transform `self` to `array` by applying them in (any) order, *without shifting* the
* other elements. This can be done (in an NSMutableArray) by calling `setObject:atIndexedSubscript:` (or just use
* [subscripting] directly) for insertions from `array` into `self` (not the seemingly more apt `insertObject:atIndex`!),
* and using the same method for deletions from `self` (*set* a `[NSNull null]` as opposed to `removeObject:atIndex:`).
* After all inserts/deletes have been applied, there will be no nulls left (except possibly at the end of the array if
* `[array count] < [self count]`)

* Some examples:
* in: ab c
* out: abdc
* diff: ..+.
*
* in: abcd
* out: dcba
* dif: ---.+++
*
* in: abcd
* out: ab d
* diff: ..-.
*
* in: a bcd
* out: adbc
* diff: .+..-
*
* If `moves` pointer is passed in, instances where one element moves to another location are detected and reported,
* possibly replacing pairs of delete/insert. The process for transforming an array remains the same, however now it is
* important to apply the moves in order and not overwrite an element that needs to be moved somewhere else.
*
* the same examples, with moves:
* in: ab c
* out: abdc
* diff: ..+.
*
* in: abcd
* out: dcba
* diff: 321.
*
* in: abcd
* out: ab d
* diff: ..-.
*
* in: abcd
* out: adbc
* diff: .312
*
* Other notes:
*
* No index will be both moved from and deleted.
* Each index 0...[self count] will be either moved from or deleted. If it is moved to the same location, we omit it.
* Each index 0...[array count] will be the destination of ONE move or ONE insert.
* Knowing these things means any two of the three (delete, move, insert) implies the third.
*/

@interface NSArray (Diffing)

/**
Expand All @@ -35,4 +89,12 @@
*/
- (void)asdk_diffWithArray:(NSArray *)array insertions:(NSIndexSet **)insertions deletions:(NSIndexSet **)deletions compareBlock:(BOOL (^)(id lhs, id rhs))comparison;

/**
* @abstract Compares two arrays, providing the insertion, deletion, and move indexes needed to transform into the target array.
* @discussion This compares the equality of each object with `isEqual:`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should add to the method above that it will not transform into the same order, only to the same contents?

Copy link
Member Author

@wiseoldduck wiseoldduck Jul 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh but it will, you only need the moves if you want to animate them.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add some comments on this, as it was not clear to me until close to the end that this was actually true (that the -/+ information is sufficient and accurate to do the reordering without moves). It requires applying the diff in a certain way (basically in order into a sparse array) that was neither documented nor being done.

* This diffing algorithm uses a bottom-up memoized longest common subsequence solution to identify differences.
* It runs in O(mn) complexity.
* The moves are returned in ascending order of their destination index.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually, the moves are returned in a form that applies either before any changes, after insertions, or after deletions - in other words, the order of insert / delete being applied (if at all) does matter for the move indices being valid.

Could you expand a bit on the comment to describe the exact order of application for insertions, deletions, and moves that results in the correct transformation?

Copy link
Member Author

@wiseoldduck wiseoldduck Jul 7, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do - these are returned as simple (previousIndex -> newIndex) moves, which is easy to work with and consistent with what vanilla [UICollectionView performBatchUpdates:completion:] wants. (This would be "before any changes")

*/
- (void)asdk_diffWithArray:(NSArray *)array insertions:(NSIndexSet **)insertions deletions:(NSIndexSet **)deletions moves:(NSArray<NSIndexPath *> **)moves;
@end
113 changes: 0 additions & 113 deletions Source/Details/NSArray+Diffing.m
View file
Open in desktop

This file was deleted.

Loading