Changeset 270151 in webkit

Timestamp:

Nov 21, 2020 12:04:56 PM (3 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Treat floats as soft wrap opportunities
​https://bugs.webkit.org/show_bug.cgi?id=219235

Reviewed by Antti Koivisto.

While floats are not part of the inline content and they are not supposed to introduce soft wrap opportunities,
e.g. [text][float box][float box][text][float box][text] is essentially just [text][text][text]
figuring out whether a float (or set of floats) should stay on the line or not (and handle potentially out of order inline items)
brings in unnecessary complexity (and apparently Blink works like this too).

• layout/inlineformatting/InlineLineBuilder.cpp:

(WebCore::Layout::isAtSoftWrapOpportunity):
(WebCore::Layout::LineBuilder::nextWrapOpportunity const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/inlineformatting/InlineLineBuilder.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-11-21  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Treat floats as soft wrap opportunities
+        https://bugs.webkit.org/show_bug.cgi?id=219235
+
+        Reviewed by Antti Koivisto.
+
+        While floats are not part of the inline content and they are not supposed to introduce soft wrap opportunities,
+        e.g. [text][float box][float box][text][float box][text] is essentially just [text][text][text]
+        figuring out whether a float (or set of floats) should stay on the line or not (and handle potentially out of order inline items)
+        brings in unnecessary complexity (and apparently Blink works like this too).
+
+        * layout/inlineformatting/InlineLineBuilder.cpp:
+        (WebCore::Layout::isAtSoftWrapOpportunity):
+        (WebCore::Layout::LineBuilder::nextWrapOpportunity const):
+
 2020-11-21  Antti Koivisto  <antti@apple.com>
 

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

@@ -79 +79 @@
     // An incoming <img> box would enable us to commit the "<span>prior_continuous_content</span>" content
     // but an incoming text content would not necessarily.
-    ASSERT(current.isText() || current.isBox());
-    ASSERT(next.isText() || next.isBox());
+    ASSERT(current.isText() || current.isBox() || current.isFloat());
+    ASSERT(next.isText() || next.isBox() || next.isFloat());
     if (current.isText() && next.isText()) {
         auto& currentInlineTextItem = downcast<InlineTextItem>(current);
@@ -102 +102 @@
         // [text-][text] : after [hyphen] position is a soft wrap opportunity.
         return endsWithSoftWrapOpportunity(currentInlineTextItem, nextInlineTextItem);
+    }
+    if (current.isFloat() || next.isFloat()) {
+        // While floats are not part of the inline content and they are not supposed to introduce soft wrap opportunities,
+        // e.g. [text][float box][float box][text][float box][text] is essentially just [text][text][text]
+        // figuring out whether a float (or set of floats) should stay on the line or not (and handle potentially out of order inline items)
+        // brings in unnecessary complexity.
+        return true;
     }
     if (current.isBox() || next.isBox()) {
@@ -516 +523 @@
             return ++index;
         }
-        if (inlineItem.isFloat()) {
-            // Floats are not part of the inline content. We ignore them as far as wrap opportunities are concerned.
-            // [text][float box][text] is essentially just [text][text]
-            continue;
-        }
         if (inlineItem.isInlineBoxStart() || inlineItem.isInlineBoxEnd()) {
             // There's no wrapping opportunity between <span>text, <span></span> or </span>text. 
             continue;
         }
-        ASSERT(inlineItem.isText() || inlineItem.isBox());
+        ASSERT(inlineItem.isText() || inlineItem.isBox() || inlineItem.isFloat());
         if (!previousInlineItemIndex) {
             previousInlineItemIndex = index;
             continue;
         }
-        if (isAtSoftWrapOpportunity(m_inlineFormattingContext, m_inlineItems[*previousInlineItemIndex], m_inlineItems[index])) {
+        auto& previousItem = m_inlineItems[*previousInlineItemIndex];
+        auto& currentItem = m_inlineItems[index];
+        if (isAtSoftWrapOpportunity(m_inlineFormattingContext, previousItem, currentItem)) {
+            if (!previousItem.isText() || !currentItem.isText())
+                return index;
             // There's a soft wrap opportunity between 'previousInlineItemIndex' and 'index'.
             // Now forward-find from the start position to see where we can actually wrap.