Address comments

AsyncDisplayKit/ASCellNode.mm

@@ -120,10 +120,7 @@ static NSMutableSet *__cellClassesForVisibilityNotifications = nil; // See +init
 
 - (void)displayNodeDidInvalidateSizeOldSize:(CGSize)oldSize
 {
-  ASSizeRange constrainedSize = ASSizeRangeMake(CGSizeZero, CGSizeMake(CGRectGetWidth(self.bounds), CGFLOAT_MAX));
-  if (_interactionDelegate != nil) {
-    constrainedSize = [_interactionDelegate constrainedSizeForNode:self];
-  }
+  ASSizeRange constrainedSize = [_interactionDelegate constrainedSizeForNode:self];
   CGSize newSize = [self layoutThatFits:constrainedSize].size;
     
   if (CGSizeEqualToSize(oldSize, newSize) == NO) {

AsyncDisplayKit/ASCollectionView.mm

@@ -765,9 +765,7 @@ static NSString * const kCellReuseIdentifier = @"_ASCollectionViewCell";
 
 - (CGSize)collectionView:(UICollectionView *)collectionView layout:(UICollectionViewLayout *)collectionViewLayout sizeForItemAtIndexPath:(NSIndexPath *)indexPath
 {
-  ASDisplayNode *node = [self nodeForItemAtIndexPath:indexPath];
-  [node layoutIfNeeded];
-  return node.calculatedSize;
+  return [[self nodeForItemAtIndexPath:indexPath] calculatedSize];
 }
 
 - (UICollectionReusableView *)collectionView:(UICollectionView *)collectionView viewForSupplementaryElementOfKind:(NSString *)kind atIndexPath:(NSIndexPath *)indexPath

AsyncDisplayKit/ASDisplayNode.h

@@ -256,9 +256,6 @@ extern NSInteger const ASDefaultDrawingPriority;
 
 // TODO: coalesc: Documentation
 - (void)invalidateSize;
-- (void)didInvalidateSize;
-- (void)sizeToFit;
-- (CGSize)sizeThatFits:(CGSize)size;
 
 /**
  * @abstract Asks the node to return a layout based on given size range.
@@ -637,21 +634,21 @@ extern NSInteger const ASDefaultDrawingPriority;
 @interface ASDisplayNode (UIViewBridge)
 
 /**
- * Marks the view as needing display. Convenience for use whether the view / layer is loaded or not. Safe to call from a background thread.
+ * Marks the view as needing display. Convenience for use whether the view / layer is loaded or not. Safe to call
+ * from a background thread.
  */
 - (void)setNeedsDisplay;
 
 /**
  * Marks the node as needing layout. Convenience for use whether the view / layer is loaded or not. Safe to call from a background thread.
- * 
- * If this node was measured, calling this method triggers an internal relayout: the calculated layout is invalidated,
- * and the supernode is notified or (if this node is the root one) a full measurement pass is executed using the old constrained size.
- *
- * Note: ASCellNode has special behavior in that calling this method will automatically notify 
- * the containing ASTableView / ASCollectionView that the cell should be resized, if necessary.
  */
 - (void)setNeedsLayout;
 
+/**
+ * Recalculate the receiver’s layout, if required.
+ *
+ * When this message is received, the layer’s super layers are traversed until a ancestor layer is found that does not require layout. Then layout is performed on the entire layer-tree beneath that ancestor.
+ */
 - (void)layoutIfNeeded;
 
 @property (nonatomic, strong, nullable) id contents;                           // default=nil

AsyncDisplayKit/ASDisplayNode.mm

@@ -713,8 +713,8 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   // Mark the node for layout in the next layout pass
   [self invalidateCalculatedLayout];
   
-  if (_supernode) {
-    ASDisplayNode *supernode = _supernode;
+  ASDisplayNode *supernode = _supernode;
+  if (supernode) {
     __instanceLock__.unlock();
     // Cause supernode's layout to be invalidated
     // We need to release the lock to prevent a deadlock
@@ -722,50 +722,39 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
     return;
   }
   
-  // Calculate a new pending layout. It will be applied in the next layout pass
-  ASLayout *layout = [self layoutThatFits:_calculatedDisplayNodeLayout->constrainedSize];
-  if (CGSizeEqualToSize(self.bounds.size, layout.size) == NO) {
-    // If the size of the layout changes inform the node of this
-    [self displayNodeDidInvalidateSizeOldSize:self.bounds.size];
-  }
+  // We are the root node and need to re-flow the layout; one of our children requested to have its size re-set.
+  CGSize boundsSize = self.bounds.size;
   
+  // Figure out constrainedSize to use
+  ASLayout *layout = nil;
+  if (_pendingDisplayNodeLayout != nullptr) {
+    layout = [self layoutThatFits:_pendingDisplayNodeLayout->constrainedSize];
+  } else {
+    layout = [self layoutThatFits:_calculatedDisplayNodeLayout->constrainedSize];
+  }
+  if (CGSizeEqualToSize(boundsSize, layout.size) == NO) {
+    // If the size of the layout changes inform our container (e.g ASTableView, ASCollectionView, ASViewController, ...)
+    // that we need it to change our bounds size.
+    [self displayNodeDidInvalidateSizeOldSize:boundsSize];
+  }
   
   __instanceLock__.unlock();
 }
 
+// TODO: Pass in oldSize and new layout
 - (void)displayNodeDidInvalidateSizeOldSize:(CGSize)size
-{
-  // The default implementation of display node changes the size of itself to the new size
-  CGRect oldBounds = self.bounds;
-  CGSize oldSize = oldBounds.size;
-  CGSize newSize = _calculatedDisplayNodeLayout->layout.size;
-  
-  if (! CGSizeEqualToSize(oldSize, newSize)) {
-    self.bounds = (CGRect){ oldBounds.origin, newSize };
-    
-    // Frame's origin must be preserved. Since it is computed from bounds size, anchorPoint
-    // and position (see frame setter in ASDisplayNode+UIViewBridge), position needs to be adjusted.
-    CGPoint anchorPoint = self.anchorPoint;
-    CGPoint oldPosition = self.position;
-    CGFloat xDelta = (newSize.width - oldSize.width) * anchorPoint.x;
-    CGFloat yDelta = (newSize.height - oldSize.height) * anchorPoint.y;
-    self.position = CGPointMake(oldPosition.x + xDelta, oldPosition.y + yDelta);
-  }
-}
-
-- (void)sizeToFit
 {
   ASDisplayNodeAssertThreadAffinity(self);
   
-  __instanceLock__.lock();
-  
-  [self setNeedsLayout];
-  
-  CGSize maxSize = _supernode ? _supernode.bounds.size : CGSizeMake(CGFLOAT_MAX, CGFLOAT_MAX);
-  CGSize newSize = [self sizeThatFits:maxSize];
-  
+  // The default implementation of display node changes the size of itself to the new size
   CGRect oldBounds = self.bounds;
   CGSize oldSize = oldBounds.size;
+  CGSize newSize = CGSizeZero;
+  if (_pendingDisplayNodeLayout != nullptr) {
+    newSize = _pendingDisplayNodeLayout->layout.size;
+  } else {
+    newSize = _calculatedDisplayNodeLayout->layout.size;
+  }
   
   if (! CGSizeEqualToSize(oldSize, newSize)) {
     self.bounds = (CGRect){ oldBounds.origin, newSize };
@@ -778,13 +767,6 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
     CGFloat yDelta = (newSize.height - oldSize.height) * anchorPoint.y;
     self.position = CGPointMake(oldPosition.x + xDelta, oldPosition.y + yDelta);
   }
-  
-  __instanceLock__.unlock();
-}
-
-- (CGSize)sizeThatFits:(CGSize)size
-{
-  return [self layoutThatFits:ASSizeRangeMake(CGSizeZero, size)].size;
 }
 
 - (ASLayout *)layoutThatFits:(ASSizeRange)constrainedSize
@@ -1418,6 +1400,8 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
 
 - (void)invalidateCalculatedLayout
 {
+  ASDN::MutexLocker l(__instanceLock__);
+  
   // This will cause the next layout pass to compute a new layout instead of returning
   // the cached layout in case the constrained or parent size did not change
   _calculatedDisplayNodeLayout->invalidate();
@@ -1448,17 +1432,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
  */
 - (void)__layoutIfNeeded
 {
-  ASDisplayNodeAssertThreadAffinity(self);
-  __instanceLock__.lock();
-  ASDisplayNode *supernode = _supernode;
-  __instanceLock__.unlock();
-  
-  if ([supernode __needsLayout]) {
-    [supernode __layoutIfNeeded];
-  } else {
-    // Layout all subviews starting from the first node that needs layout
-    [self __layoutSublayers];
-  }
+  // TODO: Nothing in here yet
 }
 
 - (void)__setNeedsDisplay
@@ -1520,16 +1494,16 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   }
 
   
-  // Check if we can reuse the calculated display node layout
-  if (_pendingDisplayNodeLayout == nullptr &&
-      _calculatedDisplayNodeLayout->isDirty() == NO &&
-      CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, bounds.size))
-  {
-    // Reuse calculatedDisplayNodeLayout for layout pass
-    return;
+  // Check if we can reuse the calculated display node layout. We prefer the _pendingDisplayNodeLayout over the
+  // _calculatedDisplayNodeLayout though
+  if (_pendingDisplayNodeLayout == nullptr) {
+    if (_calculatedDisplayNodeLayout->isDirty() == NO && CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, bounds.size)) {
+      // Reuse calculatedDisplayNodeLayout for layout pass
+      return;
+    }
   }
   
-  // Calcualted layout is not reusable we need to transform to a new one
+  // calculatedDisplayNodeLayout is not reusable we need to transition to a new one
   [self cancelLayoutTransition];
   
   BOOL didCreateNewContext = NO;
@@ -1559,14 +1533,22 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
       return _pendingDisplayNodeLayout;
     }
   
