Changeset 286807 in webkit

Timestamp:

Dec 9, 2021, 3:11:34 PM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Stop including inline box start/end inline items in the visual reordering unless they are completely empty
​https://bugs.webkit.org/show_bug.cgi?id=234035

Reviewed by Antti Koivisto.

When the visual order == logical order, we use the inline box start/end markers to
construct/finalize the inline box type of display boxes.
e.g <span>content</span> when we see the "<span>" run, we construct the inline box type of display box
and later when we come across the "</span>" run, we finalize its geometry.
Now with visual reordering, those explicit markers may be out of order. In such cases (bidi in general) we
switch over to relying solely on the content type of runs to create/finalize the required inline box type of display boxes (see InlineDisplayContentBuilder::ensureDisplayBoxForContainer).

This implicit way of constructing the inline box type of display boxes allows us to include only the minimum set of inline
items for visual reordering:
<span><span><span>content</span></span></span>
should produce only one entry for visual reordering.
It is essential to minimize the "noise" to limit the potential confusion introduced by non-content bidi runs (with their guessed levels).

• layout/formattingContexts/inline/InlineItemsBuilder.cpp:

(WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels): Reserve the guess bidi level for empty inline boxes only.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/formattingContexts/inline/InlineItemsBuilder.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-09  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Stop including inline box start/end inline items in the visual reordering unless they are completely empty
+        https://bugs.webkit.org/show_bug.cgi?id=234035
+
+        Reviewed by Antti Koivisto.
+
+        When the visual order == logical order, we use the inline box start/end markers to
+        construct/finalize the inline box type of display boxes.
+        e.g <span>content</span> when we see the "<span>" run, we construct the inline box type of display box
+        and later when we come across the "</span>" run, we finalize its geometry.
+        Now with visual reordering, those explicit markers may be out of order. In such cases (bidi in general) we
+        switch over to relying solely on the content type of runs to create/finalize the required inline box type of display boxes (see InlineDisplayContentBuilder::ensureDisplayBoxForContainer).
+
+        This implicit way of constructing the inline box type of display boxes allows us to include only the minimum set of inline
+        items for visual reordering:
+        <span><span><span>content</span></span></span>
+        should produce only one entry for visual reordering.
+        It is essential to minimize the "noise" to limit the potential confusion introduced by non-content bidi runs (with their guessed levels).
+
+        * layout/formattingContexts/inline/InlineItemsBuilder.cpp:
+        (WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels): Reserve the guess bidi level for empty inline boxes only.
+
 2021-12-09  Brent Fulgham  <bfulgham@apple.com>
 

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

@@ -322 +322 @@
             return;
         // Opaque items (inline items with no paragraph content) get their bidi level values from their adjacent items.
+        enum class InlineBoxHasContent : bool { No, Yes };
+        Vector<InlineBoxHasContent> inlineBoxContentFlagStack;
+        inlineBoxContentFlagStack.reserveInitialCapacity(inlineItems.size());
         auto lastBidiLevel = rootBidiLevel;
         for (auto index = inlineItems.size(); index--;) {
+            auto& inlineItem = inlineItems[index];
             if (inlineItemOffsets[index]) {
-                lastBidiLevel = inlineItems[index].bidiLevel();
+                lastBidiLevel = inlineItem.bidiLevel();
+                inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
                 continue;
             }
-            if (inlineItems[index].isInlineBoxStart()) {
+            if (inlineItem.isInlineBoxStart()) {
+                ASSERT(!inlineBoxContentFlagStack.isEmpty());
                 // Inline box start (e.g <span>) uses its content bidi level (next inline item).
-                inlineItems[index].setBidiLevel(lastBidiLevel);
+                inlineItems[index].setBidiLevel(inlineBoxContentFlagStack.takeLast() == InlineBoxHasContent::Yes ? InlineItem::opaqueBidiLevel : lastBidiLevel);
                 continue;
             }
-            if (inlineItems[index].isInlineBoxEnd()) {
+            if (inlineItem.isInlineBoxEnd()) {
+                inlineBoxContentFlagStack.append(InlineBoxHasContent::No);
                 // Let's not confuse ubidi with non-content entries. Opaque runs are excluded from the visual list.
-                inlineItems[index].setBidiLevel(InlineItem::opaqueBidiLevel);
+                inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
+                continue;
+            }
+            if (inlineItem.isWordBreakOpportunity()) {
+                inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
                 continue;
             }