Changeset 268922 in webkit

Timestamp:

Oct 23, 2020, 5:44:44 AM (6 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Fix inline box relative alignment when line-height is set
​https://bugs.webkit.org/show_bug.cgi?id=218107

Reviewed by Antti Koivisto.

Inline box relative vertical alignment logic should use the combination of the layout bounds
and the font metric values of the parent inline box.

The layout bound values (ascent and descent adjusted with half leading) normally match the font metric values,
unless the line-height is set to something other than normal.
In such cases the gap affects the vertical position of the child inline level boxes (e.g. text-top aligns with the
top of the text (use font metrics) and not with the layout bounds).

• layout/inlineformatting/InlineFormattingContextGeometry.cpp:

(WebCore::Layout::LineBoxBuilder::alignInlineLevelBoxesVerticallyAndComputeLineBoxHeight):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/inlineformatting/InlineFormattingContextGeometry.cpp (modified) (7 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-23  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Fix inline box relative alignment when line-height is set
+        https://bugs.webkit.org/show_bug.cgi?id=218107
+
+        Reviewed by Antti Koivisto.
+
+        Inline box relative vertical alignment logic should use the combination of the layout bounds
+        and the font metric values of the parent inline box.
+
+        The layout bound values (ascent and descent adjusted with half leading) normally match the font metric values,
+        unless the line-height is set to something other than normal.
+        In such cases the gap affects the vertical position of the child inline level boxes (e.g. text-top aligns with the
+        top of the text (use font metrics) and not with the layout bounds).
+
+        * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
+        (WebCore::Layout::LineBoxBuilder::alignInlineLevelBoxesVerticallyAndComputeLineBoxHeight):
+
 2020-10-21  Sergio Villar Senin  <svillar@igalia.com>
 

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

@@ -302 +302 @@
     // 2.2. Layout Within Line Boxes
     // https://www.w3.org/TR/css-inline-3/#line-layout
+    auto quirks = formattingContext().quirks();
     Vector<LineBox::InlineLevelBox*> lineBoxRelativeInlineLevelBoxes;
     struct AbsoluteTopAndBottom {
@@ -311 +312 @@
     auto& rootInlineBox = lineBox.rootInlineBox();
     absoluteLogicalTopAndBottomMap.add(&rootInlineBox, AbsoluteTopAndBottom { { }, rootInlineBox.layoutBounds().height(), &rootInlineBox });
+    auto maximumOffsetFromRootInlineBoxBaseline = rootInlineBox.isEmpty() ? InlineLayoutUnit() : rootInlineBox.layoutBounds().ascent;
 
     auto alignInlineBoxRelativeInlineLevelBoxes = [&] {
@@ -324 +326 @@
             auto& parentInlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox.parent());
             auto logicalTop = InlineLayoutUnit { };
+            auto offsetFromParentInlineBoxBaseline = InlineLayoutUnit { };
             switch (verticalAlignment) {
             case VerticalAlign::Baseline:
-                logicalTop = parentInlineBox.baseline() - inlineLevelBox->baseline();
-                break;
-            case VerticalAlign::TextTop:
-                logicalTop = { };
-                break;
-            case VerticalAlign::TextBottom:
-                logicalTop = parentInlineBox.layoutBounds().height() - inlineLevelBox->layoutBounds().height();
-                break;
+                offsetFromParentInlineBoxBaseline = inlineLevelBox->layoutBounds().ascent;
+                logicalTop = parentInlineBox.baseline() - offsetFromParentInlineBoxBaseline;
+                break;
+            case VerticalAlign::TextTop: {
+                // Note that TextTop aligns with the inline box's font metrics top (ascent) and not the layout bounds top.
+                auto parentAscent = parentInlineBox.fontMetrics().ascent();
+                auto parentInlineBoxLogicalTop = parentInlineBox.layoutBounds().ascent - parentAscent;
+                logicalTop = parentInlineBoxLogicalTop;
+                offsetFromParentInlineBoxBaseline = parentAscent;
+                break;
+            }
+            case VerticalAlign::TextBottom: {
+                // Note that TextBottom aligns with the inline box's font metrics bottom (descent) and not the layout bounds bottom.
+                auto& parentFontMetrics = parentInlineBox.fontMetrics();
+                auto parentInlineBoxLayoutBounds = parentInlineBox.layoutBounds();
+                auto parentInlineBoxLogicalBottom = parentInlineBoxLayoutBounds.height() - parentInlineBoxLayoutBounds.descent + parentFontMetrics.descent();
+                logicalTop = parentInlineBoxLogicalBottom - inlineLevelBox->logicalHeight();
+                offsetFromParentInlineBoxBaseline = inlineLevelBox->logicalHeight() - parentFontMetrics.descent();
+                break;
+            }
             case VerticalAlign::Middle:
-                logicalTop = parentInlineBox.baseline() - (inlineLevelBox->layoutBounds().height() / 2 + parentInlineBox.fontMetrics().xHeight() / 2);
+                offsetFromParentInlineBoxBaseline = (inlineLevelBox->layoutBounds().height() / 2 + parentInlineBox.fontMetrics().xHeight() / 2);
+                logicalTop = parentInlineBox.baseline() - offsetFromParentInlineBoxBaseline;
                 break;
             default:
@@ -345 +361 @@
             auto absoluteLogicalTop = parentAbsoluteLogicalTop + logicalTop;
             absoluteLogicalTopAndBottomMap.add(inlineLevelBox.get(), AbsoluteTopAndBottom { absoluteLogicalTop, absoluteLogicalTop + inlineLevelBox->layoutBounds().height(), inlineLevelBox.get() });
+
+            auto affectsRootInlineBoxVerticalPostion = isRootInlineBox(parentInlineBox) && quirks.shouldInlineLevelBoxStretchLineBox(lineBox, *inlineLevelBox);
+            if (affectsRootInlineBoxVerticalPostion)
+                maximumOffsetFromRootInlineBoxBaseline = std::max(maximumOffsetFromRootInlineBoxBaseline, offsetFromParentInlineBoxBaseline);
         }
     };
@@ -351 +371 @@
     auto lineBoxLogicalHeight = 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.
@@ -363 +381 @@
             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());
@@ -379 +395 @@
     auto adjustRootInlineBoxVerticalPosition = [&] {
         // FIXME: Add support for cases when the stretching inline boxes are not baseline aligned.
-        auto logicalTop = maximumBaselineAlignedAscent - rootInlineBox.layoutBounds().ascent;
-        rootInlineBox.setLogicalTop(logicalTop);
+        auto rootInlineBoxLogicalTop = maximumOffsetFromRootInlineBoxBaseline - rootInlineBox.layoutBounds().ascent;
+        rootInlineBox.setLogicalTop(rootInlineBoxLogicalTop);
     };
     adjustRootInlineBoxVerticalPosition();