Changeset 270146 in webkit

Timestamp:

Nov 21, 2020 7:15:20 AM (3 years ago)

Author:

Alan Bujtas

Message:

[LFC][Integration] LayoutIntegration::Line::rect is way too ambiguous
​https://bugs.webkit.org/show_bug.cgi?id=219230

Reviewed by Antti Koivisto.

Line::rect() is actually the line box (see ​https://www.w3.org/TR/css-inline-3/#line-box).

• layout/integration/LayoutIntegrationLine.h:

(WebCore::LayoutIntegration::Line::lineBoxTop const):
(WebCore::LayoutIntegration::Line::lineBoxBottom const):
(WebCore::LayoutIntegration::Line::lineBoxLeft const):
(WebCore::LayoutIntegration::Line::lineBoxRight const):
(WebCore::LayoutIntegration::Line::rect const): Deleted.

• layout/integration/LayoutIntegrationLineIteratorModernPath.h:

(WebCore::LayoutIntegration::LineIteratorModernPath::lineBoxTop const):
(WebCore::LayoutIntegration::LineIteratorModernPath::lineBoxBottom const):
(WebCore::LayoutIntegration::LineIteratorModernPath::contentLogicalLeft const):
(WebCore::LayoutIntegration::LineIteratorModernPath::logicalHeight const):

• layout/integration/LayoutIntegrationLineLayout.cpp:

(WebCore::LayoutIntegration::LineLayout::contentLogicalHeight const):
(WebCore::LayoutIntegration::LineLayout::firstLineBaseline const):
(WebCore::LayoutIntegration::LineLayout::lastLineBaseline const):
(WebCore::LayoutIntegration::LineLayout::adjustForPagination):
(WebCore::LayoutIntegration::LineLayout::paint):

• layout/integration/LayoutIntegrationPagination.cpp:

(WebCore::LayoutIntegration::updateMinimumPageHeight):
(WebCore::LayoutIntegration::makeAdjustedContent):

• layout/integration/LayoutIntegrationRunIteratorModernPath.h:

(WebCore::LayoutIntegration::RunIteratorModernPath::createTextRun const):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/integration/LayoutIntegrationLine.h (modified) (2 diffs)
• layout/integration/LayoutIntegrationLineIteratorModernPath.h (modified) (2 diffs)
• layout/integration/LayoutIntegrationLineLayout.cpp (modified) (5 diffs)
• layout/integration/LayoutIntegrationPagination.cpp (modified) (2 diffs)
• layout/integration/LayoutIntegrationRunIteratorModernPath.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-11-21  Zalan Bujtas  <zalan@apple.com>
+
+        [LFC][Integration] LayoutIntegration::Line::rect is way too ambiguous
+        https://bugs.webkit.org/show_bug.cgi?id=219230
+
+        Reviewed by Antti Koivisto.
+
+        Line::rect() is actually the line box (see https://www.w3.org/TR/css-inline-3/#line-box).
+
+        * layout/integration/LayoutIntegrationLine.h:
+        (WebCore::LayoutIntegration::Line::lineBoxTop const):
+        (WebCore::LayoutIntegration::Line::lineBoxBottom const):
+        (WebCore::LayoutIntegration::Line::lineBoxLeft const):
+        (WebCore::LayoutIntegration::Line::lineBoxRight const):
+        (WebCore::LayoutIntegration::Line::rect const): Deleted.
+        * layout/integration/LayoutIntegrationLineIteratorModernPath.h:
+        (WebCore::LayoutIntegration::LineIteratorModernPath::lineBoxTop const):
+        (WebCore::LayoutIntegration::LineIteratorModernPath::lineBoxBottom const):
+        (WebCore::LayoutIntegration::LineIteratorModernPath::contentLogicalLeft const):
+        (WebCore::LayoutIntegration::LineIteratorModernPath::logicalHeight const):
+        * layout/integration/LayoutIntegrationLineLayout.cpp:
+        (WebCore::LayoutIntegration::LineLayout::contentLogicalHeight const):
+        (WebCore::LayoutIntegration::LineLayout::firstLineBaseline const):
+        (WebCore::LayoutIntegration::LineLayout::lastLineBaseline const):
+        (WebCore::LayoutIntegration::LineLayout::adjustForPagination):
+        (WebCore::LayoutIntegration::LineLayout::paint):
+        * layout/integration/LayoutIntegrationPagination.cpp:
+        (WebCore::LayoutIntegration::updateMinimumPageHeight):
+        (WebCore::LayoutIntegration::makeAdjustedContent):
+        * layout/integration/LayoutIntegrationRunIteratorModernPath.h:
+        (WebCore::LayoutIntegration::RunIteratorModernPath::createTextRun const):
+
 2020-11-21  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/layout/integration/LayoutIntegrationLine.h

@@ -56 +56 @@
     size_t firstRunIndex() const { return m_firstRunIndex; }
     size_t runCount() const { return m_runCount; }
-    const FloatRect& rect() const { return m_lineBoxRect; }
+
+    float lineBoxTop() const { return m_lineBoxRect.y(); }
+    float lineBoxBottom() const { return m_lineBoxRect.maxY(); }
+    float lineBoxLeft() const { return m_lineBoxRect.x(); }
+    float lineBoxRight() const { return m_lineBoxRect.maxX(); }
+
     float enclosingContentTop() const { return m_enclosingTopAndBottom.top; }
     float enclosingContentBottom() const { return m_enclosingTopAndBottom.bottom; }
+
     const FloatRect& scrollableOverflow() const { return m_scrollableOverflow; }
     const FloatRect& inkOverflow() const { return m_inkOverflow; }
+
     float baseline() const { return m_baseline; }
+
     float contentLeftOffset() const { return m_contentLeftOffset; }
     float contentWidth() const { return m_contentWidth; }
@@ -70 +78 @@
     // This is line box geometry (see https://www.w3.org/TR/css-inline-3/#line-box).
     FloatRect m_lineBoxRect;
-    // Enclosing top and bottom includes all inline level boxes (border box) vertically. In certain cases (see line-height property)
-    // the line (and the line box) is not as tall as the inline level boxes on the line.
+    // Enclosing top and bottom includes all inline level boxes (border box) vertically.
+    // While the line box usually enclose them as well, its vertical geometry is based on
+    // the layout bounds of the inline level boxes which may be different when line-height is present.
     EnclosingTopAndBottom m_enclosingTopAndBottom;
     FloatRect m_scrollableOverflow;

trunk/Source/WebCore/layout/integration/LayoutIntegrationLineIteratorModernPath.h

@@ -52 +52 @@
     LayoutUnit top() const { return LayoutUnit::fromFloatRound(line().enclosingContentTop()); }
     LayoutUnit bottom() const { return LayoutUnit::fromFloatRound(line().enclosingContentBottom()); }
-    LayoutUnit lineBoxTop() const { return LayoutUnit::fromFloatRound(line().rect().y()); }
-    LayoutUnit lineBoxBottom() const { return LayoutUnit::fromFloatRound(line().rect().maxY()); }
+    LayoutUnit lineBoxTop() const { return LayoutUnit::fromFloatRound(line().lineBoxTop()); }
+    LayoutUnit lineBoxBottom() const { return LayoutUnit::fromFloatRound(line().lineBoxBottom()); }
 
     // FIXME: What should these really be?
@@ -60 +60 @@
     LayoutUnit selectionBottom() const { return bottom(); }
 
-    float contentLogicalLeft() const { return line().rect().x() + line().contentLeftOffset(); }
+    float contentLogicalLeft() const { return line().lineBoxLeft() + line().contentLeftOffset(); }
     float contentLogicalRight() const { return contentLogicalLeft() + line().contentWidth(); }
     float y() const { return lineBoxTop(); }
-    float logicalHeight() const { return line().rect().height(); }
+    float logicalHeight() const { return lineBoxBottom() - lineBoxTop(); }
     bool isHorizontal() const { return true; }
 

trunk/Source/WebCore/layout/integration/LayoutIntegrationLineLayout.cpp

@@ -233 +233 @@
 
     auto& lines = m_inlineContent->lines;
-    return LayoutUnit { lines.last().rect().maxY() - lines.first().rect().y() + m_inlineContent->clearGapAfterLastLine };
+    return LayoutUnit { lines.last().lineBoxBottom() - lines.first().lineBoxTop() + m_inlineContent->clearGapAfterLastLine };
 }
 
@@ -254 +254 @@
 
     auto& firstLine = m_inlineContent->lines.first();
-    return LayoutUnit { firstLine.rect().y() + firstLine.baseline() };
+    return LayoutUnit { firstLine.lineBoxTop() + firstLine.baseline() };
 }
 
