Changeset 269146 in webkit

Timestamp:

Oct 29, 2020, 7:21:34 AM (6 years ago)

Author:

Antti Koivisto

Message:

[LFC][Integration] RenderBlockFlow::positionForPointWithInlineChildren should use iterator
​https://bugs.webkit.org/show_bug.cgi?id=218283

Reviewed by Zalan Bujtas.

Reduce InlineBox access and eliminate one ensureLineBoxes() call.

• layout/integration/LayoutIntegrationLineIterator.h:

(WebCore::LayoutIntegration::PathLine::topWithLeading const):
(WebCore::LayoutIntegration::PathLine::bottomWithLeading const):
(WebCore::LayoutIntegration::PathLine::legacyRootInlineBox const):

• layout/integration/LayoutIntegrationLineIteratorLegacyPath.h:

(WebCore::LayoutIntegration::LineIteratorLegacyPath::topWithLeading const):
(WebCore::LayoutIntegration::LineIteratorLegacyPath::bottomWithLeading const):
(WebCore::LayoutIntegration::LineIteratorLegacyPath::legacyRootInlineBox const):

• layout/integration/LayoutIntegrationLineIteratorModernPath.h:

(WebCore::LayoutIntegration::LineIteratorModernPath::topWithLeading const):
(WebCore::LayoutIntegration::LineIteratorModernPath::bottomWithLeading const):
(WebCore::LayoutIntegration::LineIteratorModernPath::legacyRootInlineBox const):

• layout/integration/LayoutIntegrationRunIteratorModernPath.h:

(WebCore::LayoutIntegration::RunIteratorModernPath::legacyInlineBox const):

• rendering/RenderBlockFlow.cpp:

(WebCore::positionForRun):
(WebCore::RenderBlockFlow::positionForPointWithInlineChildren):
(WebCore::RenderBlockFlow::positionForBox const): Deleted.

• rendering/RenderBlockFlow.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/integration/LayoutIntegrationLineIterator.h (modified) (5 diffs)
• layout/integration/LayoutIntegrationLineIteratorLegacyPath.h (modified) (2 diffs)
• layout/integration/LayoutIntegrationLineIteratorModernPath.h (modified) (2 diffs)
• layout/integration/LayoutIntegrationRunIteratorModernPath.h (modified) (1 diff)
• rendering/RenderBlockFlow.cpp (modified) (5 diffs)
• rendering/RenderBlockFlow.h (modified) (1 diff)
• rendering/RootInlineBox.cpp (modified) (1 diff)
• rendering/RootInlineBox.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2020-10-29  Antti Koivisto  <antti@apple.com>
+
+        [LFC][Integration] RenderBlockFlow::positionForPointWithInlineChildren should use iterator
+        https://bugs.webkit.org/show_bug.cgi?id=218283
+
+        Reviewed by Zalan Bujtas.
+
+        Reduce InlineBox access and eliminate one ensureLineBoxes() call.
+
+        * layout/integration/LayoutIntegrationLineIterator.h:
+        (WebCore::LayoutIntegration::PathLine::topWithLeading const):
+        (WebCore::LayoutIntegration::PathLine::bottomWithLeading const):
+        (WebCore::LayoutIntegration::PathLine::legacyRootInlineBox const):
+        * layout/integration/LayoutIntegrationLineIteratorLegacyPath.h:
+        (WebCore::LayoutIntegration::LineIteratorLegacyPath::topWithLeading const):
+        (WebCore::LayoutIntegration::LineIteratorLegacyPath::bottomWithLeading const):
+        (WebCore::LayoutIntegration::LineIteratorLegacyPath::legacyRootInlineBox const):
+        * layout/integration/LayoutIntegrationLineIteratorModernPath.h:
+        (WebCore::LayoutIntegration::LineIteratorModernPath::topWithLeading const):
+        (WebCore::LayoutIntegration::LineIteratorModernPath::bottomWithLeading const):
+        (WebCore::LayoutIntegration::LineIteratorModernPath::legacyRootInlineBox const):
+        * layout/integration/LayoutIntegrationRunIteratorModernPath.h:
+        (WebCore::LayoutIntegration::RunIteratorModernPath::legacyInlineBox const):
+        * rendering/RenderBlockFlow.cpp:
+        (WebCore::positionForRun):
+        (WebCore::RenderBlockFlow::positionForPointWithInlineChildren):
+        (WebCore::RenderBlockFlow::positionForBox const): Deleted.
+        * rendering/RenderBlockFlow.h:
+
 2020-10-29  Zalan Bujtas  <zalan@apple.com>
 

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

