Changeset 253032 in webkit

Timestamp:

Dec 3, 2019 2:51:46 AM (4 years ago)

Author:

Antti Koivisto

Message:

[LFC][Integration] Setup root box properties
​https://bugs.webkit.org/show_bug.cgi?id=204743

Reviewed by Zalan Bujtas.

Source/WebCore:

Test: fast/layoutformattingcontext/flow-integration-basic.html

Line layout needs to know about flow borders and padding so that boxes are offset correctly.

• CMakeLists.txt:
• Sources.txt:
• WebCore.xcodeproj/project.pbxproj:
• layout/LayoutContext.cpp:

(WebCore::Layout::LayoutContext::layout):
(WebCore::Layout::LayoutContext::layoutWithPreparedRootGeometry):

Split setup and layout into separate functions.

• layout/LayoutContext.h:
• layout/LayoutState.cpp:

(WebCore::Layout::LayoutState::displayBoxForLayoutRoot):

• layout/LayoutState.h:
• layout/displaytree/DisplayPainter.cpp:

(WebCore::Display::paintInlineContent):
(WebCore::Display::Painter::paint):
(WebCore::Display::Painter::paintInlineFlow):

Avoid accessing tree root box properties when painting (since margins are not set up).

• layout/integration/RenderBlockFlowLineLayout.cpp: Renamed from Source/WebCore/layout/RenderBlockFlowLineLayout.cpp.

Moved to integration subdirectory.

(WebCore::Layout::RenderBlockFlowLineLayout::layout):

Drop the content size paramater, the caller is responsible of setting up the root display box.

(WebCore::Layout::RenderBlockFlowLineLayout::prepareRootDisplayBoxForLayout):

Setup padding and borders.

(WebCore::Layout::RenderBlockFlowLineLayout::displayInlineContent const):
(WebCore::Layout::RenderBlockFlowLineLayout::rootLayoutBox const):

• layout/integration/RenderBlockFlowLineLayout.h: Renamed from Source/WebCore/layout/RenderBlockFlowLineLayout.h.

(WebCore::Layout::RenderBlockFlowLineLayout::contentLogicalHeight const):

Use a member to pass content height.

• layout/layouttree/LayoutTreeBuilder.h:

(WebCore::Layout::LayoutTreeContent::layoutBoxForRenderer const):

• rendering/RenderBlockFlow.cpp:

(WebCore::RenderBlockFlow::layoutLFCLines):

LayoutTests:

