Improve Handling of Rasterized Node Interface & Hierarchy States (#2731)

* Improve handling of rasterize node interface states and testing

* Fix harder

* Finish fixes and move rasterization flag into beta header

* Re-enable rasterization in ASDKgram

* Re-enable working test

* Only do it in debug

Author: Adlai Holler

AsyncDisplayKit/ASDisplayNode+Beta.h

@@ -123,6 +123,28 @@ typedef struct {
  */
 + (void)setRangeModeForMemoryWarnings:(ASLayoutRangeMode)rangeMode;
 
+/**
+ * @abstract Whether to draw all descendant nodes' layers/views into this node's layer/view's backing store.
+ *
+ * @discussion
+ * When set to YES, causes all descendant nodes' layers/views to be drawn directly into this node's layer/view's backing
+ * store.  Defaults to NO.
+ *
+ * If a node's descendants are static (never animated or never change attributes after creation) then that node is a
+ * good candidate for rasterization.  Rasterizing descendants has two main benefits:
+ * 1) Backing stores for descendant layers are not created.  Instead the layers are drawn directly into the rasterized
+ * container.  This can save a great deal of memory.
+ * 2) Since the entire subtree is drawn into one backing store, compositing and blending are eliminated in that subtree
+ * which can help improve animation/scrolling/etc performance.
+ *
+ * Rasterization does not currently support descendants with transform, sublayerTransform, or alpha. Those properties
+ * will be ignored when rasterizing descendants.
+ *
+ * Note: this has nothing to do with -[CALayer shouldRasterize], which doesn't work with ASDisplayNode's asynchronous
+ * rendering model.
+ */
+@property (nonatomic, assign) BOOL shouldRasterizeDescendants;
+
 @end
 
 NS_ASSUME_NONNULL_END

AsyncDisplayKit/ASDisplayNode.h

@@ -416,28 +416,6 @@ extern NSInteger const ASDefaultDrawingPriority;
  */
 @property (nonatomic, assign) BOOL displaysAsynchronously;
 
-/** 
- * @abstract Whether to draw all descendant nodes' layers/views into this node's layer/view's backing store.
- *
- * @discussion
- * When set to YES, causes all descendant nodes' layers/views to be drawn directly into this node's layer/view's backing 
- * store.  Defaults to NO.
- *
- * If a node's descendants are static (never animated or never change attributes after creation) then that node is a 
- * good candidate for rasterization.  Rasterizing descendants has two main benefits:
- * 1) Backing stores for descendant layers are not created.  Instead the layers are drawn directly into the rasterized
- * container.  This can save a great deal of memory.
- * 2) Since the entire subtree is drawn into one backing store, compositing and blending are eliminated in that subtree
- * which can help improve animation/scrolling/etc performance.
- *
- * Rasterization does not currently support descendants with transform, sublayerTransform, or alpha. Those properties 
- * will be ignored when rasterizing descendants.
- *
- * Note: this has nothing to do with -[CALayer shouldRasterize], which doesn't work with ASDisplayNode's asynchronous 
- * rendering model.
- */
-@property (nonatomic, assign) BOOL shouldRasterizeDescendants;
-
 /** 
  * @abstract Prevent the node's layer from displaying.
  *

AsyncDisplayKit/ASDisplayNode.mm

@@ -1422,13 +1422,15 @@ - (BOOL)shouldRasterizeDescendants
 - (void)setShouldRasterizeDescendants:(BOOL)shouldRasterize
 {
   ASDisplayNodeAssertThreadAffinity(self);
+  BOOL rasterizedFromSelfOrAncestor = NO;
   {
     ASDN::MutexLocker l(__instanceLock__);
 
     if (_flags.shouldRasterizeDescendants == shouldRasterize)
       return;
 
     _flags.shouldRasterizeDescendants = shouldRasterize;
+    rasterizedFromSelfOrAncestor = shouldRasterize || ASHierarchyStateIncludesRasterized(_hierarchyState);
   }
 
   if (self.isNodeLoaded) {
@@ -1438,18 +1440,18 @@ - (void)setShouldRasterizeDescendants:(BOOL)shouldRasterize
     [self recursivelyClearContents];
 
     ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
-      if (shouldRasterize) {
+      if (rasterizedFromSelfOrAncestor) {
         [node enterHierarchyState:ASHierarchyStateRasterized];
-        [node __unloadNode];
+        if (node.isNodeLoaded) {
+          [node __unloadNode];
+        }
       } else {
         [node exitHierarchyState:ASHierarchyStateRasterized];
-        [node __loadNode];
+        // We can avoid eagerly loading this node. We will load it on-demand as usual.
       }
     });
-    if (!shouldRasterize) {
-      // At this point all of our subnodes have their layers or views recreated, but we haven't added
-      // them to ours yet.  This is because our node is already loaded, and the above recursion
-      // is only performed on our subnodes -- not self.
+    if (!rasterizedFromSelfOrAncestor) {
+      // If we are not going to rasterize at all, go ahead and set up our view hierarchy.
       [self _addSubnodeViewsAndLayers];
     }
 
@@ -1460,7 +1462,7 @@ - (void)setShouldRasterizeDescendants:(BOOL)shouldRasterize
     }
   } else {
     ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode *node) {
-      if (shouldRasterize) {
+      if (rasterizedFromSelfOrAncestor) {
         [node enterHierarchyState:ASHierarchyStateRasterized];
       } else {
         [node exitHierarchyState:ASHierarchyStateRasterized];
@@ -1887,6 +1889,7 @@ ASDISPLAYNODE_INLINE BOOL nodeIsInRasterizedTree(ASDisplayNode *node) {
 /*
  * Central private helper method that should eventually be called if submethods add, insert or replace subnodes
  * You must hold __instanceLock__ to call this.
+ * This method is called with thread affinity.
  *
  * @param subnode       The subnode to insert
  * @param subnodeIndex  The index in _subnodes to insert it
@@ -1930,15 +1933,22 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode atSubnodeIndex:(NSInteger)subnod
   // This call will apply our .hierarchyState to the new subnode.
   // If we are a managed hierarchy, as in ASCellNode trees, it will also apply our .interfaceState.
   [subnode __setSupernode:self];
-  
-  // Don't bother inserting the view/layer if in a rasterized subtree, because there are no layers in the hierarchy and
-  // none of this could possibly work.
-  if (nodeIsInRasterizedTree(self) == NO && self.nodeLoaded) {
-    // If node is loaded insert the subnode otherwise wait until the node get's loaded
-    ASPerformBlockOnMainThread(^{
-      [self _insertSubnodeSubviewOrSublayer:subnode atIndex:sublayerIndex];
+
+  // If this subnode will be rasterized, update its hierarchy state & enter hierarchy if needed
+  if (nodeIsInRasterizedTree(self)) {
+    ASDisplayNodePerformBlockOnEveryNodeBFS(subnode, ^(ASDisplayNode * _Nonnull node) {
+      [node enterHierarchyState:ASHierarchyStateRasterized];
+      if (node.isNodeLoaded) {
+        [node __unloadNode];
+      }
     });
-  }
+    if (_flags.isInHierarchy) {
+      [subnode __enterHierarchy];
+    }
+  } else if (self.nodeLoaded) {
+    // If not rasterizing, and node is loaded insert the subview/sublayer now.
+    [self _insertSubnodeSubviewOrSublayer:subnode atIndex:sublayerIndex];
+  } // Otherwise we will insert subview/sublayer when we get loaded
 
   ASDisplayNodeAssert(disableNotifications == shouldDisableNotificationsForMovingBetweenParents(oldParent, self), @"Invariant violated");
   if (disableNotifications) {
@@ -1968,10 +1978,7 @@ - (void)_insertSubnodeSubviewOrSublayer:(ASDisplayNode *)subnode atIndex:(NSInte
   if (canUseViewAPI(self, subnode)) {
     [_view insertSubview:subnode.view atIndex:idx];
   } else {
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wconversion"
-    [_layer insertSublayer:subnode.layer atIndex:idx];
-#pragma clang diagnostic pop
+    [_layer insertSublayer:subnode.layer atIndex:(unsigned int)idx];
   }
 }
 
@@ -2032,7 +2039,7 @@ - (void)_replaceSubnode:(ASDisplayNode *)oldSubnode withSubnode:(ASDisplayNode *
     return;
   }
 
-  if ([oldSubnode _deallocSafeSupernode] != self) {
+  if (oldSubnode.supernode != self) {
     ASDisplayNodeFailAssert(@"Old Subnode to replace must be a subnode");
     return;
   }
@@ -2076,7 +2083,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)be
     return;
   }
 
-  if ([below _deallocSafeSupernode] != self) {
+  if (below.supernode != self) {
     ASDisplayNodeFailAssert(@"Node to insert below must be a subnode");
     return;
   }
@@ -2101,7 +2108,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode belowSubnode:(ASDisplayNode *)be
 
     // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to
     // insert it will mess up our calculation
-    if ([subnode _deallocSafeSupernode] == self) {
+    if (subnode.supernode == self) {
       NSInteger currentIndexInSubnodes = [_subnodes indexOfObjectIdenticalTo:subnode];
       if (currentIndexInSubnodes < belowSubnodeIndex) {
         belowSubnodeIndex--;
@@ -2138,7 +2145,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode aboveSubnode:(ASDisplayNode *)ab
     return;
   }
 
-  if ([above _deallocSafeSupernode] != self) {
+  if (above.supernode != self) {
     ASDisplayNodeFailAssert(@"Node to insert above must be a subnode");
     return;
   }
@@ -2162,7 +2169,7 @@ - (void)_insertSubnode:(ASDisplayNode *)subnode aboveSubnode:(ASDisplayNode *)ab
 
     // If the subnode is already in the subnodes array / sublayers and it's before the below node, removing it to
     // insert it will mess up our calculation
-    if ([subnode _deallocSafeSupernode] == self) {
+    if (subnode.supernode == self) {
       NSInteger currentIndexInSubnodes = [_subnodes indexOfObjectIdenticalTo:subnode];
       if (currentIndexInSubnodes <= aboveSubnodeIndex) {
         aboveSubnodeIndex--;
@@ -2228,7 +2235,7 @@ - (void)_removeSubnode:(ASDisplayNode *)subnode
 
   // Don't call self.supernode here because that will retain/autorelease the supernode.  This method -_removeSupernode: is often called while tearing down a node hierarchy, and the supernode in question might be in the middle of its -dealloc.  The supernode is never messaged, only compared by value, so this is safe.
   // The particular issue that triggers this edge case is when a node calls -removeFromSupernode on a subnode from within its own -dealloc method.
-  if (!subnode || [subnode _deallocSafeSupernode] != self) {
+  if (!subnode || subnode.supernode != self) {
     return;
   }
 
@@ -2253,23 +2260,17 @@ - (void)_removeFromSupernode
     __weak ASDisplayNode *supernode = _supernode;
     __weak UIView *view = _view;
     __weak CALayer *layer = _layer;
-    BOOL layerBacked = _flags.layerBacked;
-    BOOL isNodeLoaded = (layer != nil || view != nil);
   __instanceLock__.unlock();
 
   // Clear supernode's reference to us before removing the view from the hierarchy, as _ASDisplayView
   // will trigger us to clear our _supernode pointer in willMoveToSuperview:nil.
   // This may result in removing the last strong reference, triggering deallocation after this method.
   [supernode _removeSubnode:self];
 
-  if (isNodeLoaded && (supernode == nil || supernode.isNodeLoaded)) {
-    ASPerformBlockOnMainThread(^{
-      if (layerBacked || supernode.layerBacked) {
-        [layer removeFromSuperlayer];
-      } else {
-        [view removeFromSuperview];
-      }
-    });
+  if (view != nil) {
+    [view removeFromSuperview];
+  } else if (layer != nil) {
+    [layer removeFromSuperlayer];
   }
 }
 
@@ -2328,12 +2329,10 @@ - (void)__enterHierarchy
   if (!_flags.isInHierarchy && !_flags.visibilityNotificationsDisabled && ![self __selfOrParentHasVisibilityNotificationsDisabled]) {
     _flags.isEnteringHierarchy = YES;
     _flags.isInHierarchy = YES;
-    
-    if (_flags.shouldRasterizeDescendants) {
-      // Nodes that are descendants of a rasterized container do not have views or layers, and so cannot receive visibility notifications directly via orderIn/orderOut CALayer actions.  Manually send visibility notifications to rasterized descendants.
-      [self _recursiveWillEnterHierarchy];
-    } else {
-      [self willEnterHierarchy];
+
+    [self willEnterHierarchy];
+    for (ASDisplayNode *subnode in self.subnodes) {
+      [subnode __enterHierarchy];
     }
     _flags.isEnteringHierarchy = NO;
 
@@ -2369,13 +2368,10 @@ - (void)__exitHierarchy
 
     [self.asyncLayer cancelAsyncDisplay];
 
-    if (_flags.shouldRasterizeDescendants) {
-      // Nodes that are descendants of a rasterized container do not have views or layers, and so cannot receive visibility notifications directly via orderIn/orderOut CALayer actions.  Manually send visibility notifications to rasterized descendants.
-      [self _recursiveDidExitHierarchy];
-    } else {
-      [self didExitHierarchy];
+    [self didExitHierarchy];
+    for (ASDisplayNode *subnode in self.subnodes) {
+      [subnode __exitHierarchy];
     }
-    
     _flags.isExitingHierarchy = NO;
   }
 }
@@ -2416,19 +2412,13 @@ - (NSArray *)subnodes
   return ([_subnodes copy] ?: @[]);
 }
 
+// NOTE: This method must be dealloc-safe (should not retain self).
 - (ASDisplayNode *)supernode
 {
   ASDN::MutexLocker l(__instanceLock__);
   return _supernode;
 }
 
-// This is a thread-method to return the supernode without causing it to be retained autoreleased.  See -_removeSubnode: for details.
-- (ASDisplayNode *)_deallocSafeSupernode
-{
-  ASDN::MutexLocker l(__instanceLock__);
-  return _supernode;
-}
-
 - (void)__setSupernode:(ASDisplayNode *)newSupernode
 {
   BOOL supernodeDidChange = NO;
@@ -2471,7 +2461,7 @@ - (void)__setSupernode:(ASDisplayNode *)newSupernode
 
             // Propagate down the new pending transition id
             ASDisplayNodePerformBlockOnEverySubnode(self, NO, ^(ASDisplayNode * _Nonnull node) {
-              node.pendingTransitionID = _pendingTransitionID;
+              node.pendingTransitionID = pendingTransitionId;
             });
           }
         }
@@ -2485,6 +2475,14 @@ - (void)__setSupernode:(ASDisplayNode *)newSupernode
       stateToEnterOrExit |= ASHierarchyStateLayoutPending;
 
       [self exitHierarchyState:stateToEnterOrExit];
+
+      // We only need to explicitly exit hierarchy here if we were rasterized.
+      // Otherwise we will exit the hierarchy when our view/layer does so
+      // which has some nice carry-over machinery to handle cases where we are removed from a hierarchy
+      // and then added into it again shortly after.
+      if (parentWasOrIsRasterized && _flags.isInHierarchy) {
+        [self __exitHierarchy];
+      }
     }
   }
 }
@@ -3559,12 +3557,6 @@ - (BOOL)isInHierarchy
   return _flags.isInHierarchy;
 }
 
-- (void)setInHierarchy:(BOOL)inHierarchy
-{
-  ASDN::MutexLocker l(__instanceLock__);
-  _flags.isInHierarchy = inHierarchy;
-}
-
 - (id<ASLayoutElement>)finalLayoutElement
 {
   return self;
@@ -3610,7 +3602,7 @@ - (ASEventLog *)eventLog
   }
 
   // Check supernode so that if we are cell node we don't find self.
-  ASCellNode *cellNode = ASDisplayNodeFindFirstSupernodeOfClass([self _deallocSafeSupernode], [ASCellNode class]);
+  ASCellNode *cellNode = ASDisplayNodeFindFirstSupernodeOfClass(self.supernode, [ASCellNode class]);
   if (cellNode != nil) {
     [result addObject:@{ @"cellNode" : ASObjectDescriptionMakeTiny(cellNode) }];
   }

AsyncDisplayKit/ASDisplayNodeExtras.h

@@ -14,6 +14,17 @@
 #import <AsyncDisplayKit/ASBaseDefines.h>
 #import <AsyncDisplayKit/ASDisplayNode.h>
 
+/**
+ * Sets the debugName field for these nodes to the given symbol names, within the domain of "self.class"
+ * For instance, in `MYButtonNode` if you call `ASSetDebugNames(self.titleNode, _countNode)` the debug names
+ * for the nodes will be set to `MYButtonNode.titleNode` and `MYButtonNode.countNode`.
+ */
+#if DEBUG
+  #define ASSetDebugNames(...) _ASSetDebugNames(self.class, @"" # __VA_ARGS__, __VA_ARGS__, nil)
+#else
+  #define ASSetDebugNames(...)
+#endif
+
 /// For deallocation of objects on the main thread across multiple run loops.
 extern void ASPerformMainThreadDeallocation(_Nullable id object);
 
@@ -163,6 +174,9 @@ extern UIColor *ASDisplayNodeDefaultTintColor() AS_WARN_UNUSED_RESULT;
 extern void ASDisplayNodeDisableHierarchyNotifications(ASDisplayNode *node);
 extern void ASDisplayNodeEnableHierarchyNotifications(ASDisplayNode *node);
 
+// Not to be called directly.
+extern void _ASSetDebugNames(Class _Nonnull owningClass, NSString * _Nonnull names, ASDisplayNode * _Nullable object, ...);
+
 ASDISPLAYNODE_EXTERN_C_END
 
 NS_ASSUME_NONNULL_END

AsyncDisplayKit/ASDisplayNodeExtras.mm

@@ -32,6 +32,24 @@ extern void ASPerformMainThreadDeallocation(_Nullable id object)
   }
 }
 
