Attachment 370909 Details for Bug 198360 – Patch

[patch] Patch

bug-198360-20190529193838.patch (text/plain), 18.38 KB, created by zalan on 2019-05-29 19:38:40 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-05-29 19:38:40 PDT

Size: 18.38 KB

patch

obsolete

>Subversion Revision: 245776
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index dd4203cfe642293cd9e0c7ae6b62a06eeffbc918..a0d1d08d8306f0706b060559670e3d4a7ca7398f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,55 @@
>+2019-05-29  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] Move final runs to a dedicated class (Line::Content)
>+        https://bugs.webkit.org/show_bug.cgi?id=198360
>+        <rdar://problem/51247717>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        It decouples the line and the final line content. So when we process the runs after closing the line,
>+        LineContent should be able to answer all the content and geometry related questions.
>+        This is also in preparation for
>+        transfering the ownership of the line content when calling Line::close(). 
>+
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::processInlineRuns const):
>+        * layout/inlineformatting/InlineLine.cpp:
>+        (WebCore::Layout::Line::Content::isVisuallyEmpty const):
>+        (WebCore::Layout::Line::Content::Run::Run):
>+        (WebCore::Layout::Line::reset):
>+        (WebCore::Layout::Line::close):
>+        (WebCore::Layout::Line::moveLogicalLeft):
>+        (WebCore::Layout::Line::appendNonBreakableSpace):
>+        (WebCore::Layout::Line::appendTextContent):
>+        (WebCore::Layout::Line::appendNonReplacedInlineBox):
>+        (WebCore::Layout::Line::appendHardLineBreak):
>+        (WebCore::Layout::Line::LineItem::LineItem): Deleted.
>+        (WebCore::Layout::Line::hasContent const): Deleted.
>+        * layout/inlineformatting/InlineLine.h:
>+        (WebCore::Layout::Line::Content::runs const):
>+        (WebCore::Layout::Line::Content::isEmpty const):
>+        (WebCore::Layout::Line::Content::logicalTop const):
>+        (WebCore::Layout::Line::Content::logicalLeft const):
>+        (WebCore::Layout::Line::Content::logicalRight const):
>+        (WebCore::Layout::Line::Content::logicalBottom const):
>+        (WebCore::Layout::Line::Content::logicalWidth const):
>+        (WebCore::Layout::Line::Content::logicalHeight const):
>+        (WebCore::Layout::Line::Content::setLogicalRect):
>+        (WebCore::Layout::Line::Content::runs):
>+        (WebCore::Layout::Line::hasContent const):
>+        (WebCore::Layout::Line::availableWidth const):
>+        (WebCore::Layout::Line::contentLogicalRight const):
>+        (WebCore::Layout::Line::logicalTop const):
>+        (WebCore::Layout::Line::logicalBottom const):
>+        (WebCore::Layout::Line::logicalLeft const):
>+        (WebCore::Layout::Line::logicalRight const):
>+        (WebCore::Layout::Line::logicalWidth const):
>+        (WebCore::Layout::Line::logicalHeight const):
>+        (WebCore::Layout::Line::contentLogicalWidth const):
>+        * page/FrameViewLayoutContext.cpp:
>+        (WebCore::layoutUsingFormattingContext):
>+
> 2019-05-29  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Move Line class to a dedicated file
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index 8186329d11fa5a686811a904a1fbd90cb889b50b..a2113115ea2694b7cd0b2e01de8759eee359ed07 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -216,10 +216,10 @@ LayoutUnit InlineFormattingContext::LineLayout::computedIntrinsicWidth(LayoutUni
> 
> void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
> {
>-    auto& lineItems = line.close();
>-    if (lineItems.isEmpty()) {
>+    auto& lineContent = line.close();
>+    if (lineContent.isEmpty()) {
>         // Spec tells us to create a zero height, empty line box.
>-        auto lineBox = Display::Rect { line.logicalTop(), line.logicalLeft(), 0 , 0 };
>+        auto lineBox = Display::Rect { lineContent.logicalTop(), lineContent.logicalLeft(), 0 , 0 };
>         m_formattingState.addLineBox({ lineBox });
>         return;
>     }
>@@ -230,13 +230,14 @@ void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
>     // A line box is always tall enough for all of the boxes it contains.
> 
>     // Ignore the initial strut.
>-    auto lineBox = Display::Rect { line.logicalTop(), line.logicalLeft(), 0 , line.hasContent() ? line.logicalHeight() : LayoutUnit { } };
>+    auto lineBox = Display::Rect { lineContent.logicalTop(), lineContent.logicalLeft(), 0 , !lineContent.isVisuallyEmpty() ? lineContent.logicalHeight() : LayoutUnit { } };
>     // Create final display runs.
>-    for (unsigned index = 0; index < lineItems.size(); ++index) {
>-        auto& lineItem = lineItems.at(index);
>+    auto& lineRuns = lineContent.runs();
>+    for (unsigned index = 0; index < lineRuns.size(); ++index) {
>+        auto& lineRun = lineRuns.at(index);
> 
>-        auto& inlineItem = lineItem->inlineItem;
>-        auto& inlineRun = lineItem->inlineRun;
>+        auto& inlineItem = lineRun->inlineItem;
>+        auto& inlineRun = lineRun->inlineRun;
>         auto& layoutBox = inlineItem.layoutBox();
>         auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
> 
>@@ -284,11 +285,11 @@ void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
> 
>         // Text content. Try to join multiple text runs when possible.
>         ASSERT(inlineRun.textContext());        
>-        const Line::LineItem* previousLineItem = !index ? nullptr : lineItems[index - 1].get();
>-        if (!lineItem->isCollapsed) {
>+        const Line::Content::Run* previousLineRun = !index ? nullptr : lineRuns[index - 1].get();
>+        if (!lineRun->isCollapsed) {
>             auto& inlineTextItem = downcast<InlineTextItem>(inlineItem);
>-            auto previousRunCanBeExtended = previousLineItem ? previousLineItem->canBeExtended : false;
>-            auto requiresNewRun = !index || !previousRunCanBeExtended || &layoutBox != &previousLineItem->inlineItem.layoutBox();
>+            auto previousRunCanBeExtended = previousLineRun ? previousLineRun->canBeExtended : false;
>+            auto requiresNewRun = !index || !previousRunCanBeExtended || &layoutBox != &previousLineRun->inlineItem.layoutBox();
>             if (requiresNewRun)
>                 m_formattingState.addInlineRun(std::make_unique<Display::Run>(inlineRun));
>             else {
>@@ -299,20 +300,20 @@ void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
>             lineBox.expandHorizontally(inlineRun.logicalWidth());
>         }
>         // FIXME take content breaking into account when part of the layout box is on the previous line.
>-        auto firstInlineRunForLayoutBox = !previousLineItem || &previousLineItem->inlineItem.layoutBox() != &layoutBox;
>+        auto firstInlineRunForLayoutBox = !previousLineRun || &previousLineRun->inlineItem.layoutBox() != &layoutBox;
>         if (firstInlineRunForLayoutBox) {
>             // Setup display box for the associated layout box.
>             displayBox.setTopLeft(inlineRun.logicalTopLeft());
>-            displayBox.setContentBoxWidth(lineItem->isCollapsed ? LayoutUnit() : inlineRun.logicalWidth());
>+            displayBox.setContentBoxWidth(lineRun->isCollapsed ? LayoutUnit() : inlineRun.logicalWidth());
>             displayBox.setContentBoxHeight(inlineRun.logicalHeight());
>-        } else if (!lineItem->isCollapsed) {
>+        } else if (!lineRun->isCollapsed) {
>             // FIXME fix it for multirun/multiline.
>             displayBox.setContentBoxWidth(displayBox.contentBoxWidth() + inlineRun.logicalWidth());
>         }
>     }
>     // FIXME linebox needs to be ajusted after content alignment.
>     m_formattingState.addLineBox({ lineBox });
>-    if (line.hasContent())
>+    if (!lineContent.isVisuallyEmpty())
>         alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, line.availableWidth());
> }
> 
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.cpp b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>index c27bea3f7fda3834aef2f6cec52930955bd146e1..0dd297df2d731fa31d7d71a7c9d6c6c371bc0c74 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>@@ -31,7 +31,19 @@
> namespace WebCore {
> namespace Layout {
> 
>-Line::LineItem::LineItem(Display::Run inlineRun, const InlineItem& inlineItem, bool isCollapsed, bool canBeExtended)
>+bool Line::Content::isVisuallyEmpty() const
>+{
>+    // Return true for empty inline containers like <span></span>.
>+    for (auto& run : m_runs) {
>+        if (run->inlineItem.isContainerStart() || run->inlineItem.isContainerEnd())
>+            continue;
>+        if (!run->isCollapsed)
>+            return false;
>+    }
>+    return true;
>+}
>+
>+Line::Content::Run::Run(Display::Run inlineRun, const InlineItem& inlineItem, bool isCollapsed, bool canBeExtended)
>     : inlineRun(inlineRun)
>     , inlineItem(inlineItem)
>     , isCollapsed(isCollapsed)
>@@ -52,19 +64,21 @@ void Line::reset(const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUn
> 
>     m_contentLogicalWidth = { };
> 
>-    m_lineItems.clear();
>+    m_content = { };
>+
>     m_trimmableContent.clear();
> }
> 
>-const Line::LineItems& Line::close()
>+const Line::Content& Line::close()
> {
>     removeTrailingTrimmableContent();
>     // Convert inline run geometry from relative to the baseline to relative to logical top.
>-    for (auto& lineItem : m_lineItems) {
>-        auto adjustedLogicalTop = lineItem->inlineRun.logicalTop() + m_logicalHeight.height + m_logicalTopLeft.y();
>-        lineItem->inlineRun.setLogicalTop(adjustedLogicalTop);
>+    for (auto& run : m_content.runs()) {
>+        auto adjustedLogicalTop = run->inlineRun.logicalTop() + m_logicalHeight.height + m_logicalTopLeft.y();
>+        run->inlineRun.setLogicalTop(adjustedLogicalTop);
>     }
>-    return m_lineItems;
>+    m_content.setLogicalRect({ logicalTop(), logicalLeft(), contentLogicalWidth(), logicalHeight() });
>+    return m_content;
> }
> 
> void Line::removeTrailingTrimmableContent()
>@@ -86,8 +100,8 @@ void Line::moveLogicalLeft(LayoutUnit delta)
>     // Shrink the line and move the items.
>     m_logicalTopLeft.move(delta, 0);
>     m_lineLogicalWidth -= delta;
>-    for (auto& lineItem : m_lineItems)
>-        lineItem->inlineRun.moveHorizontally(delta);
>+    for (auto& run : m_content.runs())
>+        run->inlineRun.moveHorizontally(delta);
> }
> 
> void Line::moveLogicalRight(LayoutUnit delta)
>@@ -104,23 +118,9 @@ LayoutUnit Line::trailingTrimmableWidth() const
>     return trimmableWidth;
> }
> 
>-bool Line::hasContent() const
>-{
>-    // Return false for empty containers like <span></span>.
>-    if (m_lineItems.isEmpty())
>-        return false;
>-    for (auto& lineItem : m_lineItems) {
>-        if (lineItem->inlineItem.isContainerStart() || lineItem->inlineItem.isContainerEnd())
>-            continue;
>-        if (!lineItem->isCollapsed)
>-            return true;
>-    }
>-    return false;
>-}
>-
> void Line::appendNonBreakableSpace(const InlineItem& inlineItem, const Display::Rect& logicalRect)
> {
>-    m_lineItems.append(std::make_unique<LineItem>(Display::Run { logicalRect }, inlineItem, false, false));
>+    m_content.runs().append(std::make_unique<Content::Run>(Display::Run { logicalRect }, inlineItem, false, false));
>     m_contentLogicalWidth += inlineItem.width();
> }
> 
>@@ -165,11 +165,12 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>         if (!isTrimmable)
>             return false;
>         // Leading whitespace.
>-        if (m_lineItems.isEmpty())
>+        auto& runs = m_content.runs();
>+        if (runs.isEmpty())
>             return true;
>         // Check if the last item is trimmable as well.
>-        for (int index = m_lineItems.size() - 1; index >= 0; --index) {
>-            auto& inlineItem = m_lineItems[index]->inlineItem;
>+        for (int index = runs.size() - 1; index >= 0; --index) {
>+            auto& inlineItem = runs[index]->inlineItem;
>             if (inlineItem.isBox())
>                 return false;
>             if (inlineItem.isText())
>@@ -186,11 +187,11 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>     auto textContext = Display::Run::TextContext { inlineItem.start(), inlineItem.isCollapsed() ? 1 : inlineItem.length() };
>     auto displayRun = Display::Run(logicalRect, textContext);
> 
>-    auto lineItem = std::make_unique<LineItem>(displayRun, inlineItem, isCompletelyCollapsed, canBeExtended);
>+    auto lineItem = std::make_unique<Content::Run>(displayRun, inlineItem, isCompletelyCollapsed, canBeExtended);
>     if (isTrimmable)
>         m_trimmableContent.add(lineItem.get());
> 
>-    m_lineItems.append(WTFMove(lineItem));
>+    m_content.runs().append(WTFMove(lineItem));
>     m_contentLogicalWidth += isCompletelyCollapsed ? LayoutUnit() : runSize.width();
> }
> 
>@@ -218,7 +219,7 @@ void Line::appendNonReplacedInlineBox(const InlineItem& inlineItem, LayoutSize r
>     auto horizontalMargin = displayBox.horizontalMargin();
>     auto logicalRect = Display::Rect { logicalTop, contentLogicalRight() + horizontalMargin.start, runSize.width(), runSize.height() };
> 
>-    m_lineItems.append(std::make_unique<LineItem>(Display::Run { logicalRect }, inlineItem, false, false));
>+    m_content.runs().append(std::make_unique<Content::Run>(Display::Run { logicalRect }, inlineItem, false, false));
>     m_contentLogicalWidth += (runSize.width() + horizontalMargin.start + horizontalMargin.end);
>     m_trimmableContent.clear();
> }
>@@ -233,7 +234,7 @@ void Line::appendHardLineBreak(const InlineItem& inlineItem)
> {
>     auto ascent = inlineItem.layoutBox().style().fontMetrics().ascent();
>     auto logicalRect = Display::Rect { -ascent, contentLogicalRight(), { }, logicalHeight() };
>-    m_lineItems.append(std::make_unique<LineItem>(Display::Run { logicalRect }, inlineItem, false, false));
>+    m_content.runs().append(std::make_unique<Content::Run>(Display::Run { logicalRect }, inlineItem, false, false));
> }
> 
> Line::UsedHeightAndDepth Line::halfLeadingMetrics(const FontMetrics& fontMetrics, LayoutUnit lineLogicalHeight)
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.h b/Source/WebCore/layout/inlineformatting/InlineLine.h
>index 38665cb351b21a7ffa28ef140d5e318279f27a8e..b9cf9d9545567ff5c930e1a3acc84463d4b44ba9 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.h
>@@ -39,18 +39,40 @@ public:
> 
>     void reset(const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUnit minimumLineHeight, LayoutUnit baselineOffset);
> 
>-    struct LineItem {
>-        LineItem(Display::Run, const InlineItem&, bool isCollapsed, bool canBeExtended);
>-
>-        // Relative to the baseline.
>-        Display::Run inlineRun;
>-        const InlineItem& inlineItem;
>-        bool isCollapsed { false };
>-        bool canBeExtended { false };
>+    class Content {
>+    public:
>+        struct Run {
>+            Run(Display::Run, const InlineItem&, bool isCollapsed, bool canBeExtended);
>+
>+            // Relative to the baseline.
>+            Display::Run inlineRun;
>+            const InlineItem& inlineItem;
>+            bool isCollapsed { false };
>+            bool canBeExtended { false };
>+        };
>+        using Runs = Vector<std::unique_ptr<Run>>;
>+        const Runs& runs() const { return m_runs; }
>+        bool isEmpty() const { return m_runs.isEmpty(); }
>+        // Not in painting sense though.
>+        bool isVisuallyEmpty() const;
>+
>+        LayoutUnit logicalTop() const { return m_logicalRect.top(); }
>+        LayoutUnit logicalLeft() const { return m_logicalRect.left(); }
>+        LayoutUnit logicalRight() const { return logicalLeft() + logicalWidth(); }
>+        LayoutUnit logicalBottom() const { return logicalTop() + logicalHeight(); }
>+        LayoutUnit logicalWidth() const { return m_logicalRect.width(); }
>+        LayoutUnit logicalHeight() const { return m_logicalRect.height(); }
>+
>+    private:
>+        friend class Line;
>+
>+        void setLogicalRect(const Display::Rect& logicalRect) { m_logicalRect = logicalRect; }
>+        Runs& runs() { return m_runs; }
>+
>+        Display::Rect m_logicalRect;
>+        Runs m_runs;
>     };
>-
>-    using LineItems = Vector<std::unique_ptr<LineItem>>;
>-    const LineItems& close();
>+    const Content& close();
> 
>     void appendTextContent(const InlineTextItem&, LayoutSize);
>     void appendNonReplacedInlineBox(const InlineItem&, LayoutSize);
>@@ -59,7 +81,7 @@ public:
>     void appendInlineContainerEnd(const InlineItem&);
>     void appendHardLineBreak(const InlineItem&);
> 
>-    bool hasContent() const;
>+    bool hasContent() const { return !m_content.isVisuallyEmpty(); }
> 
>     LayoutUnit trailingTrimmableWidth() const;
> 
>@@ -67,16 +89,9 @@ public:
>     void moveLogicalRight(LayoutUnit);
> 
>     LayoutUnit availableWidth() const { return logicalWidth() - contentLogicalWidth(); }
>-    
>     LayoutUnit contentLogicalRight() const { return logicalLeft() + contentLogicalWidth(); }
>-    LayoutUnit contentLogicalWidth() const { return m_contentLogicalWidth; }
>-
>     LayoutUnit logicalTop() const { return m_logicalTopLeft.y(); }
>-    LayoutUnit logicalLeft() const { return m_logicalTopLeft.x(); }
>-    LayoutUnit logicalRight() const { return logicalLeft() + logicalWidth(); }
>     LayoutUnit logicalBottom() const { return logicalTop() + logicalHeight(); }
>-    LayoutUnit logicalWidth() const { return m_lineLogicalWidth; }
>-    LayoutUnit logicalHeight() const { return m_logicalHeight.height + m_logicalHeight.depth; }
> 
>     struct UsedHeightAndDepth {
>         LayoutUnit height;
>@@ -85,12 +100,20 @@ public:
>     static UsedHeightAndDepth halfLeadingMetrics(const FontMetrics&, LayoutUnit lineLogicalHeight);
> 
> private:
>+    LayoutUnit logicalLeft() const { return m_logicalTopLeft.x(); }
>+    LayoutUnit logicalRight() const { return logicalLeft() + logicalWidth(); }
>+
>+    LayoutUnit logicalWidth() const { return m_lineLogicalWidth; }
>+    LayoutUnit logicalHeight() const { return m_logicalHeight.height + m_logicalHeight.depth; }
>+
>+    LayoutUnit contentLogicalWidth() const { return m_contentLogicalWidth; }
>+
>     void appendNonBreakableSpace(const InlineItem&, const Display::Rect& logicalRect);
>     void removeTrailingTrimmableContent();
> 
>     const LayoutState& m_layoutState;
>-    LineItems m_lineItems;
>-    ListHashSet<LineItem*> m_trimmableContent;
>+    Content m_content;
>+    ListHashSet<Content::Run*> m_trimmableContent;
> 
>     LayoutPoint m_logicalTopLeft;
>     LayoutUnit m_contentLogicalWidth;

Actions: View | Formatted Diff | Diff

Attachments on bug 198360: 370909 | 370966 | 370980