Attachment 371056 Details for Bug 198419 – Patch

[patch] Patch

bug-198419-20190531071657.patch (text/plain), 12.99 KB, created by zalan on 2019-05-31 07:16:59 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-05-31 07:16:59 PDT

Size: 12.99 KB

patch

obsolete

>Subversion Revision: 245776
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 53c4b63a4566b72f83f98e97e163a060e3b25cec..b943cea84967439a07f81086fb9e7852e2c08932 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-05-31  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][IFC] InlineFormattingContext::LineLayout::processInlineItemsForLine should create and destroy Line.
>+        https://bugs.webkit.org/show_bug.cgi?id=198419
>+        <rdar://problem/51300837>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This is in preparation for using "createInlineRunsForLine" logic when computing preferred width.
>+        1. Line object is now constructed and destroyed in processInlineItemsForLine (caller does not need to know about Line).
>+        2. processInlineItemsForLine returns a Line::Content instance. 
>+
>+        * layout/inlineformatting/InlineFormattingContext.h:
>+        * layout/inlineformatting/InlineFormattingContextLineLayout.cpp:
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::LineInput::LineInput):
>+        (WebCore::Layout::constructLine):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::processInlineItemsForLine const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::layout const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createDisplayRuns const):
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createLine const): Deleted.
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::createInlineRunsForLine const): Deleted.
>+        (WebCore::Layout::InlineFormattingContext::LineLayout::processInlineRuns const): Deleted.
>+
> 2019-05-29  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC[IFC] Do not reuse the same Line object.
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>index cc038ac55131723883a5e66aaa4d024c0e767d75..13f205611754fa831bd9f4aadecc191ee78f9dad 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h
>@@ -58,9 +58,22 @@ private:
> 
>     private:
>         LayoutState& layoutState() const { return m_formattingContext.layoutState(); }
>-        std::unique_ptr<Line> createLine(LayoutUnit lineLogicalTop, LayoutUnit widthConstraint) const;
>-        unsigned createInlineRunsForLine(Line&, unsigned firstInlineItemIndex) const;
>-        void processInlineRuns(const Line::Content&, LayoutUnit availableWidth) const;
>+
>+        struct LineContent {
>+            Optional<unsigned> lastInlineItemIndex;
>+            std::unique_ptr<Line::Content> runs;    
>+        };
>+
>+        struct LineInput {
>+            LineInput(LayoutUnit logicalTop, LayoutUnit availableLogicalWidth, unsigned firstInlineItemIndex, const InlineItems&);
>+
>+            LayoutUnit logicalTop;
>+            LayoutUnit availableLogicalWidth;
>+            unsigned firstInlineItemIndex { 0 };
>+            const InlineItems& inlineItems;
>+        };
>+        LineContent processInlineItemsForLine(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 998feda949d466b0bbda99ad7fef771f2abff255..6e02d087c273921e943f85e19815963904f4beda 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp
>@@ -67,6 +67,14 @@ void UncommittedContent::reset()
>     m_width = 0;
> }
> 
>+InlineFormattingContext::LineLayout::LineInput::LineInput(LayoutUnit logicalTop, LayoutUnit availableLogicalWidth, unsigned firstInlineItemIndex, const InlineItems& inlineItems)
>+    : logicalTop(logicalTop)
>+    , availableLogicalWidth(availableLogicalWidth)
>+    , firstInlineItemIndex(firstInlineItemIndex)
>+    , inlineItems(inlineItems)
>+{
>+}
>+
> InlineFormattingContext::LineLayout::LineLayout(const InlineFormattingContext& inlineFormattingContext)
>     : m_formattingContext(inlineFormattingContext)
>     , m_formattingState(m_formattingContext.formattingState())
>@@ -75,14 +83,15 @@ InlineFormattingContext::LineLayout::LineLayout(const InlineFormattingContext& i
> {
> }
> 
>-std::unique_ptr<Line> InlineFormattingContext::LineLayout::createLine(LayoutUnit lineLogicalTop, LayoutUnit availableWidth) const
>+static std::unique_ptr<Line> constructLine(const LayoutState& layoutState, const FloatingState& floatingState, const Box& formattingRoot,
>+    LayoutUnit lineLogicalTop, LayoutUnit availableWidth)
> {
>-    auto& formattingRootDisplayBox = layoutState().displayBoxForLayoutBox(m_formattingRoot);
>+    auto& formattingRootDisplayBox = layoutState.displayBoxForLayoutBox(formattingRoot);
>     auto lineLogicalLeft = formattingRootDisplayBox.contentBoxLeft();
> 
>     // Check for intruding floats and adjust logical left/available width for this line accordingly.
>-    if (!m_floatingState.isEmpty()) {
>-        auto floatConstraints = m_floatingState.constraints({ lineLogicalTop }, m_formattingRoot);
>+    if (!floatingState.isEmpty()) {
>+        auto floatConstraints = floatingState.constraints({ lineLogicalTop }, formattingRoot);
>         // Check if these constraints actually put limitation on the line.
>         if (floatConstraints.left && *floatConstraints.left <= formattingRootDisplayBox.contentBoxLeft())
>             floatConstraints.left = { };
>@@ -104,59 +113,64 @@ std::unique_ptr<Line> InlineFormattingContext::LineLayout::createLine(LayoutUnit
>         }
>     }
> 
>-    auto& formattingRootStyle = m_formattingRoot.style();
>+    auto& formattingRootStyle = formattingRoot.style();
>     auto mimimumLineHeight = formattingRootStyle.computedLineHeight();
>     auto baselineOffset = Line::halfLeadingMetrics(formattingRootStyle.fontMetrics(), mimimumLineHeight).height;
>-    return std::make_unique<Line>(layoutState(), LayoutPoint { 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
>+InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLayout::processInlineItemsForLine(const LineInput& lineInput) const
> {
>+    auto line = constructLine(layoutState(), m_floatingState, m_formattingRoot, lineInput.logicalTop, lineInput.availableLogicalWidth);
>     auto floatingContext = FloatingContext { m_floatingState };
>-    Optional<unsigned> lastCommittedIndex;
>+    unsigned committedInlineItemCount = 0;
> 
>     UncommittedContent uncommittedContent;
>     auto commitPendingContent = [&] {
>         if (uncommittedContent.isEmpty())
>             return;
>-
>-        lastCommittedIndex = lastCommittedIndex.valueOr(startInlineItemIndex) + uncommittedContent.size();
>+        committedInlineItemCount += uncommittedContent.size(); 
>         for (auto* uncommitted : uncommittedContent.inlineItems())
>-            commitInlineItemToLine(line, *uncommitted);
>+            commitInlineItemToLine(*line, *uncommitted);
>         uncommittedContent.reset();
>     };
> 
>+    auto closeLine = [&] {
>+        // This might change at some point.
>+        ASSERT(committedInlineItemCount);
>+        return LineContent { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1), line->close() };
>+    };
>     LineBreaker lineBreaker(layoutState());
>     // Iterate through the inline content and place the inline boxes on the current line.
>-    auto& inlineContent = m_formattingState.inlineItems();
>-    for (auto inlineItemIndex = startInlineItemIndex; inlineItemIndex < inlineContent.size(); ++inlineItemIndex) {
>-        auto& inlineItem = inlineContent[inlineItemIndex];
>+    for (auto inlineItemIndex = lineInput.firstInlineItemIndex; inlineItemIndex < lineInput.inlineItems.size(); ++inlineItemIndex) {
>+        auto& inlineItem = lineInput.inlineItems[inlineItemIndex];
>         if (inlineItem->isHardLineBreak()) {
>             uncommittedContent.add(*inlineItem);
>             commitPendingContent();
>-            return *lastCommittedIndex;
>+            return closeLine();
>         }
>-        auto availableWidth = line.availableWidth() - uncommittedContent.width();
>-        auto currentLogicalRight = line.contentLogicalRight() + uncommittedContent.width();
>+        auto availableWidth = line->availableWidth() - uncommittedContent.width();
>+        auto currentLogicalRight = line->contentLogicalRight() + uncommittedContent.width();
>         // FIXME: Ensure LineContext::trimmableWidth includes uncommitted content if needed.
>-        auto breakingContext = lineBreaker.breakingContext(*inlineItem, { availableWidth, currentLogicalRight, line.trailingTrimmableWidth(), !line.hasContent() });
>+        auto breakingContext = lineBreaker.breakingContext(*inlineItem, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
>         if (breakingContext.isAtBreakingOpportunity)
>             commitPendingContent();
> 
>         // Content does not fit the current line.
>         if (breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Wrap)
>-            return *lastCommittedIndex;
>+            return closeLine();
> 
>         // Partial content stays on the current line. 
>         if (breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Break) {
>             ASSERT(inlineItem->isText());
> 
>             ASSERT_NOT_IMPLEMENTED_YET();
>-            return *lastCommittedIndex;
>+            return closeLine();
>         }
> 
>         if (inlineItem->isFloat()) {
>-            handleFloat(line, floatingContext, *inlineItem);
>+            handleFloat(*line, floatingContext, *inlineItem);
>+            ++committedInlineItemCount;
>             continue;
>         }
> 
>@@ -165,24 +179,23 @@ unsigned InlineFormattingContext::LineLayout::createInlineRunsForLine(Line& line
>             commitPendingContent();
>     }
>     commitPendingContent();
>-    return *lastCommittedIndex;
>+    return closeLine();
> }
> 
> void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) const
> {
>     ASSERT(!m_formattingState.inlineItems().isEmpty());
> 
>-    unsigned startInlineItemIndex = 0;
>     auto lineLogicalTop = layoutState().displayBoxForLayoutBox(m_formattingRoot).contentBoxTop();
>-    while (true) {
>-        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;
>-        lineLogicalTop = lineContent->logicalBottom();
>+    auto& inlineItems = m_formattingState.inlineItems();
>+    unsigned currentInlineItemIndex = 0;
>+    while (currentInlineItemIndex < inlineItems.size()) {
>+        auto lineContent = processInlineItemsForLine({ lineLogicalTop, widthConstraint, currentInlineItemIndex, inlineItems });
>+        createDisplayRuns(*lineContent.runs, widthConstraint);
>+        // We should always put at least one run on the line atm. This might change later on though.
>+        ASSERT(lineContent.lastInlineItemIndex);
>+        currentInlineItemIndex = *lineContent.lastInlineItemIndex + 1;
>+        lineLogicalTop = lineContent.runs->logicalBottom();
>     }
> }
> 
>@@ -214,7 +227,7 @@ LayoutUnit InlineFormattingContext::LineLayout::computedIntrinsicWidth(LayoutUni
>     return std::max(maximumLineWidth, lineLogicalRight - trimmableTrailingWidth);
> }
> 
>-void InlineFormattingContext::LineLayout::processInlineRuns(const Line::Content& lineContent, LayoutUnit availableWidth) const
>+void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content& lineContent, LayoutUnit widthConstraint) const
> {
>     if (lineContent.isEmpty()) {
>         // Spec tells us to create a zero height, empty line box.
>@@ -313,7 +326,7 @@ void InlineFormattingContext::LineLayout::processInlineRuns(const Line::Content&
>     // 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, availableWidth);
>+        alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, widthConstraint - lineContent.logicalWidth());
> }
> 
> void InlineFormattingContext::LineLayout::handleFloat(Line& line, const FloatingContext& floatingContext, const InlineItem& floatItem) const

Actions: View | Formatted Diff | Diff

Attachments on bug 198419: 371056 | 371063