Attachment 373697 Details for Bug 199608 – Patch

[patch] Patch

bug-199608-20190708203502.patch (text/plain), 21.81 KB, created by zalan on 2019-07-08 20:35:10 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-08 20:35:10 PDT

Size: 21.81 KB

patch

obsolete

>Subversion Revision: 247199
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 112b805305f1c58dd833bdc94edbbff27efbee8e..d41cc973dabab1d798383dc28256f6b0c46666dc 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2019-07-08  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] Remove InlineItem references from inline runs.
>+        https://bugs.webkit.org/show_bug.cgi?id=199608
>+        <rdar://problem/52812775>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Constructing the inline runs is where we stop using InlineItems in the process of processing the inline content.
>+        InlineItems are redundant in the context of Display::Runs.
>+        It also enables us to create temporary InlineItems to process split content.  
>+
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createDisplayRuns const):
>+        * layout/inlineformatting/InlineFormattingContextQuirks.cpp:
>+        (WebCore::Layout::InlineFormattingContext::Quirks::lineDescentNeedsCollapsing):
>+        * layout/inlineformatting/InlineLine.cpp:
>+        (WebCore::Layout::Line::Content::Run::Run):
>+        (WebCore::Layout::Line::isVisuallyEmpty const):
>+        (WebCore::Layout::Line::close):
>+        (WebCore::Layout::Line::removeTrailingTrimmableContent):
>+        (WebCore::Layout::Line::trailingTrimmableWidth const):
>+        (WebCore::Layout::Line::appendNonBreakableSpace):
>+        (WebCore::Layout::Line::appendTextContent):
>+        (WebCore::Layout::Line::appendNonReplacedInlineBox):
>+        (WebCore::Layout::Line::appendHardLineBreak):
>+        * layout/inlineformatting/InlineLine.h:
>+        (WebCore::Layout::Line::Content::Run::layoutBox const):
>+        (WebCore::Layout::Line::Content::Run::logicalRect const):
>+        (WebCore::Layout::Line::Content::Run::textContext const):
>+        (WebCore::Layout::Line::Content::Run::type const):
>+        (WebCore::Layout::Line::Content::Run::isText const):
>+        (WebCore::Layout::Line::Content::Run::isBox const):
>+        (WebCore::Layout::Line::Content::Run::isLineBreak const):
>+        (WebCore::Layout::Line::Content::Run::isContainerStart const):
>+        (WebCore::Layout::Line::Content::Run::isContainerEnd const):
>+        (WebCore::Layout::Line::Content::Run::adjustLogicalTop):
>+        (WebCore::Layout::Line::Content::Run::moveVertically):
>+        (WebCore::Layout::Line::Content::Run::moveHorizontally):
>+        (WebCore::Layout::Line::Content::Run::setTextIsCollapsed):
>+
> 2019-07-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Introduce splitPosition to LineLayout
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index c0ffa16a510f98a959b4aab7fff32506f7b2dc94..91e5997961f97d13d2238cf30ab4d377d140ff9c 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -317,13 +317,11 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
>     auto& lineRuns = lineContent.runs();
>     for (unsigned index = 0; index < lineRuns.size(); ++index) {
>         auto& lineRun = lineRuns.at(index);
>-
>-        auto& inlineItem = lineRun->inlineItem;
>-        auto& logicalRect = lineRun->logicalRect;
>-        auto& layoutBox = inlineItem.layoutBox();
>+        auto& logicalRect = lineRun->logicalRect();
>+        auto& layoutBox = lineRun->layoutBox();
>         auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
> 
>-        if (inlineItem.isHardLineBreak()) {
>+        if (lineRun->isLineBreak()) {
>             displayBox.setTopLeft(logicalRect.topLeft());
>             displayBox.setContentBoxWidth(logicalRect.width());
>             displayBox.setContentBoxHeight(logicalRect.height());
>@@ -332,7 +330,7 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
>         }
> 
>         // Inline level box (replaced or inline-block)
>-        if (inlineItem.isBox()) {
>+        if (lineRun->isBox()) {
>             auto topLeft = logicalRect.topLeft();
>             if (layoutBox.isInFlowPositioned())
>                 topLeft += Geometry::inFlowPositionedPositionOffset(layoutState(), layoutBox);
>@@ -343,14 +341,14 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
>         }
> 
>         // Inline level container start (<span>)
>-        if (inlineItem.isContainerStart()) {
>+        if (lineRun->isContainerStart()) {
>             displayBox.setTopLeft(logicalRect.topLeft());
>             lineBoxRect.expandHorizontally(logicalRect.width());
>             continue;
>         }
> 
>         // Inline level container end (</span>)
>-        if (inlineItem.isContainerEnd()) {
>+        if (lineRun->isContainerEnd()) {
>             if (layoutBox.isInFlowPositioned()) {
>                 auto inflowOffset = Geometry::inFlowPositionedPositionOffset(layoutState(), layoutBox);
>                 displayBox.moveHorizontally(inflowOffset.width());
>@@ -366,28 +364,29 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
>         }
> 
>         // Text content. Try to join multiple text runs when possible.
>-        ASSERT(lineRun->textContext);        
>+        ASSERT(lineRun->isText());
>+        auto textContext = lineRun->textContext();   
>         const Line::Content::Run* previousLineRun = !index ? nullptr : lineRuns[index - 1].get();
>-        if (!lineRun->isCollapsed) {
>-            auto previousRunCanBeExtended = previousLineRun ? previousLineRun->canBeExtended : false;
>-            auto requiresNewRun = !index || !previousRunCanBeExtended || &layoutBox != &previousLineRun->inlineItem.layoutBox();
>+        if (!textContext->isCollapsed) {
>+            auto previousRunCanBeExtended = previousLineRun && previousLineRun->textContext() ? previousLineRun->textContext()->canBeExtended : false;
>+            auto requiresNewRun = !index || !previousRunCanBeExtended || &layoutBox != &previousLineRun->layoutBox();
>             if (requiresNewRun)
>-                m_formattingState.addInlineRun(std::make_unique<Display::Run>(logicalRect, Display::Run::TextContext { lineRun->textContext->start, lineRun->textContext->length }));
>+                m_formattingState.addInlineRun(std::make_unique<Display::Run>(logicalRect, Display::Run::TextContext { textContext->start, textContext->length }));
>             else {
>                 auto& lastDisplayRun = m_formattingState.inlineRuns().last();
>                 lastDisplayRun->expandHorizontally(logicalRect.width());
>-                lastDisplayRun->textContext()->expand(lineRun->textContext->length);
>+                lastDisplayRun->textContext()->expand(textContext->length);
>             }
>             lineBoxRect.expandHorizontally(logicalRect.width());
>         }
>         // FIXME take content breaking into account when part of the layout box is on the previous line.
>-        auto firstInlineRunForLayoutBox = !previousLineRun || &previousLineRun->inlineItem.layoutBox() != &layoutBox;
>+        auto firstInlineRunForLayoutBox = !previousLineRun || &previousLineRun->layoutBox() != &layoutBox;
>         if (firstInlineRunForLayoutBox) {
>             // Setup display box for the associated layout box.
>             displayBox.setTopLeft(logicalRect.topLeft());
>-            displayBox.setContentBoxWidth(lineRun->isCollapsed ? LayoutUnit() : logicalRect.width());
>+            displayBox.setContentBoxWidth(textContext->isCollapsed ? LayoutUnit() : logicalRect.width());
>             displayBox.setContentBoxHeight(logicalRect.height());
>-        } else if (!lineRun->isCollapsed) {
>+        } else if (!textContext->isCollapsed) {
>             // FIXME fix it for multirun/multiline.
>             displayBox.setContentBoxWidth(displayBox.contentBoxWidth() + logicalRect.width());
>         }
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp
>index c2a7903910c77c2921d81f11c03366e76a0018a1..9d2e8ab60c79312c433bf67b37a0d0d525170be8 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp
>@@ -44,19 +44,19 @@ bool InlineFormattingContext::Quirks::lineDescentNeedsCollapsing(const LayoutSta
>         return false;
> 
>     for (auto& run : lineContent.runs()) {
>-        auto& inlineItem = run->inlineItem;
>-        if (inlineItem.style().verticalAlign() != VerticalAlign::Baseline)
>+        auto& layoutBox = run->layoutBox();
>+        if (layoutBox.style().verticalAlign() != VerticalAlign::Baseline)
>             continue;
> 
>-        switch (inlineItem.type()) {
>+        switch (run->type()) {
>         case InlineItem::Type::Text:
>-            if (!run->isCollapsed)
>+            if (!run->textContext() || !run->textContext()->isCollapsed)
>                 return false;
>             break;
>         case InlineItem::Type::HardLineBreak:
>             return false;
>         case InlineItem::Type::ContainerStart: {
>-            auto& displayBox = layoutState.displayBoxForLayoutBox(inlineItem.layoutBox());
>+            auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>             if (displayBox.horizontalBorder() || (displayBox.horizontalPadding() && displayBox.horizontalPadding().value()))
>                 return false;
>             break;
>@@ -64,7 +64,6 @@ bool InlineFormattingContext::Quirks::lineDescentNeedsCollapsing(const LayoutSta
>         case InlineItem::Type::ContainerEnd:
>             break;
>         case InlineItem::Type::Box: {
>-            auto& layoutBox = inlineItem.layoutBox();
>             if (layoutBox.isInlineBlockBox() && layoutBox.establishesInlineFormattingContext()) {
>                 auto& formattingState = downcast<InlineFormattingState>(layoutState.establishedFormattingState(layoutBox));
>                 ASSERT(!formattingState.lineBoxes().isEmpty());
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.cpp b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>index 645a47de02ee360dfba0c1a355b65133a10e8834..e86cb75d3ec637c14662ab0fe37cc865271a8703 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>@@ -35,12 +35,18 @@ namespace Layout {
> 
> WTF_MAKE_ISO_ALLOCATED_IMPL(Line);
> 
>-Line::Content::Run::Run(const InlineItem& inlineItem, const Display::Rect& logicalRect, TextContext textContext, bool isCollapsed, bool canBeExtended)
>-    : inlineItem(inlineItem)
>-    , logicalRect(logicalRect)
>-    , textContext(textContext)
>-    , isCollapsed(isCollapsed)
>-    , canBeExtended(canBeExtended)
>+Line::Content::Run::Run(const InlineItem& inlineItem, const Display::Rect& logicalRect)
>+    : m_layoutBox(inlineItem.layoutBox())
>+    , m_type(inlineItem.type())
>+    , m_logicalRect(logicalRect)
>+{
>+}
>+
>+Line::Content::Run::Run(const InlineItem& inlineItem, const TextContext& textContext, const Display::Rect& logicalRect)
>+    : m_layoutBox(inlineItem.layoutBox())
>+    , m_type(inlineItem.type())
>+    , m_logicalRect(logicalRect)
>+    , m_textContext(textContext)
> {
> }
> 
>@@ -68,23 +74,23 @@ bool Line::isVisuallyEmpty() const
>     // FIXME: This should be cached instead -as the inline items are being added.
>     // Return true for empty inline containers like <span></span>.
>     for (auto& run : m_content->runs()) {
>-        if (run->inlineItem.isContainerStart()) {
>-            if (!isInlineContainerConsideredEmpty(m_layoutState, run->inlineItem.layoutBox()))
>+        if (run->isContainerStart()) {
>+            if (!isInlineContainerConsideredEmpty(m_layoutState, run->layoutBox()))
>                 return false;
>             continue;
>         }
>-        if (run->inlineItem.isContainerEnd())
>+        if (run->isContainerEnd())
>             continue;
>-        if (run->inlineItem.layoutBox().establishesFormattingContext()) {
>-            ASSERT(run->inlineItem.layoutBox().isInlineBlockBox());
>-            auto& displayBox = m_layoutState.displayBoxForLayoutBox(run->inlineItem.layoutBox());
>+        if (run->layoutBox().establishesFormattingContext()) {
>+            ASSERT(run->layoutBox().isInlineBlockBox());
>+            auto& displayBox = m_layoutState.displayBoxForLayoutBox(run->layoutBox());
>             if (!displayBox.width())
>                 continue;
>             if (m_skipVerticalAligment || displayBox.height())
>                 return false;
>             continue;
>         }
>-        if (!run->isCollapsed)
>+        if (!run->textContext() || !run->textContext()->isCollapsed)
>             return false;
>     }
>     return true;
>@@ -108,16 +114,15 @@ std::unique_ptr<Line::Content> Line::close()
> 
>         for (auto& run : m_content->runs()) {
>             LayoutUnit logicalTop;
>-            auto& inlineItem = run->inlineItem;
>-            auto& layoutBox = inlineItem.layoutBox();
>-            auto verticalAlign = inlineItem.style().verticalAlign();
>-            auto ascent = inlineItem.style().fontMetrics().ascent();
>+            auto& layoutBox = run->layoutBox();
>+            auto verticalAlign = layoutBox.style().verticalAlign();
>+            auto ascent = layoutBox.style().fontMetrics().ascent();
> 
>             switch (verticalAlign) {
>             case VerticalAlign::Baseline:
>-                if (inlineItem.isLineBreak() || inlineItem.isText())
>+                if (run->isLineBreak() || run->isText())
>                     logicalTop = baselineOffset() - ascent;
>-                else if (inlineItem.isContainerStart()) {
>+                else if (run->isContainerStart()) {
>                     auto& displayBox = m_layoutState.displayBoxForLayoutBox(layoutBox);
>                     logicalTop = baselineOffset() - ascent - displayBox.borderTop() - displayBox.paddingTop().valueOr(0);
>                 } else if (layoutBox.isInlineBlockBox() && layoutBox.establishesInlineFormattingContext()) {
>@@ -127,22 +132,22 @@ std::unique_ptr<Line::Content> Line::close()
>                     auto inlineBlockBaseline = formattingState.lineBoxes().last().baseline();
>                     logicalTop = baselineOffset() - inlineBlockBaseline.ascent;
>                 } else
>-                    logicalTop = baselineOffset() - run->logicalRect.height();
>+                    logicalTop = baselineOffset() - run->logicalRect().height();
>                 break;
>             case VerticalAlign::Top:
>                 logicalTop = { };
>                 break;
>             case VerticalAlign::Bottom:
>-                logicalTop = logicalBottom() - run->logicalRect.height();
>+                logicalTop = logicalBottom() - run->logicalRect().height();
>                 break;
>             default:
>                 ASSERT_NOT_IMPLEMENTED_YET();
>                 break;
>             }
>-            run->logicalRect.setTop(logicalTop);
>+            run->adjustLogicalTop(logicalTop);
>             // Convert runs from relative to the line top/left to the formatting root's border box top/left.
>-            run->logicalRect.moveVertically(this->logicalTop());
>-            run->logicalRect.moveHorizontally(this->logicalLeft());
>+            run->moveVertically(this->logicalTop());
>+            run->moveHorizontally(this->logicalLeft());
>         }
>     }
>     m_content->setLogicalRect({ logicalTop(), logicalLeft(), contentLogicalWidth(), logicalHeight() });
>@@ -156,8 +161,9 @@ void Line::removeTrailingTrimmableContent()
>     // Collapse trimmable trailing content
>     LayoutUnit trimmableWidth;
>     for (auto* trimmableRun : m_trimmableContent) {
>-        trimmableRun->isCollapsed = true;
>-        trimmableWidth += trimmableRun->logicalRect.width();
>+        ASSERT(trimmableRun->isText());
>+        trimmableRun->setTextIsCollapsed();
>+        trimmableWidth += trimmableRun->logicalRect().width();
>     }
>     m_contentLogicalWidth -= trimmableWidth;
> }
>@@ -182,8 +188,8 @@ LayoutUnit Line::trailingTrimmableWidth() const
> {
>     LayoutUnit trimmableWidth;
>     for (auto* trimmableRun : m_trimmableContent) {
>-        ASSERT(!trimmableRun->isCollapsed);
>-        trimmableWidth += trimmableRun->logicalRect.width();
>+        ASSERT(!trimmableRun->textContext()->isCollapsed);
>+        trimmableWidth += trimmableRun->logicalRect().width();
>     }
>     return trimmableWidth;
> }
>@@ -205,7 +211,7 @@ void Line::append(const InlineItem& inlineItem, LayoutUnit logicalWidth)
> 
> void Line::appendNonBreakableSpace(const InlineItem& inlineItem, const Display::Rect& logicalRect)
> {
>-    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect, Content::Run::TextContext { }, false, false));
>+    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect));
>     m_contentLogicalWidth += logicalRect.width();
> }
> 
>@@ -250,12 +256,12 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutUnit logica
>             return true;
>         // Check if the last item is trimmable as well.
>         for (int index = runs.size() - 1; index >= 0; --index) {
>-            auto& inlineItem = runs[index]->inlineItem;
>-            if (inlineItem.isBox())
>+            auto& run = runs[index];
>+            if (run->isBox())
>                 return false;
>-            if (inlineItem.isText())
>-                return TextUtil::isTrimmableContent(inlineItem);
>-            ASSERT(inlineItem.isContainerStart() || inlineItem.isContainerEnd());
>+            if (run->isText())
>+                return run->textContext()->isWhitespace && run->layoutBox().style().collapseWhiteSpace();
>+            ASSERT(run->isContainerStart() || run->isContainerEnd());
>         }
>         return true;
>     };
>@@ -273,8 +279,8 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutUnit logica
>         adjustBaselineAndLineHeight(inlineItem, runHeight);
>     }
> 
>-    auto textContext = Content::Run::TextContext { inlineItem.start(), inlineItem.isCollapsed() ? 1 : inlineItem.length() };
>-    auto lineItem = std::make_unique<Content::Run>(inlineItem, logicalRect, textContext, isCompletelyCollapsed, canBeExtended);
>+    auto textContext = Content::Run::TextContext { inlineItem.start(), inlineItem.isCollapsed() ? 1 : inlineItem.length(), isCompletelyCollapsed, canBeExtended, inlineItem.isWhitespace() };
>+    auto lineItem = std::make_unique<Content::Run>(inlineItem, textContext, logicalRect);
>     if (isTrimmable && !isCompletelyCollapsed)
>         m_trimmableContent.add(lineItem.get());
> 
>@@ -295,7 +301,7 @@ void Line::appendNonReplacedInlineBox(const InlineItem& inlineItem, LayoutUnit l
>         logicalRect.setHeight(inlineItemContentHeight(inlineItem));
>     }
> 
>-    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect, Content::Run::TextContext { }, false, false));
>+    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect));
>     m_contentLogicalWidth += (logicalWidth + horizontalMargin.start + horizontalMargin.end);
>     m_trimmableContent.clear();
> }
>@@ -315,7 +321,7 @@ void Line::appendHardLineBreak(const InlineItem& inlineItem)
>         adjustBaselineAndLineHeight(inlineItem, { });
>         logicalRect.setHeight(logicalHeight());
>     }
>-    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect, Content::Run::TextContext { }, false, false));
>+    m_content->runs().append(std::make_unique<Content::Run>(inlineItem, logicalRect));
> }
> 
> void Line::adjustBaselineAndLineHeight(const InlineItem& inlineItem, LayoutUnit runHeight)
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.h b/Source/WebCore/layout/inlineformatting/InlineLine.h
>index cb69abb25748bfd1aaa51d5bd97dfcd58b438950..4f6f30b46b14bbf646613b8a8721136b32861d5e 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.h
>@@ -57,15 +57,35 @@ public:
>             struct TextContext {
>                 unsigned start { 0 };
>                 unsigned length { 0 };
>+                bool isCollapsed { false };
>+                bool isWhitespace { false };
>+                bool canBeExtended { false };
>             };
>-            Run(const InlineItem&, const Display::Rect&, TextContext, bool isCollapsed, bool canBeExtended);
>-
>-            const InlineItem& inlineItem;
>-            Display::Rect logicalRect;
>-            LayoutUnit baseline;
>-            Optional<TextContext> textContext;
>-            bool isCollapsed { false };
>-            bool canBeExtended { false };
>+            Run(const InlineItem&, const Display::Rect&);
>+            Run(const InlineItem&, const TextContext&, const Display::Rect&);
>+
>+            const Box& layoutBox() const { return m_layoutBox; }
>+            const Display::Rect& logicalRect() const { return m_logicalRect; }
>+            const Optional<TextContext> textContext() const { return m_textContext; }
>+            InlineItem::Type type() const { return m_type; }
>+
>+            bool isText() const { return m_type == InlineItem::Type::Text; }
>+            bool isBox() const { return m_type == InlineItem::Type::Box; }
>+            bool isLineBreak() const { return m_type == InlineItem::Type::HardLineBreak; }
>+            bool isContainerStart() const { return m_type == InlineItem::Type::ContainerStart; }
>+            bool isContainerEnd() const { return m_type == InlineItem::Type::ContainerEnd; }
>+
>+        private:
>+            friend class Line;
>+            void adjustLogicalTop(LayoutUnit logicalTop) { m_logicalRect.setTop(logicalTop); }
>+            void moveVertically(LayoutUnit offset) { m_logicalRect.moveVertically(offset); }
>+            void moveHorizontally(LayoutUnit offset) { m_logicalRect.moveHorizontally(offset); }
>+            void setTextIsCollapsed() { m_textContext->isCollapsed = true; }
>+
>+            const Box& m_layoutBox;
>+            const InlineItem::Type m_type;
>+            Display::Rect m_logicalRect;
>+            Optional<TextContext> m_textContext;
>         };
>         using Runs = Vector<std::unique_ptr<Run>>;
>         const Runs& runs() const { return m_runs; }

Actions: View | Formatted Diff | Diff

Attachments on bug 199608: 373697