Attachment 371359 Details for Bug 198550 – Patch

[patch] Patch

bug-198550-20190604184331.patch (text/plain), 15.33 KB, created by zalan on 2019-06-04 18:43:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-04 18:43:32 PDT

Size: 15.33 KB

patch

obsolete

>Subversion Revision: 246068
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ce97d1d8dfb9d3d9c0cdb9e031f2b073905c6594..3b3542e11905dce6c3ee700b80ce597ba9648b36 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-06-04  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] Move inline item height computation to a dedicated function
>+        https://bugs.webkit.org/show_bug.cgi?id=198550
>+        <rdar://problem/51424223>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The inlineItem height value is not available during preferred width computation (hence optional). 
>+
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::UncommittedContent::add):
>+        (WebCore::Layout::inlineItemWidth):
>+        (WebCore::Layout::inlineItemHeight):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::placeInlineItems const):
>+        * layout/inlineformatting/InlineLine.cpp:
>+        (WebCore::Layout::Line::appendInlineContainerStart):
>+        (WebCore::Layout::Line::appendInlineContainerEnd):
>+        (WebCore::Layout::Line::appendTextContent):
>+        (WebCore::Layout::Line::appendNonReplacedInlineBox):
>+        (WebCore::Layout::Line::appendReplacedInlineBox):
>+        * layout/inlineformatting/InlineLine.h:
>+
> 2019-06-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Decouple float placement and line shrinking
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>index 8c7884ad614f4fa42455940fecb342223b370491..0e2514758bafec90ca777b613eacecfb5457f1e8 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -44,10 +44,10 @@ namespace Layout {
> struct UncommittedContent {
>     struct Run {
>         const InlineItem& inlineItem;
>-        LayoutUnit logicalWidth;
>+        Line::InlineItemSize size;
>         // FIXME: add optional breaking context (start and end position) for split text content.
>     };
>-    void add(const InlineItem&, LayoutUnit logicalWidth);
>+    void add(const InlineItem&, const Line::InlineItemSize&);
>     void reset();
> 
>     Vector<Run> runs() { return m_uncommittedRuns; }
>@@ -60,10 +60,10 @@ private:
>     LayoutUnit m_width;
> };
> 
>-void UncommittedContent::add(const InlineItem& inlineItem, LayoutUnit logicalWidth)
>+void UncommittedContent::add(const InlineItem& inlineItem, const Line::InlineItemSize& size)
> {
>-    m_uncommittedRuns.append({ inlineItem, logicalWidth });
>-    m_width += logicalWidth;
>+    m_uncommittedRuns.append({ inlineItem, size });
>+    m_width += size.logicalWidth;
> }
> 
> void UncommittedContent::reset()
>@@ -103,18 +103,44 @@ static LayoutUnit inlineItemWidth(const LayoutState& layoutState, const InlineIt
>     ASSERT(layoutState.hasDisplayBox(layoutBox));
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
> 
>+    if (layoutBox.isFloatingPositioned())
>+        return displayBox.marginBoxWidth();
>+
>+    if (layoutBox.isReplaced())
>+        return displayBox.width();
>+
>     if (inlineItem.isContainerStart())
>         return displayBox.marginStart() + displayBox.borderLeft() + displayBox.paddingLeft().valueOr(0);
> 
>     if (inlineItem.isContainerEnd())
>         return displayBox.marginEnd() + displayBox.borderRight() + displayBox.paddingRight().valueOr(0);
> 
>-    if (inlineItem.isFloat())
>-        return displayBox.marginBoxWidth();
>-
>+    // Non-replaced inline box (e.g. inline-block)
>     return displayBox.width();
> }
> 
>+static LayoutUnit inlineItemHeight(const LayoutState& layoutState, const InlineItem& inlineItem)
>+{
>+    auto& fontMetrics = inlineItem.style().fontMetrics();
>+    if (inlineItem.isLineBreak() || is<InlineTextItem>(inlineItem))
>+        return fontMetrics.height();
>+
>+    auto& layoutBox = inlineItem.layoutBox();
>+    ASSERT(layoutState.hasDisplayBox(layoutBox));
>+    auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>+
>+    if (layoutBox.isFloatingPositioned())
>+        return displayBox.marginBox().height();
>+
>+    if (layoutBox.isReplaced())
>+        return displayBox.height();
>+
>+    if (inlineItem.isContainerStart() || inlineItem.isContainerEnd())
>+        return fontMetrics.height() + displayBox.verticalBorder() + displayBox.verticalPadding().valueOr(0);
>+
>+    // Non-replaced inline box (e.g. inline-block)
>+    return displayBox.height();
>+}
> 
> static std::unique_ptr<Line> constructLine(const LayoutState& layoutState, const FloatingState& floatingState, const Box& formattingRoot,
>     LayoutUnit lineLogicalTop, LayoutUnit availableWidth)
>@@ -165,38 +191,18 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>         committedInlineItemCount += uncommittedContent.size();
>         for (auto& uncommittedRun : uncommittedContent.runs()) {
>             auto& inlineItem = uncommittedRun.inlineItem;
>-            if (inlineItem.isHardLineBreak()) {
>+            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() });
>+            else if (is<InlineTextItem>(inlineItem))
>+                line->appendTextContent(downcast<InlineTextItem>(inlineItem), uncommittedRun.size);
>+            else if (inlineItem.isContainerStart())
>+                line->appendInlineContainerStart(inlineItem, uncommittedRun.size);
>+            else if (inlineItem.isContainerEnd())
>+                line->appendInlineContainerEnd(inlineItem, uncommittedRun.size);
>+            else if (inlineItem.layoutBox().isReplaced())
>+                line->appendReplacedInlineBox(inlineItem, uncommittedRun.size);
>+            else
>+                line->appendNonReplacedInlineBox(inlineItem, uncommittedRun.size);
>         }
>         uncommittedContent.reset();
>     };
>@@ -212,10 +218,10 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>         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);
>+        auto itemLogicalWidth = inlineItemWidth(layoutState(), *inlineItem, currentLogicalRight);
> 
>         // FIXME: Ensure LineContext::trimmableWidth includes uncommitted content if needed.
>-        auto breakingContext = lineBreaker.breakingContext(*inlineItem, inlineItemWidth, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>+        auto breakingContext = lineBreaker.breakingContext(*inlineItem, itemLogicalWidth, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
> 
>@@ -243,12 +249,12 @@ InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLa
>             continue;
>         }
>         if (inlineItem->isHardLineBreak()) {
>-            uncommittedContent.add(*inlineItem, inlineItemWidth);
>+            uncommittedContent.add(*inlineItem, { itemLogicalWidth, inlineItemHeight(layoutState(), *inlineItem) });
>             commitPendingContent();
>             return closeLine();
>         }
> 
>-        uncommittedContent.add(*inlineItem, inlineItemWidth);
>+        uncommittedContent.add(*inlineItem, { itemLogicalWidth, inlineItemHeight(layoutState(), *inlineItem) });
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
>     }
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.cpp b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>index 2f0f5d5c096b69ad7be034c27bd21443de781537..07405ae747471d52fb353ace498a2cc9b87ef6a3 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.cpp
>@@ -119,7 +119,7 @@ void Line::appendNonBreakableSpace(const InlineItem& inlineItem, const Display::
>     m_contentLogicalWidth += logicalRect.width();
> }
> 
>-void Line::appendInlineContainerStart(const InlineItem& inlineItem, LayoutSize runSize)
>+void Line::appendInlineContainerStart(const InlineItem& inlineItem, InlineItemSize runSize)
> {
>     auto& layoutBox = inlineItem.layoutBox();
>     auto& style = layoutBox.style();
>@@ -138,18 +138,18 @@ void Line::appendInlineContainerStart(const InlineItem& inlineItem, LayoutSize r
>     alignAndAdjustLineHeight();
>     auto& displayBox = m_layoutState.displayBoxForLayoutBox(layoutBox);
>     auto logicalTop = -fontMetrics.ascent() - displayBox.borderTop() - displayBox.paddingTop().valueOr(0);
>-    auto logicalRect = Display::Rect { logicalTop, contentLogicalRight(), runSize.width(), runSize.height() };
>+    auto logicalRect = Display::Rect { logicalTop, contentLogicalRight(), runSize.logicalWidth, runSize.logicalHeight.valueOr(0) };
>     appendNonBreakableSpace(inlineItem, logicalRect);
> }
> 
>-void Line::appendInlineContainerEnd(const InlineItem& inlineItem, LayoutSize runSize)
>+void Line::appendInlineContainerEnd(const InlineItem& inlineItem, InlineItemSize runSize)
> {
>     // This is really just a placeholder to mark the end of the inline level container.
>-    auto logicalRect = Display::Rect { 0, contentLogicalRight(), runSize.width(), runSize.height() };
>+    auto logicalRect = Display::Rect { 0, contentLogicalRight(), runSize.logicalWidth, runSize.logicalHeight.valueOr(0) };
>     appendNonBreakableSpace(inlineItem, logicalRect);
> }
> 
>-void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSize)
>+void Line::appendTextContent(const InlineTextItem& inlineItem, InlineItemSize runSize)
> {
>     auto isTrimmable = TextUtil::isTrimmableContent(inlineItem);
>     if (!isTrimmable)
>@@ -177,7 +177,7 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>     // Collapsed line items don't contribute to the line width.
>     auto isCompletelyCollapsed = shouldCollapseCompletely();
>     auto canBeExtended = !isCompletelyCollapsed && !inlineItem.isCollapsed();
>-    auto logicalRect = Display::Rect { -inlineItem.style().fontMetrics().ascent(), contentLogicalRight(), runSize.width(), runSize.height() };
>+    auto logicalRect = Display::Rect { -inlineItem.style().fontMetrics().ascent(), contentLogicalRight(), runSize.logicalWidth, runSize.logicalHeight.valueOr(0) };
>     auto textContext = Display::Run::TextContext { inlineItem.start(), inlineItem.isCollapsed() ? 1 : inlineItem.length() };
>     auto displayRun = Display::Run(logicalRect, textContext);
> 
>@@ -186,13 +186,13 @@ void Line::appendTextContent(const InlineTextItem& inlineItem, LayoutSize runSiz
>         m_trimmableContent.add(lineItem.get());
> 
>     m_content->runs().append(WTFMove(lineItem));
>-    m_contentLogicalWidth += isCompletelyCollapsed ? LayoutUnit() : runSize.width();
>+    m_contentLogicalWidth += isCompletelyCollapsed ? LayoutUnit() : runSize.logicalWidth;
> }
> 
>-void Line::appendNonReplacedInlineBox(const InlineItem& inlineItem, LayoutSize runSize)
>+void Line::appendNonReplacedInlineBox(const InlineItem& inlineItem, InlineItemSize runSize)
> {
>+    auto inlineBoxHeight = runSize.logicalHeight.valueOr(0);
>     auto alignAndAdjustLineHeight = [&] {
>-        auto inlineBoxHeight = runSize.height();
>         // FIXME: We need to look inside the inline-block's formatting context and check the lineboxes (if any) to be able to baseline align.
>         if (inlineItem.layoutBox().establishesInlineFormattingContext()) {
>             if (inlineBoxHeight == logicalHeight())
>@@ -209,16 +209,16 @@ void Line::appendNonReplacedInlineBox(const InlineItem& inlineItem, LayoutSize r
> 
>     alignAndAdjustLineHeight();
>     auto& displayBox = m_layoutState.displayBoxForLayoutBox(inlineItem.layoutBox());
>-    auto logicalTop = -runSize.height();
>+    auto logicalTop = -inlineBoxHeight;
>     auto horizontalMargin = displayBox.horizontalMargin();
>-    auto logicalRect = Display::Rect { logicalTop, contentLogicalRight() + horizontalMargin.start, runSize.width(), runSize.height() };
>+    auto logicalRect = Display::Rect { logicalTop, contentLogicalRight() + horizontalMargin.start, runSize.logicalWidth, inlineBoxHeight };
> 
>     m_content->runs().append(std::make_unique<Content::Run>(Display::Run { logicalRect }, inlineItem, false, false));
>-    m_contentLogicalWidth += (runSize.width() + horizontalMargin.start + horizontalMargin.end);
>+    m_contentLogicalWidth += (runSize.logicalWidth + horizontalMargin.start + horizontalMargin.end);
>     m_trimmableContent.clear();
> }
> 
>-void Line::appendReplacedInlineBox(const InlineItem& inlineItem, LayoutSize runSize)
>+void Line::appendReplacedInlineBox(const InlineItem& inlineItem, InlineItemSize runSize)
> {
>     // FIXME Surely replaced boxes behave differently.
>     appendNonReplacedInlineBox(inlineItem, runSize);
>diff --git a/Source/WebCore/layout/inlineformatting/InlineLine.h b/Source/WebCore/layout/inlineformatting/InlineLine.h
>index bca1bc732e88843fdf780c6c17a2323577122fd2..5857499d6797c06270fc913a83de66e7b4ab3648 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineLine.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineLine.h
>@@ -75,11 +75,15 @@ public:
>     };
>     std::unique_ptr<Content> close();
> 
>-    void appendTextContent(const InlineTextItem&, LayoutSize);
>-    void appendNonReplacedInlineBox(const InlineItem&, LayoutSize);
>-    void appendReplacedInlineBox(const InlineItem&, LayoutSize);
>-    void appendInlineContainerStart(const InlineItem&, LayoutSize);
>-    void appendInlineContainerEnd(const InlineItem&, LayoutSize);
>+    struct InlineItemSize {
>+        LayoutUnit logicalWidth;
>+        Optional<LayoutUnit> logicalHeight;
>+    };
>+    void appendTextContent(const InlineTextItem&, InlineItemSize);
>+    void appendNonReplacedInlineBox(const InlineItem&, InlineItemSize);
>+    void appendReplacedInlineBox(const InlineItem&, InlineItemSize);
>+    void appendInlineContainerStart(const InlineItem&, InlineItemSize);
>+    void appendInlineContainerEnd(const InlineItem&, InlineItemSize);
>     void appendHardLineBreak(const InlineItem&);
> 
>     bool hasContent() const { return !m_content->isVisuallyEmpty(); }

Actions: View | Formatted Diff | Diff

Attachments on bug 198550: 371359