From 6414e778b4899df1a4f366c0db170fadbce27f5e Mon Sep 17 00:00:00 2001 From: Jordan Smith Date: Tue, 17 Dec 2019 16:55:23 -0800 Subject: [PATCH] Add an experiment for inline CATransaction commits Summary: **Context** When performing non-animated updates, `IGListKit` does so by beginning a `CATransaction` before `performBatchUpdates:`, and ending it in the completion block. While this disables animations for the collection view updates, it is feasible that some other changes might be bundled into the same transaction, given that we have no guarantee of the completion block being called inline. It seems that this is happening in practice and occasionally disrupting `UIViewPropertyAnimator` animations, forcing the completion immediately. **This Change** To fix this issue, we can commit the transaction inline. Here we use `performWithoutAnimation:`, although it's possible that this is just a wrapper around `CATransaction` calls anyway. Because the change may have unknown side effects, this change is gated by an experiment. Reviewed By: maxolls Differential Revision: D19150307 fbshipit-source-id: f7cff46ac7141609f42caafaf69bad71a980ad29 --- Source/IGListDiffKit/IGListExperiments.h | 4 +++- Source/IGListKit/IGListAdapterUpdater.m | 28 +++++++++++++++--------- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/Source/IGListDiffKit/IGListExperiments.h b/Source/IGListDiffKit/IGListExperiments.h index 009f3df9..ca452536 100644 --- a/Source/IGListDiffKit/IGListExperiments.h +++ b/Source/IGListDiffKit/IGListExperiments.h @@ -31,7 +31,9 @@ typedef NS_OPTIONS (NSInteger, IGListExperiment) { /// Test invalidating layout when cell reloads/updates in IGListBindingSectionController. IGListExperimentInvalidateLayoutForUpdates = 1 << 8, /// Test using the collection view when asking for layout instead of accessing the data source. Only apply to IGListCollectionViewLayout. - IGListExperimentUseCollectionViewInsteadOfDataSourceInLayout = 1 << 9 + IGListExperimentUseCollectionViewInsteadOfDataSourceInLayout = 1 << 9, + /// Test committing CATransactions inline instead of batching potentially unrelated animations + IGListExperimentPerformUpdatesWithoutDeferringCATransactionCommit = 1 << 10 }; /** diff --git a/Source/IGListKit/IGListAdapterUpdater.m b/Source/IGListKit/IGListAdapterUpdater.m index 2fcf6a9b..67c864dc 100644 --- a/Source/IGListKit/IGListAdapterUpdater.m +++ b/Source/IGListKit/IGListAdapterUpdater.m @@ -194,7 +194,7 @@ IGListIndexSetResult *(^performDiff)(void) = ^{ return IGListDiffExperiment(fromObjects, toObjects, IGListDiffEquality, experiments); }; - + // block executed in the first param block of -[UICollectionView performBatchUpdates:completion:] void (^batchUpdatesBlock)(IGListIndexSetResult *result) = ^(IGListIndexSetResult *result){ executeUpdateBlocks(); @@ -205,7 +205,7 @@ [collectionView moveSection:move.from toSection:move.to]; } // NOTE: for section updates, it's updated in the IGListSectionController's -didUpdateToObject:, since there is *only* 1 cell for the section, we can just update that cell. - + self.applyingUpdateData = [[IGListBatchUpdateData alloc] initWithInsertSections:result.inserts deleteSections:result.deletes @@ -260,14 +260,22 @@ willPerformBatchUpdatesWithCollectionView:collectionView batchUpdatesBlock(result); } completion:batchUpdatesCompletionBlock]; } else { - [CATransaction begin]; - [CATransaction setDisableActions:YES]; - [collectionView performBatchUpdates:^{ - batchUpdatesBlock(result); - } completion:^(BOOL finished) { - [CATransaction commit]; - batchUpdatesCompletionBlock(finished); - }]; + if (IGListExperimentEnabled(experiments, IGListExperimentPerformUpdatesWithoutDeferringCATransactionCommit)) { + [UIView performWithoutAnimation:^{ + [collectionView performBatchUpdates:^{ + batchUpdatesBlock(result); + } completion:batchUpdatesCompletionBlock]; + }]; + } else { + [CATransaction begin]; + [CATransaction setDisableActions:YES]; + [collectionView performBatchUpdates:^{ + batchUpdatesBlock(result); + } completion:^(BOOL finished) { + [CATransaction commit]; + batchUpdatesCompletionBlock(finished); + }]; + } } } @catch (NSException *exception) { [delegate listAdapterUpdater:self