From 01fed69b264a1e118c9cc8e05e907d39adb1036c Mon Sep 17 00:00:00 2001 From: Michael Schneider Date: Tue, 28 Jun 2016 16:41:57 -0700 Subject: [PATCH] Adds trampoline for inserting and deletion of nodes Currently measurement always needs to happen on the main thread if implicit hierarchy management is enabled as adding and removing from nodes needs to happen on the main thread. We now will trampoline to the main thread to do the insertion and deletion of nodes. This also resolves the issue that can occur if a node is already loaded deep in the layout hierarchy in the layout that the node is transforming to. Before insertion or deletion is happening we need to crawl the layout hierarchy to check that though. --- AsyncDisplayKit/ASDisplayNode.mm | 52 ++++++++---- AsyncDisplayKit/Layout/ASLayoutSpec.mm | 5 ++ AsyncDisplayKit/Layout/ASLayoutable.h | 8 ++ .../Private/ASDisplayNodeInternal.h | 1 + AsyncDisplayKit/Private/ASLayoutTransition.h | 37 +++++++- AsyncDisplayKit/Private/ASLayoutTransition.mm | 39 +++++++++ .../ASDisplayNodeImplicitHierarchyTests.m | 84 +++++++++++++++++++ 7 files changed, 206 insertions(+), 20 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index cf38ddd804..ce0eb1edb5 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -15,6 +15,7 @@ #import #import +#import #import "_ASAsyncTransaction.h" #import "_ASAsyncTransactionContainer+Private.h" @@ -626,7 +627,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) if (! [self shouldMeasureWithSizeRange:constrainedSize]) { return _layout; } - + [self cancelLayoutTransitionsInProgress]; ASLayout *previousLayout = _layout; @@ -637,13 +638,14 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) pendingLayout:newLayout previousLayout:previousLayout]; } else { - ASLayoutTransition *layoutContext; + ASLayoutTransition *layoutTransition = nil; if (self.usesImplicitHierarchyManagement) { - layoutContext = [[ASLayoutTransition alloc] initWithNode:self - pendingLayout:newLayout - previousLayout:previousLayout]; + layoutTransition = [[ASLayoutTransition alloc] initWithNode:self + pendingLayout:newLayout + previousLayout:previousLayout]; } - [self applyLayout:newLayout layoutContext:layoutContext]; + + [self _applyLayout:newLayout layoutTransition:layoutTransition]; [self _completeLayoutCalculation]; } @@ -680,6 +682,11 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) return ASLayoutableTypeDisplayNode; } +- (BOOL)canLayoutAsynchronous +{ + return !self.isNodeLoaded; +} + #pragma mark - Layout Transition - (void)transitionLayoutWithAnimation:(BOOL)animated @@ -750,10 +757,10 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) } ASLayout *previousLayout = _layout; - [self applyLayout:newLayout layoutContext:nil]; + [self _applyLayout:newLayout layoutTransition:nil]; ASDisplayNodePerformBlockOnEverySubnode(self, ^(ASDisplayNode * _Nonnull node) { - [node applyPendingLayoutContext]; + [node _applyPendingLayoutContext]; [node _completeLayoutCalculation]; node.hierarchyState &= (~ASHierarchyStateLayoutPending); }); @@ -776,6 +783,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) }); }; + // TODO ihm: Can we always push the measure to the background thread and remove the parameter from the API? if (shouldMeasureAsync) { ASPerformBlockOnBackgroundThread(transitionBlock); } else { @@ -838,8 +846,8 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) - (BOOL)_hasTransitionInProgress { - ASDN::MutexLocker l(_propertyLock); - return _transitionInProgress; + ASDN::MutexLocker l(_propertyLock); + return _transitionInProgress; } /// Starts a new transition and returns the transition id @@ -2414,16 +2422,16 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) }); } -- (void)applyPendingLayoutContext +- (void)_applyPendingLayoutContext { ASDN::MutexLocker l(_propertyLock); if (_pendingLayoutTransition) { - [self applyLayout:_pendingLayoutTransition.pendingLayout layoutContext:_pendingLayoutTransition]; + [self _applyLayout:_pendingLayoutTransition.pendingLayout layoutTransition:_pendingLayoutTransition]; _pendingLayoutTransition = nil; } } -- (void)applyLayout:(ASLayout *)layout layoutContext:(ASLayoutTransition *)layoutContext +- (void)_applyLayout:(ASLayout *)layout layoutTransition:(ASLayoutTransition *)layoutTransition { ASDN::MutexLocker l(_propertyLock); _layout = layout; @@ -2432,10 +2440,22 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) ASDisplayNodeAssertTrue(layout.size.width >= 0.0); ASDisplayNodeAssertTrue(layout.size.height >= 0.0); - if (self.usesImplicitHierarchyManagement && layoutContext != nil) { - [layoutContext applySubnodeInsertions]; - [layoutContext applySubnodeRemovals]; + if (layoutTransition == nil || self.usesImplicitHierarchyManagement == NO) { + return; } + + // Trampoline to the main thread if necessary + if (ASDisplayNodeThreadIsMain() == NO && layoutTransition.canTransitionAsynchronous == NO) { + + // Subnode insertions and removals need to happen always on the main thread if at least one subnode is already loaded + ASPerformBlockOnMainThread(^{ + [layoutTransition applySubnodeTransition]; + }); + + return; + } + + [layoutTransition applySubnodeTransition]; } - (void)layout diff --git a/AsyncDisplayKit/Layout/ASLayoutSpec.mm b/AsyncDisplayKit/Layout/ASLayoutSpec.mm index b0bd8ee8ed..eed668782e 100644 --- a/AsyncDisplayKit/Layout/ASLayoutSpec.mm +++ b/AsyncDisplayKit/Layout/ASLayoutSpec.mm @@ -54,6 +54,11 @@ typedef std::map, std::less> ASCh return ASLayoutableTypeLayoutSpec; } +- (BOOL)canLayoutAsynchronous +{ + return YES; +} + #pragma mark - Layout - (ASLayout *)measureWithSizeRange:(ASSizeRange)constrainedSize diff --git a/AsyncDisplayKit/Layout/ASLayoutable.h b/AsyncDisplayKit/Layout/ASLayoutable.h index 1199c89fdf..0f68d5e2dd 100644 --- a/AsyncDisplayKit/Layout/ASLayoutable.h +++ b/AsyncDisplayKit/Layout/ASLayoutable.h @@ -46,8 +46,16 @@ NS_ASSUME_NONNULL_BEGIN */ @protocol ASLayoutable +/** + * @abstract Returns type of layoutable + */ @property (nonatomic, readonly) ASLayoutableType layoutableType; +/** + * @abstract Returns if the layoutable can be used to layout in an asynchronous way on a background thread. + */ +@property (nonatomic, readonly) BOOL canLayoutAsynchronous; + /** * @abstract Calculate a layout based on given size range. * diff --git a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h index ee3da6b041..db6d818f7a 100644 --- a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h +++ b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h @@ -118,6 +118,7 @@ FOUNDATION_EXPORT NSString * const ASRenderingEngineDidDisplayNodesScheduledBefo ASEnvironmentState _environmentState; ASLayout *_layout; + UIEdgeInsets _hitTestSlop; NSMutableArray *_subnodes; diff --git a/AsyncDisplayKit/Private/ASLayoutTransition.h b/AsyncDisplayKit/Private/ASLayoutTransition.h index c972f41aef..5c38a3d5cd 100644 --- a/AsyncDisplayKit/Private/ASLayoutTransition.h +++ b/AsyncDisplayKit/Private/ASLayoutTransition.h @@ -18,16 +18,45 @@ @interface ASLayoutTransition : NSObject <_ASTransitionContextLayoutDelegate> +/** + * Node to apply layout transition on + */ @property (nonatomic, readonly, weak) ASDisplayNode *node; -@property (nonatomic, readonly, strong) ASLayout *pendingLayout; + +/** + * Previous layout to transition from + */ @property (nonatomic, readonly, strong) ASLayout *previousLayout; -- (instancetype)initWithNode:(ASDisplayNode *)node - pendingLayout:(ASLayout *)pendingLayout - previousLayout:(ASLayout *)previousLayout; +/** + * Pending layout to transition to + */ +@property (nonatomic, readonly, strong) ASLayout *pendingLayout; +/** + * Returns if the layout transition can happen asynchronously + */ +@property (nonatomic, readonly, assign) BOOL canTransitionAsynchronous; + +/** + * Returns a newly initialized layout transition + */ +- (instancetype)initWithNode:(ASDisplayNode *)node pendingLayout:(ASLayout *)pendingLayout previousLayout:(ASLayout *)previousLayout NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_UNAVAILABLE; + +/** + * Insert and remove subnodes that where added or removed between the previousLayout and the pendingLayout + */ +- (void)applySubnodeTransition; + +/** + * Insert all new subnodes that where added between the previous layout and the pending layout + */ - (void)applySubnodeInsertions; +/** + * Remove all subnodes that are removed between the previous layout and the pending layout + */ - (void)applySubnodeRemovals; @end diff --git a/AsyncDisplayKit/Private/ASLayoutTransition.mm b/AsyncDisplayKit/Private/ASLayoutTransition.mm index 560ac6135e..b86fe572f8 100644 --- a/AsyncDisplayKit/Private/ASLayoutTransition.mm +++ b/AsyncDisplayKit/Private/ASLayoutTransition.mm @@ -18,10 +18,37 @@ #import "ASLayout.h" #import +#import #import "NSArray+Diffing.h" #import "ASEqualityHelpers.h" +/** + * Search the whole layout stack if at least one layout has a layoutable object that can not be layed out asynchronous. + * This can be the case for example if a node was already loaded + */ +static BOOL ASLayoutCanTransitionAsynchronous(ASLayout *layout) { + // Queue used to keep track of sublayouts while traversing this layout in a BFS fashion. + std::queue queue; + queue.push(layout); + + while (!queue.empty()) { + layout = queue.front(); + queue.pop(); + + if (layout.layoutableObject.canLayoutAsynchronous == NO) { + return NO; + } + + // Add all sublayouts to process in next step + for (int i = 0; i < layout.sublayouts.count; i++) { + queue.push(layout.sublayouts[0]); + } + } + + return YES; +} + @implementation ASLayoutTransition { ASDN::RecursiveMutex _propertyLock; BOOL _calculatedSubnodeOperations; @@ -44,6 +71,18 @@ return self; } +- (BOOL)canTransitionAsynchronous +{ + ASDN::MutexLocker l(_propertyLock); + return ASLayoutCanTransitionAsynchronous(_pendingLayout); +} + +- (void)applySubnodeTransition +{ + [self applySubnodeInsertions]; + [self applySubnodeRemovals]; +} + - (void)applySubnodeInsertions { ASDN::MutexLocker l(_propertyLock); diff --git a/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m b/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m index a701fa050b..627960d12d 100644 --- a/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m +++ b/AsyncDisplayKitTests/ASDisplayNodeImplicitHierarchyTests.m @@ -130,4 +130,88 @@ XCTAssertEqual(node.subnodes[2], node2); } +- (void)testMeasurementInBackgroundThreadWithLoadedNode +{ + ASDisplayNode *node1 = [[ASDisplayNode alloc] init]; + ASDisplayNode *node2 = [[ASDisplayNode alloc] init]; + + ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; + node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize) { + ASSpecTestDisplayNode *strongNode = (ASSpecTestDisplayNode *)weakNode; + if ([strongNode.layoutState isEqualToNumber:@1]) { + return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[node1]]; + } else { + return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[node2]]; + } + }; + + // Intentionally trigger view creation + [node2 view]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Fix IHM layout also if one node is already loaded"]; + + dispatch_async(dispatch_get_global_queue( DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{ + + [node measureWithSizeRange:ASSizeRangeMake(CGSizeZero, CGSizeZero)]; + XCTAssertEqual(node.subnodes[0], node1); + + node.layoutState = @2; + [node invalidateCalculatedLayout]; + [node measureWithSizeRange:ASSizeRangeMake(CGSizeZero, CGSizeZero)]; + + // Dispatch back to the main thread to let the insertion / deletion of subnodes happening + dispatch_async(dispatch_get_main_queue(), ^{ + XCTAssertEqual(node.subnodes[0], node2); + [expectation fulfill]; + }); + }); + + [self waitForExpectationsWithTimeout:5.0 handler:^(NSError *error) { + if (error) { + NSLog(@"Timeout Error: %@", error); + } + }]; +} + +- (void)testTransitionLayoutWithAnimationWithLoadedNodes +{ + ASDisplayNode *node1 = [[ASDisplayNode alloc] init]; + ASDisplayNode *node2 = [[ASDisplayNode alloc] init]; + + ASSpecTestDisplayNode *node = [[ASSpecTestDisplayNode alloc] init]; + + node.layoutSpecBlock = ^(ASDisplayNode *weakNode, ASSizeRange constrainedSize) { + ASSpecTestDisplayNode *strongNode = (ASSpecTestDisplayNode *)weakNode; + if ([strongNode.layoutState isEqualToNumber:@1]) { + return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[node1]]; + } else { + return [ASStaticLayoutSpec staticLayoutSpecWithChildren:@[node2]]; + } + }; + + // Intentionally trigger view creation + [node2 view]; + + XCTestExpectation *expectation = [self expectationWithDescription:@"Fix IHM layout transition also if one node is already loaded"]; + + [node measureWithSizeRange:ASSizeRangeMake(CGSizeZero, CGSizeZero)]; + XCTAssertEqual(node.subnodes[0], node1); + + node.layoutState = @2; + [node invalidateCalculatedLayout]; + [node transitionLayoutWithAnimation:YES shouldMeasureAsync:YES measurementCompletion:^{ + // Push this to the next runloop to let async insertion / removing of nodes finished before checking + dispatch_async(dispatch_get_main_queue(), ^{ + XCTAssertEqual(node.subnodes[0], node2); + [expectation fulfill]; + }); + }]; + + [self waitForExpectationsWithTimeout:5.0 handler:^(NSError *error) { + if (error) { + NSLog(@"Timeout Error: %@", error); + } + }]; +} + @end