Attachment 372199 Details for Bug 198891 – Patch

[patch] Patch

bug-198891-20190615144037.patch (text/plain), 12.63 KB, created by zalan on 2019-06-15 14:40:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-15 14:40:37 PDT

Size: 12.63 KB

patch

obsolete

>Subversion Revision: 246468
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ca2b6fc3c4e2f53777e8cc6d9c74444e05fdcb1e..5e48abc2f0d7b3a0a906254d422b9ed09b014b9b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,25 @@
>+2019-06-15  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] Intruding float may prevent adding any inline box
>+        https://bugs.webkit.org/show_bug.cgi?id=198891
>+        <rdar://problem/51779956>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Take the intruding left/right float pair and find the vertical position where the next line might go
>+        if these floats prevent us from adding even one inline box to the current line.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::mapPointToAncestor):
>+        (WebCore::Layout::FormattingContext::mapPointToDescendent):
>+        * layout/FormattingContext.h:
>+        * layout/LayoutUnits.h:
>+        (WebCore::Layout::Point::max):
>+        * layout/inlineformatting/InlineFormattingContext.h:
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::placeInlineItems const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::layout const):
>+
> 2019-06-15  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floats] Add bottom value to FloatingState::Constraints
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 9d5f878f4ff3385e6dec0f0bdf6aed1c26a4dba8..ff653743f0d16c01f160286df7977bfa55ff4b77 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -211,17 +211,28 @@ LayoutUnit FormattingContext::mapTopToAncestor(const LayoutState& layoutState, c
>     return top;
> }
> 
>-Point FormattingContext::mapPointToAncestor(const LayoutState& layoutState, Point position, const Container& containingBlock, const Container& ancestor)
>+Point FormattingContext::mapPointToAncestor(const LayoutState& layoutState, Point position, const Container& from, const Container& to)
> {
>-    if (&containingBlock == &ancestor)
>+    if (&from == &to)
>         return position;
>-    ASSERT(containingBlock.isContainingBlockDescendantOf(ancestor));
>+    ASSERT(from.isContainingBlockDescendantOf(to));
>     auto mappedPosition = position;
>-    for (auto* container = &containingBlock; container && container != &ancestor; container = container->containingBlock())
>+    for (auto* container = &from; container && container != &to; container = container->containingBlock())
>         mappedPosition.moveBy(layoutState.displayBoxForLayoutBox(*container).topLeft());
>     return mappedPosition;
> }
> 
>+Point FormattingContext::mapPointToDescendent(const LayoutState& layoutState, Point point, const Container& from, const Container& to)
>+{
>+    // "point" is in the coordinate system of the "from" container.
>+    if (&from == &to)
>+        return point;
>+    ASSERT(to.isContainingBlockDescendantOf(from));
>+    for (auto* container = &to; container && container != &from; container = container->containingBlock())
>+        point.moveBy(-layoutState.displayBoxForLayoutBox(*container).topLeft());
>+    return point;
>+}
>+
> #ifndef NDEBUG
> void FormattingContext::validateGeometryConstraintsAfterLayout() const
> {
>diff --git a/Source/WebCore/layout/FormattingContext.h b/Source/WebCore/layout/FormattingContext.h
>index 85781199697dd491e2d7e4cc1855877ca3be682e..69537fbc8202491d7355e55a0a5e61bbba276512 100644
>--- a/Source/WebCore/layout/FormattingContext.h
>+++ b/Source/WebCore/layout/FormattingContext.h
>@@ -62,7 +62,8 @@ public:
>     static LayoutUnit mapTopToAncestor(const LayoutState&, const Box&, const Container& ancestor);
>     static LayoutUnit mapLeftToAncestor(const LayoutState&, const Box&, const Container& ancestor);
>     static LayoutUnit mapRightToAncestor(const LayoutState&, const Box&, const Container& ancestor);
>-    static Point mapPointToAncestor(const LayoutState&, Point, const Container& containingBlock, const Container& ancestor);
>+    static Point mapPointToAncestor(const LayoutState&, Point, const Container& from, const Container& to);
>+    static Point mapPointToDescendent(const LayoutState&, Point, const Container& from, const Container& to);
> 
> protected:
>     using LayoutQueue = Vector<const Box*>;
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index 24193145f9a97ba1c4f3b6d330a7a1f7830dbb2d..ad53730d1729852bd0de06baa845b073609adbfc 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -59,6 +59,8 @@ struct Point {
>     Point() = default;
>     Point(LayoutUnit, LayoutUnit);
>     Point(LayoutPoint);
>+    static Point max() { return Point(LayoutUnit::max(), LayoutUnit::max()); }
>+
>     void moveBy(LayoutPoint);
>     void moveBy(LayoutSize);
>     operator LayoutPoint() const { return { x, y }; }
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>index 561e38ed0e8fde8a7348950ef40e8b6d8ed8b2d9..fd60d5b9b5fe04d45dfbc0564933d2e89e167d9d 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>@@ -79,6 +79,7 @@ private:
>             SkipVerticalAligment skipVerticalAligment;
>             unsigned firstInlineItemIndex { 0 };
>             const InlineItems& inlineItems;
>+            Optional<LayoutUnit> floatMinimumLogicalBottom;
>         };
>         LineContent placeInlineItems(const LineInput&) const;
>         void createDisplayRuns(const Line::Content&, const Vector<WeakPtr<InlineItem>>& floats, LayoutUnit widthConstraint) const;
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index 62ce23b4d684a14c6513dd78e898b14fc99e3dcb..a08ac41af105500c8ca4c25c4f95b299cbfa5fed 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -161,10 +161,11 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>         uncommittedContent.reset();
>     };
> 
>+    auto lineHasFloatBox = lineInput.floatMinimumLogicalBottom.hasValue();
>     auto closeLine = [&] {
>-        // This might change at some point.
>-        ASSERT(committedInlineItemCount);
>-        return LineContent { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1), WTFMove(floats), line->close() };
>+        ASSERT(committedInlineItemCount || lineHasFloatBox);
>+        auto lastCommittedIndex = committedInlineItemCount ? Optional<unsigned> { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1) } : WTF::nullopt;
>+        return LineContent { lastCommittedIndex, WTFMove(floats), line->close() };
>     };
>     LineBreaker lineBreaker;
>     // Iterate through the inline content and place the inline boxes on the current line.
>@@ -175,7 +176,8 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>         auto itemLogicalWidth = inlineItemWidth(layoutState(), *inlineItem, currentLogicalRight);
> 
>         // FIXME: Ensure LineContext::trimmableWidth includes uncommitted content if needed.
>-        auto breakingContext = lineBreaker.breakingContext(*inlineItem, itemLogicalWidth, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>+        auto lineIsConsideredEmpty = !line->hasContent() && !lineHasFloatBox;
>+        auto breakingContext = lineBreaker.breakingContext(*inlineItem, itemLogicalWidth, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), lineIsConsideredEmpty });
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
> 
>@@ -200,6 +202,7 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>             floatBox.isLeftFloatingPositioned() ? line->moveLogicalLeft(floatBoxWidth) : line->moveLogicalRight(floatBoxWidth);
>             floats.append(makeWeakPtr(*inlineItem));
>             ++committedInlineItemCount;
>+            lineHasFloatBox = true;
>             continue;
>         }
> 
>@@ -222,12 +225,12 @@ void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) con
>     auto lineLogicalTop = formattingRootDisplayBox.contentBoxTop();
>     auto lineLogicalLeft = formattingRootDisplayBox.contentBoxLeft();
> 
>-    auto applyFloatConstraint = [&](auto& lineHorizontalConstraint) {
>+    auto applyFloatConstraint = [&](auto& lineInput) {
>         // Check for intruding floats and adjust logical left/available width for this line accordingly.
>         if (m_floatingState.isEmpty())
>             return;
>-        auto availableWidth = lineHorizontalConstraint.availableLogicalWidth;
>-        auto lineLogicalLeft = lineHorizontalConstraint.logicalTopLeft.x();
>+        auto availableWidth = lineInput.horizontalConstraint.availableLogicalWidth;
>+        auto lineLogicalLeft = lineInput.horizontalConstraint.logicalTopLeft.x();
>         auto floatConstraints = m_floatingState.constraints({ lineLogicalTop }, m_formattingRoot);
>         // Check if these constraints actually put limitation on the line.
>         if (floatConstraints.left && floatConstraints.left->x <= formattingRootDisplayBox.contentBoxLeft())
>@@ -236,33 +239,44 @@ void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) con
>         if (floatConstraints.right && floatConstraints.right->x >= formattingRootDisplayBox.contentBoxRight())
>             floatConstraints.right = { };
> 
>+        // Set the minimum float bottom value as a hint for the next line if needed.
>+        static auto inifitePoint = PointInContextRoot::max();
>+        auto floatMinimumLogicalBottom = std::min(floatConstraints.left.valueOr(inifitePoint).y, floatConstraints.right.valueOr(inifitePoint).y);
>+        if (floatMinimumLogicalBottom != inifitePoint.y)
>+            lineInput.floatMinimumLogicalBottom = FormattingContext::mapPointToDescendent(layoutState(), { 0, floatMinimumLogicalBottom }, downcast<Container>(m_floatingState.root()), m_formattingRoot).y;
>+
>         if (floatConstraints.left && floatConstraints.right) {
>-            ASSERT(floatConstraints.left->x < floatConstraints.right->x);
>+            ASSERT(floatConstraints.left->x <= floatConstraints.right->x);
>             availableWidth = floatConstraints.right->x - floatConstraints.left->x;
>             lineLogicalLeft = floatConstraints.left->x;
>         } else if (floatConstraints.left) {
>-            ASSERT(floatConstraints.left->x > lineLogicalLeft);
>+            ASSERT(floatConstraints.left->x >= lineLogicalLeft);
>             availableWidth -= (floatConstraints.left->x - lineLogicalLeft);
>             lineLogicalLeft = floatConstraints.left->x;
>         } else if (floatConstraints.right) {
>-            ASSERT(floatConstraints.right->x > lineLogicalLeft);
>+            ASSERT(floatConstraints.right->x >= lineLogicalLeft);
>             availableWidth = floatConstraints.right->x - lineLogicalLeft;
>         }
>-        lineHorizontalConstraint.availableLogicalWidth = availableWidth;
>-        lineHorizontalConstraint.logicalTopLeft.setX(lineLogicalLeft);
>+        lineInput.horizontalConstraint.availableLogicalWidth = availableWidth;
>+        lineInput.horizontalConstraint.logicalTopLeft.setX(lineLogicalLeft);
>     };
> 
>     auto& inlineItems = m_formattingState.inlineItems();
>     unsigned currentInlineItemIndex = 0;
>     while (currentInlineItemIndex < inlineItems.size()) {
>         auto lineInput = LineInput { { lineLogicalLeft, lineLogicalTop }, widthConstraint, LineInput::SkipVerticalAligment::No, currentInlineItemIndex, inlineItems };
>-        applyFloatConstraint(lineInput.horizontalConstraint);
>+        applyFloatConstraint(lineInput);
>         auto lineContent = placeInlineItems(lineInput);
>         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;
>-        lineLogicalTop = lineContent.runs->logicalBottom();
>+        if (!lineContent.lastInlineItemIndex) {
>+            // Floats prevented us putting any content on the line.
>+            ASSERT(lineInput.floatMinimumLogicalBottom);
>+            ASSERT(lineContent.runs->isEmpty());
>+            lineLogicalTop = *lineInput.floatMinimumLogicalBottom;
>+        } else {
>+            currentInlineItemIndex = *lineContent.lastInlineItemIndex + 1;
>+            lineLogicalTop = lineContent.runs->logicalBottom();
>+        }
>     }
> }
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 198891: 372199