• fast/layoutformattingcontext/flow-integration-basic.html: Added.
• platform/mac/fast/layoutformattingcontext/flow-integration-basic-expected.txt: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/layoutformattingcontext (added)
• LayoutTests/fast/layoutformattingcontext/flow-integration-basic.html (added)
• LayoutTests/platform/ios-simulator/fast/layoutformattingcontext (added)
• LayoutTests/platform/ios-simulator/fast/layoutformattingcontext/flow-integration-basic-expected.txt (added)
• LayoutTests/platform/mac/fast/layoutformattingcontext (added)
• LayoutTests/platform/mac/fast/layoutformattingcontext/flow-integration-basic-expected.txt (added)
• Source/WebCore/CMakeLists.txt (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Sources.txt (modified) (2 diffs)
• Source/WebCore/WebCore.xcodeproj/project.pbxproj (modified) (5 diffs)
• Source/WebCore/layout/LayoutContext.cpp (modified) (3 diffs)
• Source/WebCore/layout/LayoutContext.h (modified) (1 diff)
• Source/WebCore/layout/LayoutState.cpp (modified) (1 diff)
• Source/WebCore/layout/LayoutState.h (modified) (1 diff)
• Source/WebCore/layout/displaytree/DisplayPainter.cpp (modified) (5 diffs)
• Source/WebCore/layout/integration (added)
• Source/WebCore/layout/integration/RenderBlockFlowLineLayout.cpp (moved) (moved from trunk/Source/WebCore/layout/RenderBlockFlowLineLayout.cpp) (2 diffs)
• Source/WebCore/layout/integration/RenderBlockFlowLineLayout.h (moved) (moved from trunk/Source/WebCore/layout/RenderBlockFlowLineLayout.h) (2 diffs)
• Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp (modified) (1 diff)
• Source/WebCore/layout/layouttree/LayoutTreeBuilder.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderBlockFlow.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-12-03  Antti Koivisto  <antti@apple.com>
+
+        [LFC][Integration] Setup root box properties
+        https://bugs.webkit.org/show_bug.cgi?id=204743
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/layoutformattingcontext/flow-integration-basic.html: Added.
+        * platform/mac/fast/layoutformattingcontext/flow-integration-basic-expected.txt: Added.
+
 2019-12-03  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/CMakeLists.txt

@@ -106 +106 @@
     "${WEBCORE_DIR}/layout/inlineformatting"
     "${WEBCORE_DIR}/layout/inlineformatting/text"
+    "${WEBCORE_DIR}/layout/integration"
     "${WEBCORE_DIR}/layout/layouttree"
     "${WEBCORE_DIR}/loader"

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-12-03  Antti Koivisto  <antti@apple.com>
+
+        [LFC][Integration] Setup root box properties
+        https://bugs.webkit.org/show_bug.cgi?id=204743
+
+        Reviewed by Zalan Bujtas.
+
+        Test: fast/layoutformattingcontext/flow-integration-basic.html
+
+        Line layout needs to know about flow borders and padding so that boxes are offset correctly.
+
+        * CMakeLists.txt:
+        * Sources.txt:
+        * WebCore.xcodeproj/project.pbxproj:
+        * layout/LayoutContext.cpp:
+        (WebCore::Layout::LayoutContext::layout):
+        (WebCore::Layout::LayoutContext::layoutWithPreparedRootGeometry):
+
+        Split setup and layout into separate functions.
+
+        * layout/LayoutContext.h:
+        * layout/LayoutState.cpp:
+        (WebCore::Layout::LayoutState::displayBoxForLayoutRoot):
+        * layout/LayoutState.h:
+        * layout/displaytree/DisplayPainter.cpp:
+        (WebCore::Display::paintInlineContent):
+        (WebCore::Display::Painter::paint):
+        (WebCore::Display::Painter::paintInlineFlow):
+
+        Avoid accessing tree root box properties when painting (since margins are not set up).
+
+        * layout/integration/RenderBlockFlowLineLayout.cpp: Renamed from Source/WebCore/layout/RenderBlockFlowLineLayout.cpp.
+
+        Moved to integration subdirectory.
+
+        (WebCore::Layout::RenderBlockFlowLineLayout::layout):
+
+        Drop the content size paramater, the caller is responsible of setting up the root display box.
+
+        (WebCore::Layout::RenderBlockFlowLineLayout::prepareRootDisplayBoxForLayout):
+
+        Setup padding and borders.
+
+        (WebCore::Layout::RenderBlockFlowLineLayout::displayInlineContent const):
+        (WebCore::Layout::RenderBlockFlowLineLayout::rootLayoutBox const):
+        * layout/integration/RenderBlockFlowLineLayout.h: Renamed from Source/WebCore/layout/RenderBlockFlowLineLayout.h.
+        (WebCore::Layout::RenderBlockFlowLineLayout::contentLogicalHeight const):
+
+        Use a member to pass content height.
+
+        * layout/layouttree/LayoutTreeBuilder.h:
+        (WebCore::Layout::LayoutTreeContent::layoutBoxForRenderer const):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::RenderBlockFlow::layoutLFCLines):
+
 2019-12-03  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/Source/WebCore/Sources.txt

@@ -1423 +1423 @@
 layout/LayoutState.cpp
 layout/LayoutUnits.cpp
-layout/RenderBlockFlowLineLayout.cpp
 layout/Verification.cpp
 layout/blockformatting/BlockFormattingContext.cpp
@@ -1446 +1445 @@
 layout/inlineformatting/LineLayoutContext.cpp
 layout/inlineformatting/text/TextUtil.cpp
+layout/integration/RenderBlockFlowLineLayout.cpp
 layout/invalidation/InvalidationContext.cpp
 layout/invalidation/InvalidationState.cpp

trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -16630 +16630 @@
                                 6FCFC055212DACC2007695D2 /* floats */,
                                 115CFA9A208BC140001E6991 /* inlineformatting */,
+                                E4FB4B1E2395356F003C336A /* integration */,
                                 6F95DE4A237B883000E517E1 /* invalidation */,
                                 115CFA90208B9441001E6991 /* layouttree */,
@@ -16648 +16649 @@
                                 6F73918C2106CEDD006AF262 /* LayoutUnits.h */,
                                 6FE7AA2621C37B6300296DCD /* MarginTypes.h */,