@@ -265 +265 @@
 
     auto& lastLine = m_inlineContent->lines.last();
-    return LayoutUnit { lastLine.rect().y() + lastLine.baseline() };
+    return LayoutUnit { lastLine.lineBoxTop() + lastLine.baseline() };
 }
 
@@ -277 +277 @@
 
     auto& lines = paginedInlineContent->lines;
-    m_paginatedHeight = LayoutUnit { lines.last().rect().maxY() - lines.first().rect().y() };
+    m_paginatedHeight = LayoutUnit { lines.last().lineBoxBottom() - lines.first().lineBoxTop() };
 
     m_inlineContent = WTFMove(paginedInlineContent);
@@ -412 +412 @@
 
         auto& line = inlineContent.lineForRun(run);
-        auto baseline = paintOffset.y() + line.rect().y() + line.baseline();
+        auto baseline = paintOffset.y() + line.lineBoxTop() + line.baseline();
         auto expansion = run.expansion();
         // TextRun expects the xPos to be adjusted with the aligment offset (e.g. when the line is center aligned
         // and the run starts at 100px, due to the horizontal aligment, the xpos is supposed to be at 0px).
-        auto xPos = rect.x() - (line.rect().x() + line.contentLeftOffset());
+        auto xPos = rect.x() - (line.lineBoxLeft() + line.contentLeftOffset());
         WebCore::TextRun textRun { textContent.renderedContent(), xPos, expansion.horizontalExpansion, expansion.behavior };
         textRun.setTabSize(!style.collapseWhiteSpace(), style.tabSize());

