Changeset 295633 in webkit

Timestamp:

Jun 17, 2022 6:35:02 AM (2 years ago)

Author:

Alan Bujtas

Message:

Remove redundant logical right computation for grid items in RenderBlock::computeOverflow
​https://bugs.webkit.org/show_bug.cgi?id=241689

Reviewed by Simon Fraser.

If the grid content produces layout overflow, we should not need to re-compute it again by looping through the grid items.

1. Decouple "include padding end" and "include child's margin end" logic
2. Decouple "include padding after" and "include padding end" logic.
3. Restore RenderFlexibleBox and RenderGrid computeOverflow calls to pre-r282463 (when clientLogicalRightAndBottomAfterRepositioning was introduced)

• LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end-expected.html: Added.
• LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end.html: Added.
• Source/WebCore/rendering/RenderBlock.cpp:

(WebCore::RenderBlock::computeOverflow):
(WebCore::RenderBlock::layoutOverflowLogicalBottom):
(WebCore::RenderBlock::clientLogicalRightAndBottomAfterRepositioning const): Deleted.

• Source/WebCore/rendering/RenderBlock.h:
• Source/WebCore/rendering/RenderBox.cpp:

(WebCore::RenderBox::layoutOverflowRectForPropagation const):

• Source/WebCore/rendering/RenderFlexibleBox.cpp:

(WebCore::RenderFlexibleBox::layoutBlock):

• Source/WebCore/rendering/RenderGrid.cpp:

(WebCore::RenderGrid::layoutBlock):

Canonical link: ​https://commits.webkit.org/251638@main

Location:

trunk

Files:

• LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end-expected.html (added)
• LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end.html (added)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderBlock.h (modified) (2 diffs)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderFlexibleBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderGrid.cpp (modified) (1 diff)

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -682 +682 @@
 }
 
-LayoutSize RenderBlock::clientLogicalRightAndBottomAfterRepositioning() const
-{
-    LayoutUnit maxChildLogicalRight;
-    LayoutUnit maxChildLogicalBottom;
-    for (RenderBox* child = firstChildBox(); child; child = child->nextSiblingBox()) {
-        if (child->isOutOfFlowPositioned())
-            continue;
-        LayoutUnit childLogicalRight = logicalLeftForChild(*child) + logicalWidthForChild(*child) + marginEndForChild(*child);
-        LayoutUnit childLogicalBottom = logicalTopForChild(*child) + logicalHeightForChild(*child) + marginAfterForChild(*child);
-        maxChildLogicalRight = std::max(maxChildLogicalRight, childLogicalRight);
-        maxChildLogicalBottom = std::max(maxChildLogicalBottom, childLogicalBottom);
-
-    }
-    return LayoutSize(maxChildLogicalRight + paddingRight(), std::max(clientLogicalBottom(), maxChildLogicalBottom + paddingAfter()));
-}
-
-
 // Overflow is always relative to the border-box of the element in question.
 // Therefore, if the element has a vertical scrollbar placed on the left, an overflow rect at x=2px would conceptually intersect the scrollbar.
