From df1650f518a22736c9971a1676e4231ea97fefd6 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sat, 8 Jul 2017 21:35:16 -0700 Subject: [PATCH 1/3] Use a sentinel NSUInteger for node layout data --- Source/ASDisplayNode+Layout.mm | 26 ++++++++++++++------------ Source/ASDisplayNode.mm | 9 ++------- Source/Private/ASDisplayNodeInternal.h | 4 +++- Source/Private/ASDisplayNodeLayout.h | 23 +++++++---------------- Source/Private/ASDisplayNodeLayout.mm | 23 +++++------------------ 5 files changed, 31 insertions(+), 54 deletions(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 6cc2d4059..9488553f5 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -82,13 +82,11 @@ - (ASLayout *)layoutThatFits:(ASSizeRange)constrainedSize parentSize:(CGSize)par } ASLayout *layout = nil; - if (_calculatedDisplayNodeLayout->isValidForConstrainedSizeParentSize(constrainedSize, parentSize)) { + NSUInteger version = _layoutVersion; + if (_calculatedDisplayNodeLayout != nullptr && _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 @@ - (ASLayout *)layoutThatFits:(ASSizeRange)constrainedSize parentSize:(CGSize)par layout = [self calculateLayoutThatFits:constrainedSize restrictedToSize:self.style.size relativeToParentSize:parentSize]; - _pendingDisplayNodeLayout = std::make_shared(layout, constrainedSize, parentSize); + _pendingDisplayNodeLayout = std::make_shared(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(layout, constrainedSize, boundsSizeForLayout); + nextLayout = std::make_shared(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; @@ -570,6 +569,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize // Perform a full layout creation pass with passed in constrained size to create the new layout for the transition ASLayout *newLayout; + NSUInteger newLayoutVersion; { ASDN::MutexLocker l(__instanceLock__); @@ -579,6 +579,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize BOOL automaticallyManagesSubnodesDisabled = (self.automaticallyManagesSubnodes == NO); self.automaticallyManagesSubnodes = YES; // Temporary flag for 1.9.x + newLayoutVersion = _layoutVersion; newLayout = [self calculateLayoutThatFits:constrainedSize restrictedToSize:self.style.size relativeToParentSize:constrainedSize.max]; @@ -609,7 +610,8 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize auto previousLayout = _calculatedDisplayNodeLayout; auto pendingLayout = std::make_shared(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 index 95547a084..44fadc768 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -285,6 +285,7 @@ - (void)_initializeInstance _calculatedDisplayNodeLayout = std::make_shared(); _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 index 10f48bced..dda21c189 100644 --- 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,9 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo std::shared_ptr _calculatedDisplayNodeLayout; std::shared_ptr _pendingDisplayNodeLayout; + /// Sentinel for layout data. Incremented when we get -setNeedsLayout / -invalidateCalculatedLayout. + std::atomic _layoutVersion; + ASDisplayNodeViewBlock _viewBlock; ASDisplayNodeLayerBlock _layerBlock; NSMutableArray *_onDidLoadBlocks; diff --git a/Source/Private/ASDisplayNodeLayout.h b/Source/Private/ASDisplayNodeLayout.h index 230071f41..f741b6b47 100644 --- 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 layout data. */ - 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 index c01b6a4cd..694adab0b 100644 --- a/Source/Private/ASDisplayNodeLayout.mm +++ b/Source/Private/ASDisplayNodeLayout.mm @@ -17,23 +17,10 @@ #import -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); } From 05a0e52c74f8025681753869cbbdbafd784338e9 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sat, 8 Jul 2017 21:38:54 -0700 Subject: [PATCH 2/3] Add a comment --- Source/Private/ASDisplayNodeInternal.h | 1 + 1 file changed, 1 insertion(+) diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h index dda21c189..7555e0938 100644 --- a/Source/Private/ASDisplayNodeInternal.h +++ b/Source/Private/ASDisplayNodeInternal.h @@ -160,6 +160,7 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo std::shared_ptr _pendingDisplayNodeLayout; /// Sentinel for layout data. Incremented when we get -setNeedsLayout / -invalidateCalculatedLayout. + /// Starts at 1. std::atomic _layoutVersion; ASDisplayNodeViewBlock _viewBlock; From 8f2be9ac38e9a8279ca4eab4c2a47a9cb9ea8715 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Sun, 9 Jul 2017 11:37:34 -0700 Subject: [PATCH 3/3] Address feedback from @appleguy --- Source/ASDisplayNode+Layout.mm | 5 ++--- Source/Private/ASDisplayNodeLayout.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index 9488553f5..6b73c1bc5 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -83,7 +83,7 @@ - (ASLayout *)layoutThatFits:(ASSizeRange)constrainedSize parentSize:(CGSize)par ASLayout *layout = nil; NSUInteger version = _layoutVersion; - if (_calculatedDisplayNodeLayout != nullptr && _calculatedDisplayNodeLayout->isValid(constrainedSize, parentSize, version)) { + if (_calculatedDisplayNodeLayout->isValid(constrainedSize, parentSize, version)) { ASDisplayNodeAssertNotNil(_calculatedDisplayNodeLayout->layout, @"-[ASDisplayNode layoutThatFits:parentSize:] _calculatedDisplayNodeLayout->layout should not be nil! %@", self); layout = _calculatedDisplayNodeLayout->layout; } else if (_pendingDisplayNodeLayout != nullptr && _pendingDisplayNodeLayout->isValid(constrainedSize, parentSize, version)) { @@ -568,8 +568,8 @@ - (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; - NSUInteger newLayoutVersion; { ASDN::MutexLocker l(__instanceLock__); @@ -579,7 +579,6 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize BOOL automaticallyManagesSubnodesDisabled = (self.automaticallyManagesSubnodes == NO); self.automaticallyManagesSubnodes = YES; // Temporary flag for 1.9.x - newLayoutVersion = _layoutVersion; newLayout = [self calculateLayoutThatFits:constrainedSize restrictedToSize:self.style.size relativeToParentSize:constrainedSize.max]; diff --git a/Source/Private/ASDisplayNodeLayout.h b/Source/Private/ASDisplayNodeLayout.h index f741b6b47..3bc7782a7 100644 --- a/Source/Private/ASDisplayNodeLayout.h +++ b/Source/Private/ASDisplayNodeLayout.h @@ -37,7 +37,7 @@ struct ASDisplayNodeLayout { * @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 layout data. + * @param version The version of the source layout data – see ASDisplayNode's _layoutVersion. */ ASDisplayNodeLayout(ASLayout *layout, ASSizeRange constrainedSize, CGSize parentSize, NSUInteger version) : layout(layout), constrainedSize(constrainedSize), parentSize(parentSize), requestedLayoutFromAbove(NO), version(version) {};