Changeset 283390 in webkit

Timestamp:

Oct 1, 2021, 12:38:16 PM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Add a dedicated function to set the line break box's vertical geometry
​https://bugs.webkit.org/show_bug.cgi?id=231080

Reviewed by Antti Koivisto.

Line break boxes should use the parent inline box's font-metrics when computing vertical geometry.
It also makes the code read better as we don't use a function called "setInitialVerticalGeometryForInlineBox()" to set vertical geometry on a non-inline box (line break box).

• layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:

(WebCore::Layout::computedHeightAndLayoutBounds):
(WebCore::Layout::LineBoxBuilder::setVerticalGeometryForLineBreakBox const):
(WebCore::Layout::LineBoxBuilder::setInitialVerticalGeometryForInlineBox const):
(WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes):

• layout/formattingContexts/inline/InlineLineBoxBuilder.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/formattingContexts/inline/InlineLineBoxBuilder.cpp (modified) (3 diffs)
• layout/formattingContexts/inline/InlineLineBoxBuilder.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-10-01  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Add a dedicated function to set the line break box's vertical geometry
+        https://bugs.webkit.org/show_bug.cgi?id=231080
+
+        Reviewed by Antti Koivisto.
+
+        Line break boxes should use the parent inline box's font-metrics when computing vertical geometry.
+        It also makes the code read better as we don't use a function called "setInitialVerticalGeometryForInlineBox()" to set vertical geometry on a non-inline box (line break box).
+
+        * layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
+        (WebCore::Layout::computedHeightAndLayoutBounds):
+        (WebCore::Layout::LineBoxBuilder::setVerticalGeometryForLineBreakBox const):
+        (WebCore::Layout::LineBoxBuilder::setInitialVerticalGeometryForInlineBox const):
+        (WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes):
+        * layout/formattingContexts/inline/InlineLineBoxBuilder.h:
+
 2021-10-01  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp

@@ -193 +193 @@
 }
 
