Title: [266783] trunk/Source/WebCore
Revision
266783
Author
[email protected]
Date
2020-09-09 09:05:49 -0700 (Wed, 09 Sep 2020)

Log Message

[LFC][IFC] LineBuilder::rebuildLine should just re-initialize the current line
https://bugs.webkit.org/show_bug.cgi?id=216260

Reviewed by Antti Koivisto.

Now that LineBuilder manages the Line, it can properly reset it when we need it clean for the "rebuild" case.
This patch also makes "line closing" more explicit by calling collapse trailing content + apply run expansion separately.

* layout/inlineformatting/InlineLine.cpp:
(WebCore::Layout::Line::open):
(WebCore::Layout::Line::moveLogicalLeft):
(WebCore::Layout::Line::moveLogicalRight):
(WebCore::Layout::Line::clearContent): Deleted.
(WebCore::Layout::Line::availableWidth const):
(WebCore::Layout::Line::lineLogicalWidth const): Deleted.
* layout/inlineformatting/InlineLineBuilder.cpp:
(WebCore::Layout::LineBuilder::layoutInlineContent):
(WebCore::Layout::LineBuilder::handleFloatsAndInlineContent):
(WebCore::Layout::LineBuilder::rebuildLine):
* layout/inlineformatting/InlineLineBuilder.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (266782 => 266783)


--- trunk/Source/WebCore/ChangeLog	2020-09-09 16:00:27 UTC (rev 266782)
+++ trunk/Source/WebCore/ChangeLog	2020-09-09 16:05:49 UTC (rev 266783)
@@ -1,3 +1,26 @@
+2020-09-09  Zalan Bujtas  <[email protected]>
+
+        [LFC][IFC] LineBuilder::rebuildLine should just re-initialize the current line
+        https://bugs.webkit.org/show_bug.cgi?id=216260
+
+        Reviewed by Antti Koivisto.
+
+        Now that LineBuilder manages the Line, it can properly reset it when we need it clean for the "rebuild" case.
+        This patch also makes "line closing" more explicit by calling collapse trailing content + apply run expansion separately.
+
+        * layout/inlineformatting/InlineLine.cpp:
+        (WebCore::Layout::Line::open):
+        (WebCore::Layout::Line::moveLogicalLeft):
+        (WebCore::Layout::Line::moveLogicalRight):
+        (WebCore::Layout::Line::clearContent): Deleted.
+        (WebCore::Layout::Line::availableWidth const):
+        (WebCore::Layout::Line::lineLogicalWidth const): Deleted.
+        * layout/inlineformatting/InlineLineBuilder.cpp:
+        (WebCore::Layout::LineBuilder::layoutInlineContent):
+        (WebCore::Layout::LineBuilder::handleFloatsAndInlineContent):
+        (WebCore::Layout::LineBuilder::rebuildLine):
+        * layout/inlineformatting/InlineLineBuilder.h:
+
 2020-09-04  Sergio Villar Senin  <[email protected]>
 
         [WebXR] Implement XRSession end event

Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLine.cpp (266782 => 266783)


--- trunk/Source/WebCore/layout/inlineformatting/InlineLine.cpp	2020-09-09 16:00:27 UTC (rev 266782)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLine.cpp	2020-09-09 16:05:49 UTC (rev 266783)
@@ -55,17 +55,9 @@
 {
 }
 
-void Line::open(InlineLayoutUnit horizontalConstraint)
+void Line::initialize(InlineLayoutUnit horizontalConstraint)
 {
     m_horizontalConstraint = horizontalConstraint;
-    clearContent();
-#if ASSERT_ENABLED
-    m_isClosed = false;
-#endif
-}
-
-void Line::clearContent()
-{
     m_contentLogicalWidth = { };
     m_isVisuallyEmpty = true;
     m_runs.clear();
@@ -73,59 +65,53 @@
     m_lineIsVisuallyEmptyBeforeTrimmableTrailingContent = { };
 }
 
-void Line::close(bool isLastLineWithInlineContent)
+void Line::removeCollapsibleContent()
 {
-#if ASSERT_ENABLED
-    m_isClosed = true;
-#endif
     removeTrailingTrimmableContent();
     visuallyCollapsePreWrapOverflowContent();
+}
 
