From bd742e6727b322d05bfd1a718bbd4e60f2590d05 Mon Sep 17 00:00:00 2001 From: Adlai Holler Date: Mon, 15 May 2017 16:33:44 -0700 Subject: [PATCH] Simplify Layout Transition State #trivial (#269) * Port the changes to the latest master * Remove extra s --- Source/ASDisplayNode+Layout.mm | 75 ++++++------------- Source/ASDisplayNode.mm | 7 +- .../Private/ASDisplayNode+FrameworkPrivate.h | 7 +- Source/Private/ASDisplayNodeInternal.h | 5 +- 4 files changed, 27 insertions(+), 67 deletions(-) diff --git a/Source/ASDisplayNode+Layout.mm b/Source/ASDisplayNode+Layout.mm index add82bab8..468b164ef 100644 --- a/Source/ASDisplayNode+Layout.mm +++ b/Source/ASDisplayNode+Layout.mm @@ -454,12 +454,6 @@ - (void)setAutomaticallyManagesSubnodes:(BOOL)automaticallyManagesSubnodes @implementation ASDisplayNode (ASLayoutTransition) -- (BOOL)_isTransitionInProgress -{ - ASDN::MutexLocker l(__instanceLock__); - return _transitionInProgress; -} - - (BOOL)_isLayoutTransitionInvalid { ASDN::MutexLocker l(__instanceLock__); @@ -475,40 +469,17 @@ - (BOOL)_isLayoutTransitionInvalid /// Starts a new transition and returns the transition id - (int32_t)_startNewTransition { - ASDN::MutexLocker l(__instanceLock__); - _transitionInProgress = YES; - _transitionID = OSAtomicAdd32(1, &_transitionID); - return _transitionID; -} - -- (void)_finishOrCancelTransition -{ - ASDN::MutexLocker l(__instanceLock__); - _transitionInProgress = NO; + static std::atomic gNextTransitionID; + int32_t newTransitionID = gNextTransitionID.fetch_add(1) + 1; + _transitionID = newTransitionID; + return newTransitionID; } -- (void)setPendingTransitionID:(int32_t)pendingTransitionID -{ - ASDN::MutexLocker l(__instanceLock__); - ASDisplayNodeAssertTrue(_pendingTransitionID < pendingTransitionID); - _pendingTransitionID = pendingTransitionID; -} - -- (int32_t)pendingTransitionID +/// Returns NO if there was no transition to cancel/finish. +- (BOOL)_finishOrCancelTransition { - ASDN::MutexLocker l(__instanceLock__); - return _pendingTransitionID; -} - -- (BOOL)_shouldAbortTransitionWithID:(int32_t)transitionID -{ - ASDN::MutexLocker l(__instanceLock__); - return [self _locked_shouldAbortTransitionWithID:transitionID]; -} - -- (BOOL)_locked_shouldAbortTransitionWithID:(int32_t)transitionID -{ - return (!_transitionInProgress || _transitionID != transitionID); + int32_t oldValue = _transitionID.exchange(ASLayoutElementContextInvalidTransitionID); + return oldValue != ASLayoutElementContextInvalidTransitionID; } #pragma mark Layout Transition @@ -554,17 +525,21 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize // Every new layout transition has a transition id associated to check in subsequent transitions for cancelling int32_t transitionID = [self _startNewTransition]; - + // NOTE: This block captures self. It's cheaper than hitting the weak table. + asdisplaynode_iscancelled_block_t isCancelled = ^{ + return (BOOL)(_transitionID != transitionID); + }; + // Move all subnodes in layout pending state for this transition ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) { - ASDisplayNodeAssert([node _isTransitionInProgress] == NO, @"Can't start a transition when one of the subnodes is performing one."); + ASDisplayNodeAssert(node->_transitionID == ASLayoutElementContextInvalidTransitionID, @"Can't start a transition when one of the subnodes is performing one."); node.hierarchyState |= ASHierarchyStateLayoutPending; - node.pendingTransitionID = transitionID; + node->_pendingTransitionID = transitionID; }); // Transition block that executes the layout transition void (^transitionBlock)(void) = ^{ - if ([self _shouldAbortTransitionWithID:transitionID]) { + if (isCancelled()) { return; } @@ -587,11 +562,14 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize ASLayoutElementClearCurrentContext(); } - if ([self _shouldAbortTransitionWithID:transitionID]) { + if (isCancelled()) { return; } ASPerformBlockOnMainThread(^{ + if (isCancelled()) { + return; + } ASLayoutTransition *pendingLayoutTransition; _ASTransitionContext *pendingLayoutTransitionContext; { @@ -599,10 +577,6 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize // right after it passed the validation test and before it proceeds ASDN::MutexLocker l(__instanceLock__); - if ([self _locked_shouldAbortTransitionWithID:transitionID]) { - return; - } - // Update calculated layout auto previousLayout = _calculatedDisplayNodeLayout; auto pendingLayout = std::make_shared(newLayout, @@ -654,14 +628,7 @@ - (void)transitionLayoutWithSizeRange:(ASSizeRange)constrainedSize - (void)cancelLayoutTransition { - __instanceLock__.lock(); - BOOL transitionInProgress = _transitionInProgress; - __instanceLock__.unlock(); - - if (transitionInProgress) { - // Cancel transition in progress - [self _finishOrCancelTransition]; - + if ([self _finishOrCancelTransition]) { // Tell subnodes to exit layout pending state and clear related properties ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) { node.hierarchyState &= (~ASHierarchyStateLayoutPending); diff --git a/Source/ASDisplayNode.mm b/Source/ASDisplayNode.mm index f491939bf..58dac12d4 100644 --- a/Source/ASDisplayNode.mm +++ b/Source/ASDisplayNode.mm @@ -916,7 +916,7 @@ - (void)__layout // If a current layout transition is in progress there is no need to do a measurement and layout pass in here as // this is supposed to happen within the layout transition process - if (_transitionInProgress) { + if (_transitionID != ASLayoutElementContextInvalidTransitionID) { return; } @@ -1807,15 +1807,14 @@ - (void)_setSupernode:(ASDisplayNode *)newSupernode // This is especially important as with automatic subnode management, adding subnodes can happen while a transition // is in fly if (ASHierarchyStateIncludesLayoutPending(stateToEnterOrExit)) { - int32_t pendingTransitionId = newSupernode.pendingTransitionID; + int32_t pendingTransitionId = newSupernode->_pendingTransitionID; if (pendingTransitionId != ASLayoutElementContextInvalidTransitionID) { { - ASDN::MutexLocker l(__instanceLock__); _pendingTransitionID = pendingTransitionId; // Propagate down the new pending transition id ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) { - node.pendingTransitionID = pendingTransitionId; + node->_pendingTransitionID = pendingTransitionId; }); } } diff --git a/Source/Private/ASDisplayNode+FrameworkPrivate.h b/Source/Private/ASDisplayNode+FrameworkPrivate.h index 080557534..9bdfe0a46 100644 --- a/Source/Private/ASDisplayNode+FrameworkPrivate.h +++ b/Source/Private/ASDisplayNode+FrameworkPrivate.h @@ -229,7 +229,7 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyState(ASHierarchyStat @end -@interface ASDisplayNode (ASsLayoutInternal) +@interface ASDisplayNode (ASLayoutInternal) /** * @abstract Informs the root node that the intrinsic size of the receiver is no longer valid. @@ -260,11 +260,6 @@ __unused static NSString * _Nonnull NSStringFromASHierarchyState(ASHierarchyStat @interface ASDisplayNode (ASLayoutTransitionInternal) -/** - * Sentinel of the current layout transition - */ -@property (atomic, assign) int32_t pendingTransitionID; - /** * If one or multiple layout transitions are in flight this methods returns if the current layout transition that * happens in in this particular thread was invalidated through another thread is starting a transition for this node diff --git a/Source/Private/ASDisplayNodeInternal.h b/Source/Private/ASDisplayNodeInternal.h index 488f2513c..502d381d5 100644 --- a/Source/Private/ASDisplayNodeInternal.h +++ b/Source/Private/ASDisplayNodeInternal.h @@ -150,10 +150,9 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo ASLayoutSpecBlock _layoutSpecBlock; - int32_t _transitionID; - BOOL _transitionInProgress; + std::atomic _transitionID; - int32_t _pendingTransitionID; + std::atomic _pendingTransitionID; ASLayoutTransition *_pendingLayoutTransition; std::shared_ptr _calculatedDisplayNodeLayout; std::shared_ptr _pendingDisplayNodeLayout;