Attachment 371225 Details for Bug 198502 – Patch

[patch] Patch

bug-198502-20190603160231.patch (text/plain), 13.18 KB, created by zalan on 2019-06-03 16:02:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-03 16:02:33 PDT

Size: 13.18 KB

patch

obsolete

>Subversion Revision: 246027
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8d1cbdd3b973e77bbf73a66b4db559dae11b37ca..92f3ad00cbcf12694f0eee5a889bf1cf5bb686df 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-06-03  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] Remove InlineItem::width
>+        https://bugs.webkit.org/show_bug.cgi?id=198502
>+        <rdar://problem/51371744>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        InlineItems are supposd to work across subsequent layouts (and in preferred width computation as well) so they should
>+        not hold on to layout information (run width). This would not work with split runs either.
>+
>+        * layout/inlineformatting/InlineFormattingContext.h:
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::UncommittedContent::runs):
>+        (WebCore::Layout::UncommittedContent::isEmpty const):
>+        (WebCore::Layout::UncommittedContent::size const):
>+        (WebCore::Layout::UncommittedContent::add):
>+        (WebCore::Layout::UncommittedContent::reset):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::placeInlineItems const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::computedIntrinsicWidth const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createDisplayRuns const):
>+        (): Deleted.
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::commitInlineItemToLine const): Deleted.
>+        * layout/inlineformatting/InlineItem.h:
>+        (WebCore::Layout::InlineItem::style const):
>+        (): Deleted.
>+        (WebCore::Layout::InlineItem::setWidth): Deleted.
>+        (WebCore::Layout::InlineItem::width const): Deleted.
>+
> 2019-06-03  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Move run width measuring out of LineBreaker
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>index 3a5d7bcfbddd096b94dd183d8e3858bd71b2bd7a..28fbad34984b067a36539a6932282e6353e5fde8 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>@@ -74,7 +74,6 @@ private:
>         };
>         LineContent placeInlineItems(const LineInput&) const;
>         void createDisplayRuns(const Line::Content&, LayoutUnit widthConstraint) 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 77d2a44763b0b81f1c58f427cbb1ec4219a3b21c..b7d19cb6a614a889bf78841eb7dfe1f4cd105b91 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -42,28 +42,33 @@ namespace WebCore {
> namespace Layout {
> 
> struct UncommittedContent {
>-    void add(InlineItem&);
>+    struct Run {
>+        const InlineItem& inlineItem;
>+        LayoutUnit logicalWidth;
>+        // FIXME: add optional breaking context (start and end position) for split text content.
>+    };
>+    void add(const InlineItem&, LayoutUnit logicalWidth);
>     void reset();
> 
>-    Vector<InlineItem*> inlineItems() { return m_inlineItems; }
>-    bool isEmpty() const { return m_inlineItems.isEmpty(); }
>-    unsigned size() const { return m_inlineItems.size(); }
>+    Vector<Run> runs() { return m_uncommittedRuns; }
>+    bool isEmpty() const { return m_uncommittedRuns.isEmpty(); }
>+    unsigned size() const { return m_uncommittedRuns.size(); }
>     LayoutUnit width() const { return m_width; }
> 
> private:
>-    Vector<InlineItem*> m_inlineItems;
>+    Vector<Run> m_uncommittedRuns;
>     LayoutUnit m_width;
> };
> 
>-void UncommittedContent::add(InlineItem& inlineItem)
>+void UncommittedContent::add(const InlineItem& inlineItem, LayoutUnit logicalWidth)
> {
>-    m_inlineItems.append(&inlineItem);
>-    m_width += inlineItem.width();
>+    m_uncommittedRuns.append({ inlineItem, logicalWidth });
>+    m_width += logicalWidth;
> }
> 
> void UncommittedContent::reset()
> {
>-    m_inlineItems.clear();
>+    m_uncommittedRuns.clear();
>     m_width = 0;
> }
> 
>@@ -158,8 +163,41 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>         if (uncommittedContent.isEmpty())
>             return;
>         committedInlineItemCount += uncommittedContent.size();
>-        for (auto* uncommitted : uncommittedContent.inlineItems())
>-            commitInlineItemToLine(*line, *uncommitted);
>+        for (auto& uncommittedRun : uncommittedContent.runs()) {
>+            auto& inlineItem = uncommittedRun.inlineItem;
>+            if (inlineItem.isHardLineBreak()) {
>+                line->appendHardLineBreak(inlineItem);
>+                continue;
>+            }
>+
>+            auto width = uncommittedRun.logicalWidth;
>+            auto& fontMetrics = inlineItem.style().fontMetrics();
>+            if (is<InlineTextItem>(inlineItem)) {
>+                line->appendTextContent(downcast<InlineTextItem>(inlineItem), { width, fontMetrics.height() });
>+                continue;
>+            }
>+
>+            auto& layoutBox = inlineItem.layoutBox();
>+            auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
>+
>+            if (inlineItem.isContainerStart()) {
>+                auto containerHeight = fontMetrics.height() + displayBox.verticalBorder() + displayBox.verticalPadding().valueOr(0);
>+                line->appendInlineContainerStart(inlineItem, { width, containerHeight });
>+                continue;
>+            }
>+
>+            if (inlineItem.isContainerEnd()) {
>+                line->appendInlineContainerEnd(inlineItem, { width, 0 });
>+                continue;
>+            }
>+
>+            if (layoutBox.isReplaced()) {
>+                line->appendReplacedInlineBox(inlineItem, { width, displayBox.height() });
>+                continue;
>+            }
>+
>+            line->appendNonReplacedInlineBox(inlineItem, { width, displayBox.height() });
>+        }
>         uncommittedContent.reset();
>     };
> 
>@@ -171,17 +209,18 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>     LineBreaker lineBreaker;
>     // Iterate through the inline content and place the inline boxes on the current line.
>     for (auto inlineItemIndex = lineInput.firstInlineItemIndex; inlineItemIndex < lineInput.inlineItems.size(); ++inlineItemIndex) {
>+        auto availableWidth = line->availableWidth() - uncommittedContent.width();
>+        auto currentLogicalRight = line->contentLogicalRight() + uncommittedContent.width();
>         auto& inlineItem = lineInput.inlineItems[inlineItemIndex];
>+        auto inlineItemWidth = WebCore::Layout::inlineItemWidth(layoutState(), *inlineItem, currentLogicalRight);
>+
>         if (inlineItem->isHardLineBreak()) {
>-            uncommittedContent.add(*inlineItem);
>+            uncommittedContent.add(*inlineItem, inlineItemWidth);
>             commitPendingContent();
>             return closeLine();
>         }
>-        auto availableWidth = line->availableWidth() - uncommittedContent.width();
>-        auto currentLogicalRight = line->contentLogicalRight() + uncommittedContent.width();
>-        inlineItem->setWidth(inlineItemWidth(layoutState(), *inlineItem, currentLogicalRight));
>         // FIXME: Ensure LineContext::trimmableWidth includes uncommitted content if needed.
>-        auto breakingContext = lineBreaker.breakingContext(*inlineItem, inlineItem->width(), { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>+        auto breakingContext = lineBreaker.breakingContext(*inlineItem, inlineItemWidth, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
> 
>@@ -203,7 +242,7 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>             continue;
>         }
> 
>-        uncommittedContent.add(*inlineItem);
>+        uncommittedContent.add(*inlineItem, inlineItemWidth);
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
>     }
>@@ -239,7 +278,6 @@ LayoutUnit InlineFormattingContext::LineLayout::computedIntrinsicWidth(LayoutUni
>     auto& inlineContent = m_formattingState.inlineItems();
>     for (auto& inlineItem : inlineContent) {
>         auto logicalWidth = inlineItemWidth(layoutState(), *inlineItem, lineLogicalRight);
>-        inlineItem->setWidth(logicalWidth);
>         auto breakingContext = lineBreaker.breakingContext(*inlineItem, logicalWidth, { widthConstraint, lineLogicalRight, !lineLogicalRight });
>         if (breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Wrap) {
>             maximumLineWidth = std::max(maximumLineWidth, lineLogicalRight - trimmableTrailingWidth);
>@@ -330,14 +368,13 @@ void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content&
>         ASSERT(inlineRun.textContext());        
>         const Line::Content::Run* previousLineRun = !index ? nullptr : lineRuns[index - 1].get();
>         if (!lineRun->isCollapsed) {
>-            auto& inlineTextItem = downcast<InlineTextItem>(inlineItem);
>             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 {
>                 auto& lastDisplayRun = m_formattingState.inlineRuns().last();
>-                lastDisplayRun->expandHorizontally(inlineTextItem.width());
>+                lastDisplayRun->expandHorizontally(inlineRun.logicalWidth());
>                 lastDisplayRun->textContext()->expand(inlineRun.textContext()->length());
>             }
>             lineBox.expandHorizontally(inlineRun.logicalWidth());
>@@ -375,33 +412,6 @@ void InlineFormattingContext::LineLayout::handleFloat(Line& line, const Floating
>     floatBox.isLeftFloatingPositioned() ? line.moveLogicalLeft(floatBoxWidth) : line.moveLogicalRight(floatBoxWidth);
> }
> 
>-void InlineFormattingContext::LineLayout::commitInlineItemToLine(Line& line, const InlineItem& inlineItem) const
>-{
>-    if (inlineItem.isHardLineBreak())
>-        return line.appendHardLineBreak(inlineItem);
>-
>-    auto width = inlineItem.width();
>-    auto& fontMetrics = inlineItem.style().fontMetrics();
>-    if (is<InlineTextItem>(inlineItem))
>-        return line.appendTextContent(downcast<InlineTextItem>(inlineItem), { width, fontMetrics.height() });
>-
>-    auto& layoutBox = inlineItem.layoutBox();
>-    auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
>-
>-    if (inlineItem.isContainerStart()) {
>-        auto containerHeight = fontMetrics.height() + displayBox.verticalBorder() + displayBox.verticalPadding().valueOr(0);
>-        return line.appendInlineContainerStart(inlineItem, { width, containerHeight });
>-    }
>-
>-    if (inlineItem.isContainerEnd())
>-        return line.appendInlineContainerEnd(inlineItem, { width, 0 });
>-
>-    if (layoutBox.isReplaced())
>-        return line.appendReplacedInlineBox(inlineItem, { width, displayBox.height() });
>-
>-    line.appendNonReplacedInlineBox(inlineItem, { width, displayBox.height() });
>-}
>-
> static Optional<LayoutUnit> horizontalAdjustmentForAlignment(TextAlignMode align, LayoutUnit remainingWidth)
> {
>     switch (align) {
>diff --git a/Source/WebCore/layout/inlineformatting/InlineItem.h b/Source/WebCore/layout/inlineformatting/InlineItem.h
>index 87ed9c36f6c923ae8e4f8997496ebcac5acfe9c4..a8a78699fb3f71c3e5215481c39680baa5705c1d 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineItem.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineItem.h
>@@ -42,8 +42,6 @@ public:
>     Type type() const { return m_type; }
>     const Box& layoutBox() const { return m_layoutBox; }
>     const RenderStyle& style() const { return m_layoutBox.style(); }
>-    void setWidth(LayoutUnit);
>-    LayoutUnit width() const;
> 
>     bool isText() const { return type() == Type::Text; }
>     bool isBox() const { return type() == Type::Box; }
>@@ -56,10 +54,6 @@ public:
> private:
>     const Box& m_layoutBox;
>     const Type m_type;
>-    LayoutUnit m_width;
>-#if !ASSERT_DISABLED
>-    bool m_hasValidWidth { false };
>-#endif
> };
> 
> inline InlineItem::InlineItem(const Box& layoutBox, Type type)
>@@ -68,20 +62,6 @@ inline InlineItem::InlineItem(const Box& layoutBox, Type type)
> {
> }
> 
>-inline void InlineItem::setWidth(LayoutUnit width)
>-{
>-#if !ASSERT_DISABLED
>-    m_hasValidWidth = true;
>-#endif
>-    m_width = width;
>-}
>-
>-inline LayoutUnit InlineItem::width() const
>-{
>-    ASSERT(m_hasValidWidth);
>-    return m_width;
>-}
>-
> #define SPECIALIZE_TYPE_TRAITS_INLINE_ITEM(ToValueTypeName, predicate) \
> SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::Layout::ToValueTypeName) \
>     static bool isType(const WebCore::Layout::InlineItem& inlineItem) { return inlineItem.predicate; } \

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 198502: 371225