[ASLayout] Revisit the flattening algorithm (#395)

* Implement tests for the layout flattening process

* Refactor the flattening algorithm
- Remove flattened flag
- No more self check
- Stop traversing a layout tree branch when hits a displaynode node.
- Reuse as many existing ASLayout objects as possible

* Update changelog

* Ceil position values before comparing

* Explain why sublayout elements must be retained

AsyncDisplayKit.xcodeproj/project.pbxproj

@@ -421,6 +421,7 @@
 		DEFAD8131CC48914000527C4 /* ASVideoNode.mm in Sources */ = {isa = PBXBuildFile; fileRef = AEEC47E01C20C2DD00EC1693 /* ASVideoNode.mm */; };
 		E516FC7F1E9FE24200714FF4 /* ASHashing.h in Headers */ = {isa = PBXBuildFile; fileRef = E516FC7D1E9FE24200714FF4 /* ASHashing.h */; };
 		E516FC801E9FE24200714FF4 /* ASHashing.m in Sources */ = {isa = PBXBuildFile; fileRef = E516FC7E1E9FE24200714FF4 /* ASHashing.m */; };
+		E51B78BF1F028ABF00E32604 /* ASLayoutFlatteningTests.m in Sources */ = {isa = PBXBuildFile; fileRef = E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */; };
 		E54E81FC1EB357BD00FFE8E1 /* ASPageTable.h in Headers */ = {isa = PBXBuildFile; fileRef = E54E81FA1EB357BD00FFE8E1 /* ASPageTable.h */; };
 		E54E81FD1EB357BD00FFE8E1 /* ASPageTable.m in Sources */ = {isa = PBXBuildFile; fileRef = E54E81FB1EB357BD00FFE8E1 /* ASPageTable.m */; };
 		E55D86331CA8A14000A0C26F /* ASLayoutElement.mm in Sources */ = {isa = PBXBuildFile; fileRef = E55D86311CA8A14000A0C26F /* ASLayoutElement.mm */; };
@@ -890,6 +891,7 @@
 		DECBD6E61BE56E1900CF4905 /* ASButtonNode.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASButtonNode.mm; sourceTree = "<group>"; };
 		E516FC7D1E9FE24200714FF4 /* ASHashing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASHashing.h; sourceTree = "<group>"; };
 		E516FC7E1E9FE24200714FF4 /* ASHashing.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASHashing.m; sourceTree = "<group>"; };
+		E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ASLayoutFlatteningTests.m; sourceTree = "<group>"; };
 		E52405B21C8FEF03004DC8E7 /* ASLayoutTransition.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ASLayoutTransition.mm; sourceTree = "<group>"; };
 		E52405B41C8FEF16004DC8E7 /* ASLayoutTransition.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASLayoutTransition.h; sourceTree = "<group>"; };
 		E54E81FA1EB357BD00FFE8E1 /* ASPageTable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ASPageTable.h; sourceTree = "<group>"; };
@@ -1173,6 +1175,7 @@
 				058D0A30195D057000B7D73C /* ASDisplayNodeTestsHelper.h */,
 				058D0A31195D057000B7D73C /* ASDisplayNodeTestsHelper.m */,
 				697B31591CFE4B410049936F /* ASEditableTextNodeTests.m */,
+				E51B78BD1F01A0EE00E32604 /* ASLayoutFlatteningTests.m */,
 				052EE0651A159FEF002C6279 /* ASMultiplexImageNodeTests.m */,
 				058D0A32195D057000B7D73C /* ASMutableAttributedStringBuilderTests.m */,
 				3C9C128419E616EF00E942A0 /* ASTableViewTests.mm */,
@@ -2044,6 +2047,7 @@
 			isa = PBXSourcesBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				E51B78BF1F028ABF00E32604 /* ASLayoutFlatteningTests.m in Sources */,
 				29CDC2E21AAE70D000833CA4 /* ASBasicImageDownloaderContextTests.m in Sources */,
 				CC051F1F1D7A286A006434CB /* ASCALayerTests.m in Sources */,
 				242995D31B29743C00090100 /* ASBasicImageDownloaderTests.m in Sources */,

CHANGELOG.md

@@ -9,6 +9,8 @@
 - [Yoga] Implement ASYogaLayoutSpec, a simplified integration strategy for Yoga-powered layout calculation. [Scott Goodson](https://github.com/appleguy)
 - Fixed an issue where calls to setNeedsDisplay and setNeedsLayout would stop working on loaded nodes. [Garrett Moon](https://github.com/garrettmoon)
 - Migrated unit tests to OCMock 3.4 (from 2.2) and improved the multiplex image node tests. [Adlai Holler](https://github.com/Adlai-Holler)
+- Fix CollectionNode double-load issue. This should significantly improve performance in cases where a collection node has content immediately available on first layout i.e. not fetched from the network. [Adlai Holler](https://github.com/Adlai-Holler)
+- Overhaul layout flattening algorithm [Huy Nguyen](https://github.com/nguyenhuy) [#395](https://github.com/TextureGroup/Texture/pull/395).
 
 ## 2.3.3
 - [ASTextKitFontSizeAdjuster] Replace use of NSAttributedString's boundingRectWithSize:options:context: with NSLayoutManager's boundingRectForGlyphRange:inTextContainer: [Ricky Cancro](https://github.com/rcancro)

Source/ASDisplayNode+Yoga.mm

@@ -36,10 +36,6 @@ @interface ASDisplayNode (YogaInternal)
 - (ASSizeRange)_locked_constrainedSizeForLayoutPass;
 @end
 
-@interface ASLayout (YogaInternal)
-@property (nonatomic, getter=isFlattened) BOOL flattened;
-@end
-
 @implementation ASDisplayNode (Yoga)
 
 - (void)setYogaChildren:(NSArray *)yogaChildren
@@ -174,9 +170,7 @@ - (void)setupYogaCalculatedLayout
 
   NSMutableArray *sublayouts = [NSMutableArray arrayWithCapacity:childCount];
   for (ASDisplayNode *subnode in self.yogaChildren) {
-    ASLayout *sublayout = [subnode layoutForYogaNode];
-    sublayout.flattened = YES;
-    [sublayouts addObject:sublayout];
+    [sublayouts addObject:[subnode layoutForYogaNode]];
   }
 
   // The layout for self should have position CGPointNull, but include the calculated size.

Source/Layout/ASLayout.h

@@ -134,14 +134,6 @@ ASDISPLAYNODE_EXTERN_C_END
  */
 + (instancetype)layoutWithLayoutElement:(id<ASLayoutElement>)layoutElement
                                    size:(CGSize)size AS_WARN_UNUSED_RESULT;
-/**
- * Convenience initializer that creates a layout based on the values of the given layout, with a new position
- *
- * @param layout           The layout to use to create the new layout
- * @param position         The position of the new layout
- */
-+ (instancetype)layoutWithLayout:(ASLayout *)layout position:(CGPoint)position AS_WARN_UNUSED_RESULT;
-
 /**
  * Traverses the existing layout tree and generates a new tree that represents only ASDisplayNode layouts
  */

Source/Layout/ASLayout.mm

@@ -37,7 +37,7 @@ extern BOOL ASPointIsNull(CGPoint point)
 /**
  * Creates an defined number of "    |" indent blocks for the recursive description.
  */
-static inline NSString * descriptionIndents(NSUInteger indents)
+ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT NSString * descriptionIndents(NSUInteger indents)
 {
   NSMutableString *description = [NSMutableString string];
   for (NSUInteger i = 0; i < indents; i++) {
@@ -49,14 +49,31 @@ extern BOOL ASPointIsNull(CGPoint point)
   return description;
 }
 
+ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT BOOL ASLayoutIsDisplayNodeType(ASLayout *layout)
+{
+  return layout.type == ASLayoutElementTypeDisplayNode;
+}
+
+ASDISPLAYNODE_INLINE AS_WARN_UNUSED_RESULT BOOL ASLayoutIsFlattened(ASLayout *layout)
+{
+  // A layout is flattened if its position is null, and all of its sublayouts are of type displaynode with no sublayouts.
+  if (! ASPointIsNull(layout.position)) {
+    return NO;
+  }
+
+  for (ASLayout *sublayout in layout.sublayouts) {
+    if (ASLayoutIsDisplayNodeType(sublayout) == NO || sublayout.sublayouts.count > 0) {
+      return NO;
+    }
+  }
+
+  return YES;
+}
+
 @interface ASLayout () <ASDescriptionProvider>
 {
   ASLayoutElementType _layoutElementType;
 }
-/**
- * A boolean describing if the current layout has been flattened.
- */
-@property (nonatomic, getter=isFlattened) BOOL flattened;
 
 /*
  * Caches all sublayouts if set to YES or destroys the sublayout cache if set to NO. Defaults to YES
@@ -117,7 +134,7 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
     _size = size;
 
     if (ASPointIsNull(position) == NO) {
-      _position = CGPointMake(ASCeilPixelValue(position.x), ASCeilPixelValue(position.y));
+      _position = ASCeilPointValues(position);
     } else {
       _position = position;
     }
@@ -129,7 +146,6 @@ - (instancetype)initWithLayoutElement:(id<ASLayoutElement>)layoutElement
       [_elementToRectTable setRect:layout.frame forKey:layout.layoutElement];
     }
 
-    _flattened = NO;
     self.retainSublayoutLayoutElements = [ASLayout shouldRetainSublayoutLayoutElements];
   }
 
@@ -173,14 +189,6 @@ + (instancetype)layoutWithLayoutElement:(id<ASLayoutElement>)layoutElement size:
                             sublayouts:nil];
 }
 
-+ (instancetype)layoutWithLayout:(ASLayout *)layout position:(CGPoint)position
-{
-  return [self layoutWithLayoutElement:layout.layoutElement
-                                  size:layout.size
-                              position:position
-                            sublayouts:layout.sublayouts];
-}
-
 #pragma mark - Sublayout Elements Caching
 
 - (void)setRetainSublayoutLayoutElements:(BOOL)retainSublayoutLayoutElements
@@ -207,7 +215,12 @@ - (void)setRetainSublayoutLayoutElements:(BOOL)retainSublayoutLayoutElements
 
 - (ASLayout *)filteredNodeLayoutTree
 {
-  NSMutableArray *flattenedSublayouts = [NSMutableArray array];
+  if (ASLayoutIsFlattened(self)) {
+    // All flattened layouts must have this flag enabled
+    // to ensure sublayout elements are retained until the layouts are applied.
+    self.retainSublayoutLayoutElements = YES;
+    return self;
+  }
 
   struct Context {
     ASLayout *layout;
@@ -216,28 +229,42 @@ - (ASLayout *)filteredNodeLayoutTree
 
   // Queue used to keep track of sublayouts while traversing this layout in a DFS fashion.
   std::deque<Context> queue;
-  queue.push_front({self, CGPointZero});
+  for (ASLayout *sublayout in self.sublayouts) {
+    queue.push_back({sublayout, sublayout.position});
+  }
+
+  NSMutableArray *flattenedSublayouts = [NSMutableArray array];
 
   while (!queue.empty()) {
-    Context context = queue.front();
+    const Context context = queue.front();
     queue.pop_front();
-
-    if (self != context.layout && context.layout.type == ASLayoutElementTypeDisplayNode) {
-      ASLayout *layout = [ASLayout layoutWithLayout:context.layout position:context.absolutePosition];
-      layout.flattened = YES;
-      [flattenedSublayouts addObject:layout];
-    }
 
-    std::vector<Context> sublayoutContexts;
-    for (ASLayout *sublayout in context.layout.sublayouts) {
-      if (sublayout.isFlattened == NO) {
+    ASLayout *layout = context.layout;
+    const NSArray<ASLayout *> *sublayouts = layout.sublayouts;
+    const NSUInteger sublayoutsCount = sublayouts.count;
+    const CGPoint absolutePosition = context.absolutePosition;
+
+    if (ASLayoutIsDisplayNodeType(layout)) {
+      if (sublayoutsCount > 0 || CGPointEqualToPoint(ASCeilPointValues(absolutePosition), layout.position) == NO) {
+        // Only create a new layout if the existing one can't be reused, which means it has either some sublayouts or an invalid absolute position.
+        layout = [ASLayout layoutWithLayoutElement:layout.layoutElement
+                                              size:layout.size
+                                          position:absolutePosition
+                                        sublayouts:@[]];
+      }
+      [flattenedSublayouts addObject:layout];
+    } else if (sublayoutsCount > 0){
+      std::vector<Context> sublayoutContexts;
+      for (ASLayout *sublayout in sublayouts) {
         sublayoutContexts.push_back({sublayout, context.absolutePosition + sublayout.position});
       }
+      queue.insert(queue.cbegin(), sublayoutContexts.begin(), sublayoutContexts.end());
     }
-    queue.insert(queue.cbegin(), sublayoutContexts.begin(), sublayoutContexts.end());
   }
 
-  ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size position:CGPointZero sublayouts:flattenedSublayouts];
+  ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size sublayouts:flattenedSublayouts];
+  // All flattened layouts must have this flag enabled
+  // to ensure sublayout elements are retained until the layouts are applied.
   layout.retainSublayoutLayoutElements = YES;
   return layout;
 }

Source/Layout/ASYogaLayoutSpec.mm

@@ -42,7 +42,6 @@ - (ASLayout *)layoutForYogaNode:(YGNodeRef)yogaNode
     return [ASLayout layoutWithLayoutElement:layoutElement size:size sublayouts:sublayouts];
   } else {
     CGPoint position = CGPointMake(YGNodeLayoutGetLeft(yogaNode), YGNodeLayoutGetTop(yogaNode));
-    // TODO: If it were possible to set .flattened = YES, it would be valid to do so here.
     return [ASLayout layoutWithLayoutElement:layoutElement size:size position:position sublayouts:nil];
   }
 }

Source/Private/ASInternalHelpers.h

@@ -46,6 +46,8 @@ CGSize ASFloorSizeValues(CGSize s);
 
 CGFloat ASFloorPixelValue(CGFloat f);
 
+CGPoint ASCeilPointValues(CGPoint p);
+
 CGSize ASCeilSizeValues(CGSize s);
 
 CGFloat ASCeilPixelValue(CGFloat f);

Source/Private/ASInternalHelpers.m

@@ -153,6 +153,11 @@ CGFloat ASFloorPixelValue(CGFloat f)
   return floor(f * scale) / scale;
 }
 
+CGPoint ASCeilPointValues(CGPoint p)
+{
+  return CGPointMake(ASCeilPixelValue(p.x), ASCeilPixelValue(p.y));
+}
+
 CGSize ASCeilSizeValues(CGSize s)
 {
   return CGSizeMake(ASCeilPixelValue(s.width), ASCeilPixelValue(s.height));