+    // By default use the bounds
     CGSize parentSize = bounds.size;
     ASSizeRange constrainedSize = ASSizeRangeMake(parentSize);
     
     // Checkout if constrained size of layouts can be reused
-    if (_pendingDisplayNodeLayout != nullptr) {
+    if (_pendingDisplayNodeLayout != nullptr && CGSizeEqualToSize(_pendingDisplayNodeLayout->layout.size, bounds.size)) {
+      // We assume the size for the last returned layoutThatFits: layout was applied use it's constrainedSizes
       constrainedSize = _pendingDisplayNodeLayout->constrainedSize;
-    } else if (CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, CGSizeZero) == NO) {
+    } else if (CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, CGSizeZero) == NO &&
+               CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, bounds.size)) {
+      // We assume the  _calculatedDisplayNodeLayout is still valid
       constrainedSize = _calculatedDisplayNodeLayout->constrainedSize;
+    } else {
+      // In this case neither the _pendingDisplayNodeLayout or the _calculatedDisplayNodeLayout constrained size can
+      // be reused, so the current bounds is used. This is usual the case if a frame was set manually that differs to
+      // the one returned from layoutThatFits:
     }
     
     return std::make_shared<ASDisplayNodeLayout>(
@@ -1576,8 +1558,8 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
     );
   }();
   
