[ASLayout] Remove flattened property

[maicki]

The flattened property on ASLayout is not needed anymore in the algorithm to flatten a layout: filteredNodeLayoutTree.

This should fix a bug where if a layout is reused that has the flattened property set to YES will be ignored in the flattening process although it should not have been.

[nguyenhuy]

Thanks for working on this, @maicki!

[Adlai-Holler]

I think we should not do this, and instead fix the logic issue in this method.

• This diff will cause a lot more layout copying.
• The flattened flag actually makes perfect sense
• Right now the logic in this method is the real problem.

I'm on phone right now so I can't articulate the correct solution, but it's basically to stop skipping self and change the loop structure.

[appleguy]

I was looking at this code recently with some of the Yoga stuff (setting flattened on the layouts created that way, since they are already flat). It's definitely key for performance, which IIRC is the main reason the flattened flag was added (maybe we should add comments internal documentation about this, since it's unusual?)

The great thing about ASLayout is that it provides an incredibly powerful & useful hook for both developers' own custom layouts (calculateLayoutThatFits:, etc) and entire layout engines to be integrated. I bet we could even use it in the new "ASCollectionLayout" (or ASLayoutManager) class.

Because ASLayout is the #1 most frequently created ASDK object, it's a worthwhile pursuit to make it as fast as possible. The good news is that after the perf work that @maicki did last year, ASLayout is now quite well optimized!

[nguyenhuy]

@maicki and I discussed and agreed on the changes in this PR before he pushed it, so I accepted it a bit too fast.

I think we're looking at 2 different flags here:

• visited flag set to a context and used solely by the traversal algorithm to avoid processing a context multiple times.
• isFlattened flag persistently set to an ASLayout object to indicate that it doesn't contain non-node-based layout elements.

The fact that we merged these 2 flags into one isFlattened flag is problematic because as @maicki said, ASLayout objects now can't be reused.

After looking at this method again, I started to wondering if we need either of these flags at all! Please correct me if I'm wrong, but I think visited is not need because of the way our DFS algorithm is being implemented. isFlattened is not used anywhere, even though the flag itself makes perfect sense. So I've been testing a slightly different implementation of this method against the test suite proposed in this PR and ASDKgram. It works perfectly so far.

// Returns an ASLayout that contains only **direct** subnodes
- (ASLayout *)filteredNodeLayoutTree
{
  NSMutableArray *flattenedSublayouts = [NSMutableArray array];
  
  struct Context {
    ASLayout *layout;
    CGPoint absolutePosition;
  };
  
  // Queue used to keep track of sublayouts while traversing this layout in a DFS fashion.
  std::deque<Context> queue;
  queue.push_front({self, CGPointZero});
  
  while (!queue.empty()) {
    Context context = queue.front();
    queue.pop_front();

    if (self != context.layout && context.layout.type == ASLayoutElementTypeDisplayNode) {
      ASLayout *layout = [ASLayout layoutWithLayoutElement:context.layout.layoutElement
                                                      size:context.layout.size
                                                  position:context.absolutePosition
                                                sublayouts:nil]; // Avoid grabbing context.layout.sublayouts here because we don't really need them
      [flattenedSublayouts addObject:layout];
    } else {
      std::vector<Context> sublayoutContexts;
      for (ASLayout *sublayout in context.layout.sublayouts) {
        sublayoutContexts.push_back({sublayout, context.absolutePosition + sublayout.position});
      }
      queue.insert(queue.cbegin(), sublayoutContexts.begin(), sublayoutContexts.end());
    }
  }
  
  ASLayout *layout = [ASLayout layoutWithLayoutElement:_layoutElement size:_size position:CGPointZero sublayouts:flattenedSublayouts];
  layout.retainSublayoutLayoutElements = YES;
  return layout;
}

Let me know what you all think.

[Adlai-Holler]

Let's revisit this in the future. The flattening process needs work but I don't think this diff is the way, and we've got bigger fish to fry.