Use a sentinel NSUInteger for node layout data #trivial (TextureGroup…

…#428) * Use a sentinel NSUInteger for node layout data * Add a comment * Address feedback from @appleguy
AtomTickets · Apr 25, 2018 · 9dfe118 · 9dfe118
1 parent f14f7ac
commit 9dfe118
Show file tree

Hide file tree

Showing 5 changed files with 31 additions and 54 deletions.
diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm
@@ -82,21 +82,19 @@ - (ASLayout *)layoutThatFits:(ASSizeRange)constrainedSize parentSize:(CGSize)par
   }
 
   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 {
     // Create a pending display node layout for the layout pass
     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 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
   // 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 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
   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 @@ - (void)_locked_measureNodeWithBoundsIfNecessary:(CGRect)bounds
   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 @@ - (void)_layoutSublayouts
   ASLayout *layout;
   {
     ASDN::MutexLocker l(__instanceLock__);
-    if (_calculatedDisplayNodeLayout->isDirty()) {
+    if (_calculatedDisplayNodeLayout->version < _layoutVersion) {
       return;
     }
     layout = _calculatedDisplayNodeLayout->layout;
@@ -569,6 +568,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize
     }
 
     // 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 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize
         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

diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm
@@ -285,6 +285,7 @@ - (void)_initializeInstance
 
   _calculatedDisplayNodeLayout = std::make_shared<ASDisplayNodeLayout>();
   _pendingDisplayNodeLayout = nullptr;
+  _layoutVersion = 1;
 
   _defaultLayoutTransitionDuration = 0.2;
   _defaultLayoutTransitionDelay = 0.0;
@@ -882,12 +883,7 @@ - (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();
-  if (_pendingDisplayNodeLayout != nullptr) {
-    _pendingDisplayNodeLayout->invalidate();
-  }
+  _layoutVersion++;
 
   _unflattenedLayout = nil;
 
@@ -924,7 +920,6 @@ - (void)__layout
     // 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];
   }

diff --git a/Source/Private/ASDisplayNodeInternal.h b/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;

diff --git a/Source/Private/ASDisplayNodeLayout.h b/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);
 };
diff --git a/Source/Private/ASDisplayNodeLayout.mm b/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);
 }