[LFC][IFC] Decouple float placement and line shrinking
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Jun 2019 15:52:25 +0000 (15:52 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 4 Jun 2019 15:52:25 +0000 (15:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198528
<rdar://problem/51397638>

Reviewed by Antti Koivisto.

In LineLayout::placeInlineItems() float handling should be only about shrinking the current line, the actual
float placement should happen later when we construct the the display boxes/runs. It enables the preferred width
computation to call placeInlineItems() to gather line widths without accidentally mutating the layout context.

* layout/inlineformatting/InlineFormattingContext.h:
* layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
(WebCore::Layout::InlineFormattingContext::LineLayout::placeInlineItems const):
(WebCore::Layout::InlineFormattingContext::LineLayout::layout const):
(WebCore::Layout::InlineFormattingContext::LineLayout::createDisplayRuns const):
(WebCore::Layout::InlineFormattingContext::LineLayout::handleFloat const): Deleted.
* layout/inlineformatting/InlineItem.h:

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

Source/WebCore/ChangeLog
Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
Source/WebCore/layout/inlineformatting/InlineItem.h

index 5090ea0..ce97d1d 100644 (file)
@@ -1,5 +1,25 @@
 2019-06-04  Zalan Bujtas  <zalan@apple.com>
 
+        [LFC][IFC] Decouple float placement and line shrinking
+        https://bugs.webkit.org/show_bug.cgi?id=198528
+        <rdar://problem/51397638>
+
+        Reviewed by Antti Koivisto.
+
+        In LineLayout::placeInlineItems() float handling should be only about shrinking the current line, the actual
+        float placement should happen later when we construct the the display boxes/runs. It enables the preferred width
+        computation to call placeInlineItems() to gather line widths without accidentally mutating the layout context. 
+
+        * layout/inlineformatting/InlineFormattingContext.h:
+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
+        (WebCore::Layout::InlineFormattingContext::LineLayout::placeInlineItems const):
+        (WebCore::Layout::InlineFormattingContext::LineLayout::layout const):
+        (WebCore::Layout::InlineFormattingContext::LineLayout::createDisplayRuns const):
+        (WebCore::Layout::InlineFormattingContext::LineLayout::handleFloat const): Deleted.
+        * layout/inlineformatting/InlineItem.h:
+
+2019-06-04  Zalan Bujtas  <zalan@apple.com>
+
         [LFC][IFC] Add hard line break handling to LineBreaker
         https://bugs.webkit.org/show_bug.cgi?id=198503
         <rdar://problem/51373482>
index 28fbad3..ac761f6 100644 (file)
@@ -61,6 +61,7 @@ private:
 
         struct LineContent {
             Optional<unsigned> lastInlineItemIndex;
+            Vector<WeakPtr<InlineItem>> floats;
             std::unique_ptr<Line::Content> runs;
         };
 
@@ -73,8 +74,7 @@ private:
             const InlineItems& inlineItems;
         };
         LineContent placeInlineItems(const LineInput&) const;
-        void createDisplayRuns(const Line::Content&, LayoutUnit widthConstraint) const;
-        void handleFloat(Line&, const FloatingContext&, const InlineItem& floatBox) const;
+        void createDisplayRuns(const Line::Content&, const Vector<WeakPtr<InlineItem>>& floats, LayoutUnit widthConstraint) const;
         void alignRuns(TextAlignMode, unsigned firstRunIndex, LayoutUnit availableWidth) const;
 
     private:
index b93cd9e..8c7884a 100644 (file)
@@ -155,7 +155,7 @@ static std::unique_ptr<Line> constructLine(const LayoutState& layoutState, const
 InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLayout::placeInlineItems(const LineInput& lineInput) const
 {
     auto line = constructLine(layoutState(), m_floatingState, m_formattingRoot, lineInput.logicalTop, lineInput.availableLogicalWidth);
-    auto floatingContext = FloatingContext { m_floatingState };
+    Vector<WeakPtr<InlineItem>> floats;
     unsigned committedInlineItemCount = 0;
 
     UncommittedContent uncommittedContent;
@@ -204,7 +204,7 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
     auto closeLine = [&] {
         // This might change at some point.
         ASSERT(committedInlineItemCount);
-        return LineContent { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1), line->close() };
+        return LineContent { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1), WTFMove(floats), line->close() };
     };
     LineBreaker lineBreaker;
     // Iterate through the inline content and place the inline boxes on the current line.
@@ -233,7 +233,12 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
 
         ASSERT(breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Keep);
         if (inlineItem->isFloat()) {
-            handleFloat(*line, floatingContext, *inlineItem);
+            auto& floatBox = inlineItem->layoutBox();
+            ASSERT(layoutState().hasDisplayBox(floatBox));
+            // Shrink availble space for current line and move existing inline runs.
+            auto floatBoxWidth = layoutState().displayBoxForLayoutBox(floatBox).marginBoxWidth();
+            floatBox.isLeftFloatingPositioned() ? line->moveLogicalLeft(floatBoxWidth) : line->moveLogicalRight(floatBoxWidth);
+            floats.append(makeWeakPtr(*inlineItem));
             ++committedInlineItemCount;
             continue;
         }
@@ -260,7 +265,7 @@ void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) con
     unsigned currentInlineItemIndex = 0;
     while (currentInlineItemIndex < inlineItems.size()) {
         auto lineContent = placeInlineItems({ lineLogicalTop, widthConstraint, currentInlineItemIndex, inlineItems });
-        createDisplayRuns(*lineContent.runs, widthConstraint);
+        createDisplayRuns(*lineContent.runs, lineContent.floats, widthConstraint);
         // We should always put at least one run on the line atm. This might change later on though.
         ASSERT(lineContent.lastInlineItemIndex);
         currentInlineItemIndex = *lineContent.lastInlineItemIndex + 1;
@@ -297,8 +302,22 @@ LayoutUnit InlineFormattingContext::LineLayout::computedIntrinsicWidth(LayoutUni
     return std::max(maximumLineWidth, lineLogicalRight - trimmableTrailingWidth);
 }
 
-void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content& lineContent, LayoutUnit widthConstraint) const
+void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content& lineContent, const Vector<WeakPtr<InlineItem>>& floats, LayoutUnit widthConstraint) const
 {
+    auto floatingContext = FloatingContext { m_floatingState };
+
+    // Move floats to their final position.
+    for (auto floatItem : floats) {
+        auto& floatBox = floatItem->layoutBox();
+        ASSERT(layoutState().hasDisplayBox(floatBox));
+        auto& displayBox = layoutState().displayBoxForLayoutBox(floatBox);
+        // Set static position first.
+        displayBox.setTopLeft({ lineContent.logicalLeft(), lineContent.logicalTop() });
+        // Float it.
+        displayBox.setTopLeft(floatingContext.positionForFloat(floatBox));
+        m_floatingState.append(floatBox);
+    }
+
     if (lineContent.isEmpty()) {
         // Spec tells us to create a zero height, empty line box.
         auto lineBox = Display::Rect { lineContent.logicalTop(), lineContent.logicalLeft(), 0 , 0 };
@@ -398,21 +417,6 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
         alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, widthConstraint - lineContent.logicalWidth());
 }
 