-                                E4ABABDE2360893D00FA4345 /* RenderBlockFlowLineLayout.cpp */,
-                                E4ABABDB236088FD00FA4345 /* RenderBlockFlowLineLayout.h */,
                                 11FF02D520BA3C810083F25B /* Verification.cpp */,
                         );
@@ -16692 +16691 @@
                         isa = PBXGroup;
                         children = (
+                                E435505223953457004829C0 /* integration */,
                                 6FE7DDDD20EC6E8B008B5B4E /* text */,
                                 6F7CA3C9208C2B2E002F29AB /* InlineFormattingContext.cpp */,
@@ -27022 +27022 @@
                         sourceTree = "<group>";
                 };
+                E435505223953457004829C0 /* integration */ = {
+                        isa = PBXGroup;
+                        children = (
+                        );
+                        path = integration;
+                        sourceTree = "<group>";
+                };
                 E46E97860DAAD61B0071E894 /* animation */ = {
                         isa = PBXGroup;
@@ -27143 +27150 @@
                         );
                         path = updating;
+                        sourceTree = "<group>";
+                };
+                E4FB4B1E2395356F003C336A /* integration */ = {
+                        isa = PBXGroup;
+                        children = (
+                                E4ABABDE2360893D00FA4345 /* RenderBlockFlowLineLayout.cpp */,
+                                E4ABABDB236088FD00FA4345 /* RenderBlockFlowLineLayout.h */,
+                        );
+                        path = integration;
                         sourceTree = "<group>";
                 };

trunk/Source/WebCore/layout/LayoutContext.cpp

@@ -59 +59 @@
 void LayoutContext::layout(const LayoutSize& rootContentBoxSize, InvalidationState& invalidationState)
 {
-    PhaseScope scope(Phase::Type::Layout);
     // Set the geometry on the root.
     // Note that we never layout the root box. It has to have an already computed geometry (in case of ICB, it's the view geometry).
     // ICB establishes the initial BFC, but it does not live in a formatting context and while a non-ICB root(subtree layout) has to have a formatting context,
     // we could not lay it out even if we wanted to since it's outside of this LayoutContext.
-    auto& displayBox = layoutState().displayBoxForLayoutBox(layoutState().root());
+    auto& displayBox = layoutState().displayBoxForRootLayoutBox();
     displayBox.setHorizontalMargin({ });
     displayBox.setHorizontalComputedMargin({ });
@@ -74 +73 @@
     displayBox.setContentBoxWidth(rootContentBoxSize.width());
 
+    layoutWithPreparedRootGeometry(invalidationState);
+}
+
+void LayoutContext::layoutWithPreparedRootGeometry(InvalidationState& invalidationState)
+{
+    PhaseScope scope(Phase::Type::Layout);
+
     auto& formattingContextRootsForLayout = invalidationState.formattingContextRoots();
     // When invalidation is empty, we assume constraint mutation and start running layout on the context root. Layout logic should be able to figure out the damage.
@@ -82 +88 @@
         layoutFormattingContextSubtree(formattingContextRoot, invalidationState);
 }
+
 
 void LayoutContext::layoutFormattingContextSubtree(const Container& formattingContextRoot, InvalidationState& invalidationState)

trunk/Source/WebCore/layout/LayoutContext.h

@@ -53 +53 @@
 public:
     LayoutContext(LayoutState&);
+
     void layout(const LayoutSize& rootContentBoxSize, InvalidationState&);
+    void layoutWithPreparedRootGeometry(InvalidationState&);
 
     static std::unique_ptr<FormattingContext> createFormattingContext(const Container& formattingContextRoot, LayoutState&);

trunk/Source/WebCore/layout/LayoutState.cpp

@@ -57 +57 @@
 
 LayoutState::~LayoutState() = default;
+
+Display::Box& LayoutState::displayBoxForRootLayoutBox()
+{
+    return displayBoxForLayoutBox(m_layoutTreeContent->rootLayoutBox());
+}
 
 Display::Box& LayoutState::displayBoxForLayoutBox(const Box& layoutBox)

trunk/Source/WebCore/layout/LayoutState.h

@@ -63 +63 @@
 #endif
 
+    Display::Box& displayBoxForRootLayoutBox();
     Display::Box& displayBoxForLayoutBox(const Box& layoutBox);
     const Display::Box& displayBoxForLayoutBox(const Box& layoutBox) const;

trunk/Source/WebCore/layout/displaytree/DisplayPainter.cpp

@@ -111 +111 @@
 }
 
