Title: [295633] trunk
Revision
295633
Author
za...@apple.com
Date
2022-06-17 06:35:02 -0700 (Fri, 17 Jun 2022)

Log 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

Modified Paths

Added Paths

Diff

Added: trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end-expected.html (0 => 295633)


--- trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end-expected.html	2022-06-17 13:35:02 UTC (rev 295633)
@@ -0,0 +1,30 @@
+<style>
+.container {
+  box-sizing: border-box;
+  width: 320px;
+  height: 188px;
+  overflow-x: scroll;
+  border: 50px solid blue;
+  background-color: cyan;
+}
+
+.container > div {
+  margin-left: 10px;
+  width: 20px;
+  height: 20px;
+  background-color: green;
+}
+
+#overflow {
+  background-color: transparent;
+  width: 211px;
+  height: 10px;
+}
+</style>
+<div class=container><div></div></div>
+<div class=container><div style="width: 21px"></div><div id=overflow></div></div>
+<script>
+let scrollers = document.body.getElementsByClassName("container");
+for (let scroller of scrollers)
+  scroller.scrollTo(100, 0);
+</script>

Added: trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end.html (0 => 295633)


--- trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end.html	                        (rev 0)
+++ trunk/LayoutTests/fast/overflow/grid-horizontal-overflow-with-padding-end.html	2022-06-17 13:35:02 UTC (rev 295633)
@@ -0,0 +1,26 @@
+<style>
+.container {
+  box-sizing: border-box;
+  width: 320px;
+  height: 188px;
+  overflow-x: scroll;
+  padding-left: 10px;
+  padding-right: 190px;
+  border: 50px solid blue;
+  background-color: cyan;
+  display: grid;
+}
+
+.container > div {
+  width: 20px;
+  height: 20px;
+  background-color: green;
+}        
+</style>
+<div class=container><div></div></div>
+<div class=container><div style="width: 21px"></div></div>
+<script>
+let scrollers = document.body.getElementsByClassName("container");
+for (let scroller of scrollers)
+  scroller.scrollTo(100, 0);
+</script>

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (295632 => 295633)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2022-06-17 11:57:06 UTC (rev 295632)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2022-06-17 13:35:02 UTC (rev 295633)
@@ -681,23 +681,6 @@
     }
 }
 
-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.
 void RenderBlock::computeOverflow(LayoutUnit oldClientAfterEdge, bool)
@@ -708,23 +691,55 @@
     addOverflowFromPositionedObjects();
 
     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();
+        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;
 
-        // 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 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);
     }
@@ -3510,5 +3525,18 @@
     return makeStringByReplacingAll(string, discCharacterToReplace, textSecurityDiscPUACodePoint);
 #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

Modified: trunk/Source/WebCore/rendering/RenderBlock.h (295632 => 295633)


--- trunk/Source/WebCore/rendering/RenderBlock.h	2022-06-17 11:57:06 UTC (rev 295632)
+++ trunk/Source/WebCore/rendering/RenderBlock.h	2022-06-17 13:35:02 UTC (rev 295633)
@@ -231,7 +231,6 @@
     LayoutUnit adjustIntrinsicLogicalHeightForBoxSizing(LayoutUnit height) const override;
     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 };
     LayoutUnit logicalWidthForChild(const RenderBox& child) const { return isHorizontalWritingMode() ? child.width() : child.height(); }
@@ -378,6 +377,8 @@
 
     bool childBoxIsUnsplittableForFragmentation(const RenderBox& child) const;
 
+    static LayoutUnit layoutOverflowLogicalBottom(const RenderBlock&);
+
 public:
     virtual void computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeFloats = false);
     void clearLayoutOverflow();

Modified: trunk/Source/WebCore/rendering/RenderBox.cpp (295632 => 295633)


--- trunk/Source/WebCore/rendering/RenderBox.cpp	2022-06-17 11:57:06 UTC (rev 295632)
+++ trunk/Source/WebCore/rendering/RenderBox.cpp	2022-06-17 13:35:02 UTC (rev 295633)
@@ -5233,7 +5233,12 @@
 LayoutRect RenderBox::layoutOverflowRectForPropagation(const RenderStyle* parentStyle) const
 {
     // 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) {
             LayoutRect clippedOverflowRect = layoutOverflowRect();

Modified: trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp (295632 => 295633)


--- trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2022-06-17 11:57:06 UTC (rev 295632)
+++ trunk/Source/WebCore/rendering/RenderFlexibleBox.cpp	2022-06-17 13:35:02 UTC (rev 295633)
@@ -407,7 +407,7 @@
 
         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();
 

Modified: trunk/Source/WebCore/rendering/RenderGrid.cpp (295632 => 295633)


--- trunk/Source/WebCore/rendering/RenderGrid.cpp	2022-06-17 11:57:06 UTC (rev 295632)
+++ trunk/Source/WebCore/rendering/RenderGrid.cpp	2022-06-17 13:35:02 UTC (rev 295633)
@@ -369,7 +369,7 @@
         layoutPositionedObjects(relayoutChildren || isDocumentElementRenderer());
         m_trackSizingAlgorithm.reset();
 
-        computeOverflow(clientLogicalRightAndBottomAfterRepositioning().height());
+        computeOverflow(layoutOverflowLogicalBottom(*this));
     }
 
     updateLayerTransform();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to