-  // If the size of the new layout we wan to apply did change from the current bounds invalidate the whole tree up
-  // so the root node can resize in case it needs to be
+  // If the size of the new layout to apply did change from the current bounds, invalidate the whole tree up
+  // so the root node can handle a resizing if necessary
   if (CGSizeEqualToSize(self.bounds.size, pendingLayout->layout.size) == NO) {
     [self invalidateSize];
   }
@@ -1604,6 +1586,19 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   }
 }
 
+- (ASSizeRange)conrainedSizeForCurrentLayout
+{
+  ASSizeRange constrainedSize = ASSizeRangeMake(self.bounds.size);
+
+  // Checkout if constrained size of layouts can be reused
+  if (_pendingDisplayNodeLayout != nullptr) {
+    constrainedSize = _pendingDisplayNodeLayout->constrainedSize;
+  } else if (CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, CGSizeZero) == NO) {
+    constrainedSize = _calculatedDisplayNodeLayout->constrainedSize;
+  }
+  return constrainedSize;
+}
+
 - (void)layoutDidFinish
 {
   // Hook for subclasses
@@ -2691,12 +2686,18 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock)
 - (CGSize)calculatedSize
 {
   ASDN::MutexLocker l(__instanceLock__);
+  if (_pendingDisplayNodeLayout != nullptr) {
+    return _pendingDisplayNodeLayout->layout.size;
+  }
   return _calculatedDisplayNodeLayout->layout.size;
 }
 
 - (ASSizeRange)constrainedSizeForCalculatedLayout
 {
   ASDN::MutexLocker l(__instanceLock__);
+  if (_pendingDisplayNodeLayout != nullptr) {
+    return _pendingDisplayNodeLayout->constrainedSize;
+  }
   return _calculatedDisplayNodeLayout->constrainedSize;
 }
 

