Use a sentinel NSUInteger for node layout data #trivial (#428)

* Use a sentinel NSUInteger for node layout data

* Add a comment

* Address feedback from @appleguy

Source/ASDisplayNode+Layout.mm

@@ -82,13 +82,11 @@
   }
 
   ASLayout *layout = nil;
-  if (_calculatedDisplayNodeLayout->isValidForConstrainedSizeParentSize(constrainedSize, parentSize)) {
+  NSUInteger version = _layoutVersion;
+  if (_calculatedDisplayNodeLayout->isValid(constrainedSize, parentSize, version)) {
     ASDisplayNodeAssertNotNil(_calculatedDisplayNodeLayout->layout, @"-[ASDisplayNode layoutThatFits:parentSize:] _calculatedDisplayNodeLayout->layout should not be nil! %@", self);
-    // Our calculated layout is suitable for this constrainedSize, so keep using it and
-    // invalidate any pending layout that has been generated in the past.
-    _pendingDisplayNodeLayout = nullptr;
     layout = _calculatedDisplayNodeLayout->layout;
-  } else if (_pendingDisplayNodeLayout != nullptr && _pendingDisplayNodeLayout->isValidForConstrainedSizeParentSize(constrainedSize, parentSize)) {
+  } else if (_pendingDisplayNodeLayout != nullptr && _pendingDisplayNodeLayout->isValid(constrainedSize, parentSize, version)) {
     ASDisplayNodeAssertNotNil(_pendingDisplayNodeLayout->layout, @"-[ASDisplayNode layoutThatFits:parentSize:] _pendingDisplayNodeLayout->layout should not be nil! %@", self);
     layout = _pendingDisplayNodeLayout->layout;
   } else {
@@ -96,7 +94,7 @@
     layout = [self calculateLayoutThatFits:constrainedSize
                           restrictedToSize:self.style.size
                       relativeToParentSize:parentSize];
-    _pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, constrainedSize, parentSize);
+    _pendingDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>(layout, constrainedSize, parentSize, version);
     ASDisplayNodeAssertNotNil(layout, @"-[ASDisplayNode layoutThatFits:parentSize:] newly calculated layout should not be nil! %@", self);
   }
   