-    auto computeExpansionForRunsIfNeeded = [&] {
-        if (formattingContext().root().style().textAlign() != TextAlignMode::Justify)
-            return;
-        // Text is justified according to the method specified by the text-justify property,
-        // in order to exactly fill the line box. Unless otherwise specified by text-align-last,
-        // the last line before a forced break or the end of the block is start-aligned.
-        if (isLastLineWithInlineContent)
-            return;
-        if (m_runs.isEmpty() || m_runs.last().isLineBreak())
-            return;
+void Line::applyRunExpansion()
+{
+    ASSERT(formattingContext().root().style().textAlign() == TextAlignMode::Justify);
+    // Text is justified according to the method specified by the text-justify property,
+    // in order to exactly fill the line box. Unless otherwise specified by text-align-last,
+    // the last line before a forced break or the end of the block is start-aligned.
+    if (m_runs.isEmpty() || m_runs.last().isLineBreak())
+        return;
 
-        auto expansionOpportunityCount = 0;
-        Run* lastRunWithContent = nullptr;
-        // Collect and distribute the expansion opportunities.
-        for (auto& run : m_runs) {
-            expansionOpportunityCount += run.expansionOpportunityCount();
-            if (run.isText() || run.isBox())
-                lastRunWithContent = &run;
-        }
-        // Need to fix up the last run's trailing expansion.
-        if (lastRunWithContent && lastRunWithContent->hasExpansionOpportunity()) {
-            // Turn off the trailing bits first and add the forbid trailing expansion.
-            auto leadingExpansion = lastRunWithContent->expansionBehavior() & LeftExpansionMask;
-            lastRunWithContent->setExpansionBehavior(leadingExpansion | ForbidRightExpansion);
-        }
-        // Anything to distribute?
-        if (expansionOpportunityCount && availableWidth()) {
-            // Distribute the extra space.
-            auto expansionToDistribute = availableWidth() / expansionOpportunityCount;
-            auto accumulatedExpansion = InlineLayoutUnit { };
-            for (auto& run : m_runs) {
-                // Expand and move runs by the accumulated expansion.
-                run.moveHorizontally(accumulatedExpansion);
-                if (!run.hasExpansionOpportunity())
-                    continue;
-                ASSERT(run.expansionOpportunityCount());
-                auto computedExpansion = expansionToDistribute * run.expansionOpportunityCount();
-                // FIXME: Check why we need to set both.
-                run.setHorizontalExpansion(computedExpansion);
-                run.shrinkHorizontally(-computedExpansion);
-                accumulatedExpansion += computedExpansion;
-            }
-        }
-    };
-    computeExpansionForRunsIfNeeded();
+    auto expansionOpportunityCount = 0;
+    Run* lastRunWithContent = nullptr;
+    // Collect and distribute the expansion opportunities.
+    for (auto& run : m_runs) {
+        expansionOpportunityCount += run.expansionOpportunityCount();
+        if (run.isText() || run.isBox())
+            lastRunWithContent = &run;
+    }
+    // Need to fix up the last run's trailing expansion.
+    if (lastRunWithContent && lastRunWithContent->hasExpansionOpportunity()) {
+        // Turn off the trailing bits first and add the forbid trailing expansion.
+        auto leadingExpansion = lastRunWithContent->expansionBehavior() & LeftExpansionMask;
+        lastRunWithContent->setExpansionBehavior(leadingExpansion | ForbidRightExpansion);
+    }
+    // Anything to distribute?
+    if (!expansionOpportunityCount || !availableWidth())
+        return;
+    // Distribute the extra space.
+    auto expansionToDistribute = availableWidth() / expansionOpportunityCount;
+    auto accumulatedExpansion = InlineLayoutUnit { };
+    for (auto& run : m_runs) {
+        // Expand and move runs by the accumulated expansion.
+        run.moveHorizontally(accumulatedExpansion);
+        if (!run.hasExpansionOpportunity())
+            continue;
+        ASSERT(run.expansionOpportunityCount());
+        auto computedExpansion = expansionToDistribute * run.expansionOpportunityCount();
+        // FIXME: Check why we need to set both.
+        run.setHorizontalExpansion(computedExpansion);
+        run.shrinkHorizontally(-computedExpansion);
+        accumulatedExpansion += computedExpansion;
+    }
 }
 
 void Line::removeTrailingTrimmableContent()
