Changeset 266978 in webkit

Timestamp:

Sep 12, 2020 10:07:38 AM (4 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Add support for non-root-inline-box line spacing
​https://bugs.webkit.org/show_bug.cgi?id=216433

Reviewed by Antti Koivisto.

Source/WebCore:

While the LineBox vertically contains all the LineBox::InlineBoxes, its height value does not
include any line spacing (FontMetrics::lineSpacing).
Each LineBox::InlineBox may have a different line spacing value (this value is zero in many cases).
This patch collects line spacing values from the LineBox::InlineBoxes on the line and adjusts the final
line height value accordingly.

Test: fast/layoutformattingcontext/inline-box-with-line-spacing-simple.html

• layout/inlineformatting/InlineFormattingContext.cpp:

(WebCore::Layout::InlineFormattingContext::computedLineLogicalRect const):

• layout/inlineformatting/InlineLineBox.cpp:

(WebCore::Layout::LineBox::constructInlineBoxes):

• layout/inlineformatting/InlineLineBox.h:

(WebCore::Layout::LineBox::InlineBox::lineSpacing const):
(WebCore::Layout::LineBox::InlineBox::setLineSpacing):
(WebCore::Layout::LineBox::inlineBoxList const):

LayoutTests:

• fast/layoutformattingcontext/inline-box-with-line-spacing-simple-expected.html: Added.
• fast/layoutformattingcontext/inline-box-with-line-spacing-simple.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/layoutformattingcontext/inline-box-with-line-spacing-simple-expected.html (added)
• LayoutTests/fast/layoutformattingcontext/inline-box-with-line-spacing-simple.html (added)
• LayoutTests/platform/win/TestExpectations (modified) (2 diffs)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp (modified) (1 diff)
• Source/WebCore/layout/inlineformatting/InlineLineBox.cpp (modified) (1 diff)
• Source/WebCore/layout/inlineformatting/InlineLineBox.h (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2020-09-12  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Add support for non-root-inline-box line spacing
+        https://bugs.webkit.org/show_bug.cgi?id=216433
+
+        Reviewed by Antti Koivisto.
+
+        * fast/layoutformattingcontext/inline-box-with-line-spacing-simple-expected.html: Added.
+        * fast/layoutformattingcontext/inline-box-with-line-spacing-simple.html: Added.
+
 2020-09-12  Rob Buis  <rbuis@igalia.com>
 

trunk/LayoutTests/platform/win/TestExpectations

@@ -3692 +3692 @@
 fast/forms/file/entries-api/webkitdirectory-open-panel.html [ Skip ]
 
+fast/layoutformattingcontext [ Skip ]
+
 # Requires WK2 loading support
 http/wpt/fetch/dnt-header-after-redirection.html [ Skip ]
@@ -4446 +4448 @@
 webkit.org/b/204794 webanimations/accelerated-animation-removal-upon-transition-completion.html [ Timeout ]
 
-webkit.org/b/204795 fast/layoutformattingcontext/flow-integration-basic.html [ Failure ]
-
 webkit.org/b/204974 http/tests/xmlhttprequest/sync-xhr-in-unload.html [ Failure ]
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-09-12  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Add support for non-root-inline-box line spacing
+        https://bugs.webkit.org/show_bug.cgi?id=216433
+
+        Reviewed by Antti Koivisto.
+
+        While the LineBox vertically contains all the LineBox::InlineBoxes, its height value does not
+        include any line spacing (FontMetrics::lineSpacing).
+        Each LineBox::InlineBox may have a different line spacing value (this value is zero in many cases).
+        This patch collects line spacing values from the LineBox::InlineBoxes on the line and adjusts the final
+        line height value accordingly.
+
+        Test: fast/layoutformattingcontext/inline-box-with-line-spacing-simple.html
+
+        * layout/inlineformatting/InlineFormattingContext.cpp:
+        (WebCore::Layout::InlineFormattingContext::computedLineLogicalRect const):
+        * layout/inlineformatting/InlineLineBox.cpp:
+        (WebCore::Layout::LineBox::constructInlineBoxes):
+        * layout/inlineformatting/InlineLineBox.h:
+        (WebCore::Layout::LineBox::InlineBox::lineSpacing const):
+        (WebCore::Layout::LineBox::InlineBox::setLineSpacing):
+        (WebCore::Layout::LineBox::inlineBoxList const):
+
 2020-09-12  Myles C. Maxfield  <mmaxfield@apple.com>
 

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

@@ -410 +410 @@
     // |____________________v_______________________|  scrollable overflow
     //
-    if (lineContent.runs.isEmpty() || lineContent.lineBox.isLineVisuallyEmpty())
+    auto& lineBox = lineContent.lineBox;
+    if (lineContent.runs.isEmpty() || lineBox.isLineVisuallyEmpty())
         return { { }, { lineContent.logicalTopLeft, lineContent.lineLogicalWidth, { } } };
+
     auto& rootStyle = root().style();
-    auto& fontMetrics = rootStyle.fontMetrics();
-    InlineLayoutUnit lineSpacing = fontMetrics.lineSpacing();
-    auto& lineBox = lineContent.lineBox;
-    auto lineLogicalHeight = rootStyle.lineHeight().isNegative() ? std::max(lineBox.logicalHeight(), lineSpacing) : rootStyle.computedLineHeight();
+    auto lineLogicalHeight = InlineLayoutUnit { };
+    if (rootStyle.lineHeight().isNegative()) {
+        // Negative line height value means the line height is driven by the content.
+        auto usedLineSpacing = [&] {
+            auto lineBoxLogicalHeight = lineBox.logicalHeight();
+            auto logicalTopWithLineSpacing = InlineLayoutUnit { };
+            auto logicalBottomWithLineSpacing = lineBoxLogicalHeight;
+            for (auto& inlineBox : lineBox.inlineBoxList()) {
+                if (auto lineSpacing = inlineBox->lineSpacing()) {
+                    // FIXME: check if line spacing is distributed evenly.
+                    logicalTopWithLineSpacing = std::min(logicalTopWithLineSpacing, inlineBox->logicalTop() - *lineSpacing / 2);
+                    logicalBottomWithLineSpacing = std::max(logicalBottomWithLineSpacing, inlineBox->logicalBottom() + *lineSpacing / 2);
+                }
+            }
+            return -logicalTopWithLineSpacing + (logicalBottomWithLineSpacing - lineBoxLogicalHeight);
+        };
+        lineLogicalHeight = lineBox.logicalHeight() + usedLineSpacing();
+    } else
+        lineLogicalHeight = rootStyle.computedLineHeight();
+
     auto logicalRect = Display::InlineRect { lineContent.logicalTopLeft, lineContent.lineLogicalWidth, lineLogicalHeight};
-
     auto halfLeadingOffset = [&] {
+        auto& fontMetrics = rootStyle.fontMetrics();
         InlineLayoutUnit ascent = fontMetrics.ascent();
         InlineLayoutUnit descent = fontMetrics.descent();

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

@@ -165 +165 @@
         inlineBox.setBaseline(baseline);
         inlineBox.setDescent(logicalHeight - baseline);
+        if (auto lineSpacing = fontMetrics.lineSpacing() - logicalHeight)
+            inlineBox.setLineSpacing(lineSpacing);
     };
 

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

@@ -77 +77 @@
         void setIsNonEmpty() { m_isEmpty = false; }
 
+        Optional<InlineLayoutUnit> lineSpacing() const { return m_lineSpacing; }
         const FontMetrics& fontMetrics() const { return layoutBox().style().fontMetrics(); }
         const Box& layoutBox() const { return *m_layoutBox; }
@@ -88 +89 @@
         void setBaseline(InlineLayoutUnit baseline) { m_baseline = baseline; }
         void setDescent(InlineLayoutUnit descent) { m_descent = descent; }
+        void setLineSpacing(InlineLayoutUnit lineSpacing) { m_lineSpacing = lineSpacing; }
 
         WeakPtr<const Box> m_layoutBox;
@@ -93 +95 @@
         InlineLayoutUnit m_baseline { 0 };
         Optional<InlineLayoutUnit> m_descent;
+        Optional<InlineLayoutUnit> m_lineSpacing;
         bool m_isEmpty { true };
     };
@@ -109 +112 @@
     const InlineBox& inlineBoxForLayoutBox(const Box& layoutBox) const { return *m_inlineBoxRectMap.get(&layoutBox); }
     Display::InlineRect logicalRectForTextRun(const Line::Run&) const;
+
+    using InlineBoxMap = HashMap<const Box*, InlineBox*>;
+    auto inlineBoxList() const { return m_inlineBoxRectMap.values(); }
 
     InlineLayoutUnit alignmentBaseline() const { return m_rootInlineBox.logicalTop() + m_rootInlineBox.baseline(); }
@@ -130 +136 @@
 
     Optional<InlineLayoutUnit> m_horizontalAlignmentOffset;
-    HashMap<const Box*, InlineBox*> m_inlineBoxRectMap;
+    InlineBoxMap m_inlineBoxRectMap;
     Vector<std::unique_ptr<InlineBox>> m_inlineBoxList;
     const InlineFormattingContext& m_inlineFormattingContext;