-static void paintInlineContent(GraphicsContext& context, const Box& rootAbsoluteDisplayBox, const Layout::InlineFormattingState& formattingState)
+static void paintInlineContent(GraphicsContext& context, LayoutPoint absoluteOffset, const Layout::InlineFormattingState& formattingState)
 {
     auto* displayInlineContent = formattingState.displayInlineContent();
@@ -127 +127 @@
             context.setFillColor(style.color());
 
-            auto logicalLeft = rootAbsoluteDisplayBox.left() + run.logicalLeft();
+            auto logicalLeft = absoluteOffset.x() + run.logicalLeft();
             // FIXME: Add non-baseline align painting
             auto& lineBox = displayInlineContent->lineBoxForRun(run);
-            auto baselineOffset = rootAbsoluteDisplayBox.top() + lineBox.logicalTop() + lineBox.baselineOffset();
+            auto baselineOffset = absoluteOffset.y() + lineBox.logicalTop() + lineBox.baselineOffset();
             if (auto expansionContext = textContext->expansion())
                 context.drawText(style.fontCascade(), TextRun { textContext->content(), logicalLeft, expansionContext->horizontalExpansion, expansionContext->behavior }, { logicalLeft, baselineOffset });
@@ -136 +136 @@
                 context.drawText(style.fontCascade(), TextRun { textContext->content(), logicalLeft }, { logicalLeft, baselineOffset });
         } else if (auto* cachedImage = run.image()) {
-            auto runAbsoluteRect = FloatRect { rootAbsoluteDisplayBox.left() + run.logicalLeft(), rootAbsoluteDisplayBox.top() + run.logicalTop(), run.logicalWidth(), run.logicalHeight() };
+            auto runAbsoluteRect = FloatRect { absoluteOffset.x() + run.logicalLeft(), absoluteOffset.y() + run.logicalTop(), run.logicalWidth(), run.logicalHeight() };
             context.drawImage(*cachedImage->image(), runAbsoluteRect);
         }
@@ -195 +195 @@
         if (layoutBox.establishesInlineFormattingContext()) {
             auto& container = downcast<Layout::Container>(layoutBox);
-            paintInlineContent(context, absoluteDisplayBox, downcast<Layout::InlineFormattingState>(layoutState.establishedFormattingState(container)));
+            paintInlineContent(context, absoluteDisplayBox.topLeft(), downcast<Layout::InlineFormattingState>(layoutState.establishedFormattingState(container)));
             continue;
         }
@@ -207 +207 @@
     ASSERT(layoutRoot.establishesInlineFormattingContext());
 
-    auto& displayBox = layoutState.displayBoxForLayoutBox(layoutRoot);
-
-    paintInlineContent(context, displayBox, downcast<Layout::InlineFormattingState>(layoutState.establishedFormattingState(layoutRoot)));
+    paintInlineContent(context, { }, downcast<Layout::InlineFormattingState>(layoutState.establishedFormattingState(layoutRoot)));
 }
 

trunk/Source/WebCore/layout/integration/RenderBlockFlowLineLayout.cpp

@@ -75 +75 @@
         m_layoutState = makeUnique<LayoutState>(*m_treeContent);
 
-    auto& rootContainer = m_layoutState->root();
+    prepareRootGeometryForLayout();
+
     auto layoutContext = LayoutContext { *m_layoutState };
     auto invalidationState = InvalidationState { };
-    layoutContext.layout(m_flow.contentSize(), invalidationState);
 
-    auto& lineBoxes = downcast<InlineFormattingState>(m_layoutState->establishedFormattingState(rootContainer)).displayInlineContent()->lineBoxes;
-    auto height = lineBoxes.last().logicalBottom();
+    layoutContext.layoutWithPreparedRootGeometry(invalidationState);
 
-    auto& displayBox = m_layoutState->displayBoxForLayoutBox(rootContainer);
-    displayBox.setContentBoxHeight(height);
+    auto& lineBoxes = downcast<InlineFormattingState>(m_layoutState->establishedFormattingState(rootLayoutBox())).displayInlineContent()->lineBoxes;
+    m_contentLogicalHeight = lineBoxes.last().logicalBottom() - lineBoxes.first().logicalTop();
 }
 