@@ -222,7 +208,6 @@
 
 void Line::append(const InlineItem& inlineItem, InlineLayoutUnit logicalWidth)
 {
-    ASSERT(!m_isClosed);
     appendWith(inlineItem, { logicalWidth, false });
 }
 

Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLine.h (266782 => 266783)


--- trunk/Source/WebCore/layout/inlineformatting/InlineLine.h	2020-09-09 16:00:27 UTC (rev 266782)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLine.h	2020-09-09 16:05:49 UTC (rev 266783)
@@ -42,9 +42,7 @@
     Line(const InlineFormattingContext&);
     ~Line();
 
-    void open(InlineLayoutUnit horizontalConstraint);
-    void close(bool isLastLineWithInlineContent);
-    void clearContent();
+    void initialize(InlineLayoutUnit horizontalConstraint);
 
     void append(const InlineItem&, InlineLayoutUnit logicalWidth);
     void appendPartialTrailingTextItem(const InlineTextItem&, InlineLayoutUnit logicalWidth, bool needsHypen);
@@ -61,6 +59,9 @@
     void moveLogicalLeft(InlineLayoutUnit);
     void moveLogicalRight(InlineLayoutUnit);
 
+    void removeCollapsibleContent();
+    void applyRunExpansion();
+
     struct Run {
         bool isText() const { return m_type == InlineItem::Type::Text; }
         bool isBox() const { return m_type == InlineItem::Type::Box; }
@@ -181,9 +182,6 @@
     bool m_isVisuallyEmpty { true };
     Optional<bool> m_lineIsVisuallyEmptyBeforeTrimmableTrailingContent;
     bool m_shouldIgnoreTrailingLetterSpacing { false };
-#if ASSERT_ENABLED
-    bool m_isClosed { false };
-#endif
 };
 
 inline void Line::TrimmableTrailingContent::reset()

Modified: trunk/Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp (266782 => 266783)


--- trunk/Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp	2020-09-09 16:00:27 UTC (rev 266782)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineLineBuilder.cpp	2020-09-09 16:05:49 UTC (rev 266783)
@@ -323,7 +323,7 @@
     m_partialLeadingTextItem = { };
     m_lastWrapOpportunityItem = { };
 
-    m_line.open(lineConstraints.availableLogicalWidth);
+    m_line.initialize(lineConstraints.availableLogicalWidth);
     m_contentIsConstrainedByFloat = lineConstraints.isConstrainedByFloat;
 }
 
@@ -373,7 +373,11 @@
     ASSERT(lineRange.end <= needsLayoutRange.end);
     // Adjust hyphenated line count.
     m_successiveHyphenatedLineCount = committedContent.partialTrailingContent && committedContent.partialTrailingContent->trailingContentHasHyphen ? m_successiveHyphenatedLineCount + 1 : 0;
-    m_line.close(isLastLineWithInlineContent(lineRange, needsLayoutRange.end, committedContent.partialTrailingContent.hasValue()));
+    m_line.removeCollapsibleContent();
+    auto horizontalAlignment = root().style().textAlign();
+    auto runsExpandHorizontally = horizontalAlignment == TextAlignMode::Justify && !isLastLineWithInlineContent(lineRange, needsLayoutRange.end, committedContent.partialTrailingContent.hasValue());
+    if (runsExpandHorizontally)
+        m_line.applyRunExpansion();
     return lineRange;
 }
 
@@ -612,8 +616,10 @@
 
 size_t LineBuilder::rebuildLine(const InlineItemRange& layoutRange)
 {
-    // Clear the line and start appending the inline items closing with the last wrap opportunity run.
-    m_line.clearContent();
+    ASSERT(m_lastWrapOpportunityItem);
+    // We might already have added intrusive floats. They shrink the avilable horizontal space for the line.
+    // Let's just reuse what the line has at this point.
+    m_line.initialize(m_line.horizontalConstraint());
     auto currentItemIndex = layoutRange.start;
     auto logicalRight = InlineLayoutUnit { };
     if (m_partialLeadingTextItem) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to