-void InlineFormattingContext::LineLayout::handleFloat(Line& line, const FloatingContext& floatingContext, const InlineItem& floatItem) const
-{
-    auto& floatBox = floatItem.layoutBox();
-    ASSERT(layoutState().hasDisplayBox(floatBox));
-    auto& displayBox = layoutState().displayBoxForLayoutBox(floatBox);
-    // Set static position first.
-    displayBox.setTopLeft({ line.contentLogicalRight(), line.logicalTop() });
-    // Float it.
-    displayBox.setTopLeft(floatingContext.positionForFloat(floatBox));
-    m_floatingState.append(floatBox);
-    // Shrink availble space for current line and move existing inline runs.
-    auto floatBoxWidth = displayBox.marginBoxWidth();
-    floatBox.isLeftFloatingPositioned() ? line.moveLogicalLeft(floatBoxWidth) : line.moveLogicalRight(floatBoxWidth);
-}
-
 static Optional<LayoutUnit> horizontalAdjustmentForAlignment(TextAlignMode align, LayoutUnit remainingWidth)
 {
     switch (align) {
index a8a7869..85a3de3 100644 (file)
 #include "LayoutBox.h"
 #include "LayoutInlineBox.h"
 #include "LayoutLineBreakBox.h"
+#include <wtf/WeakPtr.h>
 
 namespace WebCore {
 namespace Layout {
 
-class InlineItem {
+class InlineItem : public CanMakeWeakPtr<InlineItem> {
 public:
     enum class Type { Text, HardLineBreak, Box, Float, ContainerStart, ContainerEnd };
     InlineItem(const Box& layoutBox, Type);