Changeset 285784 in webkit

Timestamp:

Nov 13, 2021, 3:28:17 PM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Inline box end's padding/border/margin should be taken into account when computing horizontal position for bidi content
​https://bugs.webkit.org/show_bug.cgi?id=233083

Reviewed by Antti Koivisto.

Let's decouple the "display box rect" and the "content right in visual order" computation.
There are runs (e.g. inline box end) that don't need to call displayBoxRect() but they
still affect the "content right in visual order" (<span style="border-right: 10px solid green">).

• layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:

(WebCore::Layout::InlineDisplayContentBuilder::createBoxesAndUpdateGeometryForLineContent): add the additional lineRun.isInlineBoxEnd() case.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp (modified) (8 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-11-13  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Inline box end's padding/border/margin should be taken into account when computing horizontal position for bidi content
+        https://bugs.webkit.org/show_bug.cgi?id=233083
+
+        Reviewed by Antti Koivisto.
+
+        Let's decouple the "display box rect" and the "content right in visual order" computation.
+        There are runs (e.g. inline box end) that don't need to call displayBoxRect() but they
+        still affect the "content right in visual order" (<span style="border-right: 10px solid green">).
+
+        * layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:
+        (WebCore::Layout::InlineDisplayContentBuilder::createBoxesAndUpdateGeometryForLineContent): add the additional lineRun.isInlineBoxEnd() case.
+
 2021-11-13  Alan Bujtas  <zalan@apple.com>
 

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

@@ -104 +104 @@
             else {
                 auto& boxGeometry = formattingState.boxGeometry(layoutBox);
-                marginStart = boxGeometry.marginStart();
-                if (lineRun.isBox())
+                if (lineRun.isBox()) {
+                    marginStart = boxGeometry.marginStart();
                     logicalRect = lineBox.logicalBorderBoxForAtomicInlineLevelBox(layoutBox, boxGeometry);
-                else if (lineRun.isInlineBoxStart() || lineRun.isLineSpanningInlineBoxStart())
+                } else if (lineRun.isInlineBoxStart()) {
+                    marginStart = boxGeometry.marginStart();
+                    logicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
+                } else if (lineRun.isLineSpanningInlineBoxStart())
                     logicalRect = lineBox.logicalBorderBoxForInlineBox(layoutBox, boxGeometry);
                 else
@@ -124 +127 @@
             auto contentLeft = contentRightInVisualOrder + distanceFromLogicalPreviousRun + marginStart.value_or(0);
             visualOrderRect.setLeft(contentLeft);
-            // The inline box right edge includes its content as well as the inline box end (padding-right etc).
-            // What we need here is the inline box start run's width.
-            // Note that content width does not refer to content _box_ here (it does include padding and border for inline level boxes).
-            auto contentWidth = lineRun.isInlineBoxStart() || lineRun.isLineSpanningInlineBoxStart() ? lineRun.logicalWidth() - *marginStart : logicalRect.width();
-            contentRightInVisualOrder = visualOrderRect.left() + contentWidth;
             return visualOrderRect;
         };
@@ -134 +132 @@
         if (lineRun.isText()) {
             auto textRunRect = displayBoxRect();
+            contentRightInVisualOrder = textRunRect.right();
+
             auto inkOverflow = [&] {
                 auto initialContaingBlockSize = RuntimeEnabledFeatures::sharedFeatures().layoutFormattingContextIntegrationEnabled()
@@ -191 +191 @@
             ASSERT(layoutBox.isAtomicInlineLevelBox());
             auto borderBoxRect = displayBoxRect();
+            contentRightInVisualOrder = borderBoxRect.right();
             // FIXME: Add ink overflow support for atomic inline level boxes (e.g. box shadow).
             boxes.append({ lineIndex, InlineDisplay::Box::Type::AtomicInlineLevelBox, layoutBox, lineRun.bidiLevel(), borderBoxRect, borderBoxRect, lineRun.expansion(), { } });
@@ -211 +212 @@
         }
         if (lineRun.isInlineBoxStart()) {
+            auto& boxGeometry = formattingState.boxGeometry(layoutBox);
             // This inline box showed up first on this line.
             auto inlineBoxBorderBox = displayBoxRect();
+            contentRightInVisualOrder += lineRun.logicalWidth();
             if (lineBox.hasContent()) {
                 // FIXME: It's expected to not have any boxes on empty lines. We should reconsider this.
@@ -223 +226 @@
             }
 
-            auto& boxGeometry = formattingState.boxGeometry(layoutBox);
             auto inlineBoxSize = LayoutSize { LayoutUnit::fromFloatCeil(inlineBoxBorderBox.width()), LayoutUnit::fromFloatCeil(inlineBoxBorderBox.height()) };
             auto logicalRect = Rect { LayoutPoint { inlineBoxBorderBox.topLeft() }, inlineBoxSize };
@@ -244 +246 @@
             auto& inlineBox = lineBox.inlineLevelBoxForLayoutBox(layoutBox);
             auto inlineBoxBorderBox = displayBoxRect();
+            // The content right edge should not include the entire inline box here (including its content and right edge).
+            contentRightInVisualOrder += lineRun.logicalWidth();
             ASSERT(!inlineBox.isFirstBox());
             boxes.append({ lineIndex, InlineDisplay::Box::Type::NonRootInlineBox, layoutBox, lineRun.bidiLevel(), inlineBoxBorderBox, inlineBoxBorderBox, { }, { }, inlineBox.hasContent(), isFirstLastBox(inlineBox) });
@@ -259 +263 @@
             continue;
         }
-        ASSERT(lineRun.isInlineBoxEnd() || lineRun.isWordBreakOpportunity());
+        if (lineRun.isInlineBoxEnd()) {
+            contentRightInVisualOrder += lineRun.logicalWidth();
+            continue;
+        }
+        ASSERT(lineRun.isWordBreakOpportunity());
     }
 }