+extern void _ASSetDebugNames(Class _Nonnull owningClass, NSString * _Nonnull names, ASDisplayNode * _Nullable object, ...)
+{
+  NSString *owningClassName = NSStringFromClass(owningClass);
+  NSArray *nameArray = [names componentsSeparatedByString:@", "];
+  va_list args;
+  va_start(args, object);
+  NSInteger i = 0;
+  for (ASDisplayNode *node = object; node != nil; node = va_arg(args, id), i++) {
+    NSMutableString *symbolName = [nameArray[i] mutableCopy];
+    // Remove any `self.` or `_` prefix
+    [symbolName replaceOccurrencesOfString:@"self." withString:@"" options:NSAnchoredSearch range:NSMakeRange(0, symbolName.length)];
+    [symbolName replaceOccurrencesOfString:@"_" withString:@"" options:NSAnchoredSearch range:NSMakeRange(0, symbolName.length)];
+    node.debugName = [NSString stringWithFormat:@"%@.%@", owningClassName, symbolName];
+  }
+  ASDisplayNodeCAssert(nameArray.count == i, @"Malformed call to ASSetDebugNames: %@", names);
+  va_end(args);
+}
+
 extern ASInterfaceState ASInterfaceStateForDisplayNode(ASDisplayNode *displayNode, UIWindow *window)
 {
     ASDisplayNodeCAssert(![displayNode isLayerBacked], @"displayNode must not be layer backed as it may have a nil window");