Merge pull request #643 from nguyenhuy/fixRelayoutDuringInitialConfig

Avoid doing relayout during initial configuration

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];
 }
 
 

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 -

AsyncDisplayKitTests/ASTableViewTests.m

@@ -57,7 +57,8 @@
 @end
 
 @interface ASTableViewFilledDataSource : NSObject <ASTableViewDataSource, ASTableViewDelegate>
-
+/** 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];