From 27ee8e0b9f6dbfb5e8aa0b3ebf6a9744cdd77aa8 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Wed, 5 Dec 2018 19:43:47 -0800 Subject: [PATCH 1/6] Introduce ASCellLayoutMode --- Source/ASCollectionNode+Beta.h | 27 ++-- Source/ASCollectionNode.mm | 16 +- Source/ASCollectionView.mm | 138 ++++++++++++------ Source/ASCollectionViewProtocols.h | 35 +++++ Source/ASSectionController.h | 13 ++ Source/ASSupplementaryNodeSource.h | 8 + Source/ASTableView.mm | 38 ++++- Source/Details/ASCollectionInternal.h | 2 +- Source/Details/ASDataController.h | 19 ++- Source/Details/ASDataController.mm | 38 +++-- Source/Details/ASRangeController.h | 2 + Source/Details/ASRangeController.mm | 6 +- Source/IGListAdapter+AsyncDisplayKit.mm | 2 +- .../Private/ASIGListAdapterBasedDataSource.h | 2 +- .../Private/ASIGListAdapterBasedDataSource.mm | 22 ++- Source/Private/_ASHierarchyChangeSet.h | 3 + Source/Private/_ASHierarchyChangeSet.mm | 2 + Tests/ASCollectionViewTests.mm | 1 + 18 files changed, 289 insertions(+), 85 deletions(-) diff --git a/Source/ASCollectionNode+Beta.h b/Source/ASCollectionNode+Beta.h index 136e02401..769054cee 100644 --- a/Source/ASCollectionNode+Beta.h +++ b/Source/ASCollectionNode+Beta.h @@ -33,21 +33,23 @@ NS_ASSUME_NONNULL_BEGIN @property (nullable, nonatomic, weak) id batchFetchingDelegate; /** - * When this mode is enabled, ASCollectionView matches the timing of UICollectionView as closely as possible, - * ensuring that all reload and edit operations are performed on the main thread as blocking calls. + * When this mode is enabled, ASCollectionView matches the timing of UICollectionView as closely as + * possible, ensuring that all reload and edit operations are performed on the main thread as + * blocking calls. * - * This mode is useful for applications that are debugging issues with their collection view implementation. - * In particular, some applications do not properly conform to the API requirement of UICollectionView, and these - * applications may experience difficulties with ASCollectionView. Providing this mode allows for developers to - * work towards resolving technical debt in their collection view data source, while ramping up asynchronous - * collection layout. + * This mode is useful for applications that are debugging issues with their collection view + * implementation. In particular, some applications do not properly conform to the API requirement + * of UICollectionView, and these applications may experience difficulties with ASCollectionView. + * Providing this mode allows for developers to work towards resolving technical debt in their + * collection view data source, while ramping up asynchronous collection layout. * - * NOTE: Because this mode results in expensive operations like cell layout being performed on the main thread, - * it should be used as a tool to resolve data source conformance issues with Apple collection view API. + * NOTE: Because this mode results in expensive operations like cell layout being performed on the + * main thread, it should be used as a tool to resolve data source conformance issues with Apple + * collection view API. * - * @default defaults to NO. + * @default defaults to ASCellLayoutModeNone. */ -@property (nonatomic) BOOL usesSynchronousDataLoading; +@property (nonatomic) ASCellLayoutMode cellLayoutMode; /** * Returns YES if the ASCollectionNode contents are completely synchronized with the underlying collection-view layout. @@ -71,9 +73,6 @@ NS_ASSUME_NONNULL_BEGIN - (void)endUpdatesAnimated:(BOOL)animated ASDISPLAYNODE_DEPRECATED_MSG("Use -performBatchUpdates:completion: instead."); - (void)endUpdatesAnimated:(BOOL)animated completion:(nullable void (^)(BOOL))completion ASDISPLAYNODE_DEPRECATED_MSG("Use -performBatchUpdates:completion: instead."); - -- (void)invalidateFlowLayoutDelegateMetrics; - @end NS_ASSUME_NONNULL_END diff --git a/Source/ASCollectionNode.mm b/Source/ASCollectionNode.mm index 11fe678fb..cd533418a 100644 --- a/Source/ASCollectionNode.mm +++ b/Source/ASCollectionNode.mm @@ -42,7 +42,7 @@ @interface _ASCollectionPendingState : NSObject { @property (nonatomic) BOOL allowsSelection; // default is YES @property (nonatomic) BOOL allowsMultipleSelection; // default is NO @property (nonatomic) BOOL inverted; //default is NO -@property (nonatomic) BOOL usesSynchronousDataLoading; +@property (nonatomic) ASCellLayoutMode cellLayoutMode; @property (nonatomic) CGFloat leadingScreensForBatching; @property (nonatomic, weak) id layoutInspector; @property (nonatomic) BOOL alwaysBounceVertical; @@ -193,7 +193,7 @@ - (void)didLoad view.inverted = pendingState.inverted; view.allowsSelection = pendingState.allowsSelection; view.allowsMultipleSelection = pendingState.allowsMultipleSelection; - view.usesSynchronousDataLoading = pendingState.usesSynchronousDataLoading; + view.cellLayoutMode = pendingState.cellLayoutMode; view.layoutInspector = pendingState.layoutInspector; view.showsVerticalScrollIndicator = pendingState.showsVerticalScrollIndicator; view.showsHorizontalScrollIndicator = pendingState.showsHorizontalScrollIndicator; @@ -628,21 +628,21 @@ - (void)setBatchFetchingDelegate:(id)batchFetchingDeleg return _batchFetchingDelegate; } -- (BOOL)usesSynchronousDataLoading +- (ASCellLayoutMode)cellLayoutMode { if ([self pendingState]) { - return _pendingState.usesSynchronousDataLoading; + return _pendingState.cellLayoutMode; } else { - return self.view.usesSynchronousDataLoading; + return self.view.cellLayoutMode; } } -- (void)setUsesSynchronousDataLoading:(BOOL)usesSynchronousDataLoading +- (void)setCellLayoutMode:(ASCellLayoutMode)cellLayoutMode { if ([self pendingState]) { - _pendingState.usesSynchronousDataLoading = usesSynchronousDataLoading; + _pendingState.cellLayoutMode = cellLayoutMode; } else { - self.view.usesSynchronousDataLoading = usesSynchronousDataLoading; + self.view.cellLayoutMode = cellLayoutMode; } } diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index a14853c1c..915eb168c 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -63,6 +63,10 @@ flowLayout ? flowLayout.property : default; \ }) +// ASCellLayoutMode is an NSUInteger-based NS_OPTIONS field. Be careful with BOOL handling on the +// 32-bit Objective-C runtime, and pattern after ASInterfaceStateIncludesVisible() & friends. +#define ASCellLayoutModeIncludes(layoutMode) ((_cellLayoutMode & layoutMode) == layoutMode) + /// What, if any, invalidation should we perform during the next -layoutSubviews. typedef NS_ENUM(NSUInteger, ASCollectionViewInvalidationStyle) { /// Perform no invalidation. @@ -685,9 +689,10 @@ - (CGSize)sizeForElement:(ASCollectionElement *)element if (wrapperNode.sizeForItemBlock) { return wrapperNode.sizeForItemBlock(wrapperNode, element.constrainedSize.max); } else { - // In this case, we should use the exact value that was stashed earlier by calling sizeForItem:, referenceSizeFor*, etc. - // Although the node would use the preferredSize in layoutThatFits, we can skip this because there's no constrainedSize. - return wrapperNode.style.preferredSize; + // In this case, it is possible the model indexPath for this element will be nil. Attempt to convert it, + // and call out to the delegate directly. If it has been deleted from the model, the size returned will be the layout's default. + NSIndexPath *indexPath = [_dataController.visibleMap indexPathForElement:element]; + return [self _sizeForUIKitCellWithKind:element.supplementaryElementKind atIndexPath:indexPath]; } } else { return [node layoutThatFits:element.constrainedSize].size; @@ -786,37 +791,7 @@ - (NSArray *)visibleNodes return visibleNodes; } -- (BOOL)usesSynchronousDataLoading -{ - return self.dataController.usesSynchronousDataLoading; -} - -- (void)setUsesSynchronousDataLoading:(BOOL)usesSynchronousDataLoading -{ - self.dataController.usesSynchronousDataLoading = usesSynchronousDataLoading; -} - - (void)invalidateFlowLayoutDelegateMetrics { - for (ASCollectionElement *element in self.dataController.pendingMap) { - // This may be either a Supplementary or Item type element. - // For UIKit passthrough cells of either type, re-fetch their sizes from the standard UIKit delegate methods. - ASCellNode *node = element.node; - if (node.shouldUseUIKitCell) { - ASWrapperCellNode *wrapperNode = (ASWrapperCellNode *)node; - if (wrapperNode.sizeForItemBlock) { - continue; - } - NSIndexPath *indexPath = [_dataController.pendingMap indexPathForElement:element]; - NSString *kind = [element supplementaryElementKind]; - CGSize previousSize = node.style.preferredSize; - CGSize size = [self _sizeForUIKitCellWithKind:kind atIndexPath:indexPath]; - - if (!CGSizeEqualToSize(previousSize, size)) { - node.style.preferredSize = size; - [node invalidateCalculatedLayout]; - } - } - } } #pragma mark Internal @@ -879,9 +854,21 @@ Performing nested batch updates with super (e.g. resizing a cell node & updating - (void)_superPerformBatchUpdates:(void(^)())updates completion:(void(^)(BOOL finished))completion { ASDisplayNodeAssertMainThread(); - + _superBatchUpdateCount++; - [super performBatchUpdates:updates completion:completion]; + + if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData)) { + if (updates) { + updates(); + } + [super reloadData]; + if (completion) { + completion(YES); + } + } else { + [super performBatchUpdates:updates completion:completion]; + } + _superBatchUpdateCount--; } @@ -1757,7 +1744,12 @@ - (void)layoutSubviews switch (invalidationStyle) { case ASCollectionViewInvalidationStyleWithAnimation: if (0 == _superBatchUpdateCount) { - [self _superPerformBatchUpdates:^{ } completion:nil]; + BOOL shouldReloadData = ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData); + if (shouldReloadData) { + [super reloadData]; + } else { + [self _superPerformBatchUpdates:^{ } completion:nil]; + } } break; case ASCollectionViewInvalidationStyleWithoutAnimation: @@ -1864,6 +1856,41 @@ - (void)_beginBatchFetching #pragma mark - ASDataControllerSource +- (BOOL)dataController:(ASDataController *)dataController shouldEagerlyLayoutNode:(ASCellNode *)node +{ + NSAssert(!ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysLazy), + @"ASCellLayoutModeAlwaysLazy flag is no longer supported"); + return !node.shouldUseUIKitCell; +} + +- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet +{ + // If we have AlwaysSync set, block and donate main priority. + if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysSync)) { + return YES; + } + // Prioritize AlwaysAsync over the remaining heuristics for the Default mode. + if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysAsync)) { + return NO; + } + // The heuristics below apply to the ASCellLayoutModeDefault. + // If we have very few ASCellNodes (besides UIKit passthrough ones), match UIKit by blocking. + if (changeSet.countForAsyncLayout < 2) { + return YES; + } + CGSize contentSize = self.contentSize; + CGSize boundsSize = self.bounds.size; + if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) { + return YES; + } + return NO; +} + +- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController +{ + return ASCellLayoutModeIncludes(ASCellLayoutModeSerializeNodeCreation); +} + - (id)dataController:(ASDataController *)dataController nodeModelForItemAtIndexPath:(NSIndexPath *)indexPath { if (!_asyncDataSourceFlags.nodeModelForItem) { @@ -1874,7 +1901,7 @@ - (id)dataController:(ASDataController *)dataController nodeModelForItemAtIndexP return [_asyncDataSource collectionNode:collectionNode nodeModelForItemAtIndexPath:indexPath]; } -- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath +- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath shouldAsyncLayout:(BOOL *)shouldAsyncLayout { ASDisplayNodeAssertMainThread(); ASCellNodeBlock block = nil; @@ -1904,22 +1931,29 @@ - (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAt // or it is an error. if (_asyncDataSourceFlags.interop) { cell = [[ASWrapperCellNode alloc] init]; - cell.style.preferredSize = [self _sizeForUIKitCellWithKind:nil atIndexPath:indexPath]; } else { ASDisplayNodeFailAssert(@"ASCollection could not get a node block for item at index path %@: %@, %@. If you are trying to display a UICollectionViewCell, make sure your dataSource conforms to the protocol!", indexPath, cell, block); cell = [[ASCellNode alloc] init]; } } + + // This condition is intended to run for either cells received from the datasource, or created just above. + if (cell.shouldUseUIKitCell) { + *shouldAsyncLayout = NO; + } } // Wrap the node block + BOOL disableRangeController = ASCellLayoutModeIncludes(ASCellLayoutModeDisableRangeController); __weak __typeof__(self) weakSelf = self; return ^{ __typeof__(self) strongSelf = weakSelf; ASCellNode *node = (block ? block() : cell); ASDisplayNodeAssert([node isKindOfClass:[ASCellNode class]], @"ASCollectionNode provided a non-ASCellNode! %@, %@", node, strongSelf); - [node enterHierarchyState:ASHierarchyStateRangeManaged]; - + + if (!disableRangeController) { + [node enterHierarchyState:ASHierarchyStateRangeManaged]; + } if (node.interactionDelegate == nil) { node.interactionDelegate = strongSelf; } @@ -2001,7 +2035,6 @@ - (ASCellNodeBlock)dataController:(ASDataController *)dataController supplementa BOOL useUIKitCell = _asyncDataSourceFlags.interopViewForSupplementaryElement; if (useUIKitCell) { cell = [[ASWrapperCellNode alloc] init]; - cell.style.preferredSize = [self _sizeForUIKitCellWithKind:kind atIndexPath:indexPath]; } else { cell = [[ASCellNode alloc] init]; } @@ -2113,6 +2146,12 @@ - (NSString *)nameForRangeControllerDataSource #pragma mark - ASRangeControllerDelegate +- (BOOL)rangeControllerShouldUpdateRanges:(ASRangeController *)rangeController +{ + BOOL disableRangeController = ASCellLayoutModeIncludes(ASCellLayoutModeDisableRangeController); + return !disableRangeController; +} + - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates { ASDisplayNodeAssertMainThread(); @@ -2151,7 +2190,7 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet [_layoutFacilitator collectionViewWillPerformBatchUpdates]; __block NSUInteger numberOfUpdates = 0; - id completion = ^(BOOL finished){ + void(^completion)(BOOL finished) = ^(BOOL finished){ as_activity_scope(as_activity_create("Handle collection update completion", changeSet.rootActivity, OS_ACTIVITY_FLAG_DEFAULT)); as_log_verbose(ASCollectionLog(), "Update animation finished %{public}@", self.collectionNode); // Flush any range changes that happened as part of the update animations ending. @@ -2160,6 +2199,20 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet [changeSet executeCompletionHandlerWithFinished:finished]; }; + BOOL shouldReloadData = ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData); + // TODO: Consider adding !changeSet.isEmpty as a check to also disable shouldReloadData. + if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysBatchUpdateSectionReload) && + [changeSet sectionChangesOfType:_ASHierarchyChangeTypeReload].count > 0) { + shouldReloadData = NO; + } + + if (shouldReloadData) { + // When doing a reloadData, the insert / delete calls are not necessary. + // Calling updates() is enough, as it commits .pendingMap to .visibleMap. + updates(); + [super reloadData]; + completion(YES); + } else { [self _superPerformBatchUpdates:^{ updates(); @@ -2193,6 +2246,7 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet numberOfUpdates++; } } completion:completion]; + } as_log_debug(ASCollectionLog(), "Completed batch update %{public}@", self.collectionNode); diff --git a/Source/ASCollectionViewProtocols.h b/Source/ASCollectionViewProtocols.h index a33e433fa..aa32720ff 100644 --- a/Source/ASCollectionViewProtocols.h +++ b/Source/ASCollectionViewProtocols.h @@ -10,6 +10,41 @@ #import #import +typedef NS_OPTIONS(NSUInteger, ASCellLayoutMode) { + /** No options set. Default value. */ + ASCellLayoutModeNone = 0, + /** + * If ASCellLayoutModeAlwaysSync is enabled it will cause the ASDataController to wait on the + * background queue, and this ensures that any new / changed cells are in the hierarchy by the + * very next CATransaction / frame draw. + * + * Note: Sync & Async flags force the behavior to be always one or the other, regardless of the + * items. Default: If neither ASCellLayoutModeAlwaysSync or ASCellLayoutModeAlwaysAsync is set, + * default behavior is synchronous when there are 0 or 1 ASCellNodes in the data source, and + * asynchronous when there are 2 or more. + */ + ASCellLayoutModeAlwaysSync = 1 << 1, + ASCellLayoutModeAlwaysAsync = 1 << 2, + ASCellLayoutModeForceIfNeeded = 1 << 3, // Deprecated, default OFF. + ASCellLayoutModeAlwaysPassthroughDelegate = 1 << 4, // Deprecated, default ON. + /** Instead of using performBatchUpdates: prefer using reloadData for changes for collection view */ + ASCellLayoutModeAlwaysReloadData = 1 << 5, + /** If flag is enabled nodes are *not* gonna be range managed. */ + ASCellLayoutModeDisableRangeController = 1 << 6, + ASCellLayoutModeAlwaysLazy = 1 << 7, // Deprecated, default OFF. + /** + * Defines if the node creation should happen serialized and not in parallel within the + * data controller + */ + ASCellLayoutModeSerializeNodeCreation = 1 << 8, + /** + * When set, the performBatchUpdates: API (including animation) is used when handling Section + * Reload operations. This is useful only when ASCellLayoutModeAlwaysReloadData is enabled and + * cell height animations are desired. + */ + ASCellLayoutModeAlwaysBatchUpdateSectionReload = 1 << 9 +}; + NS_ASSUME_NONNULL_BEGIN /** diff --git a/Source/ASSectionController.h b/Source/ASSectionController.h index de26770ce..5d48bbef1 100644 --- a/Source/ASSectionController.h +++ b/Source/ASSectionController.h @@ -35,6 +35,19 @@ NS_ASSUME_NONNULL_BEGIN */ - (ASCellNodeBlock)nodeBlockForItemAtIndex:(NSInteger)index; +/** + * Similar to -collectionView:cellForItemAtIndexPath:. + * + * Note: only called if nodeBlockForItemAtIndex: returns nil. + * + * @param index The index of the item. + * + * @return A node to display for the given item. This will be called on the main thread and should + * not implement reuse (it will be called once per item). Unlike UICollectionView's version, + * this method is not called when the item is about to display. + */ +- (ASCellNode *)nodeForItemAtIndex:(NSInteger)index; + @optional /** diff --git a/Source/ASSupplementaryNodeSource.h b/Source/ASSupplementaryNodeSource.h index 8f1bfcfa0..3d63a6900 100644 --- a/Source/ASSupplementaryNodeSource.h +++ b/Source/ASSupplementaryNodeSource.h @@ -25,6 +25,14 @@ NS_ASSUME_NONNULL_BEGIN */ - (ASCellNodeBlock)nodeBlockForSupplementaryElementOfKind:(NSString *)elementKind atIndex:(NSInteger)index; +/** + * Asks the controller to provide a node to display for the given supplementary element. + * + * @param kind The kind of supplementary element. + * @param index The index of the item. + */ +- (ASCellNode *)nodeForSupplementaryElementOfKind:(NSString *)kind atIndex:(NSInteger)index; + @optional /** diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index 99c1df82a..e38190dbf 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -1519,6 +1519,11 @@ - (NSString *)nameForRangeControllerDataSource #pragma mark - ASRangeControllerDelegate +- (BOOL)rangeControllerShouldUpdateRanges:(ASRangeController *)rangeController +{ + return YES; +} + - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates { ASDisplayNodeAssertMainThread(); @@ -1666,13 +1671,42 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet #pragma mark - ASDataControllerSource +- (BOOL)dataController:(ASDataController *)dataController shouldEagerlyLayoutNode:(ASCellNode *)node +{ + return YES; +} + +- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController { + return NO; +} + +- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet +{ + // For more details on this method, see the comment in the ASCollectionView implementation. + if (changeSet.countForAsyncLayout < 2) { + return YES; + } + CGSize contentSize = self.contentSize; + CGSize boundsSize = self.bounds.size; + if (contentSize.height <= boundsSize.height && contentSize.width <= boundsSize.width) { + return YES; + } + return NO; +} + +- (void)dataControllerDidFinishWaiting:(ASDataController *)dataController +{ + // ASCellLayoutMode is not currently supported on ASTableView (see ASCollectionView for details). +} + - (id)dataController:(ASDataController *)dataController nodeModelForItemAtIndexPath:(NSIndexPath *)indexPath { // Not currently supported for tables. Will be added when the collection API stabilizes. return nil; } -- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath { +- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath shouldAsyncLayout:(BOOL *)shouldAsyncLayout +{ ASCellNodeBlock block = nil; if (_asyncDataSourceFlags.tableNodeNodeBlockForRow) { @@ -1720,6 +1754,8 @@ - (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAt return ^{ __typeof__(self) strongSelf = weakSelf; ASCellNode *node = (block != nil ? block() : [[ASCellNode alloc] init]); + ASDisplayNodeAssert([node isKindOfClass:[ASCellNode class]], @"ASTableNode provided a non-ASCellNode! %@, %@", node, strongSelf); + [node enterHierarchyState:ASHierarchyStateRangeManaged]; if (node.interactionDelegate == nil) { node.interactionDelegate = strongSelf; diff --git a/Source/Details/ASCollectionInternal.h b/Source/Details/ASCollectionInternal.h index 0cd9e8db8..102356606 100644 --- a/Source/Details/ASCollectionInternal.h +++ b/Source/Details/ASCollectionInternal.h @@ -31,7 +31,7 @@ NS_ASSUME_NONNULL_BEGIN /** * @see ASCollectionNode+Beta.h for full documentation. */ -@property (nonatomic) BOOL usesSynchronousDataLoading; +@property (nonatomic) ASCellLayoutMode cellLayoutMode; /** * Attempt to get the view-layer index path for the item with the given index path. diff --git a/Source/Details/ASDataController.h b/Source/Details/ASDataController.h index fbd48731e..b64c0520c 100644 --- a/Source/Details/ASDataController.h +++ b/Source/Details/ASDataController.h @@ -52,7 +52,7 @@ AS_EXTERN NSString * const ASCollectionInvalidUpdateException; /** Fetch the ASCellNode block for specific index path. This block should return the ASCellNode for the specified index path. */ -- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath; +- (ASCellNodeBlock)dataController:(ASDataController *)dataController nodeBlockAtIndexPath:(NSIndexPath *)indexPath shouldAsyncLayout:(BOOL *)shouldAsyncLayout; /** Fetch the number of rows in specific section. @@ -72,6 +72,18 @@ AS_EXTERN NSString * const ASCollectionInvalidUpdateException; - (nullable id)dataController:(ASDataController *)dataController nodeModelForItemAtIndexPath:(NSIndexPath *)indexPath; +/** + * Called just after dispatching ASCellNode allocation and layout to the concurrent background queue. + * In some cases, for example on the first content load for a screen, it may be desirable to call + * -waitUntilAllUpdatesAreProcessed at this point. + * + * Returning YES will cause the ASDataController to wait on the background queue, and this ensures + * that any new / changed cells are in the hierarchy by the very next CATransaction / frame draw. + */ +- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet; +- (BOOL)dataController:(ASDataController *)dataController shouldEagerlyLayoutNode:(ASCellNode *)node; +- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController; + @optional /** @@ -222,11 +234,6 @@ AS_EXTERN NSString * const ASCollectionInvalidUpdateException; @property (nonatomic, readonly) ASEventLog *eventLog; #endif -/** - * @see ASCollectionNode+Beta.h for full documentation. - */ -@property (nonatomic) BOOL usesSynchronousDataLoading; - /** @name Data Updating */ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet; diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm index 962e6b2b9..329446978 100644 --- a/Source/Details/ASDataController.mm +++ b/Source/Details/ASDataController.mm @@ -156,10 +156,14 @@ - (void)_allocateNodesFromElements:(NSArray *)elements { as_activity_create_for_scope("Data controller batch"); - // TODO: Should we use USER_INITIATED here since the user is probably waiting? - dispatch_queue_t queue = dispatch_get_global_queue(QOS_CLASS_DEFAULT, 0); - ASDispatchApply(nodeCount, queue, 0, ^(size_t i) { - if (!weakDataSource) { + dispatch_queue_t queue = dispatch_get_global_queue(QOS_CLASS_USER_INITIATED, 0); + NSUInteger threadCount = 0; + if ([_dataSource dataControllerShouldSerializeNodeCreation:self]) { + threadCount = 1; + } + ASDispatchApply(nodeCount, queue, threadCount, ^(size_t i) { + __strong id strongDataSource = weakDataSource; + if (strongDataSource == nil) { return; } @@ -181,6 +185,9 @@ - (void)_allocateNodesFromElements:(NSArray *)elements */ - (void)_layoutNode:(ASCellNode *)node withConstrainedSize:(ASSizeRange)constrainedSize { + if (![_dataSource dataController:self shouldEagerlyLayoutNode:node]) { + return; + } ASDisplayNodeAssert(ASSizeRangeHasSignificantArea(constrainedSize), @"Attempt to layout cell node with invalid size range %@", NSStringFromASSizeRange(constrainedSize)); CGRect frame = CGRectZero; @@ -368,6 +375,7 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map LOG(@"Populating elements of kind: %@, for index paths: %@", kind, indexPaths); id dataSource = self.dataSource; id node = self.node; + BOOL shouldAsyncLayout = YES; for (NSIndexPath *indexPath in indexPaths) { ASCellNodeBlock nodeBlock; id nodeModel; @@ -389,13 +397,12 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map } } if (nodeBlock == nil) { - nodeBlock = [dataSource dataController:self nodeBlockAtIndexPath:indexPath]; + nodeBlock = [dataSource dataController:self nodeBlockAtIndexPath:indexPath shouldAsyncLayout:&shouldAsyncLayout]; } } else { - BOOL shouldAsyncLayout = YES; nodeBlock = [dataSource dataController:self supplementaryNodeBlockOfKind:kind atIndexPath:indexPath shouldAsyncLayout:&shouldAsyncLayout]; } - + ASSizeRange constrainedSize = ASSizeRangeUnconstrained; if (shouldFetchSizeRanges) { constrainedSize = [self constrainedSizeForNodeOfKind:kind atIndexPath:indexPath]; @@ -408,6 +415,7 @@ - (void)_insertElementsIntoMap:(ASMutableElementMap *)map owningNode:node traitCollection:traitCollection]; [map insertElement:element atIndexPath:indexPath]; + changeSet.countForAsyncLayout += (shouldAsyncLayout ? 1 : 0); } } @@ -666,8 +674,15 @@ - (void)updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet }]; --_editingTransactionGroupCount; }); - - if (_usesSynchronousDataLoading) { + + // We've now dispatched node allocation and layout to a concurrent background queue. + // In some cases, it's advantageous to prevent the main thread from returning, to ensure the next + // frame displayed to the user has the view updates in place. Doing this does slightly reduce + // total latency, by donating the main thread's priority to the background threads. As such, the + // two cases where it makes sense to block: + // 1. There is very little work to be performed in the background (UIKit passthrough) + // 2. There is a higher priority on display latency than smoothness, e.g. app startup. + if ([_dataSource dataController:self shouldSynchronouslyProcessChangeSet:changeSet]) { [self waitUntilAllUpdatesAreProcessed]; } } @@ -869,7 +884,12 @@ - (void)_relayoutAllNodes _visibleMap = _pendingMap; for (ASCollectionElement *element in _visibleMap) { + // Ignore this element if it is no longer in the latest data. It is still recognized in the UIKit world but will be deleted soon. NSIndexPath *indexPathInPendingMap = [_pendingMap indexPathForElement:element]; + if (indexPathInPendingMap == nil) { + continue; + } + NSString *kind = element.supplementaryElementKind ?: ASDataControllerRowNodeKind; ASSizeRange newConstrainedSize = [self constrainedSizeForNodeOfKind:kind atIndexPath:indexPathInPendingMap]; diff --git a/Source/Details/ASRangeController.h b/Source/Details/ASRangeController.h index 7b062c43e..1ce295259 100644 --- a/Source/Details/ASRangeController.h +++ b/Source/Details/ASRangeController.h @@ -160,6 +160,8 @@ AS_SUBCLASSING_RESTRICTED */ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates; +- (BOOL)rangeControllerShouldUpdateRanges:(ASRangeController *)rangeController; + @end @interface ASRangeController (ASRangeControllerUpdateRangeProtocol) diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm index 9c8b7313c..cb3add3e4 100644 --- a/Source/Details/ASRangeController.mm +++ b/Source/Details/ASRangeController.mm @@ -208,7 +208,11 @@ - (void)_updateVisibleNodeIndexPaths if (!_layoutController || !_dataSource) { return; } - + + if ([_delegate rangeControllerShouldUpdateRanges:self] == NO) { + return; + } + #if AS_RANGECONTROLLER_LOG_UPDATE_FREQ _updateCountThisFrame += 1; #endif diff --git a/Source/IGListAdapter+AsyncDisplayKit.mm b/Source/IGListAdapter+AsyncDisplayKit.mm index ec085da68..c3e81d3e3 100644 --- a/Source/IGListAdapter+AsyncDisplayKit.mm +++ b/Source/IGListAdapter+AsyncDisplayKit.mm @@ -31,7 +31,7 @@ - (void)setASDKCollectionNode:(ASCollectionNode *)collectionNode } // Make a data source and retain it. - dataSource = [[ASIGListAdapterBasedDataSource alloc] initWithListAdapter:self]; + dataSource = [[ASIGListAdapterBasedDataSource alloc] initWithListAdapter:self collectionDelegate:collectionNode.delegate]; objc_setAssociatedObject(self, _cmd, dataSource, OBJC_ASSOCIATION_RETAIN_NONATOMIC); // Attach the data source to the collection node. diff --git a/Source/Private/ASIGListAdapterBasedDataSource.h b/Source/Private/ASIGListAdapterBasedDataSource.h index 44380bc07..5be2185f2 100644 --- a/Source/Private/ASIGListAdapterBasedDataSource.h +++ b/Source/Private/ASIGListAdapterBasedDataSource.h @@ -20,7 +20,7 @@ NS_ASSUME_NONNULL_BEGIN AS_SUBCLASSING_RESTRICTED @interface ASIGListAdapterBasedDataSource : NSObject -- (instancetype)initWithListAdapter:(IGListAdapter *)listAdapter; +- (instancetype)initWithListAdapter:(IGListAdapter *)listAdapter collectionDelegate:(nullable id)collectionDelegate; @end diff --git a/Source/Private/ASIGListAdapterBasedDataSource.mm b/Source/Private/ASIGListAdapterBasedDataSource.mm index 1bbf65c89..0dc6615aa 100644 --- a/Source/Private/ASIGListAdapterBasedDataSource.mm +++ b/Source/Private/ASIGListAdapterBasedDataSource.mm @@ -38,6 +38,7 @@ @interface ASIGListAdapterBasedDataSource () @property (nonatomic, weak, readonly) IGListAdapter *listAdapter; @property (nonatomic, readonly) id delegate; @property (nonatomic, readonly) id dataSource; +@property (nonatomic, weak, readonly) id collectionDelegate; /** * The section controller that we will forward beginBatchFetchWithContext: to. @@ -52,7 +53,7 @@ @interface ASIGListAdapterBasedDataSource () @implementation ASIGListAdapterBasedDataSource -- (instancetype)initWithListAdapter:(IGListAdapter *)listAdapter +- (instancetype)initWithListAdapter:(IGListAdapter *)listAdapter collectionDelegate:(nullable id)collectionDelegate { if (self = [super init]) { #if IG_LIST_COLLECTION_VIEW @@ -63,6 +64,7 @@ - (instancetype)initWithListAdapter:(IGListAdapter *)listAdapter ASDisplayNodeAssert([listAdapter conformsToProtocol:@protocol(UICollectionViewDataSource)], @"Expected IGListAdapter to conform to UICollectionViewDataSource."); ASDisplayNodeAssert([listAdapter conformsToProtocol:@protocol(UICollectionViewDelegateFlowLayout)], @"Expected IGListAdapter to conform to UICollectionViewDelegateFlowLayout."); _listAdapter = listAdapter; + _collectionDelegate = collectionDelegate; } return self; } @@ -114,6 +116,10 @@ - (void)scrollViewDidEndDecelerating:(UIScrollView *)scrollView - (BOOL)shouldBatchFetchForCollectionNode:(ASCollectionNode *)collectionNode { + if ([_collectionDelegate respondsToSelector:@selector(shouldBatchFetchForCollectionNode:)]) { + return [_collectionDelegate shouldBatchFetchForCollectionNode:collectionNode]; + } + NSInteger sectionCount = [self numberOfSectionsInCollectionNode:collectionNode]; if (sectionCount == 0) { return NO; @@ -131,6 +137,10 @@ - (BOOL)shouldBatchFetchForCollectionNode:(ASCollectionNode *)collectionNode - (void)collectionNode:(ASCollectionNode *)collectionNode willBeginBatchFetchWithContext:(ASBatchContext *)context { + if ([_collectionDelegate respondsToSelector:@selector(collectionNode:willBeginBatchFetchWithContext:)]) { + [_collectionDelegate collectionNode:collectionNode willBeginBatchFetchWithContext:context]; + return; + } ASIGSectionController *ctrl = self.sectionControllerForBatchFetching; self.sectionControllerForBatchFetching = nil; [ctrl beginBatchFetchWithContext:context]; @@ -207,6 +217,11 @@ - (ASCellNodeBlock)collectionNode:(ASCollectionNode *)collectionNode nodeBlockFo return [[self sectionControllerForSection:indexPath.section] nodeBlockForItemAtIndex:indexPath.item]; } +- (ASCellNode *)collectionNode:(ASCollectionNode *)collectionNode nodeForItemAtIndexPath:(NSIndexPath *)indexPath +{ + return [[self sectionControllerForSection:indexPath.section] nodeForItemAtIndex:indexPath.item]; +} + - (ASSizeRange)collectionNode:(ASCollectionNode *)collectionNode constrainedSizeForItemAtIndexPath:(NSIndexPath *)indexPath { ASIGSectionController *ctrl = [self sectionControllerForSection:indexPath.section]; @@ -222,6 +237,11 @@ - (ASCellNodeBlock)collectionNode:(ASCollectionNode *)collectionNode nodeBlockFo return [[self supplementaryElementSourceForSection:indexPath.section] nodeBlockForSupplementaryElementOfKind:kind atIndex:indexPath.item]; } +- (ASCellNode *)collectionNode:(ASCollectionNode *)collectionNode nodeForSupplementaryElementOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath +{ + return [[self supplementaryElementSourceForSection:indexPath.section] nodeForSupplementaryElementOfKind:kind atIndex:indexPath.item]; +} + - (NSArray *)collectionNode:(ASCollectionNode *)collectionNode supplementaryElementKindsInSection:(NSInteger)section { return [[self supplementaryElementSourceForSection:section] supportedElementKinds]; diff --git a/Source/Private/_ASHierarchyChangeSet.h b/Source/Private/_ASHierarchyChangeSet.h index c617c58fd..66f5966e6 100644 --- a/Source/Private/_ASHierarchyChangeSet.h +++ b/Source/Private/_ASHierarchyChangeSet.h @@ -113,6 +113,9 @@ NSString *NSStringFromASHierarchyChangeType(_ASHierarchyChangeType changeType); /// Indicates whether the change set is empty, that is it includes neither reload data nor per item or section changes. @property (nonatomic, readonly) BOOL isEmpty; +/// The count of new ASCellNodes that can undergo async layout calculation. May be zero if all UIKit passthrough cells. +@property (nonatomic, assign) NSUInteger countForAsyncLayout; + /// The top-level activity for this update. @property (nonatomic, OS_ACTIVITY_NULLABLE) os_activity_t rootActivity; diff --git a/Source/Private/_ASHierarchyChangeSet.mm b/Source/Private/_ASHierarchyChangeSet.mm index 5ff483817..72e39c6d7 100644 --- a/Source/Private/_ASHierarchyChangeSet.mm +++ b/Source/Private/_ASHierarchyChangeSet.mm @@ -119,6 +119,7 @@ @interface _ASHierarchyChangeSet () @end @implementation _ASHierarchyChangeSet { + NSUInteger _countForAsyncLayout; std::vector _oldItemCounts; std::vector _newItemCounts; void (^_completionHandler)(BOOL finished); @@ -127,6 +128,7 @@ @implementation _ASHierarchyChangeSet { @synthesize reverseSectionMapping = _reverseSectionMapping; @synthesize itemMappings = _itemMappings; @synthesize reverseItemMappings = _reverseItemMappings; +@synthesize countForAsyncLayout = _countForAsyncLayout; - (instancetype)init { diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm index 92fb349e8..c49d58071 100644 --- a/Tests/ASCollectionViewTests.mm +++ b/Tests/ASCollectionViewTests.mm @@ -1049,6 +1049,7 @@ - (void)testInitialRangeBounds UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; ASCollectionNode *cn = testController.collectionNode; + cn.cellLayoutMode = ASCellLayoutModeAlwaysAsync; [cn setTuningParameters:{ .leadingBufferScreenfuls = 2, .trailingBufferScreenfuls = 0 } forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload]; window.rootViewController = testController; From 1354de52540670a7b2b954412aee6606afd38298 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Thu, 6 Dec 2018 08:42:39 -0800 Subject: [PATCH 2/6] Some smaller improvements --- Source/ASCollectionNode+Beta.h | 1 + Source/ASCollectionView.mm | 75 ++++++++++--------- Source/ASTableView.mm | 3 +- Source/Details/ASDataController.mm | 1 + Source/Details/ASRangeController.mm | 2 +- .../Private/ASIGListAdapterBasedDataSource.mm | 1 + 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/Source/ASCollectionNode+Beta.h b/Source/ASCollectionNode+Beta.h index 769054cee..feb485ba8 100644 --- a/Source/ASCollectionNode+Beta.h +++ b/Source/ASCollectionNode+Beta.h @@ -73,6 +73,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)endUpdatesAnimated:(BOOL)animated ASDISPLAYNODE_DEPRECATED_MSG("Use -performBatchUpdates:completion: instead."); - (void)endUpdatesAnimated:(BOOL)animated completion:(nullable void (^)(BOOL))completion ASDISPLAYNODE_DEPRECATED_MSG("Use -performBatchUpdates:completion: instead."); + @end NS_ASSUME_NONNULL_END diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 915eb168c..4825d23ae 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -791,7 +791,9 @@ - (NSArray *)visibleNodes return visibleNodes; } -- (void)invalidateFlowLayoutDelegateMetrics { +- (void)invalidateFlowLayoutDelegateMetrics +{ + // Subclass hook } #pragma mark Internal @@ -2148,8 +2150,7 @@ - (NSString *)nameForRangeControllerDataSource - (BOOL)rangeControllerShouldUpdateRanges:(ASRangeController *)rangeController { - BOOL disableRangeController = ASCellLayoutModeIncludes(ASCellLayoutModeDisableRangeController); - return !disableRangeController; + return !ASCellLayoutModeIncludes(ASCellLayoutModeDisableRangeController); } - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet:(_ASHierarchyChangeSet *)changeSet updates:(dispatch_block_t)updates @@ -2190,7 +2191,7 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet [_layoutFacilitator collectionViewWillPerformBatchUpdates]; __block NSUInteger numberOfUpdates = 0; - void(^completion)(BOOL finished) = ^(BOOL finished){ + let completion = ^(BOOL finished) { as_activity_scope(as_activity_create("Handle collection update completion", changeSet.rootActivity, OS_ACTIVITY_FLAG_DEFAULT)); as_log_verbose(ASCollectionLog(), "Update animation finished %{public}@", self.collectionNode); // Flush any range changes that happened as part of the update animations ending. @@ -2213,39 +2214,39 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet [super reloadData]; completion(YES); } else { - [self _superPerformBatchUpdates:^{ - updates(); - - for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) { - [super reloadItemsAtIndexPaths:change.indexPaths]; - numberOfUpdates++; - } - - for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeReload]) { - [super reloadSections:change.indexSet]; - numberOfUpdates++; - } - - for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeOriginalDelete]) { - [super deleteItemsAtIndexPaths:change.indexPaths]; - numberOfUpdates++; - } - - for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeOriginalDelete]) { - [super deleteSections:change.indexSet]; - numberOfUpdates++; - } - - for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeOriginalInsert]) { - [super insertSections:change.indexSet]; - numberOfUpdates++; - } - - for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeOriginalInsert]) { - [super insertItemsAtIndexPaths:change.indexPaths]; - numberOfUpdates++; - } - } completion:completion]; + [self _superPerformBatchUpdates:^{ + updates(); + + for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) { + [super reloadItemsAtIndexPaths:change.indexPaths]; + numberOfUpdates++; + } + + for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeReload]) { + [super reloadSections:change.indexSet]; + numberOfUpdates++; + } + + for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeOriginalDelete]) { + [super deleteItemsAtIndexPaths:change.indexPaths]; + numberOfUpdates++; + } + + for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeOriginalDelete]) { + [super deleteSections:change.indexSet]; + numberOfUpdates++; + } + + for (_ASHierarchySectionChange *change in [changeSet sectionChangesOfType:_ASHierarchyChangeTypeOriginalInsert]) { + [super insertSections:change.indexSet]; + numberOfUpdates++; + } + + for (_ASHierarchyItemChange *change in [changeSet itemChangesOfType:_ASHierarchyChangeTypeOriginalInsert]) { + [super insertItemsAtIndexPaths:change.indexPaths]; + numberOfUpdates++; + } + } completion:completion]; } as_log_debug(ASCollectionLog(), "Completed batch update %{public}@", self.collectionNode); diff --git a/Source/ASTableView.mm b/Source/ASTableView.mm index e38190dbf..36edc5f6c 100644 --- a/Source/ASTableView.mm +++ b/Source/ASTableView.mm @@ -1676,7 +1676,8 @@ - (BOOL)dataController:(ASDataController *)dataController shouldEagerlyLayoutNod return YES; } -- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController { +- (BOOL)dataControllerShouldSerializeNodeCreation:(ASDataController *)dataController +{ return NO; } diff --git a/Source/Details/ASDataController.mm b/Source/Details/ASDataController.mm index 329446978..4feef1f43 100644 --- a/Source/Details/ASDataController.mm +++ b/Source/Details/ASDataController.mm @@ -188,6 +188,7 @@ - (void)_layoutNode:(ASCellNode *)node withConstrainedSize:(ASSizeRange)constrai if (![_dataSource dataController:self shouldEagerlyLayoutNode:node]) { return; } + ASDisplayNodeAssert(ASSizeRangeHasSignificantArea(constrainedSize), @"Attempt to layout cell node with invalid size range %@", NSStringFromASSizeRange(constrainedSize)); CGRect frame = CGRectZero; diff --git a/Source/Details/ASRangeController.mm b/Source/Details/ASRangeController.mm index cb3add3e4..5b135f6ef 100644 --- a/Source/Details/ASRangeController.mm +++ b/Source/Details/ASRangeController.mm @@ -209,7 +209,7 @@ - (void)_updateVisibleNodeIndexPaths return; } - if ([_delegate rangeControllerShouldUpdateRanges:self] == NO) { + if (![_delegate rangeControllerShouldUpdateRanges:self]) { return; } diff --git a/Source/Private/ASIGListAdapterBasedDataSource.mm b/Source/Private/ASIGListAdapterBasedDataSource.mm index 0dc6615aa..ade87eb58 100644 --- a/Source/Private/ASIGListAdapterBasedDataSource.mm +++ b/Source/Private/ASIGListAdapterBasedDataSource.mm @@ -141,6 +141,7 @@ - (void)collectionNode:(ASCollectionNode *)collectionNode willBeginBatchFetchWit [_collectionDelegate collectionNode:collectionNode willBeginBatchFetchWithContext:context]; return; } + ASIGSectionController *ctrl = self.sectionControllerForBatchFetching; self.sectionControllerForBatchFetching = nil; [ctrl beginBatchFetchWithContext:context]; From e23bf76a320b6a02d276f2db2f76d080bd0070c8 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 7 Dec 2018 11:30:12 -0800 Subject: [PATCH 3/6] Improve logic around _superPerformBatchUpdates:completion: --- Source/ASCollectionView.mm | 47 ++++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 4825d23ae..d129e8e24 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -846,31 +846,31 @@ - (CGSize)_sizeForUIKitCellWithKind:(NSString *)kind atIndexPath:(NSIndexPath *) return size; } +- (void)_superReloadData:(void(^)())updates completion:(void(^)(BOOL finished))completion +{ + if (updates) { + updates(); + } + [super reloadData]; + if (completion) { + completion(YES); + } +} + /** - Performing nested batch updates with super (e.g. resizing a cell node & updating collection view during same frame) - can cause super to throw data integrity exceptions because it checks the data source counts before - the update is complete. - - Always call [self _superPerform:] rather than [super performBatch:] so that we can keep our `superPerformingBatchUpdates` flag updated. + * Performing nested batch updates with super (e.g. resizing a cell node & updating collection view + * during same frame) can cause super to throw data integrity exceptions because it checks the data + * source counts before the update is complete. + * + * Always call [self _superPerform:] rather than [super performBatch:] so that we can keep our + * `superPerformingBatchUpdates` flag updated. */ - (void)_superPerformBatchUpdates:(void(^)())updates completion:(void(^)(BOOL finished))completion { ASDisplayNodeAssertMainThread(); _superBatchUpdateCount++; - - if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData)) { - if (updates) { - updates(); - } - [super reloadData]; - if (completion) { - completion(YES); - } - } else { - [super performBatchUpdates:updates completion:completion]; - } - + [super performBatchUpdates:updates completion:completion]; _superBatchUpdateCount--; } @@ -1746,11 +1746,10 @@ - (void)layoutSubviews switch (invalidationStyle) { case ASCollectionViewInvalidationStyleWithAnimation: if (0 == _superBatchUpdateCount) { - BOOL shouldReloadData = ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData); - if (shouldReloadData) { - [super reloadData]; + if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysReloadData)) { + [self _superReloadData:nil completion:nil]; } else { - [self _superPerformBatchUpdates:^{ } completion:nil]; + [self _superPerformBatchUpdates:nil completion:nil]; } } break; @@ -2210,9 +2209,7 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet if (shouldReloadData) { // When doing a reloadData, the insert / delete calls are not necessary. // Calling updates() is enough, as it commits .pendingMap to .visibleMap. - updates(); - [super reloadData]; - completion(YES); + [self _superReloadData:updates completion:completion]; } else { [self _superPerformBatchUpdates:^{ updates(); From a7747637904a6ccbb9e3060321bdf002bc4a4456 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 7 Dec 2018 13:02:41 -0800 Subject: [PATCH 4/6] Add comment about default values for ASCellLayoutModeNone --- Source/ASCollectionViewProtocols.h | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Source/ASCollectionViewProtocols.h b/Source/ASCollectionViewProtocols.h index aa32720ff..16a547d7d 100644 --- a/Source/ASCollectionViewProtocols.h +++ b/Source/ASCollectionViewProtocols.h @@ -11,7 +11,10 @@ #import typedef NS_OPTIONS(NSUInteger, ASCellLayoutMode) { - /** No options set. Default value. */ + /** + * No options set. If cell layout mode is set to ASCellLayoutModeNone, the default values for + * each flag listed below is used. + */ ASCellLayoutModeNone = 0, /** * If ASCellLayoutModeAlwaysSync is enabled it will cause the ASDataController to wait on the @@ -23,26 +26,26 @@ typedef NS_OPTIONS(NSUInteger, ASCellLayoutMode) { * default behavior is synchronous when there are 0 or 1 ASCellNodes in the data source, and * asynchronous when there are 2 or more. */ - ASCellLayoutModeAlwaysSync = 1 << 1, - ASCellLayoutModeAlwaysAsync = 1 << 2, + ASCellLayoutModeAlwaysSync = 1 << 1, // Default OFF + ASCellLayoutModeAlwaysAsync = 1 << 2, // Default OFF ASCellLayoutModeForceIfNeeded = 1 << 3, // Deprecated, default OFF. ASCellLayoutModeAlwaysPassthroughDelegate = 1 << 4, // Deprecated, default ON. /** Instead of using performBatchUpdates: prefer using reloadData for changes for collection view */ - ASCellLayoutModeAlwaysReloadData = 1 << 5, + ASCellLayoutModeAlwaysReloadData = 1 << 5, // Default OFF /** If flag is enabled nodes are *not* gonna be range managed. */ - ASCellLayoutModeDisableRangeController = 1 << 6, + ASCellLayoutModeDisableRangeController = 1 << 6, // Default OFF ASCellLayoutModeAlwaysLazy = 1 << 7, // Deprecated, default OFF. /** * Defines if the node creation should happen serialized and not in parallel within the * data controller */ - ASCellLayoutModeSerializeNodeCreation = 1 << 8, + ASCellLayoutModeSerializeNodeCreation = 1 << 8, // Default OFF /** * When set, the performBatchUpdates: API (including animation) is used when handling Section * Reload operations. This is useful only when ASCellLayoutModeAlwaysReloadData is enabled and * cell height animations are desired. */ - ASCellLayoutModeAlwaysBatchUpdateSectionReload = 1 << 9 + ASCellLayoutModeAlwaysBatchUpdateSectionReload = 1 << 9 // Default OFF }; NS_ASSUME_NONNULL_BEGIN From 2ffea7d78ea868d957830150af43a9490cf1cbea Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 7 Dec 2018 13:57:13 -0800 Subject: [PATCH 5/6] Always call _superReloadData:completion: within UICollectionView --- Source/ASCollectionView.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index d129e8e24..9a0e057dc 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -348,7 +348,7 @@ - (void)dealloc */ - (void)reloadData { - [super reloadData]; + [self _superReloadData:nil completion:nil]; // UICollectionView calls -reloadData during first layoutSubviews and when the data source changes. // This fires off the first load of cell nodes. @@ -2183,7 +2183,7 @@ - (void)rangeController:(ASRangeController *)rangeController updateWithChangeSet if (changeSet.includesReloadData) { _superIsPendingDataLoad = YES; updates(); - [super reloadData]; + [self _superReloadData:nil completion:nil]; as_log_debug(ASCollectionLog(), "Did reloadData %@", self.collectionNode); [changeSet executeCompletionHandlerWithFinished:YES]; } else { From 35026364730515010f6863642dfea69128ddb002 Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Fri, 7 Dec 2018 14:33:12 -0800 Subject: [PATCH 6/6] Add initial range test for ASCellLayoutModeNone --- Source/ASCollectionView.mm | 2 +- Tests/ASCollectionViewTests.mm | 34 ++++++++++++++++++++++++++-------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Source/ASCollectionView.mm b/Source/ASCollectionView.mm index 9a0e057dc..f585bf1fd 100644 --- a/Source/ASCollectionView.mm +++ b/Source/ASCollectionView.mm @@ -1874,7 +1874,7 @@ - (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyPro if (ASCellLayoutModeIncludes(ASCellLayoutModeAlwaysAsync)) { return NO; } - // The heuristics below apply to the ASCellLayoutModeDefault. + // The heuristics below apply to the ASCellLayoutModeNone. // If we have very few ASCellNodes (besides UIKit passthrough ones), match UIKit by blocking. if (changeSet.countForAsyncLayout < 2) { return YES; diff --git a/Tests/ASCollectionViewTests.mm b/Tests/ASCollectionViewTests.mm index c49d58071..c4fc4c914 100644 --- a/Tests/ASCollectionViewTests.mm +++ b/Tests/ASCollectionViewTests.mm @@ -163,6 +163,7 @@ - (instancetype)initWithNibName:(NSString *)nibNameOrNil bundle:(NSBundle *)nibB @interface ASCollectionView (InternalTesting) - (NSArray *)dataController:(ASDataController *)dataController supplementaryNodeKindsInSections:(NSIndexSet *)sections; +- (BOOL)dataController:(ASDataController *)dataController shouldSynchronouslyProcessChangeSet:(_ASHierarchyChangeSet *)changeSet; @end @@ -1045,26 +1046,43 @@ - (void)_primitiveBatchFetchingFillTestAnimated:(BOOL)animated visible:(BOOL)vis } - (void)testInitialRangeBounds +{ + [self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeNone]; +} + +- (void)testInitialRangeBoundsCellLayoutModeAlwaysAsync +{ + [self testInitialRangeBoundsWithCellLayoutMode:ASCellLayoutModeAlwaysAsync]; +} + +- (void)testInitialRangeBoundsWithCellLayoutMode:(ASCellLayoutMode)cellLayoutMode { UIWindow *window = [[UIWindow alloc] initWithFrame:[UIScreen mainScreen].bounds]; ASCollectionViewTestController *testController = [[ASCollectionViewTestController alloc] initWithNibName:nil bundle:nil]; ASCollectionNode *cn = testController.collectionNode; - cn.cellLayoutMode = ASCellLayoutModeAlwaysAsync; + cn.cellLayoutMode = cellLayoutMode; [cn setTuningParameters:{ .leadingBufferScreenfuls = 2, .trailingBufferScreenfuls = 0 } forRangeMode:ASLayoutRangeModeMinimum rangeType:ASLayoutRangeTypePreload]; window.rootViewController = testController; + [testController.collectionNode.collectionViewLayout invalidateLayout]; + [testController.collectionNode.collectionViewLayout prepareLayout]; + [window makeKeyAndVisible]; // Trigger the initial reload to start [window layoutIfNeeded]; - // Test the APIs that monitor ASCollectionNode update handling - XCTAssertTrue(cn.isProcessingUpdates, @"ASCollectionNode should still be processing updates after initial layoutIfNeeded call (reloadData)"); - [cn onDidFinishProcessingUpdates:^{ - XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates inside -onDidFinishProcessingUpdates: block"); - }]; + // Test the APIs that monitor ASCollectionNode update handling if collection node should + // layout asynchronously + if (![cn.view dataController:nil shouldSynchronouslyProcessChangeSet:nil]) { + XCTAssertTrue(cn.isProcessingUpdates, @"ASCollectionNode should still be processing updates after initial layoutIfNeeded call (reloadData)"); - // Wait for ASDK reload to finish - [cn waitUntilAllUpdatesAreProcessed]; + [cn onDidFinishProcessingUpdates:^{ + XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates inside -onDidFinishProcessingUpdates: block"); + }]; + + // Wait for ASDK reload to finish + [cn waitUntilAllUpdatesAreProcessed]; + } XCTAssertTrue(!cn.isProcessingUpdates, @"ASCollectionNode should no longer be processing updates after -wait call");