-LayoutUnit RenderBlockFlowLineLayout::contentBoxHeight() const
+void RenderBlockFlowLineLayout::prepareRootGeometryForLayout()
 {
-    return m_layoutState ? m_layoutState->displayBoxForLayoutBox(m_layoutState->root()).contentBoxHeight() : 0_lu;
+    auto& displayBox = m_layoutState->displayBoxForRootLayoutBox();
+
+    // Don't set marging properties or height. These should not be be accessed by inline layout.
+    displayBox.setBorder(Layout::Edges { { m_flow.borderStart(), m_flow.borderEnd() }, { m_flow.borderBefore(), m_flow.borderAfter() } });
+    displayBox.setPadding(Layout::Edges { { m_flow.paddingStart(), m_flow.paddingEnd() }, { m_flow.paddingBefore(), m_flow.paddingAfter() } });
+    displayBox.setContentBoxWidth(m_flow.contentSize().width());
 }
 
 const Display::InlineContent* RenderBlockFlowLineLayout::displayInlineContent() const
 {
-    return downcast<InlineFormattingState>(m_layoutState->establishedFormattingState(m_treeContent->rootLayoutBox())).displayInlineContent();
+    return downcast<InlineFormattingState>(m_layoutState->establishedFormattingState(rootLayoutBox())).displayInlineContent();
 }
 
@@ -150 +154 @@
 }
 
+const Container& RenderBlockFlowLineLayout::rootLayoutBox() const
+{
+    return m_treeContent->rootLayoutBox();
+}
+
 }
 }

trunk/Source/WebCore/layout/integration/RenderBlockFlowLineLayout.h

@@ -56 +56 @@
 
     void layout();
-    LayoutUnit contentBoxHeight() const;
+
+    LayoutUnit contentLogicalHeight() const { return m_contentLogicalHeight; }
     const Display::InlineContent* displayInlineContent() const;
 
@@ -65 +66 @@
 
 private:
+    const Container& rootLayoutBox() const;
+    void prepareRootGeometryForLayout();
+
     const RenderBlockFlow& m_flow;
-    std::unique_ptr<LayoutTreeContent> m_treeContent;
+    std::unique_ptr<const LayoutTreeContent> m_treeContent;
     std::unique_ptr<LayoutState> m_layoutState;
+    LayoutUnit m_contentLogicalHeight;
 };
 

trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp

@@ -455 +455 @@
         auto& layoutRoot = layoutState.root();
         auto invalidationState = InvalidationState { };
+
         LayoutContext(layoutState).layout(renderView.size(), invalidationState);
         showLayoutTree(layoutRoot, &layoutState);

trunk/Source/WebCore/layout/layouttree/LayoutTreeBuilder.h

@@ -58 +58 @@
     void addBox(std::unique_ptr<Box> box) { m_boxes.add(WTFMove(box)); }
 
-    using RenderObjectToLayoutBoxMap = HashMap<const RenderObject*, Box*>;
     Box* layoutBoxForRenderer(const RenderObject& renderer) { return m_renderObjectToLayoutBox.get(&renderer); }
+    const Box* layoutBoxForRenderer(const RenderObject& renderer) const { return m_renderObjectToLayoutBox.get(&renderer); }
+    
     void addLayoutBoxForRenderer(const RenderObject& renderer, Box& layoutBox) { m_renderObjectToLayoutBox.add(&renderer, &layoutBox); }
 
@@ -66 +67 @@
     std::unique_ptr<Container> m_rootLayoutBox;
     HashSet<std::unique_ptr<Box>> m_boxes;
-    RenderObjectToLayoutBoxMap m_renderObjectToLayoutBox;
+
+    HashMap<const RenderObject*, Box*> m_renderObjectToLayoutBox;
 };
 

trunk/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -3681 +3681 @@
     lfcLineLayout.layout();
 
-    LayoutUnit lineLayoutHeight = lfcLineLayout.contentBoxHeight();
-    LayoutUnit lineLayoutTop = borderAndPaddingBefore();
-    repaintLogicalTop = lineLayoutTop;
-    repaintLogicalBottom = repaintLogicalTop + lineLayoutHeight + borderAndPaddingAfter();
-    setLogicalHeight(lineLayoutTop + lineLayoutHeight + borderAndPaddingAfter());
+    auto contentHeight = lfcLineLayout.contentLogicalHeight();
+    auto contentTop = borderAndPaddingBefore();
+    auto contentBottom = contentTop + contentHeight;
+    auto totalHeight = contentBottom + borderAndPaddingAfter();
+
+    repaintLogicalTop = contentTop;
+    repaintLogicalBottom = contentBottom;
+
+    setLogicalHeight(totalHeight);
 }
 #endif