@@ -308,7 +306,7 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
   // Prefer _pendingDisplayNodeLayout over _calculatedDisplayNodeLayout (if exists, it's the newest)
   // If there is no _pending, check if _calculated is valid to reuse (avoiding recalculation below).
   if (_pendingDisplayNodeLayout == nullptr) {
-    if (_calculatedDisplayNodeLayout->isDirty() == NO
+    if (_calculatedDisplayNodeLayout->version >= _layoutVersion
         && (_calculatedDisplayNodeLayout->requestedLayoutFromAbove == YES
             || CGSizeEqualToSize(_calculatedDisplayNodeLayout->layout.size, boundsSizeForLayout))) {
       return;
@@ -337,8 +335,8 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
   BOOL pendingLayoutApplicable = NO;
   if (nextLayout == nullptr) {
     as_log_verbose(ASLayoutLog(), "No pending layout.");
-  } else if (nextLayout->isDirty()) {
-    as_log_verbose(ASLayoutLog(), "Pending layout is invalid.");
+  } else if (nextLayout->version < _layoutVersion) {
+    as_log_verbose(ASLayoutLog(), "Pending layout is stale.");
   } else if (layoutSizeDifferentFromBounds) {
     as_log_verbose(ASLayoutLog(), "Pending layout size %@ doesn't match bounds size.", NSStringFromCGSize(nextLayout->layout.size));
   } else {
@@ -349,12 +347,13 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
   if (!pendingLayoutApplicable) {
     as_log_verbose(ASLayoutLog(), "Measuring with previous constrained size.");
     // Use the last known constrainedSize passed from a parent during layout (if never, use bounds).
+    NSUInteger version = _layoutVersion;
     ASSizeRange constrainedSize = [self _locked_constrainedSizeForLayoutPass];
     ASLayout *layout = [self calculateLayoutThatFits:constrainedSize
                                     restrictedToSize:self.style.size
                                 relativeToParentSize:boundsSizeForLayout];
     
-    nextLayout = std::make_shared<ASDisplayNodeLayout>(layout, constrainedSize, boundsSizeForLayout);
+    nextLayout = std::make_shared<ASDisplayNodeLayout>(layout, constrainedSize, boundsSizeForLayout, version);
   }
   
   if (didCreateNewContext) {
@@ -425,7 +424,7 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
   ASLayout *layout;
   {
     ASDN::MutexLocker l(__instanceLock__);
-    if (_calculatedDisplayNodeLayout->isDirty()) {
+    if (_calculatedDisplayNodeLayout->version < _layoutVersion) {
       return;
     }
     layout = _calculatedDisplayNodeLayout->layout;
@@ -569,6 +568,7 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
     }
     
     // Perform a full layout creation pass with passed in constrained size to create the new layout for the transition
+    NSUInteger newLayoutVersion = _layoutVersion;
     ASLayout *newLayout;
     {
       ASDN::MutexLocker l(__instanceLock__);
@@ -609,7 +609,8 @@ ASPrimitiveTraitCollectionDeprecatedImplementation
         auto previousLayout = _calculatedDisplayNodeLayout;
         auto pendingLayout = std::make_shared<ASDisplayNodeLayout>(newLayout,
                                                                    constrainedSize,
-                                                                   constrainedSize.max);
+                                                                   constrainedSize.max,
+                                                                   newLayoutVersion);
         [self _locked_setCalculatedDisplayNodeLayout:pendingLayout];
         
         // Setup pending layout transition for animation

Source/ASDisplayNode.mm

@@ -285,6 +285,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
   
   _calculatedDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>();
   _pendingDisplayNodeLayout = nullptr;
+  _layoutVersion = 1;
   
   _defaultLayoutTransitionDuration = 0.2;
   _defaultLayoutTransitionDelay = 0.0;
@@ -882,12 +883,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
 {
   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();
-  if (_pendingDisplayNodeLayout != nullptr) {
-    _pendingDisplayNodeLayout->invalidate();
-  }
+  _layoutVersion++;
   
   _unflattenedLayout = nil;
 
@@ -924,7 +920,6 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c)
     // This method will confirm that the layout is up to date (and update if needed).
     // Importantly, it will also APPLY the layout to all of our subnodes if (unless parent is transitioning).
     [self _locked_measureNodeWithBoundsIfNecessary:bounds];
-    _pendingDisplayNodeLayout = nullptr;
     
     [self _locked_layoutPlaceholderIfNecessary];
   }

Source/Private/ASDisplayNodeInternal.h

@@ -34,7 +34,6 @@ NS_ASSUME_NONNULL_BEGIN
 @protocol _ASDisplayLayerDelegate;
 @class _ASDisplayLayer;
 @class _ASPendingState;
-@class ASSentinel;
 struct ASDisplayNodeFlags;
 
 BOOL ASDisplayNodeSubclassOverridesSelector(Class subclass, SEL selector);
@@ -160,6 +159,10 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo
   std::shared_ptr<ASDisplayNodeLayout> _calculatedDisplayNodeLayout;
   std::shared_ptr<ASDisplayNodeLayout> _pendingDisplayNodeLayout;
   
+  /// Sentinel for layout data. Incremented when we get -setNeedsLayout / -invalidateCalculatedLayout.
+  /// Starts at 1.
+  std::atomic<NSUInteger> _layoutVersion;
+  
   ASDisplayNodeViewBlock _viewBlock;
   ASDisplayNodeLayerBlock _layerBlock;
   NSMutableArray<ASDisplayNodeDidLoadBlock> *_onDidLoadBlocks;

Source/Private/ASDisplayNodeLayout.h

@@ -30,35 +30,26 @@ struct ASDisplayNodeLayout {
   ASSizeRange constrainedSize;
   CGSize parentSize;
   BOOL requestedLayoutFromAbove;
-  BOOL _dirty;
+  NSUInteger version;
   
   /*
    * Create a new display node layout with
    * @param layout The layout to associate, usually returned from a call to -layoutThatFits:parentSize:
    * @param constrainedSize Constrained size used to create the layout
    * @param parentSize Parent size used to create the layout
+   * @param version The version of the source layout data – see ASDisplayNode's _layoutVersion. 
    */
-  ASDisplayNodeLayout(ASLayout *layout, ASSizeRange constrainedSize, CGSize parentSize)
-  : layout(layout), constrainedSize(constrainedSize), parentSize(parentSize), requestedLayoutFromAbove(NO), _dirty(NO) {};
+  ASDisplayNodeLayout(ASLayout *layout, ASSizeRange constrainedSize, CGSize parentSize, NSUInteger version)
+  : layout(layout), constrainedSize(constrainedSize), parentSize(parentSize), requestedLayoutFromAbove(NO), version(version) {};
   
   /*
    * Creates a layout without any layout associated. By default this display node layout is dirty.
    */
   ASDisplayNodeLayout()
-  : layout(nil), constrainedSize({{0, 0}, {0, 0}}), parentSize({0, 0}), requestedLayoutFromAbove(NO), _dirty(YES) {};
+  : layout(nil), constrainedSize({{0, 0}, {0, 0}}), parentSize({0, 0}), requestedLayoutFromAbove(NO), version(0) {};
   
   /**
-   * Returns if the display node layout is dirty as it was invalidated or it was created without a layout.
+   * Returns whether this is valid for a given constrained size, parent size, and version
    */
-  BOOL isDirty();
-  
-  /**
-   * Returns if ASDisplayNode is still valid for a given constrained and parent size
-   */
-  BOOL isValidForConstrainedSizeParentSize(ASSizeRange constrainedSize, CGSize parentSize);
-  
-  /**
-   * Invalidate the display node layout
-   */
-  void invalidate();
+  BOOL isValid(ASSizeRange constrainedSize, CGSize parentSize, NSUInteger version);
 };

Source/Private/ASDisplayNodeLayout.mm

@@ -17,23 +17,10 @@
 
 #import <AsyncDisplayKit/ASDisplayNodeLayout.h>
 
-BOOL ASDisplayNodeLayout::isDirty()
+BOOL ASDisplayNodeLayout::isValid(ASSizeRange theConstrainedSize, CGSize theParentSize, NSUInteger versionArg)
 {
-  return _dirty || layout == nil;
-}
-
-BOOL ASDisplayNodeLayout::isValidForConstrainedSizeParentSize(ASSizeRange theConstrainedSize, CGSize theParentSize)
-{
-  // Only generate a new layout if:
-  // - The current layout is dirty
-  // - The passed constrained size is different than the original layout's parent or constrained  size
-  return (layout != nil
-          && _dirty == NO
-          && CGSizeEqualToSize(parentSize, theParentSize)
-          && ASSizeRangeEqualToSizeRange(constrainedSize, theConstrainedSize));
-}
-
-void ASDisplayNodeLayout::invalidate()
-{
-  _dirty = YES;
+  return version >= versionArg
+      && layout != nil
+      && CGSizeEqualToSize(parentSize, theParentSize)
+      && ASSizeRangeEqualToSizeRange(constrainedSize, theConstrainedSize);
 }