AsyncDisplayKit/ASTableView.mm

@@ -751,7 +751,6 @@ static NSString * const kCellReuseIdentifier = @"_ASTableViewCell";
 - (CGFloat)tableView:(UITableView *)tableView heightForRowAtIndexPath:(NSIndexPath *)indexPath
 {
   ASCellNode *node = [_dataController nodeAtIndexPath:indexPath];
-  [node layoutIfNeeded];
   return node.calculatedSize.height;
 }
 

AsyncDisplayKit/Details/_ASDisplayView.mm

@@ -16,6 +16,7 @@
 #import "ASDisplayNode+FrameworkPrivate.h"
 #import "ASDisplayNode+Subclasses.h"
 #import "ASObjectDescriptionHelpers.h"
+#import "ASLayout.h"
 
 @interface _ASDisplayView ()
 @property (nullable, atomic, weak, readwrite) ASDisplayNode *asyncdisplaykit_node;
@@ -205,10 +206,7 @@
 - (CGSize)sizeThatFits:(CGSize)size
 {
   ASDisplayNode *node = _asyncdisplaykit_node; // Create strong reference to weak ivar.
-  if (node) {
-    return [node sizeThatFits:size];
-  }
-  return [super sizeThatFits:size];
+  return node ? [node layoutThatFits:ASSizeRangeMake(size)].size : [super sizeThatFits:size];
 }
 
 - (void)setNeedsDisplay

AsyncDisplayKit/Private/ASDisplayNodeInternal.h

@@ -189,12 +189,13 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 
 + (void)scheduleNodeForRecursiveDisplay:(ASDisplayNode *)node;
 
-// The _ASDisplayLayer backing the node, if any.
+/// The _ASDisplayLayer backing the node, if any.
 @property (nonatomic, readonly, strong) _ASDisplayLayer *asyncLayer;
 
-// Bitmask to check which methods an object overrides.
+/// Bitmask to check which methods an object overrides.
 @property (nonatomic, assign, readonly) ASDisplayNodeMethodOverrides methodOverrides;
 
+/// Thread safe way to access the bounds of the node
 @property (nonatomic, assign) CGRect threadSafeBounds;
 
 
@@ -202,23 +203,34 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 - (BOOL)__shouldLoadViewOrLayer;
 
 /**
- Invoked before a call to setNeedsLayout to the underlying view
+ * Invoked before a call to setNeedsLayout to the underlying view
  */
 - (void)__setNeedsLayout;
 
+/**
+ * The node's supernodes are traversed until a ancestor node is found that does not require layout. Then layout
+ * is performed on the entire node-tree beneath that ancestor
+ */
 - (void)__layoutIfNeeded;
 
 /**
- Invoked after a call to setNeedsDisplay to the underlying view
+ * Invoked after a call to setNeedsDisplay to the underlying view
  */
 - (void)__setNeedsDisplay;
 
+/**
+ * Called from [CALayer layoutSublayers:]. Executes the layout pass for the node
+ */
 - (void)__layoutSublayers;
+
+/*
+ * Internal method to set the supernode
+ */
 - (void)__setSupernode:(ASDisplayNode *)supernode;
 
 /**
- Internal method to add / replace / insert subnode and remove from supernode without checking if
- node has automaticallyManagesSubnodes set to YES. 
+ * Internal method to add / replace / insert subnode and remove from supernode without checking if
+ * node has automaticallyManagesSubnodes set to YES.
  */
 - (void)_addSubnode:(ASDisplayNode *)subnode;
 - (void)_replaceSubnode:(ASDisplayNode *)oldSubnode withSubnode:(ASDisplayNode *)replacementSubnode;
