From 88ff693327cee3f851d4b145b4d1d307cd02af64 Mon Sep 17 00:00:00 2001 From: Bin Liu Date: Thu, 11 Feb 2016 23:49:14 -0800 Subject: [PATCH] Fixes in ChangeSet and DataController --- .../Details/ASChangeSetDataController.m | 24 +++++++++-------- .../Details/ASDataController+Subclasses.h | 8 ++++++ AsyncDisplayKit/Details/ASDataController.mm | 11 +++++--- .../Private/_ASHierarchyChangeSet.h | 4 ++- .../Private/_ASHierarchyChangeSet.m | 26 ++++++++++++------- 5 files changed, 48 insertions(+), 25 deletions(-) diff --git a/AsyncDisplayKit/Details/ASChangeSetDataController.m b/AsyncDisplayKit/Details/ASChangeSetDataController.m index 3e2c2ce118..6448b9ec47 100644 --- a/AsyncDisplayKit/Details/ASChangeSetDataController.m +++ b/AsyncDisplayKit/Details/ASChangeSetDataController.m @@ -11,6 +11,8 @@ #import "_ASHierarchyChangeSet.h" #import "ASAssert.h" +#import "ASDataController+Subclasses.h" + @interface ASChangeSetDataController () @property (nonatomic, assign) NSUInteger batchUpdateCounter; @@ -51,15 +53,7 @@ [_changeSet markCompleted]; [super beginUpdates]; - - for (_ASHierarchySectionChange *change in [_changeSet sectionChangesOfType:_ASHierarchyChangeTypeReload]) { - [super reloadSections:change.indexSet withAnimationOptions:change.animationOptions]; - } - - for (_ASHierarchyItemChange *change in [_changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) { - [super reloadRowsAtIndexPaths:change.indexPaths withAnimationOptions:change.animationOptions]; - } - + for (_ASHierarchyItemChange *change in [_changeSet itemChangesOfType:_ASHierarchyChangeTypeDelete]) { [super deleteRowsAtIndexPaths:change.indexPaths withAnimationOptions:change.animationOptions]; } @@ -67,7 +61,15 @@ for (_ASHierarchySectionChange *change in [_changeSet sectionChangesOfType:_ASHierarchyChangeTypeDelete]) { [super deleteSections:change.indexSet withAnimationOptions:change.animationOptions]; } - + + for (_ASHierarchySectionChange *change in [_changeSet sectionChangesOfType:_ASHierarchyChangeTypeReload]) { + [super reloadSections:change.indexSet withAnimationOptions:change.animationOptions]; + } + + for (_ASHierarchyItemChange *change in [_changeSet itemChangesOfType:_ASHierarchyChangeTypeReload]) { + [super reloadRowsAtIndexPaths:change.indexPaths withIndexPathsAfterUpdates:change.indexPathsAfterUpdates withAnimationOptions:change.animationOptions]; + } + for (_ASHierarchySectionChange *change in [_changeSet sectionChangesOfType:_ASHierarchyChangeTypeInsert]) { [super insertSections:change.indexSet withAnimationOptions:change.animationOptions]; } @@ -75,7 +77,7 @@ for (_ASHierarchyItemChange *change in [_changeSet itemChangesOfType:_ASHierarchyChangeTypeInsert]) { [super insertRowsAtIndexPaths:change.indexPaths withAnimationOptions:change.animationOptions]; } - + [super endUpdatesAnimated:animated completion:completion]; _changeSet = nil; diff --git a/AsyncDisplayKit/Details/ASDataController+Subclasses.h b/AsyncDisplayKit/Details/ASDataController+Subclasses.h index 32d787910e..18b4b35ff6 100644 --- a/AsyncDisplayKit/Details/ASDataController+Subclasses.h +++ b/AsyncDisplayKit/Details/ASDataController+Subclasses.h @@ -50,6 +50,14 @@ */ - (ASSizeRange)constrainedSizeForNodeOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath; +#pragma mark - Row Editing (Internal API) + +/** + * reload a set of IndexPaths requires deleting the cells at their current indexPaths + * and then inserting new ones at their future indexPaths, and these two sets of indexPath will be different in many cases + */ +- (void)reloadRowsAtIndexPaths:(NSArray *)indexPaths withIndexPathsAfterUpdates:(NSArray *)indexPathAfterUpdates withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions; + #pragma mark - Node & Section Insertion/Deletion API /** diff --git a/AsyncDisplayKit/Details/ASDataController.mm b/AsyncDisplayKit/Details/ASDataController.mm index ddc821f133..b60ea3b44a 100644 --- a/AsyncDisplayKit/Details/ASDataController.mm +++ b/AsyncDisplayKit/Details/ASDataController.mm @@ -787,6 +787,11 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; } - (void)reloadRowsAtIndexPaths:(NSArray *)indexPaths withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions +{ + [self reloadRowsAtIndexPaths:indexPaths withIndexPathsAfterUpdates:indexPaths withAnimationOptions:animationOptions]; +} + +- (void)reloadRowsAtIndexPaths:(NSArray *)indexPaths withIndexPathsAfterUpdates:(NSArray *)indexPathAfterUpdates withAnimationOptions:(ASDataControllerAnimationOptions)animationOptions { [self performEditCommandWithBlock:^{ ASDisplayNodeAssertMainThread(); @@ -796,20 +801,20 @@ static void *kASSizingQueueContext = &kASSizingQueueContext; // Reloading requires re-fetching the data. Load it on the current calling thread, locking the data source. [self accessDataSourceWithBlock:^{ - NSMutableArray *nodes = [[NSMutableArray alloc] initWithCapacity:indexPaths.count]; + NSMutableArray *nodes = [[NSMutableArray alloc] initWithCapacity:indexPathAfterUpdates.count]; // FIXME: This doesn't currently do anything // FIXME: Shouldn't deletes be sorted in descending order? [indexPaths sortedArrayUsingSelector:@selector(compare:)]; - for (NSIndexPath *indexPath in indexPaths) { + for (NSIndexPath *indexPath in indexPathAfterUpdates) { [nodes addObject:[_dataSource dataController:self nodeBlockAtIndexPath:indexPath]]; } [_editingTransactionQueue addOperationWithBlock:^{ LOG(@"Edit Transaction - reloadRows: %@", indexPaths); [self _deleteNodesAtIndexPaths:indexPaths withAnimationOptions:animationOptions]; - [self _batchLayoutNodes:nodes atIndexPaths:indexPaths withAnimationOptions:animationOptions]; + [self _batchLayoutNodes:nodes atIndexPaths:indexPathAfterUpdates withAnimationOptions:animationOptions]; }]; }]; }]; diff --git a/AsyncDisplayKit/Private/_ASHierarchyChangeSet.h b/AsyncDisplayKit/Private/_ASHierarchyChangeSet.h index 33d183a79d..c97f3f4e3d 100644 --- a/AsyncDisplayKit/Private/_ASHierarchyChangeSet.h +++ b/AsyncDisplayKit/Private/_ASHierarchyChangeSet.h @@ -30,7 +30,9 @@ typedef NS_ENUM(NSInteger, _ASHierarchyChangeType) { @property (nonatomic, readonly) ASDataControllerAnimationOptions animationOptions; /// Index paths are sorted descending for changeType .Delete, ascending otherwise -@property (nonatomic, strong) NSArray *indexPaths; +@property (nonatomic, strong, readonly) NSArray *indexPaths; +/// Calculated indexPaths after any insertions or deletions +@property (nonatomic, strong) NSArray *indexPathsAfterUpdates; @property (nonatomic, readonly) _ASHierarchyChangeType changeType; + (NSDictionary *)sectionToIndexSetMapFromChanges:(NSArray *)changes ofType:(_ASHierarchyChangeType)changeType; diff --git a/AsyncDisplayKit/Private/_ASHierarchyChangeSet.m b/AsyncDisplayKit/Private/_ASHierarchyChangeSet.m index 39047f867e..3a76f3c4ec 100644 --- a/AsyncDisplayKit/Private/_ASHierarchyChangeSet.m +++ b/AsyncDisplayKit/Private/_ASHierarchyChangeSet.m @@ -198,9 +198,8 @@ // Ignore item inserts in reloaded(new)/inserted sections. [_ASHierarchyItemChange sortAndCoalesceChanges:_insertItemChanges ignoringChangesInSections:insertedOrReloaded]; - - // reload itemsChanges need to be adjusted so that we access the correct indexPaths in the datasource + // reload items changes need to be adjusted so that we access the correct indexPaths in the datasource NSDictionary *insertedIndexPathsMap = [_ASHierarchyItemChange sectionToIndexSetMapFromChanges:_insertItemChanges ofType:_ASHierarchyChangeTypeInsert]; NSDictionary *deletedIndexPathsMap = [_ASHierarchyItemChange sectionToIndexSetMapFromChanges:_deleteItemChanges ofType:_ASHierarchyChangeTypeDelete]; @@ -210,23 +209,30 @@ // Every indexPaths in the change need to update its section and/or row // depending on all the deletions and insertions + // For reference, when batching reloads/deletes/inserts: + // - delete/reload indexPaths that are passed in should all be their current indexPaths + // - insert indexPaths that are passed in should all be their future indexPaths after deletions for (NSIndexPath *indexPath in change.indexPaths) { NSUInteger section = indexPath.section; NSUInteger row = indexPath.row; - section -= [_deletedSections countOfIndexesInRange:NSMakeRange(0, section)]; - section += [_insertedSections countOfIndexesInRange:NSMakeRange(0, section)]; - NSIndexSet *indicesInsertedInSection = insertedIndexPathsMap[@(section).stringValue]; - row += [indicesInsertedInSection countOfIndexesInRange:NSMakeRange(0, row)]; - NSIndexSet *indicesDeletedInSection = deletedIndexPathsMap[@(section).stringValue]; - row += [indicesDeletedInSection countOfIndexesInRange:NSMakeRange(0, row)]; + // Update section number based on section insertions/deletions that are above the current section + section -= [_deletedSections countOfIndexesInRange:NSMakeRange(0, section + 1)]; + section += [_insertedSections countOfIndexesInRange:NSMakeRange(0, section + 1)]; + // Update row number based on deletions that are above the current row in the current section + NSIndexSet *indicesDeletedInSection = deletedIndexPathsMap[@(indexPath.section)]; + row -= [indicesDeletedInSection countOfIndexesInRange:NSMakeRange(0, row + 1)]; + // Update row number based on insertions that are above the current row in the future section + NSIndexSet *indicesInsertedInSection = insertedIndexPathsMap[@(section)]; + row += [indicesInsertedInSection countOfIndexesInRange:NSMakeRange(0, row + 1)]; + //TODO: reuse the old indexPath object if section and row aren't changed NSIndexPath *newIndexPath = [NSIndexPath indexPathForRow:row inSection:section]; [newIndexPaths addObject:newIndexPath]; } - change.indexPaths = newIndexPaths; + change.indexPathsAfterUpdates = newIndexPaths; } } } @@ -342,7 +348,7 @@ for (_ASHierarchyItemChange *change in changes) { NSAssert(change.changeType == changeType, @"The map we created must all be of the same changeType as of now"); for (NSIndexPath *indexPath in change.indexPaths) { - NSString *sectionKey = @(indexPath.section).stringValue; + NSString *sectionKey = @(indexPath.section); NSMutableIndexSet *indexSet = sectionToIndexSetMap[sectionKey]; if (indexSet) { [indexSet addIndex:indexPath.row];