From 0feaa2a36856e6d170b81f65c94a3b1af812d30a Mon Sep 17 00:00:00 2001 From: Scott Goodson Date: Sun, 10 Jan 2016 02:33:34 -0800 Subject: [PATCH] Improvements to the efficiency of recursivelySetInterfaceState: and the beta range controller. --- AsyncDisplayKit.xcodeproj/project.pbxproj | 13 ++++ .../xcshareddata/WorkspaceSettings.xcsettings | 8 --- AsyncDisplayKit/ASDisplayNode.mm | 65 +++++++++++++++---- AsyncDisplayKit/ASDisplayNodeExtras.h | 17 +++++ .../Details/ASRangeControllerBeta.mm | 15 ----- AsyncDisplayKit/Details/_ASDisplayLayer.mm | 2 + AsyncDisplayKit/Layout/ASLayoutSpec.mm | 19 ++++-- .../Private/ASDisplayNode+UIViewBridge.mm | 9 ++- .../Private/ASDisplayNodeInternal.h | 2 +- 9 files changed, 103 insertions(+), 47 deletions(-) delete mode 100644 AsyncDisplayKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings diff --git a/AsyncDisplayKit.xcodeproj/project.pbxproj b/AsyncDisplayKit.xcodeproj/project.pbxproj index 4ce31a9543..8437e36896 100644 --- a/AsyncDisplayKit.xcodeproj/project.pbxproj +++ b/AsyncDisplayKit.xcodeproj/project.pbxproj @@ -495,6 +495,13 @@ remoteGlobalIDString = 058D09AB195D04C000B7D73C; remoteInfo = AsyncDisplayKit; }; + DEACA2B11C425DC400FA9DDF /* PBXContainerItemProxy */ = { + isa = PBXContainerItemProxy; + containerPortal = 058D09A4195D04C000B7D73C /* Project object */; + proxyType = 1; + remoteGlobalIDString = 058D09AB195D04C000B7D73C; + remoteInfo = AsyncDisplayKit; + }; /* End PBXContainerItemProxy section */ /* Begin PBXCopyFilesBuildPhase section */ @@ -1524,6 +1531,7 @@ buildRules = ( ); dependencies = ( + DEACA2B21C425DC400FA9DDF /* PBXTargetDependency */, ); name = AsyncDisplayKitTestHost; productName = AsyncDisplayKitTestHost; @@ -1939,6 +1947,11 @@ target = 058D09AB195D04C000B7D73C /* AsyncDisplayKit */; targetProxy = 058D09C2195D04C000B7D73C /* PBXContainerItemProxy */; }; + DEACA2B21C425DC400FA9DDF /* PBXTargetDependency */ = { + isa = PBXTargetDependency; + target = 058D09AB195D04C000B7D73C /* AsyncDisplayKit */; + targetProxy = DEACA2B11C425DC400FA9DDF /* PBXContainerItemProxy */; + }; /* End PBXTargetDependency section */ /* Begin PBXVariantGroup section */ diff --git a/AsyncDisplayKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings b/AsyncDisplayKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings deleted file mode 100644 index 08de0be8d3..0000000000 --- a/AsyncDisplayKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings +++ /dev/null @@ -1,8 +0,0 @@ - - - - - IDEWorkspaceSharedSettings_AutocreateContextsIfNeeded - - - diff --git a/AsyncDisplayKit/ASDisplayNode.mm b/AsyncDisplayKit/ASDisplayNode.mm index 97c2be98ed..1346f7e910 100644 --- a/AsyncDisplayKit/ASDisplayNode.mm +++ b/AsyncDisplayKit/ASDisplayNode.mm @@ -198,12 +198,14 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) return [_ASDisplayLayer class]; } -+ (void)scheduleNodeForDisplay:(ASDisplayNode *)node ++ (void)scheduleNodeForRecursiveDisplay:(ASDisplayNode *)node { ASDisplayNodeAssertMainThread(); + ASDisplayNodeAssert([ASDisplayNode shouldUseNewRenderingRange], @"+scheduleNodeForRecursiveDisplay: should never be called without the new rendering range enabled"); static NSMutableSet *nodesToDisplay = nil; static BOOL displayScheduled = NO; static ASDN::RecursiveMutex displaySchedulerLock; + { ASDN::MutexLocker l(displaySchedulerLock); if (!nodesToDisplay) { @@ -211,6 +213,7 @@ static ASDisplayNodeMethodOverrides GetASDisplayNodeMethodOverrides(Class c) } [nodesToDisplay addObject:node]; } + if (!displayScheduled) { displayScheduled = YES; // It's essenital that any layout pass that is scheduled during the current @@ -1557,13 +1560,11 @@ void recursivelyTriggerDisplayForLayer(CALayer *layer, BOOL shouldBlock) - (void)setShouldBypassEnsureDisplay:(BOOL)shouldBypassEnsureDisplay { - ASDN::MutexLocker l(_propertyLock); _flags.shouldBypassEnsureDisplay = shouldBypassEnsureDisplay; } - (BOOL)shouldBypassEnsureDisplay { - ASDN::MutexLocker l(_propertyLock); return _flags.shouldBypassEnsureDisplay; } @@ -1612,7 +1613,7 @@ static BOOL ShouldUseNewRenderingRange = NO; - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize { ASDisplayNodeAssertThreadAffinity(self); - return [ASLayoutSpec new]; + return nil; } - (ASLayout *)calculatedLayout @@ -1772,7 +1773,7 @@ static BOOL ShouldUseNewRenderingRange = NO; oldState = _interfaceState; _interfaceState = newState; } - + if ((newState & ASInterfaceStateMeasureLayout) != (oldState & ASInterfaceStateMeasureLayout)) { // Trigger asynchronous measurement if it is not already cached or being calculated. } @@ -1782,8 +1783,11 @@ static BOOL ShouldUseNewRenderingRange = NO; // Still, the interfaceState should be updated to the current state of the node; just don't act on the transition. // Entered or exited data loading state. - if ((newState & ASInterfaceStateFetchData) != (oldState & ASInterfaceStateFetchData)) { - if (newState & ASInterfaceStateFetchData) { + BOOL nowFetchData = ASInterfaceStateIncludesFetchData(newState); + BOOL wasFetchData = ASInterfaceStateIncludesFetchData(oldState); + + if (nowFetchData != wasFetchData) { + if (nowFetchData) { [self fetchData]; } else { if ([self supportsRangeManagedInterfaceState]) { @@ -1793,21 +1797,42 @@ static BOOL ShouldUseNewRenderingRange = NO; } // Entered or exited contents rendering state. - if ((newState & ASInterfaceStateDisplay) != (oldState & ASInterfaceStateDisplay)) { + BOOL nowDisplay = ASInterfaceStateIncludesDisplay(newState); + BOOL wasDisplay = ASInterfaceStateIncludesDisplay(oldState); + + if (nowDisplay != wasDisplay) { if ([self supportsRangeManagedInterfaceState]) { - if (newState & ASInterfaceStateDisplay) { + if (nowDisplay) { // Once the working window is eliminated (ASRangeHandlerRender), trigger display directly here. [self setDisplaySuspended:NO]; } else { [self setDisplaySuspended:YES]; [self clearContents]; } + } else { + // NOTE: This case isn't currently supported as setInterfaceState: isn't exposed externally, and all + // internal use cases are range-managed. When a node is visible, don't mess with display - CA will start it. + if ([ASDisplayNode shouldUseNewRenderingRange] && !ASInterfaceStateIncludesVisible(newState)) { + // Check __implementsDisplay purely for efficiency - it's faster even than calling -asyncLayer. + if ([self __implementsDisplay]) { + if (nowDisplay) { + [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; + } else { + [[self asyncLayer] cancelAsyncDisplay]; + [self clearContents]; + } + } + } } } - // Entered or exited data loading state. - if ((newState & ASInterfaceStateVisible) != (oldState & ASInterfaceStateVisible)) { - if (newState & ASInterfaceStateVisible) { + // Became visible or invisible. When range-managed, this represents literal visibility - at least one pixel + // is onscreen. If not range-managed, we can't guarantee more than the node being present in an onscreen window. + BOOL nowVisible = ASInterfaceStateIncludesVisible(newState); + BOOL wasVisible = ASInterfaceStateIncludesVisible(oldState); + + if (nowVisible != wasVisible) { + if (nowVisible) { [self visibilityDidChange:YES]; } else { [self visibilityDidChange:NO]; @@ -1843,11 +1868,23 @@ static BOOL ShouldUseNewRenderingRange = NO; - (void)recursivelySetInterfaceState:(ASInterfaceState)interfaceState { + ASInterfaceState oldState = self.interfaceState; + ASInterfaceState newState = interfaceState; ASDisplayNodePerformBlockOnEveryNode(nil, self, ^(ASDisplayNode *node) { node.interfaceState = interfaceState; }); - // FIXME: This should also be called in setInterfaceState: if it isn't being applied recursively. - [ASDisplayNode scheduleNodeForDisplay:self]; + + 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]; + } + } } - (ASHierarchyState)hierarchyState diff --git a/AsyncDisplayKit/ASDisplayNodeExtras.h b/AsyncDisplayKit/ASDisplayNodeExtras.h index e8e72c7391..0ab9957cc3 100644 --- a/AsyncDisplayKit/ASDisplayNodeExtras.h +++ b/AsyncDisplayKit/ASDisplayNodeExtras.h @@ -12,6 +12,23 @@ #import #import +// Because inline methods can't be extern'd and need to be part of the translation unit of code +// that compiles with them to actually inline, we both declare and define these in the header. +inline BOOL ASInterfaceStateIncludesVisible(ASInterfaceState interfaceState) +{ + return ((interfaceState & ASInterfaceStateVisible) == ASInterfaceStateVisible); +} + +inline BOOL ASInterfaceStateIncludesDisplay(ASInterfaceState interfaceState) +{ + return ((interfaceState & ASInterfaceStateDisplay) == ASInterfaceStateDisplay); +} + +inline BOOL ASInterfaceStateIncludesFetchData(ASInterfaceState interfaceState) +{ + return ((interfaceState & ASInterfaceStateFetchData) == ASInterfaceStateFetchData); +} + NS_ASSUME_NONNULL_BEGIN ASDISPLAYNODE_EXTERN_C_BEGIN diff --git a/AsyncDisplayKit/Details/ASRangeControllerBeta.mm b/AsyncDisplayKit/Details/ASRangeControllerBeta.mm index bc98031e67..84eec7872e 100644 --- a/AsyncDisplayKit/Details/ASRangeControllerBeta.mm +++ b/AsyncDisplayKit/Details/ASRangeControllerBeta.mm @@ -17,21 +17,6 @@ #import "ASInternalHelpers.h" #import "ASDisplayNode+FrameworkPrivate.h" -extern BOOL ASInterfaceStateIncludesVisible(ASInterfaceState interfaceState) -{ - return ((interfaceState & ASInterfaceStateVisible) == ASInterfaceStateVisible); -} - -extern BOOL ASInterfaceStateIncludesDisplay(ASInterfaceState interfaceState) -{ - return ((interfaceState & ASInterfaceStateDisplay) == ASInterfaceStateDisplay); -} - -extern BOOL ASInterfaceStateIncludesFetchData(ASInterfaceState interfaceState) -{ - return ((interfaceState & ASInterfaceStateFetchData) == ASInterfaceStateFetchData); -} - @interface ASRangeControllerBeta () { BOOL _rangeIsValid; diff --git a/AsyncDisplayKit/Details/_ASDisplayLayer.mm b/AsyncDisplayKit/Details/_ASDisplayLayer.mm index 3152e4d7ce..24ab6b0d0e 100644 --- a/AsyncDisplayKit/Details/_ASDisplayLayer.mm +++ b/AsyncDisplayKit/Details/_ASDisplayLayer.mm @@ -110,6 +110,8 @@ ASDisplayNodeAssertMainThread(); ASDN::MutexLocker l(_displaySuspendedLock); + // FIXME: Reconsider whether we should cancel a display in progress. + // We should definitely cancel a display that is scheduled, but unstarted display. [self cancelAsyncDisplay]; // Short circuit if display is suspended. When resumed, we will setNeedsDisplay at that time. diff --git a/AsyncDisplayKit/Layout/ASLayoutSpec.mm b/AsyncDisplayKit/Layout/ASLayoutSpec.mm index dfe40084dc..abb428ebd1 100644 --- a/AsyncDisplayKit/Layout/ASLayoutSpec.mm +++ b/AsyncDisplayKit/Layout/ASLayoutSpec.mm @@ -39,7 +39,6 @@ static NSString * const kDefaultChildrenKey = @"kDefaultChildrenKey"; if (!(self = [super init])) { return nil; } - _layoutChildren = [NSMutableDictionary dictionary]; _isMutable = YES; return self; } @@ -56,11 +55,6 @@ static NSString * const kDefaultChildrenKey = @"kDefaultChildrenKey"; return self; } -- (void)setChild:(id)child; -{ - [self setChild:child forIdentifier:kDefaultChildKey]; -} - - (id)layoutableToAddFromLayoutable:(id)child { if (self.isFinalLayoutable == NO) { @@ -88,6 +82,19 @@ static NSString * const kDefaultChildrenKey = @"kDefaultChildrenKey"; return child; } +- (NSMutableDictionary *)layoutChildren +{ + if (!_layoutChildren) { + _layoutChildren = [NSMutableDictionary dictionary]; + } + return _layoutChildren; +} + +- (void)setChild:(id)child; +{ + [self setChild:child forIdentifier:kDefaultChildKey]; +} + - (void)setChild:(id)child forIdentifier:(NSString *)identifier { ASDisplayNodeAssert(self.isMutable, @"Cannot set properties when layout spec is not mutable"); diff --git a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm index 00fdf6bdde..7dd7275090 100644 --- a/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm +++ b/AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm @@ -11,6 +11,7 @@ #import "ASInternalHelpers.h" #import "ASAssert.h" #import "ASDisplayNodeInternal.h" +#import "ASDisplayNodeExtras.h" #import "ASDisplayNode+Subclasses.h" #import "ASDisplayNode+FrameworkPrivate.h" #import "ASDisplayNode+Beta.h" @@ -250,9 +251,11 @@ _messageToViewOrLayer(setNeedsDisplay); if ([ASDisplayNode shouldUseNewRenderingRange]) { - BOOL shouldDisplay = ((_interfaceState & ASInterfaceStateDisplay) == ASInterfaceStateDisplay); - if (_layer && !_flags.synchronous && shouldDisplay) { - [ASDisplayNode scheduleNodeForDisplay:self]; + BOOL nowDisplay = ASInterfaceStateIncludesDisplay(_interfaceState); + // FIXME: This should not need to recursively display, so create a non-recursive variant. + // The semantics of setNeedsDisplay (as defined by CALayer behavior) are not recursive. + if (_layer && !_flags.synchronous && nowDisplay && [self __implementsDisplay]) { + [ASDisplayNode scheduleNodeForRecursiveDisplay:self]; } } } diff --git a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h index c1f752002d..42627d39c0 100644 --- a/AsyncDisplayKit/Private/ASDisplayNodeInternal.h +++ b/AsyncDisplayKit/Private/ASDisplayNodeInternal.h @@ -109,7 +109,7 @@ typedef NS_OPTIONS(NSUInteger, ASDisplayNodeMethodOverrides) #endif } -+ (void)scheduleNodeForDisplay:(ASDisplayNode *)node; ++ (void)scheduleNodeForRecursiveDisplay:(ASDisplayNode *)node; // The _ASDisplayLayer backing the node, if any. @property (nonatomic, readonly, retain) _ASDisplayLayer *asyncLayer;