trunk/Source/WebCore/layout/integration/LayoutIntegrationPagination.cpp

@@ -115 +115 @@
     auto orphans = style.hasAutoOrphans() ? 1 : std::max<int>(style.orphans(), 1);
     auto minimumLineCount = std::min<unsigned>(std::max(widows, orphans), lineCount);
-    flow.updateMinimumPageHeight(0, LayoutUnit(inlineContent.lines[minimumLineCount - 1].rect().maxY()));
+    flow.updateMinimumPageHeight(0, LayoutUnit(inlineContent.lines[minimumLineCount - 1].lineBoxBottom()));
 }
 
@@ -130 +130 @@
             line.firstRunIndex(),
             line.runCount(),
-            moveVertically(line.rect(), offset),
+            moveVertically({ FloatPoint { line.lineBoxLeft(), line.lineBoxTop() }, FloatPoint { line.lineBoxRight(), line.lineBoxBottom() } }, offset),
             { line.enclosingContentTop() + offset, line.enclosingContentBottom() + offset },
             moveVertically(line.scrollableOverflow(), offset),

trunk/Source/WebCore/layout/integration/LayoutIntegrationRunIteratorModernPath.h

@@ -222 +222 @@
         auto expansion = run().expansion();
         auto rect = this->rect();
-        auto xPos = rect.x() - (line().rect().x() + line().contentLeftOffset());
+        auto xPos = rect.x() - (line().lineBoxLeft() + line().contentLeftOffset());
 
         auto textForRun = [&] {