Changeset 268864 in webkit

Timestamp:

Oct 22, 2020, 7:43:57 AM (6 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Handle line box sizing quirks at the line box level
​https://bugs.webkit.org/show_bug.cgi?id=218060

Reviewed by Antti Koivisto.

In this patch the line box vertical sizing behavior (mostly quirk) is moved from the inline box to the line box level.

1. Inline boxes (root and other inline boxes (<span>)) are always sized to their initial height (see layout bounds) regardless of

whether they have content or not.

2. They are set to empty initially and we change them to non-empty as they gain (non-inline-level-box) content

(e.g. <div>root inline box gains content</div>).

3. Use this empty flag as input to the line box vertical sizing logic (only non-empty inline boxes contribute to the line box height).

Note that in standard mode the root inline box starts with an imaginary strut which makes it non-empty even when it has no content.
This is most visible with simple block containers like this:

<div style="border: 1px solid green"><img></div>

In quirks mode the border hugs the image (root inline box does not contribute to the height of the line box), while in standard mode
(assume 1. the image is taller than the default font with line spacing, 2. font has 4px descent),
there's a 4px gap between the border and the image.

4. While computing the final line rect (InlineFormattingContext::Geometry::computedLineLogicalRect), just rely on the line box height

computation (no need to check whether the line has content).

5. Treat <br> as visually non-empty run as it may contribute to the line box visually.
6. Add <br> quirk behavior so that it only contributes to the line box height when the line is empty.

• layout/inlineformatting/InlineFormattingContext.h:
• layout/inlineformatting/InlineFormattingContextGeometry.cpp:

(WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
(WebCore::Layout::LineBoxBuilder::alignInlineLevelBoxesVerticallyAndComputeLineBoxHeight):
(WebCore::Layout::InlineFormattingContext::Geometry::computedLineLogicalRect const):

• layout/inlineformatting/InlineFormattingContextQuirks.cpp:

(WebCore::Layout::InlineFormattingContext::Quirks::shouldInlineLevelBoxStretchLineBox const):

• layout/inlineformatting/InlineLine.cpp:

(WebCore::Layout::Line::isRunVisuallyNonEmpty const):

• layout/inlineformatting/InlineLineBox.h:

(WebCore::Layout::LineBox::InlineLevelBox::verticalAlign const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/inlineformatting/InlineFormattingContext.h (modified) (1 diff)
• layout/inlineformatting/InlineFormattingContextGeometry.cpp (modified) (7 diffs)
• layout/inlineformatting/InlineFormattingContextQuirks.cpp (modified) (1 diff)
• layout/inlineformatting/InlineLine.cpp (modified) (1 diff)
• layout/inlineformatting/InlineLineBox.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-22  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Handle line box sizing quirks at the line box level
+        https://bugs.webkit.org/show_bug.cgi?id=218060
+
+        Reviewed by Antti Koivisto.
+
+        In this patch the line box vertical sizing behavior (mostly quirk) is moved from the inline box to the line box level.
+
+        1. Inline boxes (root and other inline boxes (<span>)) are always sized to their initial height (see layout bounds) regardless of
+        whether they have content or not.
+        2. They are set to empty initially and we change them to non-empty as they gain (non-inline-level-box) content
+        (e.g. <div>root inline box gains content</div>).
+        3. Use this empty flag as input to the line box vertical sizing logic (only non-empty inline boxes contribute to the line box height).
+
+        Note that in standard mode the root inline box starts with an imaginary strut which makes it non-empty even when it has no content.
+        This is most visible with simple block containers like this:
+            <div style="border: 1px solid green"><img></div>
+        In quirks mode the border hugs the image (root inline box does not contribute to the height of the line box), while in standard mode
+        (assume 1. the image is taller than the default font with line spacing, 2. font has 4px descent),
+        there's a 4px gap between the border and the image.
+
+        4. While computing the final line rect (InlineFormattingContext::Geometry::computedLineLogicalRect), just rely on the line box height
+        computation (no need to check whether the line has content).
+        5. Treat <br> as visually non-empty run as it may contribute to the line box visually.
+        6. Add <br> quirk behavior so that it only contributes to the line box height when the line is empty.
+
+        * layout/inlineformatting/InlineFormattingContext.h:
+        * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
+        (WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
+        (WebCore::Layout::LineBoxBuilder::alignInlineLevelBoxesVerticallyAndComputeLineBoxHeight):
+        (WebCore::Layout::InlineFormattingContext::Geometry::computedLineLogicalRect const):
+        * layout/inlineformatting/InlineFormattingContextQuirks.cpp:
+        (WebCore::Layout::InlineFormattingContext::Quirks::shouldInlineLevelBoxStretchLineBox const):
+        * layout/inlineformatting/InlineLine.cpp:
+        (WebCore::Layout::Line::isRunVisuallyNonEmpty const):
+        * layout/inlineformatting/InlineLineBox.h:
+        (WebCore::Layout::LineBox::InlineLevelBox::verticalAlign const):
+
 2020-10-22  Antti Koivisto  <antti@apple.com>
 

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

@@ -51 +51 @@
         InlineLayoutUnit initialLineHeight() const;
         bool hasSoftWrapOpportunityAtImage() const;
+        bool shouldInlineLevelBoxStretchLineBox(const LineBox&, const LineBox::InlineLevelBox&) const;
 
     private:

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

@@ -214 +214 @@
         if (isInitiallyConsideredNonEmpty)
             rootInlineBox->setIsNonEmpty();
-        if (lineHasImaginaryStrut)
-            setVerticalGeometryForInlineBox(*rootInlineBox);
+        setVerticalGeometryForInlineBox(*rootInlineBox);
         lineBox.addRootInlineBox(WTFMove(rootInlineBox));
     };
@@ -252 +251 @@
     };
     createWrappedInlineBoxes();
-
-    auto stretchRootInlineBoxIfNeededQuirk = [&] (const auto& layoutBox) {
-        auto& parentInlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent());
-        if (!isRootInlineBox(parentInlineBox) || !parentInlineBox.isEmpty())
-            return;
-        setVerticalGeometryForInlineBox(parentInlineBox);
-        parentInlineBox.setIsNonEmpty();
-    };
 
     for (auto& run : runs) {
@@ -293 +284 @@
         } else if (run.isText() || run.isSoftLineBreak()) {
             // FIXME: Adjust non-empty inline box height when glyphs from the non-primary font stretch the box.
-            stretchRootInlineBoxIfNeededQuirk(layoutBox);
+            lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent()).setIsNonEmpty();
         } else if (run.isHardLineBreak()) {
             auto lineBreakBox = LineBox::InlineLevelBox::createLineBreakBox(layoutBox, logicalLeft);
             setVerticalGeometryForInlineBox(*lineBreakBox);
+            lineBreakBox->setIsNonEmpty();
             lineBox.addInlineLevelBox(WTFMove(lineBreakBox));
-            stretchRootInlineBoxIfNeededQuirk(layoutBox);
         } else if (run.isWordBreakOpportunity())
             lineBox.addInlineLevelBox(LineBox::InlineLevelBox::createGenericInlineLevelBox(layoutBox, logicalLeft));
