[LFC Display] Simplify display tree building

https://bugs.webkit.org/show_bug.cgi?id=218152

Reviewed by Zalan Bujtas.

Rather than tracking container/current child display boxes in various places in the code,
wrap it up in a small InsertionPosition struct and TreeBuilder::insert() which handles
the setFirstChild/setNextSibling logic.

* display/DisplayTreeBuilder.cpp:
(WebCore::Display::TreeBuilder::build const):
(WebCore::Display::TreeBuilder::insert const):
(WebCore::Display::TreeBuilder::buildInlineDisplayTree const):
(WebCore::Display::TreeBuilder::recursiveBuildDisplayTree const):
* display/DisplayTreeBuilder.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@268953 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

Source/WebCore/ChangeLog

@@ -1,3 +1,21 @@
+2020-10-24  Simon Fraser  <[email protected]>
+
+        [LFC Display] Simplify display tree building
+        https://bugs.webkit.org/show_bug.cgi?id=218152
+
+        Reviewed by Zalan Bujtas.
+
+        Rather than tracking container/current child display boxes in various places in the code,
+        wrap it up in a small InsertionPosition struct and TreeBuilder::insert() which handles
+        the setFirstChild/setNextSibling logic.
+
+        * display/DisplayTreeBuilder.cpp:
+        (WebCore::Display::TreeBuilder::build const):
+        (WebCore::Display::TreeBuilder::insert const):
+        (WebCore::Display::TreeBuilder::buildInlineDisplayTree const):
+        (WebCore::Display::TreeBuilder::recursiveBuildDisplayTree const):
+        * display/DisplayTreeBuilder.h:
+
 2020-10-24  Jer Noble  <[email protected]>
 
         [BigSur] Appending a new WebM init segment between Cluster elements throws an error

Source/WebCore/display/DisplayTreeBuilder.cpp

@@ -69,18 +69,30 @@ std::unique_ptr<Tree> TreeBuilder::build(const Layout::LayoutState& layoutState)
 
     auto borderBoxRect = LayoutRect { Layout::BoxGeometry::borderBoxRect(geometry) };
     auto offset = toLayoutSize(borderBoxRect.location());
-    recursiveBuildDisplayTree(layoutState, offset, *rootLayoutBox.firstChild(), *rootDisplayContainerBox);
+    auto insertionPosition = InsertionPosition { *rootDisplayContainerBox };
+
+    recursiveBuildDisplayTree(layoutState, offset, *rootLayoutBox.firstChild(), insertionPosition);
 
     LOG_WITH_STREAM(FormattingContextLayout, stream << "Display tree:\n" << displayTreeAsText(*rootDisplayContainerBox));
 
     return makeUnique<Tree>(WTFMove(rootDisplayContainerBox));
 }
 
-void TreeBuilder::buildInlineDisplayTree(const Layout::LayoutState& layoutState, LayoutSize offsetFromRoot, const Layout::ContainerBox& inlineFormattingRoot, ContainerBox& parentDisplayBox) const
+void TreeBuilder::insert(std::unique_ptr<Box>&& box, InsertionPosition& insertionPosition) const
 {
-    auto& inlineFormattingState = layoutState.establishedInlineFormattingState(inlineFormattingRoot);
+    if (insertionPosition.currentChild) {
+        auto boxPtr = box.get();
+        insertionPosition.currentChild->setNextSibling(WTFMove(box));
+        insertionPosition.currentChild = boxPtr;
+    } else {
+        insertionPosition.currentChild = box.get();
+        insertionPosition.container.setFirstChild(WTFMove(box));
+    }
+}
 
