<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[245970] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/245970">245970</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2019-05-31 10:05:33 -0700 (Fri, 31 May 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>[LFC][IFC] InlineFormattingContext::LineLayout::processInlineItemsForLine should create and destroy Line.
https://bugs.webkit.org/show_bug.cgi?id=198419
<rdar://problem/51300837>

Reviewed by Antti Koivisto.

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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorelayoutinlineformattingInlineFormattingContexth">trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h</a></li>
<li><a href="#trunkSourceWebCorelayoutinlineformattingInlineFormattingContextLineLayoutcpp">trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (245969 => 245970)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2019-05-31 16:58:28 UTC (rev 245969)
+++ trunk/Source/WebCore/ChangeLog      2019-05-31 17:05:33 UTC (rev 245970)
</span><span class="lines">@@ -1,3 +1,26 @@
</span><ins>+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 Antti Koivisto.
+
+        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.
+
</ins><span class="cx"> 2019-05-31  Don Olmstead  <don.olmstead@sony.com>
</span><span class="cx"> 
</span><span class="cx">         [CMake] Add WebKit::WTF target
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutinlineformattingInlineFormattingContexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h (245969 => 245970)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h   2019-05-31 16:58:28 UTC (rev 245969)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.h      2019-05-31 17:05:33 UTC (rev 245970)
</span><span class="lines">@@ -58,9 +58,22 @@
</span><span class="cx"> 
</span><span class="cx">     private:
</span><span class="cx">         LayoutState& layoutState() const { return m_formattingContext.layoutState(); }
</span><del>-        std::unique_ptr<Line> createLine(LayoutUnit lineLogicalTop, LayoutUnit widthConstraint) const;
-        unsigned createInlineRunsForLine(Line&, unsigned firstInlineItemIndex) const;
-        void processInlineRuns(const Line::Content&, LayoutUnit availableWidth) const;
</del><ins>+
+        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 placeInlineItems(const LineInput&) const;
+        void createDisplayRuns(const Line::Content&, LayoutUnit widthConstraint) const;
</ins><span class="cx">         void commitInlineItemToLine(Line&, const InlineItem&) const;
</span><span class="cx">         void handleFloat(Line&, const FloatingContext&, const InlineItem& floatBox) const;
</span><span class="cx">         void alignRuns(TextAlignMode, unsigned firstRunIndex, LayoutUnit availableWidth) const;
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutinlineformattingInlineFormattingContextLineLayoutcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp (245969 => 245970)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp       2019-05-31 16:58:28 UTC (rev 245969)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextLineLayout.cpp  2019-05-31 17:05:33 UTC (rev 245970)
</span><span class="lines">@@ -67,6 +67,14 @@
</span><span class="cx">     m_width = 0;
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+InlineFormattingContext::LineLayout::LineInput::LineInput(LayoutUnit logicalTop, LayoutUnit availableLogicalWidth, unsigned firstInlineItemIndex, const InlineItems& inlineItems)
+    : logicalTop(logicalTop)
+    , availableLogicalWidth(availableLogicalWidth)
+    , firstInlineItemIndex(firstInlineItemIndex)
+    , inlineItems(inlineItems)
+{
+}
+
</ins><span class="cx"> InlineFormattingContext::LineLayout::LineLayout(const InlineFormattingContext& inlineFormattingContext)
</span><span class="cx">     : m_formattingContext(inlineFormattingContext)
</span><span class="cx">     , m_formattingState(m_formattingContext.formattingState())
</span><span class="lines">@@ -75,14 +83,15 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-std::unique_ptr<Line> InlineFormattingContext::LineLayout::createLine(LayoutUnit lineLogicalTop, LayoutUnit availableWidth) const
</del><ins>+static std::unique_ptr<Line> constructLine(const LayoutState& layoutState, const FloatingState& floatingState, const Box& formattingRoot,
+    LayoutUnit lineLogicalTop, LayoutUnit availableWidth)
</ins><span class="cx"> {
</span><del>-    auto& formattingRootDisplayBox = layoutState().displayBoxForLayoutBox(m_formattingRoot);
</del><ins>+    auto& formattingRootDisplayBox = layoutState.displayBoxForLayoutBox(formattingRoot);
</ins><span class="cx">     auto lineLogicalLeft = formattingRootDisplayBox.contentBoxLeft();
</span><span class="cx"> 
</span><span class="cx">     // Check for intruding floats and adjust logical left/available width for this line accordingly.
</span><del>-    if (!m_floatingState.isEmpty()) {
-        auto floatConstraints = m_floatingState.constraints({ lineLogicalTop }, m_formattingRoot);
</del><ins>+    if (!floatingState.isEmpty()) {
+        auto floatConstraints = floatingState.constraints({ lineLogicalTop }, formattingRoot);
</ins><span class="cx">         // Check if these constraints actually put limitation on the line.
</span><span class="cx">         if (floatConstraints.left && *floatConstraints.left <= formattingRootDisplayBox.contentBoxLeft())
</span><span class="cx">             floatConstraints.left = { };
</span><span class="lines">@@ -104,48 +113,52 @@
</span><span class="cx">         }
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    auto& formattingRootStyle = m_formattingRoot.style();
</del><ins>+    auto& formattingRootStyle = formattingRoot.style();
</ins><span class="cx">     auto mimimumLineHeight = formattingRootStyle.computedLineHeight();
</span><span class="cx">     auto baselineOffset = Line::halfLeadingMetrics(formattingRootStyle.fontMetrics(), mimimumLineHeight).height;
</span><del>-    return std::make_unique<Line>(layoutState(), LayoutPoint { lineLogicalLeft, lineLogicalTop }, availableWidth, mimimumLineHeight, baselineOffset);
</del><ins>+    return std::make_unique<Line>(layoutState, LayoutPoint { lineLogicalLeft, lineLogicalTop }, availableWidth, mimimumLineHeight, baselineOffset);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><del>-unsigned InlineFormattingContext::LineLayout::createInlineRunsForLine(Line& line, unsigned startInlineItemIndex) const
</del><ins>+InlineFormattingContext::LineLayout::LineContent InlineFormattingContext::LineLayout::placeInlineItems(const LineInput& lineInput) const
</ins><span class="cx"> {
</span><ins>+    auto line = constructLine(layoutState(), m_floatingState, m_formattingRoot, lineInput.logicalTop, lineInput.availableLogicalWidth);
</ins><span class="cx">     auto floatingContext = FloatingContext { m_floatingState };
</span><del>-    Optional<unsigned> lastCommittedIndex;
</del><ins>+    unsigned committedInlineItemCount = 0;
</ins><span class="cx"> 
</span><span class="cx">     UncommittedContent uncommittedContent;
</span><span class="cx">     auto commitPendingContent = [&] {
</span><span class="cx">         if (uncommittedContent.isEmpty())
</span><span class="cx">             return;
</span><del>-
-        lastCommittedIndex = lastCommittedIndex.valueOr(startInlineItemIndex) + uncommittedContent.size();
</del><ins>+        committedInlineItemCount += uncommittedContent.size();
</ins><span class="cx">         for (auto* uncommitted : uncommittedContent.inlineItems())
</span><del>-            commitInlineItemToLine(line, *uncommitted);
</del><ins>+            commitInlineItemToLine(*line, *uncommitted);
</ins><span class="cx">         uncommittedContent.reset();
</span><span class="cx">     };
</span><span class="cx"> 
</span><ins>+    auto closeLine = [&] {
+        // This might change at some point.
+        ASSERT(committedInlineItemCount);
+        return LineContent { lineInput.firstInlineItemIndex + (committedInlineItemCount - 1), line->close() };
+    };
</ins><span class="cx">     LineBreaker lineBreaker(layoutState());
</span><span class="cx">     // Iterate through the inline content and place the inline boxes on the current line.
</span><del>-    auto& inlineContent = m_formattingState.inlineItems();
-    for (auto inlineItemIndex = startInlineItemIndex; inlineItemIndex < inlineContent.size(); ++inlineItemIndex) {
-        auto& inlineItem = inlineContent[inlineItemIndex];
</del><ins>+    for (auto inlineItemIndex = lineInput.firstInlineItemIndex; inlineItemIndex < lineInput.inlineItems.size(); ++inlineItemIndex) {
+        auto& inlineItem = lineInput.inlineItems[inlineItemIndex];
</ins><span class="cx">         if (inlineItem->isHardLineBreak()) {
</span><span class="cx">             uncommittedContent.add(*inlineItem);
</span><span class="cx">             commitPendingContent();
</span><del>-            return *lastCommittedIndex;
</del><ins>+            return closeLine();
</ins><span class="cx">         }
</span><del>-        auto availableWidth = line.availableWidth() - uncommittedContent.width();
-        auto currentLogicalRight = line.contentLogicalRight() + uncommittedContent.width();
</del><ins>+        auto availableWidth = line->availableWidth() - uncommittedContent.width();
+        auto currentLogicalRight = line->contentLogicalRight() + uncommittedContent.width();
</ins><span class="cx">         // FIXME: Ensure LineContext::trimmableWidth includes uncommitted content if needed.
</span><del>-        auto breakingContext = lineBreaker.breakingContext(*inlineItem, { availableWidth, currentLogicalRight, line.trailingTrimmableWidth(), !line.hasContent() });
</del><ins>+        auto breakingContext = lineBreaker.breakingContext(*inlineItem, { availableWidth, currentLogicalRight, line->trailingTrimmableWidth(), !line->hasContent() });
</ins><span class="cx">         if (breakingContext.isAtBreakingOpportunity)
</span><span class="cx">             commitPendingContent();
</span><span class="cx"> 
</span><span class="cx">         // Content does not fit the current line.
</span><span class="cx">         if (breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Wrap)
</span><del>-            return *lastCommittedIndex;
</del><ins>+            return closeLine();
</ins><span class="cx"> 
</span><span class="cx">         // Partial content stays on the current line. 
</span><span class="cx">         if (breakingContext.breakingBehavior == LineBreaker::BreakingBehavior::Break) {
</span><span class="lines">@@ -152,11 +165,12 @@
</span><span class="cx">             ASSERT(inlineItem->isText());
</span><span class="cx"> 
</span><span class="cx">             ASSERT_NOT_IMPLEMENTED_YET();
</span><del>-            return *lastCommittedIndex;
</del><ins>+            return closeLine();
</ins><span class="cx">         }
</span><span class="cx"> 
</span><span class="cx">         if (inlineItem->isFloat()) {
</span><del>-            handleFloat(line, floatingContext, *inlineItem);
</del><ins>+            handleFloat(*line, floatingContext, *inlineItem);
+            ++committedInlineItemCount;
</ins><span class="cx">             continue;
</span><span class="cx">         }
</span><span class="cx"> 
</span><span class="lines">@@ -165,7 +179,7 @@
</span><span class="cx">             commitPendingContent();
</span><span class="cx">     }
</span><span class="cx">     commitPendingContent();
</span><del>-    return *lastCommittedIndex;
</del><ins>+    return closeLine();
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void InlineFormattingContext::LineLayout::layout(LayoutUnit widthConstraint) const
</span><span class="lines">@@ -172,17 +186,16 @@
</span><span class="cx"> {
</span><span class="cx">     ASSERT(!m_formattingState.inlineItems().isEmpty());
</span><span class="cx"> 
</span><del>-    unsigned startInlineItemIndex = 0;
</del><span class="cx">     auto lineLogicalTop = layoutState().displayBoxForLayoutBox(m_formattingRoot).contentBoxTop();
</span><del>-    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();
</del><ins>+    auto& inlineItems = m_formattingState.inlineItems();
+    unsigned currentInlineItemIndex = 0;
+    while (currentInlineItemIndex < inlineItems.size()) {
+        auto lineContent = placeInlineItems({ 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();
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -214,7 +227,7 @@
</span><span class="cx">     return std::max(maximumLineWidth, lineLogicalRight - trimmableTrailingWidth);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void InlineFormattingContext::LineLayout::processInlineRuns(const Line::Content& lineContent, LayoutUnit availableWidth) const
</del><ins>+void InlineFormattingContext::LineLayout::createDisplayRuns(const Line::Content& lineContent, LayoutUnit widthConstraint) const
</ins><span class="cx"> {
</span><span class="cx">     if (lineContent.isEmpty()) {
</span><span class="cx">         // Spec tells us to create a zero height, empty line box.
</span><span class="lines">@@ -313,7 +326,7 @@
</span><span class="cx">     // FIXME linebox needs to be ajusted after content alignment.
</span><span class="cx">     m_formattingState.addLineBox({ lineBox });
</span><span class="cx">     if (!lineContent.isVisuallyEmpty())
</span><del>-        alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, availableWidth);
</del><ins>+        alignRuns(m_formattingRoot.style().textAlign(), previousLineLastRunIndex.valueOr(-1) + 1, widthConstraint - lineContent.logicalWidth());
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void InlineFormattingContext::LineLayout::handleFloat(Line& line, const FloatingContext& floatingContext, const InlineItem& floatItem) const
</span></span></pre>
</div>
</div>

</body>
</html>