@@ -315 +306 @@
         InlineLayoutUnit top { 0 };
         InlineLayoutUnit bottom { 0 };
+        const LineBox::InlineLevelBox* inlineLevelBox { nullptr };
     };
     HashMap<LineBox::InlineLevelBox*, AbsoluteTopAndBottom> absoluteLogicalTopAndBottomMap;
     auto& rootInlineBox = lineBox.rootInlineBox();
-    absoluteLogicalTopAndBottomMap.add(&rootInlineBox, AbsoluteTopAndBottom { { }, rootInlineBox.layoutBounds().height() });
+    absoluteLogicalTopAndBottomMap.add(&rootInlineBox, AbsoluteTopAndBottom { { }, rootInlineBox.layoutBounds().height(), &rootInlineBox });
 
     auto alignInlineBoxRelativeInlineLevelBoxes = [&] {
@@ -352 +344 @@
             auto parentAbsoluteLogicalTop = absoluteLogicalTopAndBottomMap.get(&parentInlineBox).top;
             auto absoluteLogicalTop = parentAbsoluteLogicalTop + logicalTop;
-            absoluteLogicalTopAndBottomMap.add(inlineLevelBox.get(), AbsoluteTopAndBottom { absoluteLogicalTop, absoluteLogicalTop + inlineLevelBox->layoutBounds().height() });
+            absoluteLogicalTopAndBottomMap.add(inlineLevelBox.get(), AbsoluteTopAndBottom { absoluteLogicalTop, absoluteLogicalTop + inlineLevelBox->layoutBounds().height(), inlineLevelBox.get() });
         }
     };
@@ -358 +350 @@
 
     auto lineBoxLogicalHeight = InlineLayoutUnit { };
-    auto minimumInlineBoxRelativeLogicalTop = InlineLayoutUnit { };
     auto inlineBoxRelativeLogicalHeight = InlineLayoutUnit { };
