[LFC][IFC] LineCandidateContent can have only one float item
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 16:22:10 +0000 (16:22 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 16:22:10 +0000 (16:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207681
<rdar://problem/59413044>

Reviewed by Antti Koivisto.

Floats should not be considered as inline content. They shrink the available space but we never
add them to the line. This patch decouples InlineContent and candidate floats. Also there can only be
one float box per candidate content (since there's always a soft wrap opportunity before/after the float box).

* layout/inlineformatting/LineLayoutContext.cpp:
(WebCore::Layout::LineCandidate::InlineContent::runs const):
(WebCore::Layout::LineCandidate::InlineContent::logicalWidth const):
(WebCore::Layout::LineCandidate::InlineContent::trailingLineBreak const):
(WebCore::Layout::LineCandidate::InlineContent::appendLineBreak):
(WebCore::Layout::LineCandidate::InlineContent::setTrailingLineBreak):
(WebCore::Layout::LineCandidate::InlineContent::appendInlineItem):
(WebCore::Layout::LineCandidate::reset):
(WebCore::Layout::LineCandidate::InlineContent::reset):
(WebCore::Layout::LineLayoutContext::layoutLine):
(WebCore::Layout::LineLayoutContext::nextContentForLine):
(WebCore::Layout::LineLayoutContext::tryAddingFloatItem):
(WebCore::Layout::LineLayoutContext::tryAddingInlineItems):
(WebCore::Layout::LineCandidateContent::appendLineBreak): Deleted.
(WebCore::Layout::LineCandidateContent::appendFloat): Deleted.
(WebCore::Layout::LineCandidateContent::hasIntrusiveFloats const): Deleted.
(WebCore::Layout::LineCandidateContent::inlineRuns const): Deleted.
(WebCore::Layout::LineCandidateContent::inlineContentLogicalWidth const): Deleted.
(WebCore::Layout::LineCandidateContent::floats const): Deleted.
(WebCore::Layout::LineCandidateContent::trailingLineBreak const): Deleted.
(WebCore::Layout::LineCandidateContent::setTrailingLineBreak): Deleted.
(WebCore::Layout::LineCandidateContent::appendInlineContent): Deleted.
(WebCore::Layout::LineCandidateContent::reset): Deleted.
(WebCore::Layout::LineLayoutContext::tryAddingFloatItems): Deleted.
* layout/inlineformatting/LineLayoutContext.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256507 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/layout/inlineformatting/LineLayoutContext.cpp
Source/WebCore/layout/inlineformatting/LineLayoutContext.h

index c15b689..98cca75 100644 (file)
@@ -1,3 +1,41 @@
+2020-02-13  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] LineCandidateContent can have only one float item
+        https://bugs.webkit.org/show_bug.cgi?id=207681
+        <rdar://problem/59413044>
+
+        Reviewed by Antti Koivisto.
+
+        Floats should not be considered as inline content. They shrink the available space but we never
+        add them to the line. This patch decouples InlineContent and candidate floats. Also there can only be
+        one float box per candidate content (since there's always a soft wrap opportunity before/after the float box). 
+
+        * layout/inlineformatting/LineLayoutContext.cpp:
+        (WebCore::Layout::LineCandidate::InlineContent::runs const):
+        (WebCore::Layout::LineCandidate::InlineContent::logicalWidth const):
+        (WebCore::Layout::LineCandidate::InlineContent::trailingLineBreak const):
+        (WebCore::Layout::LineCandidate::InlineContent::appendLineBreak):
+        (WebCore::Layout::LineCandidate::InlineContent::setTrailingLineBreak):
+        (WebCore::Layout::LineCandidate::InlineContent::appendInlineItem):
+        (WebCore::Layout::LineCandidate::reset):
+        (WebCore::Layout::LineCandidate::InlineContent::reset):
+        (WebCore::Layout::LineLayoutContext::layoutLine):
+        (WebCore::Layout::LineLayoutContext::nextContentForLine):
+        (WebCore::Layout::LineLayoutContext::tryAddingFloatItem):
+        (WebCore::Layout::LineLayoutContext::tryAddingInlineItems):
+        (WebCore::Layout::LineCandidateContent::appendLineBreak): Deleted.
+        (WebCore::Layout::LineCandidateContent::appendFloat): Deleted.
+        (WebCore::Layout::LineCandidateContent::hasIntrusiveFloats const): Deleted.
+        (WebCore::Layout::LineCandidateContent::inlineRuns const): Deleted.
+        (WebCore::Layout::LineCandidateContent::inlineContentLogicalWidth const): Deleted.
+        (WebCore::Layout::LineCandidateContent::floats const): Deleted.
+        (WebCore::Layout::LineCandidateContent::trailingLineBreak const): Deleted.
+        (WebCore::Layout::LineCandidateContent::setTrailingLineBreak): Deleted.
+        (WebCore::Layout::LineCandidateContent::appendInlineContent): Deleted.
+        (WebCore::Layout::LineCandidateContent::reset): Deleted.
+        (WebCore::Layout::LineLayoutContext::tryAddingFloatItems): Deleted.
+        * layout/inlineformatting/LineLayoutContext.h:
+
 2020-02-13  Chris Lord  <clord@igalia.com>
 
         Implement OffscreenCanvas.copiedImage
index 9a5ce3f..582d22b 100644 (file)
@@ -171,40 +171,46 @@ static inline size_t nextWrapOpportunity(const InlineItems& inlineContent, size_
     return layoutRange.end;
 }
 
-struct LineCandidateContent {
-    void appendInlineContent(const InlineItem&, InlineLayoutUnit logicalWidth);
-    void appendLineBreak(const InlineItem& inlineItem) { setTrailingLineBreak(inlineItem); }
-    void appendFloat(const InlineItem& inlineItem) { m_floats.append(&inlineItem); }
-
-    bool hasIntrusiveFloats() const { return !m_floats.isEmpty(); }
-    const LineBreaker::RunList& inlineRuns() const { return m_inlineRuns; }
-    InlineLayoutUnit inlineContentLogicalWidth() const { return m_inlineContentLogicalWidth; }
-    const LineLayoutContext::FloatList& floats() const { return m_floats; }
+struct LineCandidate {
+    void reset();
 
-    const InlineItem* trailingLineBreak() const { return m_trailingLineBreak; }
+    struct InlineContent {
+        const LineBreaker::RunList& runs() const { return m_inlineRuns; }
+        InlineLayoutUnit logicalWidth() const { return m_LogicalWidth; }
+        const InlineItem* trailingLineBreak() const { return m_trailingLineBreak; }
 
-    void reset();
+        void appendInlineItem(const InlineItem&, InlineLayoutUnit logicalWidth);
+        void appendLineBreak(const InlineItem& inlineItem) { setTrailingLineBreak(inlineItem); }
+        void reset();
 
-private:
-    void setTrailingLineBreak(const InlineItem& lineBreakItem) { m_trailingLineBreak = &lineBreakItem; }
+    private:
+        void setTrailingLineBreak(const InlineItem& lineBreakItem) { m_trailingLineBreak = &lineBreakItem; }
 
-    InlineLayoutUnit m_inlineContentLogicalWidth { 0 };
-    LineBreaker::RunList m_inlineRuns;
-    LineLayoutContext::FloatList m_floats;
-    const InlineItem* m_trailingLineBreak { nullptr };
+        InlineLayoutUnit m_LogicalWidth { 0 };
+        LineBreaker::RunList m_inlineRuns;
+        const InlineItem* m_trailingLineBreak { nullptr };
+    };
+    // Candidate content is either a collection of inline items or a float box.
+    InlineContent inlineContent;
+    const InlineItem* floatItem { nullptr };
 };
 
-inline void LineCandidateContent::appendInlineContent(const InlineItem& inlineItem, InlineLayoutUnit logicalWidth)
+inline void LineCandidate::InlineContent::appendInlineItem(const InlineItem& inlineItem, InlineLayoutUnit logicalWidth)
 {
-    m_inlineContentLogicalWidth += logicalWidth;
+    m_LogicalWidth += logicalWidth;
     m_inlineRuns.append({ inlineItem, logicalWidth });
 }
 
-inline void LineCandidateContent::reset()
+inline void LineCandidate::reset()
 {
-    m_inlineContentLogicalWidth = 0;
+    floatItem = { };
+    inlineContent.reset();
+}
+
+inline void LineCandidate::InlineContent::reset()
+{
+    m_LogicalWidth = 0;
     m_inlineRuns.clear();
-    m_floats.clear();
     m_trailingLineBreak = nullptr;
 }
 
@@ -259,39 +265,42 @@ LineLayoutContext::LineContent LineLayoutContext::layoutLine(LineBuilder& line,
     auto lineBreaker = LineBreaker { };
     auto currentItemIndex = layoutRange.start;
     unsigned committedInlineItemCount = 0;
-    auto candidateContent = LineCandidateContent { };
+    auto lineCandidate = LineCandidate { };
     while (currentItemIndex < layoutRange.end) {
         // 1. Collect the set of runs that we can commit to the line as one entity e.g. <span>text_and_span_start_span_end</span>.
         // 2. Apply floats and shrink the available horizontal space e.g. <span>intru_<div style="float: left"></div>sive_float</span>.
         // 3. Check if the content fits the line and commit the content accordingly (full, partial or not commit at all).
         // 4. Return if we are at the end of the line either by not being able to fit more content or because of an explicit line break.
-        nextContentForLine(candidateContent, currentItemIndex, layoutRange, partialLeadingContentLength, line.lineBox().logicalWidth());
-        if (candidateContent.hasIntrusiveFloats()) {
-            // Add floats first because they shrink the available horizontal space for the rest of the content.
-            auto result = tryAddingFloatItems(line, candidateContent.floats());
+        nextContentForLine(lineCandidate, currentItemIndex, layoutRange, partialLeadingContentLength, line.lineBox().logicalWidth());
+        if (lineCandidate.floatItem) {
+            // Floats shrink the available horizontal space for the rest of the content, but they are not added on the line.
+            auto result = tryAddingFloatItem(line, *lineCandidate.floatItem);
             committedInlineItemCount += result.committedCount;
             if (result.isEndOfLine == LineBreaker::IsEndOfLine::Yes) {
-                // Floats take up all the horizontal space.
+                // This float takes up all the horizontal space.
                 return close(line, layoutRange, committedInlineItemCount, { });
             }
-        }
-        if (!candidateContent.inlineRuns().isEmpty()) {
-            // Now check if we can put this content on the current line.
-            auto result = tryAddingInlineItems(lineBreaker, line, candidateContent);
-            if (result.revertTo) {
-                ASSERT(!result.committedCount);
-                ASSERT(result.isEndOfLine == LineBreaker::IsEndOfLine::Yes);
-                // An earlier line wrapping opportunity turned out to be the final breaking position.
-                rebuildLineForRevert(line, *result.revertTo, layoutRange);
-            }
-            committedInlineItemCount += result.committedCount;
-            if (result.isEndOfLine == LineBreaker::IsEndOfLine::Yes) {
-                // We can't place any more items on the current line.
-                return close(line, layoutRange, committedInlineItemCount, result.partialContent);
+            ASSERT(lineCandidate.inlineContent.runs().isEmpty());
+        } else {
+            auto& inlineContent = lineCandidate.inlineContent;
+            if (!inlineContent.runs().isEmpty()) {
+                // Now check if we can put this content on the current line.
+                auto result = tryAddingInlineItems(lineBreaker, line, lineCandidate);
+                if (result.revertTo) {
+                    ASSERT(!result.committedCount);
+                    ASSERT(result.isEndOfLine == LineBreaker::IsEndOfLine::Yes);
+                    // An earlier line wrapping opportunity turned out to be the final breaking position.
+                    rebuildLineForRevert(line, *result.revertTo, layoutRange);
+                }
+                committedInlineItemCount += result.committedCount;
+                if (result.isEndOfLine == LineBreaker::IsEndOfLine::Yes) {
+                    // We can't place any more items on the current line.
+                    return close(line, layoutRange, committedInlineItemCount, result.partialContent);
+                }
+            } else if (auto* trailingLineBreak = inlineContent.trailingLineBreak()) {
+                line.append(*trailingLineBreak, 0);
+                return close(line, layoutRange, ++committedInlineItemCount, { });
             }
-        } else if (auto* trailingLineBreak = candidateContent.trailingLineBreak()) {
-            line.append(*trailingLineBreak, 0);
-            return close(line, layoutRange, ++committedInlineItemCount, { });
         }
         currentItemIndex = layoutRange.start + committedInlineItemCount;
         partialLeadingContentLength = { };
@@ -330,10 +339,10 @@ LineLayoutContext::LineContent LineLayoutContext::close(LineBuilder& line, const
     return LineContent { trailingInlineItemIndex, partialContent, WTFMove(m_floats), line.close(isLastLineWithInlineContent), line.lineBox() };
 }
 
-void LineLayoutContext::nextContentForLine(LineCandidateContent& candidateContent, unsigned currentInlineItemIndex, const InlineItemRange layoutRange, Optional<unsigned> partialLeadingContentLength, InlineLayoutUnit currentLogicalRight)
+void LineLayoutContext::nextContentForLine(LineCandidate& lineCandidate, unsigned currentInlineItemIndex, const InlineItemRange layoutRange, Optional<unsigned> partialLeadingContentLength, InlineLayoutUnit currentLogicalRight)
 {
     ASSERT(currentInlineItemIndex < layoutRange.end);
-    candidateContent.reset();
+    lineCandidate.reset();
     // 1. Simply add any overflow content from the previous line to the candidate content. It's always a text content.
     // 2. Find the next soft wrap position or explicit line break.
     // 3. Collect floats between the inline content.
@@ -346,7 +355,7 @@ void LineLayoutContext::nextContentForLine(LineCandidateContent& candidateConten
         // Construct a partial leading inline item.
         m_partialLeadingTextItem = downcast<InlineTextItem>(m_inlineItems[currentInlineItemIndex]).right(*partialLeadingContentLength);
         auto itemWidth = inlineItemWidth(*m_partialLeadingTextItem, currentLogicalRight);
-        candidateContent.appendInlineContent(*m_partialLeadingTextItem, itemWidth);
+        lineCandidate.inlineContent.appendInlineItem(*m_partialLeadingTextItem, itemWidth);
         currentLogicalRight += itemWidth;
         ++currentInlineItemIndex;
     }
@@ -354,49 +363,48 @@ void LineLayoutContext::nextContentForLine(LineCandidateContent& candidateConten
     for (auto index = currentInlineItemIndex; index < softWrapOpportunityIndex; ++index) {
         auto& inlineItem = m_inlineItems[index];
         if (inlineItem.isText() || inlineItem.isContainerStart() || inlineItem.isContainerEnd()) {
+            ASSERT(!lineCandidate.floatItem);
             auto inlineItenmWidth = inlineItemWidth(inlineItem, currentLogicalRight);
-            candidateContent.appendInlineContent(inlineItem, inlineItenmWidth);
+            lineCandidate.inlineContent.appendInlineItem(inlineItem, inlineItenmWidth);
             currentLogicalRight += inlineItenmWidth;
             continue;
         }
         if (inlineItem.isFloat()) {
             // Floats are not part of the line context.
             // FIXME: Check if their width should be added to currentLogicalRight.
-            candidateContent.appendFloat(inlineItem);
+            ASSERT(!lineCandidate.floatItem);
+            ASSERT(lineCandidate.inlineContent.runs().isEmpty());
+            lineCandidate.floatItem = &inlineItem;
             continue;
         }
         if (inlineItem.isLineBreak()) {
-            candidateContent.appendLineBreak(inlineItem);
+            lineCandidate.inlineContent.appendLineBreak(inlineItem);
             continue;
         }
         ASSERT_NOT_REACHED();
     }
 }
 
-LineLayoutContext::Result LineLayoutContext::tryAddingFloatItems(LineBuilder& line, const FloatList& floats)
+LineLayoutContext::Result LineLayoutContext::tryAddingFloatItem(LineBuilder& line, const InlineItem& floatItem)
 {
-    size_t committedFloatItemCount = 0;
-    for (auto& floatItem : floats) {
-        auto logicalWidth = inlineItemWidth(*floatItem, { });
-
-        auto lineIsConsideredEmpty = line.isVisuallyEmpty() && !line.hasIntrusiveFloat();
-        if (LineBreaker().shouldWrapFloatBox(logicalWidth, line.availableWidth() + line.trimmableTrailingWidth(), lineIsConsideredEmpty))
-            return { LineBreaker::IsEndOfLine::Yes, committedFloatItemCount };
-        // This float can sit on the current line.
-        ++committedFloatItemCount;
-        auto& floatBox = floatItem->layoutBox();
-        // Shrink available space for current line and move existing inline runs.
-        line.setHasIntrusiveFloat();
-        if (floatBox.isLeftFloatingPositioned())
-            line.moveLogicalLeft(logicalWidth);
-        else
-            line.moveLogicalRight(logicalWidth);
-        m_floats.append(floatItem);
-    }
-    return { LineBreaker::IsEndOfLine::No, committedFloatItemCount };
+    auto logicalWidth = inlineItemWidth(floatItem, { });
+
+    auto lineIsConsideredEmpty = line.isVisuallyEmpty() && !line.hasIntrusiveFloat();
+    if (LineBreaker().shouldWrapFloatBox(logicalWidth, line.availableWidth() + line.trimmableTrailingWidth(), lineIsConsideredEmpty))
+        return { LineBreaker::IsEndOfLine::Yes };
+    // This float can sit on the current line.
+    auto& floatBox = floatItem.layoutBox();
+    // Shrink available space for current line and move existing inline runs.
+    line.setHasIntrusiveFloat();
+    if (floatBox.isLeftFloatingPositioned())
+        line.moveLogicalLeft(logicalWidth);
+    else
+        line.moveLogicalRight(logicalWidth);
+    m_floats.append(&floatItem);
+    return { LineBreaker::IsEndOfLine::No, 1 };
 }
 
-LineLayoutContext::Result LineLayoutContext::tryAddingInlineItems(LineBreaker& lineBreaker, LineBuilder& line, const LineCandidateContent& candidateContent)
+LineLayoutContext::Result LineLayoutContext::tryAddingInlineItems(LineBreaker& lineBreaker, LineBuilder& line, const LineCandidate& lineCandidate)
 {
     auto shouldDisableHyphenation = [&] {
         auto& style = root().style();
@@ -409,14 +417,15 @@ LineLayoutContext::Result LineLayoutContext::tryAddingInlineItems(LineBreaker& l
     if (shouldDisableHyphenation())
         lineBreaker.setHyphenationDisabled();
 
-    auto& candidateRuns = candidateContent.inlineRuns();
-    auto result = lineBreaker.shouldWrapInlineContent(candidateRuns, candidateContent.inlineContentLogicalWidth(), lineStatus);
+    auto& inlineContent = lineCandidate.inlineContent;
+    auto& candidateRuns = inlineContent.runs();
+    auto result = lineBreaker.shouldWrapInlineContent(candidateRuns, inlineContent.logicalWidth(), lineStatus);
     if (result.action == LineBreaker::Result::Action::Keep) {
         // This continuous content can be fully placed on the current line.
         for (auto& run : candidateRuns)
             line.append(run.inlineItem, run.logicalWidth);
         // Consume trailing line break as well.
-        if (auto* lineBreakItem = candidateContent.trailingLineBreak()) {
+        if (auto* lineBreakItem = inlineContent.trailingLineBreak()) {
             line.append(*lineBreakItem, 0);
             return { LineBreaker::IsEndOfLine::Yes, candidateRuns.size() + 1 };
         }
index f8f14cb..8736f83 100644 (file)
@@ -33,7 +33,7 @@
 namespace WebCore {
 namespace Layout {
 
-struct LineCandidateContent;
+struct LineCandidate;
 
 class LineLayoutContext {
 public:
@@ -56,18 +56,17 @@ public:
         size_t end { 0 };
     };
     LineContent layoutLine(LineBuilder&, const InlineItemRange, Optional<unsigned> partialLeadingContentLength);
-    using FloatList = Vector<const InlineItem*>;
 
 private:
-    void nextContentForLine(LineCandidateContent&, unsigned inlineItemIndex, const InlineItemRange layoutRange, Optional<unsigned> overflowLength, InlineLayoutUnit currentLogicalRight);
+    void nextContentForLine(LineCandidate&, unsigned inlineItemIndex, const InlineItemRange layoutRange, Optional<unsigned> overflowLength, InlineLayoutUnit currentLogicalRight);
     struct Result {
         LineBreaker::IsEndOfLine isEndOfLine { LineBreaker::IsEndOfLine::No };
         size_t committedCount { 0 };
         Optional <LineContent::PartialContent> partialContent { };
         const InlineItem* revertTo { nullptr };
     };
-    Result tryAddingFloatItems(LineBuilder&, const FloatList&);
-    Result tryAddingInlineItems(LineBreaker&, LineBuilder&, const LineCandidateContent&);
+    Result tryAddingFloatItem(LineBuilder&, const InlineItem& floatItem);
+    Result tryAddingInlineItems(LineBreaker&, LineBuilder&, const LineCandidate&);
     void rebuildLineForRevert(LineBuilder&, const InlineItem& revertTo, const InlineItemRange layoutRange);
     void commitPartialContent(LineBuilder&, const LineBreaker::RunList&, const LineBreaker::Result::PartialTrailingContent&);
     LineContent close(LineBuilder&, const InlineItemRange layoutRange, unsigned committedInlineItemCount, Optional<LineContent::PartialContent>);
@@ -80,6 +79,7 @@ private:
     const InlineFormattingContext& m_inlineFormattingContext;
     const ContainerBox& m_formattingContextRoot;
     const InlineItems& m_inlineItems;
+    using FloatList = Vector<const InlineItem*>;
     FloatList m_floats;
     Optional<InlineTextItem> m_partialLeadingTextItem;
     unsigned m_successiveHyphenatedLineCount { 0 };