@@ -56 +56 @@
     LayoutUnit selectionTopForHitTesting() const;
     LayoutUnit selectionBottom() const;
+    LayoutUnit topWithLeading() const;
+    LayoutUnit bottomWithLeading() const;
 
     float y() const;
@@ -65 +67 @@
 
     const RenderBlockFlow& containingBlock() const;
+    const RootInlineBox* legacyRootInlineBox() const;
 
 protected:
@@ -129 +132 @@
 {
     return WTF::switchOn(m_pathVariant, [](const auto& path) {
-        return path.top();
+        return path.bottom();
     });
 }
@@ -151 +154 @@
     return WTF::switchOn(m_pathVariant, [](const auto& path) {
         return path.selectionBottom();
+    });
+}
+
+inline LayoutUnit PathLine::topWithLeading() const
+{
+    return WTF::switchOn(m_pathVariant, [](const auto& path) {
+        return path.topWithLeading();
+    });
+}
+
+inline LayoutUnit PathLine::bottomWithLeading() const
+{
+    return WTF::switchOn(m_pathVariant, [](const auto& path) {
+        return path.bottomWithLeading();
     });
 }
@@ -182 +199 @@
 }
 
-}
-}
-
+inline const RootInlineBox* PathLine::legacyRootInlineBox() const
+{
+    return WTF::switchOn(m_pathVariant, [](const auto& path) {
+        return path.legacyRootInlineBox();
+    });
+}
+
+}
+}
+

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

@@ -50 +50 @@
     LayoutUnit selectionTopForHitTesting() const { return m_rootInlineBox->selectionTop(RootInlineBox::ForHitTesting::Yes); }
     LayoutUnit selectionBottom() const { return m_rootInlineBox->selectionBottom(); }
+    LayoutUnit topWithLeading() const { return m_rootInlineBox->lineTopWithLeading(); }
+    LayoutUnit bottomWithLeading() const { return m_rootInlineBox->lineBottomWithLeading(); }
 
     float y() const { return m_rootInlineBox->y(); }
@@ -56 +58 @@
 
     const RenderBlockFlow& containingBlock() const { return m_rootInlineBox->blockFlow(); }
+    const RootInlineBox* legacyRootInlineBox() const { return m_rootInlineBox; }
 
     void traverseNext()

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

@@ -52 +52 @@
     LayoutUnit top() const { return LayoutUnit::fromFloatRound(line().rect().y()); }
     LayoutUnit bottom() const { return LayoutUnit::fromFloatRound(line().rect().maxY()); }
+    // FIXME: What should these really be?
     LayoutUnit selectionTop() const { return top(); }
     LayoutUnit selectionTopForHitTesting() const { return top(); }
     LayoutUnit selectionBottom() const { return bottom(); }
+    LayoutUnit topWithLeading() const { return top(); }
+    LayoutUnit bottomWithLeading() const { return bottom(); }
 
     float y() const { return top(); }
@@ -61 +64 @@
 
     const RenderBlockFlow& containingBlock() const { return m_inlineContent->containingBlock(); }
+    const RootInlineBox* legacyRootInlineBox() const { return nullptr; }
 
     void traverseNext()

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