+    auto quirks = formattingContext().quirks();
+    auto maximumBaselineAlignedAscent = rootInlineBox.isEmpty() ? InlineLayoutUnit() : rootInlineBox.layoutBounds().ascent;
     auto computeLineBoxLogicalHeight = [&] {
         // FIXME: Add support for layout bounds based line box height.
-        auto minimumLogicalTop = InlineLayoutUnit { };
-        auto maximumlogicalBottom = InlineLayoutUnit { };
+        auto minimumLogicalTop = Optional<InlineLayoutUnit> { };
+        auto maximumLogicalBottom = Optional<InlineLayoutUnit> { };
         for (auto absoluteLogicalTopAndBottom : absoluteLogicalTopAndBottomMap.values()) {
-            minimumLogicalTop = std::min(minimumLogicalTop, absoluteLogicalTopAndBottom.top);
-            maximumlogicalBottom = std::max(maximumlogicalBottom, absoluteLogicalTopAndBottom.bottom);
-        }
-        minimumInlineBoxRelativeLogicalTop = minimumLogicalTop;
-        inlineBoxRelativeLogicalHeight = maximumlogicalBottom - minimumLogicalTop;
+            auto& inlineLevelBox = *absoluteLogicalTopAndBottom.inlineLevelBox;
+            if (!quirks.shouldInlineLevelBoxStretchLineBox(lineBox, inlineLevelBox))
+                continue;
+            minimumLogicalTop = std::min(minimumLogicalTop.valueOr(absoluteLogicalTopAndBottom.top), absoluteLogicalTopAndBottom.top);
+            maximumLogicalBottom = std::max(maximumLogicalBottom.valueOr(absoluteLogicalTopAndBottom.bottom), absoluteLogicalTopAndBottom.bottom);
+            if (inlineLevelBox.verticalAlign() == VerticalAlign::Baseline)
+                maximumBaselineAlignedAscent = std::max(maximumBaselineAlignedAscent, inlineLevelBox.layoutBounds().ascent);
+        }
+        inlineBoxRelativeLogicalHeight = maximumLogicalBottom.valueOr(InlineLayoutUnit()) - minimumLogicalTop.valueOr(InlineLayoutUnit());
         lineBoxLogicalHeight = inlineBoxRelativeLogicalHeight;
         // Now stretch the line box with the line box relative inline level boxes.
-        for (auto* lineBoxRelativeInlineLevelBox : lineBoxRelativeInlineLevelBoxes)
+        for (auto* lineBoxRelativeInlineLevelBox : lineBoxRelativeInlineLevelBoxes) {
+            if (!quirks.shouldInlineLevelBoxStretchLineBox(lineBox, *lineBoxRelativeInlineLevelBox))
+                continue;
             lineBoxLogicalHeight = std::max(lineBoxLogicalHeight, lineBoxRelativeInlineLevelBox->layoutBounds().height());
+        }
     };
     computeLineBoxLogicalHeight();
 
     auto adjustRootInlineBoxVerticalPosition = [&] {
-        if (minimumInlineBoxRelativeLogicalTop >= 0)
-            return;
-        rootInlineBox.setLogicalTop(-minimumInlineBoxRelativeLogicalTop);
+        // FIXME: Add support for cases when the stretching inline boxes are not baseline aligned.
+        auto logicalTop = maximumBaselineAlignedAscent - rootInlineBox.layoutBounds().ascent;
+        rootInlineBox.setLogicalTop(logicalTop);
     };
     adjustRootInlineBoxVerticalPosition();
@@ -418 +418 @@
 InlineRect InlineFormattingContext::Geometry::computedLineLogicalRect(const LineBox& lineBox, const LineBuilder::LineContent& lineContent) const
 {
-    auto isConsideredEmpty = lineContent.runs.isEmpty() || lineBox.isLineVisuallyEmpty();
-    return { lineContent.logicalTopLeft, lineContent.lineLogicalWidth, isConsideredEmpty ? InlineLayoutUnit() : lineBox.logicalHeight()};
+    return { lineContent.logicalTopLeft, lineContent.lineLogicalWidth, lineBox.logicalHeight() };
 }
 

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

@@ -41 +41 @@
 }
 
+bool InlineFormattingContext::Quirks::shouldInlineLevelBoxStretchLineBox(const LineBox& lineBox, const LineBox::InlineLevelBox& inlineLevelBox) const
+{
+    if (inlineLevelBox.isEmpty())
+        return false;
+    if (layoutState().inNoQuirksMode())
+        return true;
+    if (!inlineLevelBox.isLineBreakBox())
+        return true;
+    // <br> in non-standard mode stretches the line box only when the line is empty.
+    // e.g. <div><span><br></span></div> will stretch but <div>this will not stretch to 200px<span style="font-size: 200px;"><br></span></div>
+    return lineBox.isLineVisuallyEmpty();
+}
+
 bool InlineFormattingContext::Quirks::hasSoftWrapOpportunityAtImage() const
 {

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

@@ -354 +354 @@
 
     if (run.isLineBreak())
-        return true;
+        return false;
 
     // Note that this does not check whether the inline container has content. It simply checks if the container itself is considered non-empty.

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

@@ -81 +81 @@
 
         const FontMetrics& fontMetrics() const { return layoutBox().style().fontMetrics(); }
+        VerticalAlign verticalAlign() const { return layoutBox().style().verticalAlign(); }
         const Box& layoutBox() const { return *m_layoutBox; }