Changeset 246483 in webkit

Timestamp:

Jun 16, 2019, 1:19:24 PM (7 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Intruding float may prevent adding any inline box
​https://bugs.webkit.org/show_bug.cgi?id=198891
<rdar://problem/51779956>

Reviewed by Antti Koivisto.

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/FormattingContext.cpp (modified) (1 diff)
• layout/FormattingContext.h (modified) (1 diff)
• layout/LayoutUnits.h (modified) (1 diff)
• layout/inlineformatting/InlineFormattingContext.h (modified) (1 diff)
• layout/inlineformatting/InlineFormattingContextLineLayout.cpp (modified) (6 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-16  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 Antti Koivisto.
+
+        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-16  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/layout/FormattingContext.cpp

@@ -212 +212 @@
 }
 
-Point FormattingContext::mapPointToAncestor(const LayoutState& layoutState, Point position, const Container& containingBlock, const Container& ancestor)
-{
-    if (&containingBlock == &ancestor)
+Point FormattingContext::mapPointToAncestor(const LayoutState& layoutState, Point position, const Container& from, const Container& to)
+{
+    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;
 }
 

trunk/Source/WebCore/layout/FormattingContext.h

@@ -63 +63 @@
     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:

trunk/Source/WebCore/layout/LayoutUnits.h

@@ -60 +60 @@
     Point(LayoutUnit, LayoutUnit);
     Point(LayoutPoint);
+    static Point max() { return { LayoutUnit::max(), LayoutUnit::max() }; }
+
     void move(LayoutSize);
     void moveBy(LayoutPoint);

trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h

@@ -80 +80 @@
             unsigned firstInlineItemIndex { 0 };
             const InlineItems& inlineItems;
+            Optional<LayoutUnit> floatMinimumLogicalBottom;
         };
         LineContent placeInlineItems(const LineInput&) const;

trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp

@@ -162 +162 @@
     };
 
+    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;
@@ -176 +177 @@
 
         // 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();
@@ -201 +203 @@
             floats.append(makeWeakPtr(*inlineItem));
             ++committedInlineItemCount;
+            lineHasFloatBox = true;
             continue;
         }
@@ -223 +226 @@
     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.
@@ -237 +240 @@
             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 = floatMinimumLogicalBottom;
+
         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);
     };
 
@@ -257 +266 @@
     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();
+        }
     }
 }