From e92a6ce9e3440c3dddbbcbc214d4901adb8cb0ee Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Fri, 9 Oct 2015 19:37:36 -0700 Subject: [PATCH] Initial work on measuring loaded cell nodes on the main thread --- AsyncDisplayKit.xcodeproj/project.pbxproj | 12 +++++++ AsyncDisplayKit/ASCellNode.h | 9 ++++- AsyncDisplayKit/Details/ASDataController.mm | 37 ++++++++++++++++++-- AsyncDisplayKit/Private/ASCellNodeInternal.h | 25 +++++++++++++ AsyncDisplayKit/Private/ASCellNodeInternal.m | 25 +++++++++++++ 5 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 AsyncDisplayKit/Private/ASCellNodeInternal.h create mode 100644 AsyncDisplayKit/Private/ASCellNodeInternal.m diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 1a58fabfe9..d726a29a68 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -379,6 +379,10 @@ CC7FD9DF1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.m in Sources */ = {isa = PBXBuildFile; fileRef = CC7FD9DD1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.m */; settings = {ASSET_TAGS = (); }; }; CC7FD9E11BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m in Sources */ = {isa = PBXBuildFile; fileRef = CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */; settings = {ASSET_TAGS = (); }; }; CC7FD9E21BB603FF005CCB2B /* ASPhotosFrameworkImageRequest.h in Headers */ = {isa = PBXBuildFile; fileRef = CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */; settings = {ATTRIBUTES = (Public, ); }; }; + CCBFF68F1BC8A8BA00EF0162 /* ASCellNodeInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = CCBFF68D1BC8A8BA00EF0162 /* ASCellNodeInternal.h */; settings = {ASSET_TAGS = (); }; }; + CCBFF6901BC8A8BA00EF0162 /* ASCellNodeInternal.h in Headers */ = {isa = PBXBuildFile; fileRef = CCBFF68D1BC8A8BA00EF0162 /* ASCellNodeInternal.h */; settings = {ASSET_TAGS = (); }; }; + CCBFF6911BC8A8BA00EF0162 /* ASCellNodeInternal.m in Sources */ = {isa = PBXBuildFile; fileRef = CCBFF68E1BC8A8BA00EF0162 /* ASCellNodeInternal.m */; settings = {ASSET_TAGS = (); }; }; + CCBFF6921BC8A8BA00EF0162 /* ASCellNodeInternal.m in Sources */ = {isa = PBXBuildFile; fileRef = CCBFF68E1BC8A8BA00EF0162 /* ASCellNodeInternal.m */; settings = {ASSET_TAGS = (); }; }; D785F6621A74327E00291744 /* ASScrollNode.h in Headers */ = {isa = PBXBuildFile; fileRef = D785F6601A74327E00291744 /* ASScrollNode.h */; settings = {ATTRIBUTES = (Public, ); }; }; D785F6631A74327E00291744 /* ASScrollNode.m in Sources */ = {isa = PBXBuildFile; fileRef = D785F6611A74327E00291744 /* ASScrollNode.m */; }; DB7121BCD50849C498C886FB /* libPods-AsyncDisplayKitTests.a in Frameworks */ = {isa = PBXBuildFile; fileRef = EFA731F0396842FF8AB635EE /* libPods-AsyncDisplayKitTests.a */; }; @@ -627,6 +631,8 @@ CC7FD9DC1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPhotosFrameworkImageRequest.h; sourceTree = ""; }; CC7FD9DD1BB5E962005CCB2B /* ASPhotosFrameworkImageRequest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPhotosFrameworkImageRequest.m; sourceTree = ""; }; CC7FD9E01BB5F750005CCB2B /* ASPhotosFrameworkImageRequestTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASPhotosFrameworkImageRequestTests.m; sourceTree = ""; }; + CCBFF68D1BC8A8BA00EF0162 /* ASCellNodeInternal.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASCellNodeInternal.h; sourceTree = ""; }; + CCBFF68E1BC8A8BA00EF0162 /* ASCellNodeInternal.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASCellNodeInternal.m; sourceTree = ""; }; D3779BCFF841AD3EB56537ED /* Pods-AsyncDisplayKitTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-AsyncDisplayKitTests.release.xcconfig"; path = "Pods/Target Support Files/Pods-AsyncDisplayKitTests/Pods-AsyncDisplayKitTests.release.xcconfig"; sourceTree = ""; }; D785F6601A74327E00291744 /* ASScrollNode.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASScrollNode.h; sourceTree = ""; }; D785F6611A74327E00291744 /* ASScrollNode.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASScrollNode.m; sourceTree = ""; }; @@ -925,6 +931,8 @@ 058D0A01195D050800B7D73C /* Private */ = { isa = PBXGroup; children = ( + CCBFF68D1BC8A8BA00EF0162 /* ASCellNodeInternal.h */, + CCBFF68E1BC8A8BA00EF0162 /* ASCellNodeInternal.m */, 9C65A7291BA8EA4D0084DA91 /* ASLayoutOptionsPrivate.h */, 9C8221931BA237B80037F19A /* ASStackBaselinePositionedLayout.h */, 9C8221941BA237B80037F19A /* ASStackBaselinePositionedLayout.mm */, @@ -1047,6 +1055,7 @@ 058D0A71195D05F800B7D73C /* _AS-objc-internal.h in Headers */, 058D0A68195D05EC00B7D73C /* _ASAsyncTransaction.h in Headers */, 058D0A6A195D05EC00B7D73C /* _ASAsyncTransactionContainer+Private.h in Headers */, + CCBFF68F1BC8A8BA00EF0162 /* ASCellNodeInternal.h in Headers */, 058D0A6B195D05EC00B7D73C /* _ASAsyncTransactionContainer.h in Headers */, 058D0A6D195D05EC00B7D73C /* _ASAsyncTransactionGroup.h in Headers */, 058D0A72195D05F800B7D73C /* _ASCoreAnimationExtras.h in Headers */, @@ -1150,6 +1159,7 @@ B35062481B010EFD0018CF92 /* _AS-objc-internal.h in Headers */, B350623C1B010EFD0018CF92 /* _ASAsyncTransaction.h in Headers */, B350623E1B010EFD0018CF92 /* _ASAsyncTransactionContainer+Private.h in Headers */, + CCBFF6901BC8A8BA00EF0162 /* ASCellNodeInternal.h in Headers */, B350623F1B010EFD0018CF92 /* _ASAsyncTransactionContainer.h in Headers */, B35062411B010EFD0018CF92 /* _ASAsyncTransactionGroup.h in Headers */, B35062491B010EFD0018CF92 /* _ASCoreAnimationExtras.h in Headers */, @@ -1442,6 +1452,7 @@ 058D0A23195D050800B7D73C /* _ASAsyncTransactionContainer.m in Sources */, 058D0A24195D050800B7D73C /* _ASAsyncTransactionGroup.m in Sources */, 058D0A26195D050800B7D73C /* _ASCoreAnimationExtras.mm in Sources */, + CCBFF6911BC8A8BA00EF0162 /* ASCellNodeInternal.m in Sources */, 058D0A18195D050800B7D73C /* _ASDisplayLayer.mm in Sources */, 058D0A19195D050800B7D73C /* _ASDisplayView.mm in Sources */, 058D0A27195D050800B7D73C /* _ASPendingState.m in Sources */, @@ -1552,6 +1563,7 @@ B35062401B010EFD0018CF92 /* _ASAsyncTransactionContainer.m in Sources */, B35062421B010EFD0018CF92 /* _ASAsyncTransactionGroup.m in Sources */, B350624A1B010EFD0018CF92 /* _ASCoreAnimationExtras.mm in Sources */, + CCBFF6921BC8A8BA00EF0162 /* ASCellNodeInternal.m in Sources */, 2767E9421BB19BD600EA9B77 /* ASViewController.m in Sources */, B35062101B010EFD0018CF92 /* _ASDisplayLayer.mm in Sources */, B35062121B010EFD0018CF92 /* _ASDisplayView.mm in Sources */, diff --git a/AsyncDisplayKit/ASCellNode.h b/AsyncDisplayKit/ASCellNode.h index ae061eb6ca..96970cc479 100644 --- a/AsyncDisplayKit/ASCellNode.h +++ b/AsyncDisplayKit/ASCellNode.h @@ -12,7 +12,9 @@ /** * Generic cell node. Subclass this instead of `ASDisplayNode` to use with `ASTableView` and `ASCollectionView`. */ -@interface ASCellNode : ASDisplayNode +@interface ASCellNode : ASDisplayNode { + BOOL _needsMeasure; +} /** * @abstract When enabled, ensures that the cell is completely displayed before allowed onscreen. @@ -52,6 +54,11 @@ - (void)touchesEnded:(NSSet *)touches withEvent:(UIEvent *)event ASDISPLAYNODE_REQUIRES_SUPER; - (void)touchesCancelled:(NSSet *)touches withEvent:(UIEvent *)event ASDISPLAYNODE_REQUIRES_SUPER; +/* + * + */ +@property (nonatomic) BOOL needsMeasure; + @end diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index 7b07ff76ef..251d75ecaf 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -15,6 +15,7 @@ #import "ASDisplayNode.h" #import "ASMultidimensionalArrayUtils.h" #import "ASDisplayNodeInternal.h" +#import "ASCellNodeInternal.h" //#define LOG(...) NSLog(__VA_ARGS__) #define LOG(...) @@ -96,6 +97,23 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; #pragma mark - Cell Layout +/* + * Once nodes have loaded their views, we can't layout in the background so this is a chance + * to do so immediately on the main thread. + */ +- (void)_layoutNodesWithMainThreadAffinity:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths { + NSAssert(NSThread.isMainThread, @"Main thread layout must be on the main thread."); + [indexPaths enumerateObjectsUsingBlock:^(NSIndexPath *indexPath, NSUInteger idx, __unused BOOL * stop) { + ASCellNode *node = nodes[idx]; + if (node.isNodeLoaded && node.needsMeasure) { + ASSizeRange constrainedSize = [_dataSource dataController:self constrainedSizeForNodeAtIndexPath:indexPath]; + [node measureWithSizeRange:constrainedSize]; + node.frame = CGRectMake(0, 0, node.calculatedSize.width, node.calculatedSize.height); + node.needsMeasure = NO; + } + }]; +} + - (void)_layoutNodes:(NSArray *)nodes atIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions { ASDisplayNodeAssert([NSOperationQueue currentQueue] == _editingTransactionQueue, @"Cell node layout must be initiated from edit transaction queue"); @@ -117,8 +135,12 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; for (NSUInteger k = j; k < j + batchCount; k++) { ASCellNode *node = nodes[k]; ASSizeRange constrainedSize = nodeBoundSizes[k]; - [node measureWithSizeRange:constrainedSize]; - node.frame = CGRectMake(0, 0, node.calculatedSize.width, node.calculatedSize.height); + // Nodes with main thread affinity should all have already been measured. + if (node.needsMeasure) { + [node measureWithSizeRange:constrainedSize]; + node.frame = CGRectMake(0, 0, node.calculatedSize.width, node.calculatedSize.height); + node.needsMeasure = NO; + } } }); } @@ -245,6 +267,8 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; NSMutableArray *updatedIndexPaths = [NSMutableArray array]; [self _populateFromEntireDataSourceWithMutableNodes:updatedNodes mutableIndexPaths:updatedIndexPaths]; + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - reloadData"); @@ -399,6 +423,8 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; NSMutableArray *updatedIndexPaths = [NSMutableArray array]; [self _populateFromDataSourceWithSectionIndexSet:indexSet mutableNodes:updatedNodes mutableIndexPaths:updatedIndexPaths]; + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - insertSections: %@", indexSet); NSMutableArray *sectionArray = [NSMutableArray arrayWithCapacity:indexSet.count]; @@ -448,6 +474,8 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; // For example, if an initial -reloadData call is quickly followed by -reloadSections, sizing the initial set may not be done // at this time. Thus _editingNodes could be empty and crash in ASIndexPathsForMultidimensional[...] + [self _layoutNodesWithMainThreadAffinity:updatedNodes atIndexPaths:updatedIndexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ NSArray *indexPaths = ASIndexPathsForMultidimensionalArrayAtIndexSet(_editingNodes, sections); @@ -510,6 +538,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [nodes addObject:[_dataSource dataController:self nodeAtIndexPath:sortedIndexPaths[i]]]; } + [self _layoutNodesWithMainThreadAffinity:nodes atIndexPaths:indexPaths]; [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - insertRows: %@", indexPaths); [self _batchLayoutNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions]; @@ -552,6 +581,9 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; [nodes addObject:[_dataSource dataController:self nodeAtIndexPath:indexPath]]; }]; + // FIXME: Is there any reason I should split the edit transaction queue work into two phases, and do this layout in between rather than before? + [self _layoutNodesWithMainThreadAffinity:nodes atIndexPaths:indexPaths]; + [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - reloadRows: %@", indexPaths); [self _deleteNodesAtIndexPaths:indexPaths withAnimationOptions:animationOptions]; @@ -580,6 +612,7 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; ASSizeRange constrainedSize = [_dataSource dataController:self constrainedSizeForNodeAtIndexPath:indexPath]; [node measureWithSizeRange:constrainedSize]; node.frame = CGRectMake(0.0f, 0.0f, node.calculatedSize.width, node.calculatedSize.height); + node.needsMeasure = NO; }]; }]; }]; diff --git a/AsyncDisplayKit/Private/ASCellNodeInternal.h b/AsyncDisplayKit/Private/ASCellNodeInternal.h new file mode 100644 index 0000000000..a0477d1ed7 --- /dev/null +++ b/AsyncDisplayKit/Private/ASCellNodeInternal.h @@ -0,0 +1,25 @@ +// +// ASCellNodeInternal.h +// AsyncDisplayKit +// +// Created by Adlai Holler on 10/9/15. +// Copyright © 2015 Facebook. All rights reserved. +// + +#import + +#import + +@interface ASCellNode (Internal) + +/* + * @abstract Should this node be remeasured when the data controller next adds it? + * + * @discussion If possible, cell nodes should be measured in the background. However, + * we cannot violate a node's thread affinity. When nodes are added in a data controller, + * nodes with main thread affinity will be measured immediately on the main thread and this + * flag will be cleared, so the node will be skipped during the background measurement pass. + */ +@property (nonatomic) BOOL needsMeasure; + +@end diff --git a/AsyncDisplayKit/Private/ASCellNodeInternal.m b/AsyncDisplayKit/Private/ASCellNodeInternal.m new file mode 100644 index 0000000000..eee4e317a8 --- /dev/null +++ b/AsyncDisplayKit/Private/ASCellNodeInternal.m @@ -0,0 +1,25 @@ +// +// ASCellNodeInternal.m +// AsyncDisplayKit +// +// Created by Adlai Holler on 10/9/15. +// Copyright © 2015 Facebook. All rights reserved. +// + +#import "ASCellNodeInternal.h" + +@implementation ASCellNode (Internal) + +// FIXME: Lock this + +- (BOOL)needsMeasure +{ + return _needsMeasure; +} + +- (void)setNeedsMeasure:(BOOL)needsMeasure +{ + _needsMeasure = needsMeasure; +} + +@end