Improvements to the efficiency of recursivelySetInterfaceState: and the beta range controller.

Author: Scott Goodson

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 */

AsyncDisplayKit.xcworkspace/xcshareddata/WorkspaceSettings.xcsettings

@@ -198,19 +198,22 @@ + (Class)layerClass
   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) {
       nodesToDisplay = [[NSMutableSet alloc] init];
     }
     [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)recursivelyEnsureDisplaySynchronously:(BOOL)synchronously
 
 - (void)setShouldBypassEnsureDisplay:(BOOL)shouldBypassEnsureDisplay
 {
-  ASDN::MutexLocker l(_propertyLock);
   _flags.shouldBypassEnsureDisplay = shouldBypassEnsureDisplay;
 }
 
 - (BOOL)shouldBypassEnsureDisplay
 {
-  ASDN::MutexLocker l(_propertyLock);
   return _flags.shouldBypassEnsureDisplay;
 }
 
@@ -1612,7 +1613,7 @@ - (CGSize)calculateSizeThatFits:(CGSize)constrainedSize
 - (ASLayoutSpec *)layoutSpecThatFits:(ASSizeRange)constrainedSize
 {
   ASDisplayNodeAssertThreadAffinity(self);
-  return [ASLayoutSpec new];
+  return nil;
 }
 
 - (ASLayout *)calculatedLayout
@@ -1772,7 +1773,7 @@ - (void)setInterfaceState:(ASInterfaceState)newState
     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 @@ - (void)setInterfaceState:(ASInterfaceState)newState
   // 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 @@ - (void)setInterfaceState:(ASInterfaceState)newState
   }
 
   // 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 @@ - (void)exitInterfaceState:(ASInterfaceState)interfaceState
 
 - (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

AsyncDisplayKit/ASDisplayNode.mm

@@ -12,6 +12,23 @@
 #import <AsyncDisplayKit/ASBaseDefines.h>
 #import <AsyncDisplayKit/ASDisplayNode.h>
 
+// 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

AsyncDisplayKit/ASDisplayNodeExtras.h

@@ -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;

AsyncDisplayKit/Details/ASRangeControllerBeta.mm

@@ -110,6 +110,8 @@ - (void)setNeedsDisplay
   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.

AsyncDisplayKit/Details/_ASDisplayLayer.mm

@@ -39,7 +39,6 @@ - (instancetype)init
   if (!(self = [super init])) {
     return nil;
   }
-  _layoutChildren = [NSMutableDictionary dictionary];
   _isMutable = YES;
   return self;
 }
@@ -56,11 +55,6 @@ - (ASLayout *)measureWithSizeRange:(ASSizeRange)constrainedSize
   return self;
 }
 
-- (void)setChild:(id<ASLayoutable>)child;
-{
-  [self setChild:child forIdentifier:kDefaultChildKey];
-}
-
 - (id<ASLayoutable>)layoutableToAddFromLayoutable:(id<ASLayoutable>)child
 {
   if (self.isFinalLayoutable == NO) {
@@ -88,6 +82,19 @@ - (void)setChild:(id<ASLayoutable>)child;
   return child;
 }
 
+- (NSMutableDictionary *)layoutChildren
+{
+  if (!_layoutChildren) {
+    _layoutChildren = [NSMutableDictionary dictionary];
+  }
+  return _layoutChildren;
+}
+
+- (void)setChild:(id<ASLayoutable>)child;
+{
+  [self setChild:child forIdentifier:kDefaultChildKey];
+}
+
 - (void)setChild:(id<ASLayoutable>)child forIdentifier:(NSString *)identifier
 {
   ASDisplayNodeAssert(self.isMutable, @"Cannot set properties when layout spec is not mutable");

AsyncDisplayKit/Layout/ASLayoutSpec.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 @@ - (void)setNeedsDisplay
     _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];
       }
     }
   }

AsyncDisplayKit/Private/ASDisplayNode+UIViewBridge.mm

@@ -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;