-void LineBoxBuilder::setInitialVerticalGeometryForInlineBox(InlineLevelBox& inlineLevelBox) const
-{
-    ASSERT(inlineLevelBox.isInlineBox() || inlineLevelBox.isLineBreakBox());
-    auto& primaryFontMetrics = inlineLevelBox.primaryFontMetrics();
-    InlineLayoutUnit ascent = primaryFontMetrics.ascent();
-    InlineLayoutUnit descent = primaryFontMetrics.descent();
+struct HeightAndLayoutBounds {
+    InlineLayoutUnit ascent { 0 };
+    InlineLayoutUnit descent { 0 };
+    InlineLevelBox::LayoutBounds layoutBounds { };
+};
+static auto computedHeightAndLayoutBounds(const FontMetrics& fontMetrics, std::optional<InlineLayoutUnit> preferredLineHeight)
+{
+    InlineLayoutUnit ascent = fontMetrics.ascent();
+    InlineLayoutUnit descent = fontMetrics.descent();
     auto logicalHeight = ascent + descent;
-    // We need floor/ceil to match legacy layout integral positioning.
-    inlineLevelBox.setBaseline(floorf(ascent));
-    inlineLevelBox.setDescent(ceilf(descent));
-    inlineLevelBox.setLogicalHeight(logicalHeight);
-
-    if (inlineLevelBox.isPreferredLineHeightFontMetricsBased()) {
+
+    if (preferredLineHeight) {
         // If line-height computes to normal and either text-edge is leading or this is the root inline box,
         // the font’s line gap metric may also be incorporated into A and D by adding half to each side as half-leading.
@@ -211 +210 @@
         // Since text-edge is not supported yet and the initial value is leading, we should just apply it to
         // all inline boxes.
-        auto halfLineGap = (primaryFontMetrics.lineSpacing() - logicalHeight) / 2;
-        ascent += halfLineGap;
-        descent += halfLineGap;
-    } else {
-        auto preferredLineHeight = inlineLevelBox.preferredLineHeight();
-        InlineLayoutUnit halfLeading = (preferredLineHeight - (ascent + descent)) / 2;
-        ascent += halfLeading;
-        descent += halfLeading;
+        auto halfLeading = (*preferredLineHeight - logicalHeight) / 2;
+        return HeightAndLayoutBounds { ascent, descent, { ascent + halfLeading, descent + halfLeading } };
     }
+    // Preferred line height is purely font metrics based (i.e glyphs stretch the line).
+    auto halfLineGap = (fontMetrics.lineSpacing() - logicalHeight) / 2;
+    return HeightAndLayoutBounds { ascent, descent, { ascent + halfLineGap, descent + halfLineGap } };
+}
+
+void LineBoxBuilder::setVerticalGeometryForLineBreakBox(InlineLevelBox& lineBreakBox, const InlineLevelBox& parentInlineBox) const
+{
     // We need floor/ceil to match legacy layout integral positioning.
-    inlineLevelBox.setLayoutBounds(InlineLevelBox::LayoutBounds { floorf(ascent), ceilf(descent) });
+    ASSERT(lineBreakBox.isLineBreakBox());
+    ASSERT(parentInlineBox.isInlineBox());
+
+    auto& fontMetrics = parentInlineBox.primaryFontMetrics();
+    auto preferredLineHeight = parentInlineBox.isPreferredLineHeightFontMetricsBased() ? std::nullopt : std::make_optional(parentInlineBox.preferredLineHeight());
+    auto heightAndLayoutBounds = computedHeightAndLayoutBounds(fontMetrics, preferredLineHeight);
+
+    lineBreakBox.setBaseline(floorf(heightAndLayoutBounds.ascent));
+    lineBreakBox.setDescent(ceilf(heightAndLayoutBounds.descent));
+    lineBreakBox.setLogicalHeight(heightAndLayoutBounds.ascent + heightAndLayoutBounds.descent);
+    lineBreakBox.setLayoutBounds({ floorf(heightAndLayoutBounds.layoutBounds.ascent), ceilf(heightAndLayoutBounds.layoutBounds.descent) });
+}
+
+void LineBoxBuilder::setInitialVerticalGeometryForInlineBox(InlineLevelBox& inlineBox) const
+{
+    // We need floor/ceil to match legacy layout integral positioning.
+    ASSERT(inlineBox.isInlineBox());
+
+    auto& fontMetrics = inlineBox.primaryFontMetrics();
+    auto preferredLineHeight = inlineBox.isPreferredLineHeightFontMetricsBased() ? std::nullopt : std::make_optional(inlineBox.preferredLineHeight());
+    auto heightAndLayoutBounds = computedHeightAndLayoutBounds(fontMetrics, preferredLineHeight);
+
+    inlineBox.setBaseline(floorf(heightAndLayoutBounds.ascent));
+    inlineBox.setDescent(ceilf(heightAndLayoutBounds.descent));
+    inlineBox.setLogicalHeight(heightAndLayoutBounds.ascent + heightAndLayoutBounds.descent);
+    inlineBox.setLayoutBounds({ floorf(heightAndLayoutBounds.layoutBounds.ascent), ceilf(heightAndLayoutBounds.layoutBounds.descent) });
 }
 
@@ -378 +403 @@
         if (run.isHardLineBreak()) {
             auto lineBreakBox = InlineLevelBox::createLineBreakBox(layoutBox, style, logicalLeft);
-            setInitialVerticalGeometryForInlineBox(lineBreakBox);
+            auto& parentInlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent());
+            setVerticalGeometryForLineBreakBox(lineBreakBox, parentInlineBox);
             updateCanUseSimplifiedAlignment(lineBreakBox);
             lineBox.addInlineLevelBox(WTFMove(lineBreakBox));

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.h

@@ -53 +53 @@
 private:
     void setInitialVerticalGeometryForInlineBox(InlineLevelBox&) const;
+    void setVerticalGeometryForLineBreakBox(InlineLevelBox& lineBreakBox, const InlineLevelBox& parentInlineBox) const;
     void adjustVerticalGeometryForInlineBoxWithFallbackFonts(InlineLevelBox&, const TextUtil::FallbackFontList&) const;
     InlineLayoutUnit constructAndAlignInlineLevelBoxes(LineBox&, const Line::RunList&, size_t lineIndex);