From 312de1a0849ce560d7ae885f878af4e9eb37aa70 Mon Sep 17 00:00:00 2001 From: Huy Nguyen Date: Thu, 7 Jul 2016 17:41:58 +0700 Subject: [PATCH] Make sure range controller listens to node display notifications if absolutely needed --- AsyncDisplayKit/ASDisplayNode.mm | 32 +++++++++---------- AsyncDisplayKit/Details/ASRangeController.mm | 29 +++++++++-------- .../Private/ASDisplayNode+FrameworkPrivate.h | 5 +++ 3 files changed, 36 insertions(+), 30 deletions(-) diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 32b1c80b6a..d4ff7e6499 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -245,9 +245,9 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) dispatch_once(&onceToken, ^{ renderQueue = [[ASRunLoopQueue alloc] initWithRunLoop:CFRunLoopGetMain() andHandler:^(ASDisplayNode * _Nonnull dequeuedItem, BOOL isQueueDrained) { - CFAbsoluteTime timestamp = isQueueDrained ? CFAbsoluteTimeGetCurrent() : 0; [dequeuedItem _recursivelyTriggerDisplayAndBlock:NO]; if (isQueueDrained) { + CFAbsoluteTime timestamp = CFAbsoluteTimeGetCurrent(); [[NSNotificationCenter defaultCenter] postNotificationName:ASRenderingEngineDidDisplayScheduledNodesNotification object:nil userInfo:@{ASRenderingEngineDidDisplayNodesScheduledBeforeTimestamp: @(timestamp)}]; @@ -2317,27 +2317,27 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) }); } -- (void)recursivelySetInterfaceState:(ASInterfaceState)interfaceState +- (void)recursivelySetInterfaceState:(ASInterfaceState)newInterfaceState { - ASInterfaceState oldState = self.interfaceState; - ASInterfaceState newState = interfaceState; + // Instead of each node in the recursion assuming it needs to schedule itself for display, + // setInterfaceState: skips this when handling range-managed nodes (our whole subtree has this set). + // If our range manager intends for us to be displayed right now, and didn't before, get started! + BOOL shouldScheduleDisplay = [self supportsRangeManagedInterfaceState] && [self shouldScheduleDisplayWithNewInterfaceState:newInterfaceState]; ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) { - node.interfaceState = interfaceState; + node.interfaceState = newInterfaceState; }); - - if ([self supportsRangeManagedInterfaceState]) { - // Instead of each node in the recursion assuming it needs to schedule itself for display, - // setInterfaceState: skips this when handling range-managed nodes (our whole subtree has this set). - // If our range manager intends for us to be displayed right now, and didn't before, get started! - - BOOL nowDisplay = ASInterfaceStateIncludesDisplay(newState); - BOOL wasDisplay = ASInterfaceStateIncludesDisplay(oldState); - if (nowDisplay && (nowDisplay != wasDisplay)) { - [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; - } + if (shouldScheduleDisplay) { + [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; } } +- (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfaceState +{ + BOOL willDisplay = ASInterfaceStateIncludesDisplay(newInterfaceState); + BOOL nowDisplay = ASInterfaceStateIncludesDisplay(self.interfaceState); + return willDisplay && (willDisplay != nowDisplay); +} + - (ASHierarchyState)hierarchyState { ASDN::MutexLocker l(_propertyLock); diff --git a/AsyncDisplayKit/Details/ASRangeController.mm b/AsyncDisplayKit/Details/ASRangeController.mm index 6c84935540..f1d354af10 100644 --- a/AsyncDisplayKit/Details/ASRangeController.mm +++ b/AsyncDisplayKit/Details/ASRangeController.mm @@ -27,7 +27,7 @@ NSSet *_allPreviousIndexPaths; ASLayoutRangeMode _currentRangeMode; BOOL _didUpdateCurrentRange; - BOOL _didRegisterForNotifications; + BOOL _didRegisterForNodeDisplayNotifications; CFAbsoluteTime _pendingDisplayNodesTimestamp; } @@ -56,7 +56,7 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; - (void)dealloc { - if (_didRegisterForNotifications) { + if (_didRegisterForNodeDisplayNotifications) { [[NSNotificationCenter defaultCenter] removeObserver:self name:ASRenderingEngineDidDisplayScheduledNodesNotification object:nil]; } } @@ -242,10 +242,6 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; [allIndexPaths addObjectsFromArray:ASIndexPathsForTwoDimensionalArray(allNodes)]; } - // TODO Don't register for notifications if this range update doesn't cause any node to enter rendering pipeline. - // This can be done once there is an API to observe to (or be notified upon) interface state changes or pipeline enterings - [self registerForNotificationsForInterfaceStateIfNeeded:selfInterfaceState]; - #if ASRangeControllerLoggingEnabled ASDisplayNodeAssertTrue([visibleIndexPaths isSubsetOfSet:displayIndexPaths]); NSMutableArray *modifiedIndexPaths = (ASRangeControllerLoggingEnabled ? [NSMutableArray array] : nil); @@ -309,16 +305,21 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; #if ASRangeControllerLoggingEnabled [modifiedIndexPaths addObject:indexPath]; #endif + + BOOL nodeShouldScheduleDisplay = [node shouldScheduleDisplayWithNewInterfaceState:interfaceState]; [node recursivelySetInterfaceState:interfaceState]; + + if (nodeShouldScheduleDisplay) { + [self registerForNodeDisplayNotificationsForInterfaceStateIfNeeded:selfInterfaceState]; + if (_didRegisterForNodeDisplayNotifications) { + _pendingDisplayNodesTimestamp = CFAbsoluteTimeGetCurrent(); + } + } } } } } - if (_didRegisterForNotifications) { - _pendingDisplayNodesTimestamp = CFAbsoluteTimeGetCurrent(); - } - _rangeIsValid = YES; _queuedRangeUpdate = NO; @@ -338,9 +339,9 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; #pragma mark - Notification observers -- (void)registerForNotificationsForInterfaceStateIfNeeded:(ASInterfaceState)interfaceState +- (void)registerForNodeDisplayNotificationsForInterfaceStateIfNeeded:(ASInterfaceState)interfaceState { - if (!_didRegisterForNotifications) { + if (!_didRegisterForNodeDisplayNotifications) { ASLayoutRangeMode nextRangeMode = [ASRangeController rangeModeForInterfaceState:interfaceState currentRangeMode:_currentRangeMode]; if (_currentRangeMode != nextRangeMode) { @@ -348,7 +349,7 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; selector:@selector(scheduledNodesDidDisplay:) name:ASRenderingEngineDidDisplayScheduledNodesNotification object:nil]; - _didRegisterForNotifications = YES; + _didRegisterForNodeDisplayNotifications = YES; } } } @@ -359,7 +360,7 @@ static UIApplicationState __ApplicationState = UIApplicationStateActive; if (_pendingDisplayNodesTimestamp < notificationTimestamp) { // The rendering engine has processed all the nodes this range controller scheduled. Let's schedule a range update [[NSNotificationCenter defaultCenter] removeObserver:self name:ASRenderingEngineDidDisplayScheduledNodesNotification object:nil]; - _didRegisterForNotifications = NO; + _didRegisterForNodeDisplayNotifications = NO; [self scheduleRangeUpdate]; } diff --git a/AsyncDisplayKit/Private/ASDisplayNode+FrameworkPrivate.h b/AsyncDisplayKit/Private/ASDisplayNode+FrameworkPrivate.h index 78ce6ce05e..30cf597f51 100644 --- a/AsyncDisplayKit/Private/ASDisplayNode+FrameworkPrivate.h +++ b/AsyncDisplayKit/Private/ASDisplayNode+FrameworkPrivate.h @@ -135,6 +135,11 @@ inline BOOL ASHierarchyStateIncludesRangeManaged(ASHierarchyState hierarchyState */ @property (nonatomic, assign) BOOL shouldBypassEnsureDisplay; +/** + * @abstract Checks whether a node should be scheduled for display, considering its current and new interface states. + */ +- (BOOL)shouldScheduleDisplayWithNewInterfaceState:(ASInterfaceState)newInterfaceState; + @end @interface UIView (ASDisplayNodeInternal)