Attachment 371280 Details for Bug 198528 – Patch

[patch] Patch

bug-198528-20190604074552.patch (text/plain), 8.87 KB, created by zalan on 2019-06-04 07:45:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-04 07:45:53 PDT

Size: 8.87 KB

patch

obsolete

>Subversion Revision: 246027
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d1f71cd4bd0eed8e8556df67a2e987bde91cdbc5..bc48ac61ee62e1eb06c94d7d3de1c0386031c51f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,23 @@
>+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 NOBODY (OOPS!).
>+
>+        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-03  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Add hard line break handling to LineBreaker
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>index 28fbad34984b067a36539a6932282e6353e5fde8..ac761f6733dceda60cba38ec4e834fa49f10e407 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>@@ -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:
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index b93cd9e746be4a8366d86904d4e6de8e822e1ffe..8c7884ad614f4fa42455940fecb342223b370491 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -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) {
>diff --git a/Source/WebCore/layout/inlineformatting/InlineItem.h b/Source/WebCore/layout/inlineformatting/InlineItem.h
>index a8a78699fb3f71c3e5215481c39680baa5705c1d..85a3de3b57e36d9f3e783db79f8e5d0a814d6edd 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineItem.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineItem.h
>@@ -30,11 +30,12 @@
> #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);

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 198528: 371280