@@ -171 +171 @@
     InlineBox* legacyInlineBox() const
     {
-        ASSERT_NOT_REACHED();
         return nullptr;
     }

trunk/Source/WebCore/rendering/RenderBlockFlow.cpp

@@ -3307 +3307 @@
 }
 
-Position RenderBlockFlow::positionForBox(InlineBox *box, bool start) const
-{
-    if (!box)
+static Position positionForRun(const RenderBlockFlow& flow, LayoutIntegration::RunIterator run, bool start)
+{
+    if (!run)
         return Position();
 
-    if (!box->renderer().nonPseudoNode())
-        return makeDeprecatedLegacyPosition(nonPseudoElement(), start ? caretMinOffset() : caretMaxOffset());
-
-    if (!is<InlineTextBox>(*box))
-        return makeDeprecatedLegacyPosition(box->renderer().nonPseudoNode(), start ? box->renderer().caretMinOffset() : box->renderer().caretMaxOffset());
-
-    auto& textBox = downcast<InlineTextBox>(*box);
-    return makeDeprecatedLegacyPosition(textBox.renderer().nonPseudoNode(), start ? textBox.start() : textBox.start() + textBox.len());
+    if (!run->renderer().nonPseudoNode())
+        return makeDeprecatedLegacyPosition(flow.nonPseudoElement(), start ? flow.caretMinOffset() : flow.caretMaxOffset());
+
+    if (!is<LayoutIntegration::TextRunIterator>(run))
+        return makeDeprecatedLegacyPosition(run->renderer().nonPseudoNode(), start ? run->renderer().caretMinOffset() : run->renderer().caretMaxOffset());
+
+    auto& textRun = downcast<LayoutIntegration::TextRunIterator>(run);
+    return makeDeprecatedLegacyPosition(textRun->renderer().nonPseudoNode(), start ? textRun->localStartOffset() : textRun->localEndOffset());
 }
 
@@ -3391 +3391 @@
     ASSERT(childrenInline());
 
-    ensureLineBoxes();
-
-    if (!firstRootBox())
+    auto firstLine = LayoutIntegration::firstLineFor(*this);
+
+    if (!firstLine)
         return createVisiblePosition(0, Affinity::Downstream);
 
@@ -3400 +3400 @@
 
     // look for the closest line box in the root box which is at the passed-in y coordinate
-    InlineBox* closestBox = 0;
-    RootInlineBox* firstRootBoxWithChildren = 0;
-    RootInlineBox* lastRootBoxWithChildren = 0;
-    for (RootInlineBox* root = firstRootBox(); root; root = root->nextRootBox()) {
-        if (fragment && root->containingFragment() != fragment)
+    LayoutIntegration::RunIterator closestRun;
+    LayoutIntegration::LineIterator firstLineWithChildren;
+    LayoutIntegration::LineIterator lastLineWithChildren;
+    for (auto line = firstLine; line; line.traverseNext()) {
+        if (fragment && line->legacyRootInlineBox() && line->legacyRootInlineBox()->containingFragment() != fragment)
             continue;
 
-        if (!root->firstLeafDescendant())
+        if (!line.firstRun())
             continue;
-        if (!firstRootBoxWithChildren)
-            firstRootBoxWithChildren = root;
-
-        if (!linesAreFlipped && root->isFirstAfterPageBreak() && (pointInLogicalContents.y() < root->lineTopWithLeading()
-            || (blocksAreFlipped && pointInLogicalContents.y() == root->lineTopWithLeading())))
+        if (!firstLineWithChildren)
+            firstLineWithChildren = line;
+
+        if (!linesAreFlipped && line->legacyRootInlineBox() && line->legacyRootInlineBox()->isFirstAfterPageBreak()
+            && (pointInLogicalContents.y() < line->topWithLeading() || (blocksAreFlipped && pointInLogicalContents.y() == line->topWithLeading())))
             break;
 
-        lastRootBoxWithChildren = root;
+        lastLineWithChildren = line;
 
         // check if this root line box is located at this y coordinate
-        if (pointInLogicalContents.y() < root->selectionBottom() || (blocksAreFlipped && pointInLogicalContents.y() == root->selectionBottom())) {
+        if (pointInLogicalContents.y() < line->selectionBottom() || (blocksAreFlipped && pointInLogicalContents.y() == line->selectionBottom())) {
             if (linesAreFlipped) {
-                RootInlineBox* nextRootBoxWithChildren = root->nextRootBox();
-                while (nextRootBoxWithChildren && !nextRootBoxWithChildren->firstLeafDescendant())
-                    nextRootBoxWithChildren = nextRootBoxWithChildren->nextRootBox();
-
-                if (nextRootBoxWithChildren && nextRootBoxWithChildren->isFirstAfterPageBreak() && (pointInLogicalContents.y() > nextRootBoxWithChildren->lineTopWithLeading()
-                    || (!blocksAreFlipped && pointInLogicalContents.y() == nextRootBoxWithChildren->lineTopWithLeading())))
+                auto nextLineWithChildren = line.next();
+                while (nextLineWithChildren && !nextLineWithChildren.firstRun())
+                    nextLineWithChildren.traverseNext();
+
+                if (nextLineWithChildren && nextLineWithChildren->legacyRootInlineBox() && nextLineWithChildren->legacyRootInlineBox()->isFirstAfterPageBreak()
+                    && (pointInLogicalContents.y() > nextLineWithChildren->topWithLeading() || (!blocksAreFlipped && pointInLogicalContents.y() == nextLineWithChildren->topWithLeading())))
                     continue;
             }
-            if (auto closestRun = LayoutIntegration::LineIterator(root).closestRunForLogicalLeftPosition(pointInLogicalContents.x()))
-                closestBox = closestRun->legacyInlineBox();
-            if (closestBox)
+            closestRun = line.closestRunForLogicalLeftPosition(pointInLogicalContents.x());
+            if (closestRun)
                 break;
         }
@@ -3438 +3437 @@
     bool moveCaretToBoundary = frame().editor().behavior().shouldMoveCaretToHorizontalBoundaryWhenPastTopOrBottom();
 
-    if (!moveCaretToBoundary && !closestBox && lastRootBoxWithChildren) {
+    if (!moveCaretToBoundary && !closestRun && lastLineWithChildren) {
         // y coordinate is below last root line box, pretend we hit it
-        auto closestRun = LayoutIntegration::LineIterator(lastRootBoxWithChildren).closestRunForLogicalLeftPosition(pointInLogicalContents.x());
-        closestBox = closestRun ? closestRun->legacyInlineBox() : nullptr;
-    }
-
-    if (closestBox) {
+        closestRun = lastLineWithChildren.closestRunForLogicalLeftPosition(pointInLogicalContents.x());
+    }
+
+    if (closestRun) {
         if (moveCaretToBoundary) {
-            LayoutUnit firstRootBoxWithChildrenTop = std::min(firstRootBoxWithChildren->selectionTop(RootInlineBox::ForHitTesting::Yes), LayoutUnit(firstRootBoxWithChildren->logicalTop()));
-            if (pointInLogicalContents.y() < firstRootBoxWithChildrenTop
-                || (blocksAreFlipped && pointInLogicalContents.y() == firstRootBoxWithChildrenTop)) {
-                InlineBox* box = firstRootBoxWithChildren->firstLeafDescendant();
-                if (box->isLineBreak()) {
-                    if (InlineBox* newBox = box->nextLeafOnLineIgnoringLineBreak())
-                        box = newBox;
+            LayoutUnit firstLineWithChildrenTop = std::min(firstLineWithChildren->selectionTopForHitTesting(), LayoutUnit(firstLineWithChildren->top()));
+            if (pointInLogicalContents.y() < firstLineWithChildrenTop
+                || (blocksAreFlipped && pointInLogicalContents.y() == firstLineWithChildrenTop)) {
+                auto run = firstLineWithChildren.firstRun();
+                if (run->isLineBreak()) {
+                    if (auto next = run.nextOnLineIgnoringLineBreak())
+                        run = next;
                 }
                 // y coordinate is above first root line box, so return the start of the first
-                return positionForBox(box, true);
+                return positionForRun(*this, run, true);
             }
         }
 
         // pass the box a top position that is inside it
-        LayoutPoint point(pointInLogicalContents.x(), closestBox->root().blockDirectionPointInLine());
+        LayoutPoint point(pointInLogicalContents.x(), closestRun.line()->blockDirectionPointInLine());
         if (!isHorizontalWritingMode())
             point = point.transposedPoint();
-        if (closestBox->renderer().isReplaced())
-            return positionForPointRespectingEditingBoundaries(*this, downcast<RenderBox>(closestBox->renderer()), point);
-        return closestBox->renderer().positionForPoint(point, nullptr);
-    }
-
-    if (lastRootBoxWithChildren) {
+        if (closestRun->renderer().isReplaced())
+            return positionForPointRespectingEditingBoundaries(*this, const_cast<RenderBox&>(downcast<RenderBox>(closestRun->renderer())), point);
+        return const_cast<RenderObject&>(closestRun->renderer()).positionForPoint(point, nullptr);
+    }
+
+    if (lastLineWithChildren) {
         // We hit this case for Mac behavior when the Y coordinate is below the last box.
         ASSERT(moveCaretToBoundary);
-        InlineBox* logicallyLastBox;
-        if (lastRootBoxWithChildren->getLogicalEndBoxWithNode(logicallyLastBox))
-            return positionForBox(logicallyLastBox, false);
+        if (auto logicallyLastRun = lastLineWithChildren.logicalEndRunWithNode())
+            return positionForRun(*this, logicallyLastRun, false);
     }
 
@@ -3484 +3481 @@
 Position RenderBlockFlow::positionForPoint(const LayoutPoint& point)
 {
-    // FIXME: This forces Complex Line Layout.
     return positionForPoint(point, nullptr).deepEquivalent();
 }

trunk/Source/WebCore/rendering/RenderBlockFlow.h

@@ -528 +528 @@
         LayoutUnit& lastLogicalTop, LayoutUnit& lastLogicalLeft, LayoutUnit& lastLogicalRight, const LogicalSelectionOffsetCaches&, const PaintInfo*) override;
     
-    Position positionForBox(InlineBox*, bool start = true) const;
     VisiblePosition positionForPointWithInlineChildren(const LayoutPoint& pointInLogicalContents, const RenderFragmentContainer*) override;
     void addFocusRingRectsForInlineChildren(Vector<LayoutRect>& rects, const LayoutPoint& additionalOffset, const RenderLayerModelObject*) override;

trunk/Source/WebCore/rendering/RootInlineBox.cpp

@@ -751 +751 @@
 }
 
-int RootInlineBox::blockDirectionPointInLine() const
-{
-    return !blockFlow().style().isFlippedBlocksWritingMode() ? std::max(lineTop(), selectionTop()) : std::min(lineBottom(), selectionBottom());
-}
-
 RenderBlockFlow& RootInlineBox::blockFlow() const
 {

trunk/Source/WebCore/rendering/RootInlineBox.h

@@ -80 +80 @@
     LayoutUnit selectionTopAdjustedForPrecedingBlock() const;
     LayoutUnit selectionHeightAdjustedForPrecedingBlock() const { return std::max<LayoutUnit>(0, selectionBottom() - selectionTopAdjustedForPrecedingBlock()); }
-
-    int blockDirectionPointInLine() const;
 
     LayoutUnit alignBoxesInBlockDirection(LayoutUnit heightOfBlock, GlyphOverflowAndFallbackFontsMap&, VerticalPositionCache&);