@@ -709 +692 @@
 
     if (hasNonVisibleOverflow()) {
-        // Set the axis we don't care about to be 1, since we want this overflow to always be considered reachable.
-        LayoutUnit rectWidth = 1_lu;
-        // For grid, width of the overflow rect should be the width of the grid area of the items rather than the container block.
-        // As per https://github.com/w3c/csswg-drafts/issues/3653, child's margins along with padding should contribute to the
-        // scrollable overflow area.
-        if (this->isRenderGrid())
-            rectWidth = clientLogicalRightAndBottomAfterRepositioning().width();
-
-        // When we have overflow clip, propagate the original spillout since it will include collapsed bottom margins
-        // and bottom padding.
-        LayoutRect clientRect(flippedClientBoxRect());
-        LayoutRect rectToApply;
-        if (isHorizontalWritingMode())
-            rectToApply = LayoutRect(clientRect.x(), clientRect.y(), rectWidth, std::max(0_lu, oldClientAfterEdge - clientRect.y()));
-        else
-            rectToApply = LayoutRect(clientRect.x(), clientRect.y(), std::max(0_lu, oldClientAfterEdge - clientRect.x()), rectWidth);
-        addLayoutOverflow(rectToApply);
+        auto includePaddingEnd = [&] {
+            // As per https://github.com/w3c/csswg-drafts/issues/3653 padding should contribute to the scrollable overflow area.
+            if (!paddingEnd())
+                return;
+            // FIXME: Expand it to non-grid cases when applicable.
+            if (!is<RenderGrid>(*this))
+                return;
+
+            auto layoutOverflowRect = this->layoutOverflowRect();
+            auto layoutOverflowLogicalWidthIncludingPaddingEnd = [&] {
+                if (hasHorizontalLayoutOverflow())
+                    return (isHorizontalWritingMode() ? layoutOverflowRect.width() : layoutOverflowRect.height()) + paddingEnd();
+
+                // FIXME: This is not sufficient for BFC layout (missing non-formatting-context root descendants).
+                auto contentLogicalRight = LayoutUnit { };
+                for (auto& child : childrenOfType<RenderBox>(*this)) {
+                    if (child.isOutOfFlowPositioned())
+                        continue;
+                    auto childLogicalRight = logicalLeftForChild(child) + logicalWidthForChild(child) + std::max(0_lu, marginEndForChild(child));
+                    contentLogicalRight = std::max(contentLogicalRight, childLogicalRight);
+                }
+                auto logicalRightWithPaddingEnd = contentLogicalRight + paddingEnd();
+                // Use padding box as the reference box.
+                return logicalRightWithPaddingEnd - (isHorizontalWritingMode() ? borderLeft() : borderTop());
+            };
+
+            if (isHorizontalWritingMode())
+                layoutOverflowRect.setWidth(layoutOverflowLogicalWidthIncludingPaddingEnd());
+            else
+                layoutOverflowRect.setHeight(layoutOverflowLogicalWidthIncludingPaddingEnd());
+            addLayoutOverflow(layoutOverflowRect);
+        };
+        includePaddingEnd();
+
+        auto includePaddingAfter = [&] {
+            // When we have overflow clip, propagate the original spillout since it will include collapsed bottom margins and bottom padding.
+            auto clientRect = flippedClientBoxRect();
+            auto rectToApply = clientRect;
+            // Set the axis we don't care about to be 1, since we want this overflow to always be considered reachable.
+            if (isHorizontalWritingMode()) {
+                rectToApply.setWidth(1);
+                rectToApply.setHeight(std::max(0_lu, oldClientAfterEdge - clientRect.y()));
+            } else {
+                rectToApply.setWidth(std::max(0_lu, oldClientAfterEdge - clientRect.x()));
+                rectToApply.setHeight(1);
+            }
+            addLayoutOverflow(rectToApply);
+        };
+        includePaddingAfter();
         if (hasRenderOverflow())
             m_overflow->setLayoutClientAfterEdge(oldClientAfterEdge);
@@ -3511 +3526 @@
 #endif
 }
-    
+
+LayoutUnit RenderBlock::layoutOverflowLogicalBottom(const RenderBlock& renderer)
+{
+    ASSERT(is<RenderGrid>(renderer) || is<RenderFlexibleBox>(renderer));
+    auto maxChildLogicalBottom = LayoutUnit { };
+    for (auto& child : childrenOfType<RenderBox>(renderer)) {
+        if (child.isOutOfFlowPositioned())
+            continue;
+        auto childLogicalBottom = renderer.logicalTopForChild(child) + renderer.logicalHeightForChild(child) + renderer.marginAfterForChild(child);
+        maxChildLogicalBottom = std::max(maxChildLogicalBottom, childLogicalBottom);
+    }
+    return std::max(renderer.clientLogicalBottom(), maxChildLogicalBottom + renderer.paddingAfter());
+}
+
 } // namespace WebCore

trunk/Source/WebCore/rendering/RenderBlock.h

@@ -232 +232 @@
     void paintExcludedChildrenInBorder(PaintInfo&, const LayoutPoint&);
     
-    LayoutSize clientLogicalRightAndBottomAfterRepositioning() const;
     // Accessors for logical width/height and margins in the containing block's block-flow direction.
     enum ApplyLayoutDeltaMode { ApplyLayoutDelta, DoNotApplyLayoutDelta };
@@ -379 +378 @@
     bool childBoxIsUnsplittableForFragmentation(const RenderBox& child) const;
 
+    static LayoutUnit layoutOverflowLogicalBottom(const RenderBlock&);
+
 public:
     virtual void computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeFloats = false);

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -5234 +5234 @@
 {
     // Only propagate interior layout overflow if we don't completely clip it.
-    LayoutRect rect = borderBoxRect();
+    auto rect = borderBoxRect();
+    if (isGridItem()) {
+        // As per https://github.com/w3c/csswg-drafts/issues/3653, child's margins should contribute to the scrollable overflow area.
+        // FIXME: Expand it to non-grid cases when applicable.
+        rect.setWidth(rect.width() + std::max(0_lu, marginEnd()));
+    }
     if (!shouldApplyLayoutContainment()) {
         if (style().overflowX() == Overflow::Clip && style().overflowY() == Overflow::Visible) {

trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp

@@ -408 +408 @@
         repaintChildrenDuringLayoutIfMoved(oldChildRects);
         // FIXME: css3/flexbox/repaint-rtl-column.html seems to repaint more overflow than it needs to.
-        computeOverflow(clientLogicalRightAndBottomAfterRepositioning().height());
+        computeOverflow(layoutOverflowLogicalBottom(*this));
     }
     updateLayerTransform();

trunk/Source/WebCore/rendering/RenderGrid.cpp

@@ -370 +370 @@
         m_trackSizingAlgorithm.reset();
 
-        computeOverflow(clientLogicalRightAndBottomAfterRepositioning().height());
+        computeOverflow(layoutOverflowLogicalBottom(*this));
     }