From 6383b690296422b620426bf266ee6c00859e91f0 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 8 Sep 2015 22:30:56 +0300 Subject: [PATCH 1/2] Update ASTableViewTests to detect the case when relayout is trigger during initial configuration. --- AsyncDisplayKitTests/ASTableViewTests.m | 66 +++++++++++++++++++++---- 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/AsyncDisplayKitTests/ASTableViewTests.m b/AsyncDisplayKitTests/ASTableViewTests.m index 4a23a659..39844d6a 100644 --- a/AsyncDisplayKitTests/ASTableViewTests.m +++ b/AsyncDisplayKitTests/ASTableViewTests.m @@ -57,7 +57,8 @@ @end @interface ASTableViewFilledDataSource : NSObject - +/** Calculated by counting how many times a constrained size is asked for the first node on main thread. */ +@property (atomic) int numberOfRelayouts; @end @implementation ASTableViewFilledDataSource @@ -80,6 +81,16 @@ return textCellNode; } +- (ASSizeRange)tableView:(ASTableView *)tableView constrainedSizeForNodeAtIndexPath:(NSIndexPath *)indexPath +{ + if ([NSThread isMainThread] && indexPath.section == 0 && indexPath.row == 0) { + _numberOfRelayouts++; + } + CGFloat maxWidth = tableView.bounds.size.width; + return ASSizeRangeMake(CGSizeMake(maxWidth, 0), + CGSizeMake(maxWidth, FLT_MAX)); +} + @end @interface ASTableViewTests : XCTestCase @@ -202,34 +213,71 @@ } } -- (void)testRelayoutAllRows +- (void)testRelayoutAllRowsWithNonZeroSizeInitially { - // Initial width of the table view is 0 and all nodes are measured with this size. - ASTestTableView *tableView = [[ASTestTableView alloc] initWithFrame:CGRectMake(0, 0, 0, 500) + // Initial width of the table view is non-zero and all nodes are measured with this size. + // Any subsequence size change must trigger a relayout. + CGSize tableViewFinalSize = CGSizeMake(100, 500); + // Width and height are swapped so that a later size change will simulate a rotation + ASTestTableView *tableView = [[ASTestTableView alloc] initWithFrame:CGRectMake(0, 0, tableViewFinalSize.height, tableViewFinalSize.width) style:UITableViewStylePlain asyncDataFetching:YES]; - CGSize tableViewFinalSize = CGSizeMake(100, 500); ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new]; tableView.asyncDelegate = dataSource; tableView.asyncDataSource = dataSource; + // Trigger layout measurement on all nodes [tableView reloadData]; + [self triggerSizeChangeAndAssertRelayoutAllRowsForTableView:tableView newSize:tableViewFinalSize]; +} + +- (void)testRelayoutAllRowsWithZeroSizeInitially +{ + // Initial width of the table view is 0. The first size change is part of the initial config. + // Any subsequence size change after that must trigger a relayout. + CGSize tableViewFinalSize = CGSizeMake(100, 500); + ASTestTableView *tableView = [[ASTestTableView alloc] initWithFrame:CGRectZero + style:UITableViewStylePlain + asyncDataFetching:YES]; + ASTableViewFilledDataSource *dataSource = [ASTableViewFilledDataSource new]; + + tableView.asyncDelegate = dataSource; + tableView.asyncDataSource = dataSource; + + // Initial configuration + UIView *superview = [[UIView alloc] initWithFrame:CGRectMake(0, 0, 500, 500)]; + [superview addSubview:tableView]; + // Width and height are swapped so that a later size change will simulate a rotation + tableView.frame = CGRectMake(0, 0, tableViewFinalSize.height, tableViewFinalSize.width); + // Trigger layout measurement on all nodes + [tableView layoutIfNeeded]; + + [self triggerSizeChangeAndAssertRelayoutAllRowsForTableView:tableView newSize:tableViewFinalSize]; +} + +- (void)triggerSizeChangeAndAssertRelayoutAllRowsForTableView:(ASTableView *)tableView newSize:(CGSize)newSize +{ + XCTestExpectation *nodesMeasuredUsingNewConstrainedSizeExpectation = [self expectationWithDescription:@"nodesMeasuredUsingNewConstrainedSizeExpectation"]; + [tableView beginUpdates]; - tableView.frame = CGRectMake(0, 0, tableViewFinalSize.width, tableViewFinalSize.height); + CGRect frame = tableView.frame; + frame.size = newSize; + tableView.frame = frame; [tableView layoutIfNeeded]; - XCTestExpectation *nodesMeasuredUsingNewConstrainedSizeExpectation = [self expectationWithDescription:@"nodesMeasuredUsingNewConstrainedSize"]; - [tableView endUpdatesAnimated:NO completion:^(BOOL completed) { + int numberOfRelayouts = ((ASTableViewFilledDataSource *)(tableView.asyncDataSource)).numberOfRelayouts; + XCTAssertEqual(numberOfRelayouts, 1); + for (int section = 0; section < NumberOfSections; section++) { for (int row = 0; row < NumberOfRowsPerSection; row++) { NSIndexPath *indexPath = [NSIndexPath indexPathForRow:row inSection:section]; ASCellNode *node = [tableView nodeForRowAtIndexPath:indexPath]; - XCTAssertEqual(node.constrainedSizeForCalculatedLayout.max.width, tableViewFinalSize.width); + XCTAssertEqual(node.constrainedSizeForCalculatedLayout.max.width, newSize.width); } } [nodesMeasuredUsingNewConstrainedSizeExpectation fulfill]; From a8cecbd0ad0a2c806194dabf110d8b1b2aa82ae9 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Tue, 8 Sep 2015 22:31:09 +0300 Subject: [PATCH 2/2] Avoid doing relayout during initial configuration of table and collection views. --- AsyncDisplayKit/ASCollectionView.mm | 22 +++++++++++++++++----- AsyncDisplayKit/ASTableView.mm | 22 +++++++++++++++++----- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/AsyncDisplayKit/ASCollectionView.mm b/AsyncDisplayKit/ASCollectionView.mm index 33edd6ed..cf54d6fb 100644 --- a/AsyncDisplayKit/ASCollectionView.mm +++ b/AsyncDisplayKit/ASCollectionView.mm @@ -126,6 +126,7 @@ static BOOL _isInterceptedSelector(SEL sel) ASBatchContext *_batchContext; CGSize _maxSizeForNodesConstrainedSize; + BOOL _ignoreMaxSizeChange; } @property (atomic, assign) BOOL asyncDataSourceLocked; @@ -179,6 +180,9 @@ static BOOL _isInterceptedSelector(SEL sel) _collectionViewLayoutImplementsInsetSection = [layout respondsToSelector:@selector(sectionInset)]; _maxSizeForNodesConstrainedSize = self.bounds.size; + // If the initial size is 0, expect a size change very soon which is part of the initial configuration + // and should not trigger a relayout. + _ignoreMaxSizeChange = CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, CGSizeZero); self.backgroundColor = [UIColor whiteColor]; @@ -481,14 +485,22 @@ static BOOL _isInterceptedSelector(SEL sel) - (void)layoutSubviews { - [super layoutSubviews]; - if (! CGSizeEqualToSize(_maxSizeForNodesConstrainedSize, self.bounds.size)) { _maxSizeForNodesConstrainedSize = self.bounds.size; - [self performBatchAnimated:NO updates:^{ - [_dataController relayoutAllRows]; - } completion:nil]; + + // First size change occurs during initial configuration. An expensive relayout pass is unnecessary at that time + // and should be avoided, assuming that the initial data loading automatically runs shortly afterward. + if (_ignoreMaxSizeChange) { + _ignoreMaxSizeChange = NO; + } else { + [self performBatchAnimated:NO updates:^{ + [_dataController relayoutAllRows]; + } completion:nil]; + } } + + // To ensure _maxSizeForNodesConstrainedSize is up-to-date for every usage, this call to super must be done last + [super layoutSubviews]; } diff --git a/AsyncDisplayKit/ASTableView.mm b/AsyncDisplayKit/ASTableView.mm index 8948591d..ff482d9c 100644 --- a/AsyncDisplayKit/ASTableView.mm +++ b/AsyncDisplayKit/ASTableView.mm @@ -149,6 +149,7 @@ static BOOL _isInterceptedSelector(SEL sel) BOOL _asyncDataSourceImplementsConstrainedSizeForNode; CGFloat _maxWidthForNodesConstrainedSize; + BOOL _ignoreMaxWidthChange; } @property (atomic, assign) BOOL asyncDataSourceLocked; @@ -203,6 +204,9 @@ void ASPerformBlockWithoutAnimation(BOOL withoutAnimation, void (^block)()) { _automaticallyAdjustsContentOffset = NO; _maxWidthForNodesConstrainedSize = self.bounds.size.width; + // If the initial size is 0, expect a size change very soon which is part of the initial configuration + // and should not trigger a relayout. + _ignoreMaxWidthChange = (_maxWidthForNodesConstrainedSize == 0); } - (instancetype)initWithFrame:(CGRect)frame style:(UITableViewStyle)style @@ -367,14 +371,22 @@ void ASPerformBlockWithoutAnimation(BOOL withoutAnimation, void (^block)()) { - (void)layoutSubviews { - [super layoutSubviews]; - if (_maxWidthForNodesConstrainedSize != self.bounds.size.width) { _maxWidthForNodesConstrainedSize = self.bounds.size.width; - [self beginUpdates]; - [_dataController relayoutAllRows]; - [self endUpdates]; + + // First width change occurs during initial configuration. An expensive relayout pass is unnecessary at that time + // and should be avoided, assuming that the initial data loading automatically runs shortly afterward. + if (_ignoreMaxWidthChange) { + _ignoreMaxWidthChange = NO; + } else { + [self beginUpdates]; + [_dataController relayoutAllRows]; + [self endUpdates]; + } } + + // To ensure _maxWidthForNodesConstrainedSize is up-to-date for every usage, this call to super must be done last + [super layoutSubviews]; } #pragma mark -