Attachment 370920 Details for Bug 198366 – Patch

[patch] Patch

bug-198366-20190529215226.patch (text/plain), 13.84 KB, created by zalan on 2019-05-29 21:52:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-05-29 21:52:26 PDT

Size: 13.84 KB

patch

obsolete

>Subversion Revision: 245776
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a0d1d08d8306f0706b060559670e3d4a7ca7398f..53c4b63a4566b72f83f98e97e163a060e3b25cec 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-05-29  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC[IFC] Do not reuse the same Line object.
>+        https://bugs.webkit.org/show_bug.cgi?id=198366
>+        <rdar://problem/51250279>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This is in preparation for constructing Line inside createInlineRunsForLine and return Line::Content. 
>+
>+        * layout/inlineformatting/InlineFormattingContext.h:
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createLine const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::layout const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::processInlineRuns const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::initializeLine const): Deleted.
>+        * layout/inlineformatting/InlineLine.cpp:
>+        (WebCore::Layout::Line::Line):
>+        (WebCore::Layout::m_lineLogicalWidth):
>+        (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::reset): Deleted.
>+        * layout/inlineformatting/InlineLine.h:
>+        (WebCore::Layout::Line::hasContent const):
>+
> 2019-05-29  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Move final runs to a dedicated class (Line::Content)
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>index 934a700a0240c2bcd730cb5e4888e6c2d4cdd02e..cc038ac55131723883a5e66aaa4d024c0e767d75 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>@@ -30,6 +30,7 @@
> #include "DisplayBox.h"
> #include "FormattingContext.h"
> #include "InlineFormattingState.h"
>+#include "InlineLine.h"
> #include <wtf/IsoMalloc.h>
> 
> namespace WebCore {
>@@ -37,7 +38,6 @@ namespace Layout {
> 
> class FloatingState;
> class InlineContainer;
>-class Line;
> 
> // This class implements the layout logic for inline formatting contexts.
> // https://www.w3.org/TR/CSS22/visuren.html#inline-formatting
>@@ -58,9 +58,9 @@ private:
> 
>     private:
>         LayoutState& layoutState() const { return m_formattingContext.layoutState(); }
>-        void initializeLine(Line&, LayoutUnit lineLogicalTop, LayoutUnit widthConstraint) const;
>+        std::unique_ptr<Line> createLine(LayoutUnit lineLogicalTop, LayoutUnit widthConstraint) const;
>         unsigned createInlineRunsForLine(Line&, unsigned firstInlineItemIndex) const;
>-        void processInlineRuns(Line&) const;
>+        void processInlineRuns(const Line::Content&, LayoutUnit availableWidth) const;
>         void commitInlineItemToLine(Line&, const InlineItem&) const;
>         void handleFloat(Line&, const FloatingContext&, const InlineItem& floatBox) const;
>         void alignRuns(TextAlignMode, unsigned firstRunIndex, LayoutUnit availableWidth) const;
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index a2113115ea2694b7cd0b2e01de8759eee359ed07..998feda949d466b0bbda99ad7fef771f2abff255 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -75,7 +75,7 @@ InlineFormattingContext::LineLayout::LineLayout(const InlineFormattingContext& i
> {
> }
> 
>-void InlineFormattingContext::LineLayout::initializeLine(Line& line, LayoutUnit lineLogicalTop, LayoutUnit availableWidth) const
>+std::unique_ptr<Line> InlineFormattingContext::LineLayout::createLine(LayoutUnit lineLogicalTop, LayoutUnit availableWidth) const
> {
>     auto& formattingRootDisplayBox = layoutState().displayBoxForLayoutBox(m_formattingRoot);
>     auto lineLogicalLeft = formattingRootDisplayBox.contentBoxLeft();
>@@ -107,7 +107,7 @@ void InlineFormattingContext::LineLayout::initializeLine(Line& line, LayoutUnit
>     auto& formattingRootStyle = m_formattingRoot.style();
>     auto mimimumLineHeight = formattingRootStyle.computedLineHeight();
>     auto baselineOffset = Line::halfLeadingMetrics(formattingRootStyle.fontMetrics(), mimimumLineHeight).height;
>-    line.reset({ lineLogicalLeft, lineLogicalTop }, availableWidth, mimimumLineHeight, baselineOffset);
>+    return std::make_unique<Line>(layoutState(), LayoutPoint { lineLogicalLeft, lineLogicalTop }, availableWidth, mimimumLineHeight, baselineOffset);
> }
> 
> unsigned InlineFormattingContext::LineLayout::createInlineRunsForLine(Line& line, unsigned startInlineItemIndex) const
>@@ -172,17 +172,17 @@ void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) con
> {
>     ASSERT(!m_formattingState.inlineItems().isEmpty());
> 
>-    Line line(layoutState());
>-    initializeLine(line, layoutState().displayBoxForLayoutBox(m_formattingRoot).contentBoxTop(), widthConstraint);
>-
>     unsigned startInlineItemIndex = 0;
>+    auto lineLogicalTop = layoutState().displayBoxForLayoutBox(m_formattingRoot).contentBoxTop();
>     while (true) {
>-        auto nextInlineItemIndex = createInlineRunsForLine(line, startInlineItemIndex);
>-        processInlineRuns(line);
>+        auto line = createLine(lineLogicalTop, widthConstraint);
>+        auto nextInlineItemIndex = createInlineRunsForLine(*line, startInlineItemIndex);
>+        auto lineContent = line->close();
>+        processInlineRuns(*lineContent, line->availableWidth());
>         if (nextInlineItemIndex == m_formattingState.inlineItems().size())
>             break;
>         startInlineItemIndex = nextInlineItemIndex;
>-        initializeLine(line, line.logicalBottom(), widthConstraint);
>+        lineLogicalTop = lineContent->logicalBottom();
>     }
> }
> 
>@@ -214,9 +214,8 @@ LayoutUnit InlineFormattingContext::LineLayout::computedIntrinsicWidth(LayoutUni
>     return std::max(maximumLineWidth, lineLogicalRight - trimmableTrailingWidth);
> }
> 
>-void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
>+void InlineFormattingContext::LineLayout::processInlineRuns(const Line::Content& lineContent, LayoutUnit availableWidth) const
> {
>-    auto& lineContent = line.close();
>     if (lineContent.isEmpty()) {
>         // Spec tells us to create a zero height, empty line box.
>         auto lineBox = Display::Rect { lineContent.logicalTop(), lineContent.logicalLeft(), 0 , 0 };
>@@ -314,7 +313,7 @@ void InlineFormattingContext::LineLayout::processInlineRuns(Line& line) const
>     // FIXME linebox needs to be ajusted after content alignment.
>     m_formattingState.addLineBox({ lineBox });
>     if (!lineContent.isVisuallyEmpty())
>-        alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, line.availableWidth());
>+        alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, availableWidth);
> }
> 
> void InlineFormattingContext::LineLayout::handleFloat(Line& line, const FloatingContext& floatingContext, const InlineItem& floatItem) const
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.cpp b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>index 0dd297df2d731fa31d7d71a7c9d6c6c371bc0c74..f8e2a87a2a940465effed93953bf56e91c6d9f93 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>@@ -51,34 +51,25 @@ Line::Content::Run::Run(Display::Run inlineRun, const InlineItem& inlineItem, bo
> {
> }
> 
>-Line::Line(const LayoutState& layoutState)
>+Line::Line(const LayoutState& layoutState, const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUnit minimumHeight, LayoutUnit baselineOffset)
>     : m_layoutState(layoutState)
>+    , m_content(std::make_unique<Line::Content>())
>+    , m_logicalTopLeft(topLeft)
>+    , m_logicalHeight({ baselineOffset, minimumHeight - baselineOffset })
>+    , m_lineLogicalWidth(availableWidth)
> {
> }
> 
>-void Line::reset(const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUnit minimumHeight, LayoutUnit baselineOffset)
>-{
>-    m_logicalTopLeft = topLeft;
>-    m_lineLogicalWidth = availableWidth;
>-    m_logicalHeight = { baselineOffset, minimumHeight - baselineOffset };
>-
>-    m_contentLogicalWidth = { };
>-
>-    m_content = { };
>-
>-    m_trimmableContent.clear();
>-}
>-
>-const Line::Content& Line::close()
>+std::unique_ptr<Line::Content> Line::close()
> {
>     removeTrailingTrimmableContent();
>     // Convert inline run geometry from relative to the baseline to relative to logical top.
>-    for (auto& run : m_content.runs()) {
>+    for (auto& run : m_content->runs()) {
>         auto adjustedLogicalTop = run->inlineRun.logicalTop() + m_logicalHeight.height + m_logicalTopLeft.y();
>         run->inlineRun.setLogicalTop(adjustedLogicalTop);
>     }
>-    m_content.setLogicalRect({ logicalTop(), logicalLeft(), contentLogicalWidth(), logicalHeight() });
>-    return m_content;
>+    m_content->setLogicalRect({ logicalTop(), logicalLeft(), contentLogicalWidth(), logicalHeight() });
>+    return WTFMove(m_content);
> }
> 
> void Line::removeTrailingTrimmableContent()
>@@ -100,7 +91,7 @@ void Line::moveLogicalLeft(LayoutUnit delta)
>     // Shrink the line and move the items.
>     m_logicalTopLeft.move(delta, 0);
>     m_lineLogicalWidth -= delta;
>-    for (auto& run : m_content.runs())
>+    for (auto& run : m_content->runs())
>         run->inlineRun.moveHorizontally(delta);
> }
> 
>@@ -120,7 +111,7 @@ LayoutUnit Line::trailingTrimmableWidth() const
> 
> void Line::appendNonBreakableSpace(const InlineItem& inlineItem, const Display::Rect& logicalRect)
> {
>-    m_content.runs().append(std::make_unique<Content::Run>(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,7 +156,7 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>         if (!isTrimmable)
>             return false;
>         // Leading whitespace.
>-        auto& runs = m_content.runs();
>+        auto& runs = m_content->runs();
>         if (runs.isEmpty())
>             return true;
>         // Check if the last item is trimmable as well.
>@@ -191,7 +182,7 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>     if (isTrimmable)
>         m_trimmableContent.add(lineItem.get());
> 
>-    m_content.runs().append(WTFMove(lineItem));
>+    m_content->runs().append(WTFMove(lineItem));
>     m_contentLogicalWidth += isCompletelyCollapsed ? LayoutUnit() : runSize.width();
> }
> 
>@@ -219,7 +210,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_content.runs().append(std::make_unique<Content::Run>(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();
> }
>@@ -234,7 +225,7 @@ void Line::appendHardLineBreak(const InlineItem& inlineItem)
> {
>     auto ascent = inlineItem.layoutBox().style().fontMetrics().ascent();
>     auto logicalRect = Display::Rect { -ascent, contentLogicalRight(), { }, logicalHeight() };
>-    m_content.runs().append(std::make_unique<Content::Run>(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 b9cf9d9545567ff5c930e1a3acc84463d4b44ba9..1075260a91a5bc526006bd9afce692e30ab94741 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.h
>@@ -29,15 +29,14 @@
> 
> #include "DisplayRun.h"
> #include "InlineItem.h"
>+#include "InlineTextItem.h"
> 
> namespace WebCore {
> namespace Layout {
> 
> class Line {
> public:
>-    Line(const LayoutState&);
>-
>-    void reset(const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUnit minimumLineHeight, LayoutUnit baselineOffset);
>+    Line(const LayoutState&, const LayoutPoint& topLeft, LayoutUnit availableWidth, LayoutUnit minimumLineHeight, LayoutUnit baselineOffset);
> 
>     class Content {
>     public:
>@@ -72,7 +71,7 @@ public:
>         Display::Rect m_logicalRect;
>         Runs m_runs;
>     };
>-    const Content& close();
>+    std::unique_ptr<Content> close();
> 
>     void appendTextContent(const InlineTextItem&, LayoutSize);
>     void appendNonReplacedInlineBox(const InlineItem&, LayoutSize);
>@@ -81,7 +80,7 @@ public:
>     void appendInlineContainerEnd(const InlineItem&);
>     void appendHardLineBreak(const InlineItem&);
> 
>-    bool hasContent() const { return !m_content.isVisuallyEmpty(); }
>+    bool hasContent() const { return !m_content->isVisuallyEmpty(); }
> 
>     LayoutUnit trailingTrimmableWidth() const;
> 
>@@ -112,7 +111,7 @@ private:
>     void removeTrailingTrimmableContent();
> 
>     const LayoutState& m_layoutState;
>-    Content m_content;
>+    std::unique_ptr<Content> m_content;
>     ListHashSet<Content::Run*> m_trimmableContent;
> 
>     LayoutPoint m_logicalTopLeft;

Actions: View | Formatted Diff | Diff

Attachments on bug 198366: 370920 | 371053