-    Box* previousSiblingBox = nullptr;
+void TreeBuilder::buildInlineDisplayTree(const Layout::LayoutState& layoutState, LayoutSize offsetFromRoot, const Layout::ContainerBox& inlineFormattingRoot, InsertionPosition& insertionPosition) const
+{
+    auto& inlineFormattingState = layoutState.establishedInlineFormattingState(inlineFormattingRoot);
 
     for (auto& run : inlineFormattingState.lineRuns()) {
         auto& lineGeometry = inlineFormattingState.lines().at(run.lineIndex());
@@ -94,53 +106,39 @@ void TreeBuilder::buildInlineDisplayTree(const Layout::LayoutState& layoutState,
 
         auto style = Style { run.layoutBox().style() };
         auto textBox = makeUnique<TextBox>(snapRectToDevicePixels(runRect, m_pixelSnappingFactor), WTFMove(style), run);
-        auto textBoxPtr = textBox.get();
 
-        if (previousSiblingBox)
-            previousSiblingBox->setNextSibling(WTFMove(textBox));
-        else
-            parentDisplayBox.setFirstChild(WTFMove(textBox));
-
-        previousSiblingBox = textBoxPtr;
+        insert(WTFMove(textBox), insertionPosition);
     }
 }
 
-Box* TreeBuilder::recursiveBuildDisplayTree(const Layout::LayoutState& layoutState, LayoutSize offsetFromRoot, const Layout::Box& box, ContainerBox& parentDisplayBox, Box* previousSiblingBox) const
+void TreeBuilder::recursiveBuildDisplayTree(const Layout::LayoutState& layoutState, LayoutSize offsetFromRoot, const Layout::Box& box, InsertionPosition& insertionPosition) const
 {
     auto geometry = layoutState.geometryForBox(box);
     auto displayBox = displayBoxForLayoutBox(geometry, box, offsetFromRoot);
 
-    Box* result = displayBox.get();
-
-    if (previousSiblingBox)
-        previousSiblingBox->setNextSibling(WTFMove(displayBox));
-    else
-        parentDisplayBox.setFirstChild(WTFMove(displayBox));
+    insert(WTFMove(displayBox), insertionPosition);
 
     if (!is<Layout::ContainerBox>(box))
-        return result;
+        return;
 
     auto& layoutContainerBox = downcast<Layout::ContainerBox>(box);
     if (!layoutContainerBox.hasChild())
-        return result;
+        return;
 
     auto borderBoxRect = LayoutRect { Layout::BoxGeometry::borderBoxRect(geometry) };
     offsetFromRoot += toLayoutSize(borderBoxRect.location());
 
-    auto& displayContainerBox = downcast<ContainerBox>(*result);
-
+    auto positionForChildren = InsertionPosition { downcast<ContainerBox>(*insertionPosition.currentChild) };
+    
     if (layoutContainerBox.establishesInlineFormattingContext()) {
-        buildInlineDisplayTree(layoutState, offsetFromRoot, downcast<Layout::ContainerBox>(layoutContainerBox), displayContainerBox);
-        return result;
+        buildInlineDisplayTree(layoutState, offsetFromRoot, downcast<Layout::ContainerBox>(layoutContainerBox), positionForChildren);
+        return;
     }
 
-    Display::Box* currSiblingDisplayBox = nullptr;
     for (auto& child : Layout::childrenOfType<Layout::Box>(layoutContainerBox)) {
         if (layoutState.hasBoxGeometry(child))
-            currSiblingDisplayBox = recursiveBuildDisplayTree(layoutState, offsetFromRoot, child, displayContainerBox, currSiblingDisplayBox);
+            recursiveBuildDisplayTree(layoutState, offsetFromRoot, child, positionForChildren);
     }
-
-    return result;
 }
 
 std::unique_ptr<Box> TreeBuilder::displayBoxForRootBox(const Layout::BoxGeometry& geometry, const Layout::ContainerBox& rootBox) const

Source/WebCore/display/DisplayTreeBuilder.h

@@ -57,12 +57,19 @@ class TreeBuilder {
     std::unique_ptr<Tree> build(const Layout::LayoutState&) const;
 
 private:
+    struct InsertionPosition {
+        Display::ContainerBox& container;
+        Display::Box* currentChild { nullptr };
+    };
+
     std::unique_ptr<Box> displayBoxForRootBox(const Layout::BoxGeometry&, const Layout::ContainerBox&) const;
     std::unique_ptr<Box> displayBoxForLayoutBox(const Layout::BoxGeometry&, const Layout::Box&, LayoutSize offsetFromRoot) const;
 
-    Box* recursiveBuildDisplayTree(const Layout::LayoutState&, LayoutSize offsetFromRoot, const Layout::Box&, Display::ContainerBox& parentDisplayBox, Display::Box* previousSiblingBox = nullptr) const;
+    void recursiveBuildDisplayTree(const Layout::LayoutState&, LayoutSize offsetFromRoot, const Layout::Box&, InsertionPosition&) const;
 
-    void buildInlineDisplayTree(const Layout::LayoutState&, LayoutSize offsetFromRoot, const Layout::ContainerBox&, Display::ContainerBox& parentDisplayBox) const;
+    void buildInlineDisplayTree(const Layout::LayoutState&, LayoutSize offsetFromRoot, const Layout::ContainerBox&, InsertionPosition&) const;
+    
+    void insert(std::unique_ptr<Box>&&, InsertionPosition&) const;
 
     float m_pixelSnappingFactor { 1 };
 };