@@ -233,16 +245,16 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
 - (void)__incrementVisibilityNotificationsDisabled;
 - (void)__decrementVisibilityNotificationsDisabled;
 
-// Helper method to summarize whether or not the node run through the display process
+/// Helper method to summarize whether or not the node run through the display process
 - (BOOL)__implementsDisplay;
 
-// Display the node's view/layer immediately on the current thread, bypassing the background thread rendering. Will be deprecated.
+/// Display the node's view/layer immediately on the current thread, bypassing the background thread rendering. Will be deprecated.
 - (void)displayImmediately;
 
-// Alternative initialiser for backing with a custom view class.  Supports asynchronous display with _ASDisplayView subclasses.
+/// Alternative initialiser for backing with a custom view class.  Supports asynchronous display with _ASDisplayView subclasses.
 - (instancetype)initWithViewClass:(Class)viewClass;
 
-// Alternative initialiser for backing with a custom layer class.  Supports asynchronous display with _ASDisplayLayer subclasses.
+/// Alternative initialiser for backing with a custom layer class.  Supports asynchronous display with _ASDisplayLayer subclasses.
 - (instancetype)initWithLayerClass:(Class)layerClass;
 
 @property (nonatomic, assign) CGFloat contentsScaleForDisplay;

AsyncDisplayKitTests/ASDisplayNodeTestsHelper.m

@@ -45,7 +45,7 @@ BOOL ASDisplayNodeRunRunLoopUntilBlockIsTrue(as_condition_block_t block)
 }
 
 void ASDisplayNodeSizeToFitSize(ASDisplayNode *node, CGSize size) {
-  CGSize sizeThatFits = [node sizeThatFits:size];
+  CGSize sizeThatFits = [node layoutThatFits:ASSizeRangeMake(size)].size;
   node.bounds = (CGRect){.origin = CGPointZero, .size = sizeThatFits};
 }
 void ASDisplayNodeSizeToFitSizeRange(ASDisplayNode *node, ASSizeRange sizeRange) {

AsyncDisplayKitTests/ASImageNodeSnapshotTests.m

@@ -47,14 +47,11 @@
   imageNode.style.width = ASDimensionMake(forcedImageSize.width);
   imageNode.style.height = ASDimensionMake(forcedImageSize.height);
   ASDisplayNodeSizeToFitSize(imageNode, forcedImageSize);
-  [imageNode layoutIfNeeded];
   ASSnapshotVerifyNode(imageNode, @"first");
   
   imageNode.style.width = ASDimensionMake(200);
   imageNode.style.height = ASDimensionMake(200);
   ASDisplayNodeSizeToFitSize(imageNode, CGSizeMake(200, 200));
-  [imageNode layoutIfNeeded];
-  
   ASSnapshotVerifyNode(imageNode, @"second");
   
   XCTAssert(CGImageGetWidth((CGImageRef)imageNode.contents) == forcedImageSize.width * imageNode.contentsScale &&

AsyncDisplayKitTests/ASTextNodeSnapshotTests.m

@@ -25,8 +25,7 @@
   textNode.attributedText = [[NSAttributedString alloc] initWithString:@"judar"
                                                             attributes:@{NSFontAttributeName : [UIFont italicSystemFontOfSize:24]}];
   textNode.textContainerInset = UIEdgeInsetsMake(0, 2, 0, 2);
-  ASLayout *layout = [textNode layoutThatFits:ASSizeRangeMake(CGSizeZero, CGSizeMake(CGFLOAT_MAX, CGFLOAT_MAX))];
-  textNode.frame = CGRectMake(0, 0, layout.size.width, layout.size.height);
+  ASDisplayNodeSizeToFitSizeRange(textNode, ASSizeRangeMake(CGSizeZero, CGSizeMake(CGFLOAT_MAX, CGFLOAT_MAX)));
   
   ASSnapshotVerifyNode(textNode, nil);
 }