Skip to content

Commit 3bdeec8

Browse files
fantasaifantasai
committed
Set containing block of grid-lanes items to container size in the stacking axis
https://bugs.webkit.org/show_bug.cgi?id=304715 rdar://167221488 Reviewed by Brandon Stewart. Because grid items have a grid area in both axes, there are many places in the grid code where we set its containing block to a grid area in both axes. However the containing block of a grid-lanes item in the stacking axis is its grid container, not a grid area. Trying to use a grid area results in nonsensical item sizes in cases where the item size depends on the container (e.g. fit-content, percentages). This patch factors out the places where we try to set the item's containing block and ensures we set it correctly in the stacking axis for grid-lanes items. * LayoutTests/TestExpectations: * LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/abspos/column-grid-lanes-positioned-item-dynamic-change.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/track-sizing/grid-lanes-track-sizing-check-grid-height-on-resize.html: * LayoutTests/platform/mac/TestExpectations: * Source/WebCore/rendering/GridMasonryLayout.cpp: (WebCore::GridMasonryLayout::setItemContainingBlockToGridArea): (WebCore::GridMasonryLayout::insertIntoGridAndLayoutItem): (WebCore::GridMasonryLayout::setItemGridAxisContainingBlockToGridArea): Deleted. * Source/WebCore/rendering/GridMasonryLayout.h: * Source/WebCore/rendering/RenderGrid.cpp: (WebCore::RenderGrid::selfAlignmentForGridItem const): (WebCore::RenderGrid::performPreLayoutForGridItems const): (WebCore::RenderGrid::updateGridAreaWithEstimate const): (WebCore::RenderGrid::updateGridAreaIncludingAlignment const): (WebCore::RenderGrid::updateGridAreaForAspectRatioItems): (WebCore::RenderGrid::layoutGridItems): (WebCore::RenderGrid::layoutMasonryItems): (WebCore::RenderGrid::gridAreaBreadthForGridItemIncludingAlignmentOffsets const): * Source/WebCore/rendering/RenderGrid.h: Canonical link: https://commits.webkit.org/305319@main
1 parent cb75605 commit 3bdeec8

File tree

7 files changed

+49
-24
lines changed

7 files changed

+49
-24
lines changed

‎LayoutTests/TestExpectations‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1709,7 +1709,6 @@ imported/w3c/web-platform-tests/css/css-grid/subgrid/standalone-axis-size-005.ht
17091709
imported/w3c/web-platform-tests/css/css-grid/subgrid/subgrid-button.html [ ImageOnlyFailure ]
17101710

17111711
# grid-lanes
1712-
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/abspos/row-grid-lanes-positioned-item-dynamic-change.html [ ImageOnlyFailure ]
17131712
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/alignment/column-justify-items-center-001.html [ ImageOnlyFailure ]
17141713
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/alignment/grid-lanes-align-content-001.html [ ImageOnlyFailure ]
17151714
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/alignment/grid-lanes-align-content-002.html [ ImageOnlyFailure ]
@@ -1725,7 +1724,6 @@ imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/fragmentation/
17251724
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/fragmentation/grid-lanes-fragmentation-003.html [ ImageOnlyFailure ]
17261725
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/grid-placement/column-explicit-placement-003.html [ ImageOnlyFailure ]
17271726
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/grid-placement/grid-lanes-grid-placement-named-lines-002.html [ ImageOnlyFailure ]
1728-
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/grid-placement/row-explicit-placement-008.html [ ImageOnlyFailure ]
17291727
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/item-placement/column-reverse-001.html [ ImageOnlyFailure ]
17301728
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/item-placement/column-reverse-002.html [ ImageOnlyFailure ]
17311729
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/item-placement/column-reverse-003.html [ ImageOnlyFailure ]
@@ -1770,9 +1768,6 @@ imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/column-i
17701768
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/column-item-percentage-sizes-002.html [ ImageOnlyFailure ]
17711769
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/column-item-percentage-sizes-003.html [ ImageOnlyFailure ]
17721770
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/row-item-minmax-img-002.html [ ImageOnlyFailure ]
1773-
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/row-item-percentage-sizes-001.html [ ImageOnlyFailure ]
1774-
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/row-item-percentage-sizes-002.html [ ImageOnlyFailure ]
1775-
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/items/row-item-percentage-sizes-003.html [ ImageOnlyFailure ]
17761771
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/subgrid/track-sizing/grid-lanes-subgrid-flex-001.html [ ImageOnlyFailure ]
17771772
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/subgrid/track-sizing/grid-lanes-subgrid-flex-002.html [ ImageOnlyFailure ]
17781773
imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/subgrid/track-sizing/grid-lanes-subgrid-intrinsic-sizing.html [ ImageOnlyFailure ]

‎LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/abspos/column-grid-lanes-positioned-item-dynamic-change.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<!DOCTYPE html>
2-
<html>
2+
<html class="reftest-wait">
33
<head>
44
<meta charset="utf-8">
55
<title>CSS Grid Lanes Layout Test: Grid Lanes positioned item dynamic change.</title>

‎LayoutTests/imported/w3c/web-platform-tests/css/css-grid/grid-lanes/tentative/track-sizing/grid-lanes-track-sizing-check-grid-height-on-resize.html‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
var gridItem = document.querySelector('item');
3737
gridItem.style.height = '125px';
3838
gridItem.style.width = '300px';
39-
document
4039
document.documentElement.className = '';
4140
</script>
4241
</html>

‎Source/WebCore/rendering/GridMasonryLayout.cpp‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,25 @@ LayoutUnit GridMasonryLayout::calculateMasonryIntrinsicLogicalWidth(RenderBox& g
110110
return { };
111111
}
112112

113-
void GridMasonryLayout::setItemGridAxisContainingBlockToGridArea(const GridTrackSizingAlgorithm& algorithm, RenderBox& gridItem)
113+
void GridMasonryLayout::setItemContainingBlockToGridArea(const GridTrackSizingAlgorithm& algorithm, RenderBox& gridItem)
114114
{
115-
if (auto direction = gridAxisDirection(); direction == Style::GridTrackSizingDirection::Columns)
115+
CheckedPtr<RenderGrid> containingBlock = dynamicDowncast<RenderGrid>(gridItem.containingBlock());
116+
if (!containingBlock) {
117+
ASSERT_NOT_REACHED();
118+
return;
119+
}
120+
121+
// FIXME: We need to set both axes here because RenderGrid sets and expects them all over the place.
122+
// Ideally we untangle all that and only set the grid axis that we need. webkit.org/b/305136
123+
auto direction = gridAxisDirection();
124+
if (direction == Style::GridTrackSizingDirection::Columns) {
116125
gridItem.setGridAreaContentLogicalWidth(algorithm.gridAreaBreadthForGridItem(gridItem, direction));
117-
else
126+
gridItem.setGridAreaContentLogicalHeight(containingBlock->availableLogicalHeightForContentBox());
127+
} else {
118128
gridItem.setGridAreaContentLogicalHeight(algorithm.gridAreaBreadthForGridItem(gridItem, direction));
119-
129+
gridItem.setGridAreaContentLogicalWidth(containingBlock->contentBoxLogicalWidth());
130+
}
131+
120132
// FIXME(249230): Try to cache masonry layout sizes
121133
gridItem.setChildNeedsLayout(MarkOnlyThis);
122134
}
@@ -145,7 +157,7 @@ void GridMasonryLayout::insertIntoGridAndLayoutItem(const GridTrackSizingAlgorit
145157
gridItem.setOverridingBorderBoxLogicalWidth(calculateMasonryIntrinsicLogicalWidth(gridItem, layoutPhase));
146158

147159
m_renderGrid->currentGrid().insert(gridItem, area);
148-
setItemGridAxisContainingBlockToGridArea(algorithm, gridItem);
160+
setItemContainingBlockToGridArea(algorithm, gridItem);
149161
gridItem.layoutIfNeeded();
150162
updateRunningPositions(gridItem, area);
151163
m_autoFlowNextCursor = gridAxisSpanFromArea(area).endLine() % m_gridAxisTracksCount;

‎Source/WebCore/rendering/GridMasonryLayout.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ class GridMasonryLayout {
6363
GridArea gridAreaForDefiniteGridAxisItem(const RenderBox&) const;
6464

6565
void placeMasonryItems(const GridTrackSizingAlgorithm&, GridMasonryLayout::MasonryLayoutPhase);
66-
void setItemGridAxisContainingBlockToGridArea(const GridTrackSizingAlgorithm&, RenderBox&);
66+
void setItemContainingBlockToGridArea(const GridTrackSizingAlgorithm&, RenderBox&);
6767
void insertIntoGridAndLayoutItem(const GridTrackSizingAlgorithm&, RenderBox&, const GridArea&, GridMasonryLayout::MasonryLayoutPhase);
6868
LayoutUnit calculateMasonryIntrinsicLogicalWidth(RenderBox&, GridMasonryLayout::MasonryLayoutPhase);
6969

‎Source/WebCore/rendering/RenderGrid.cpp‎

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,7 +1253,7 @@ void RenderGrid::performPreLayoutForGridItems(const GridTrackSizingAlgorithm& al
12531253
// Orthogonal items should be laid out in order to properly compute content-sized tracks that may depend on item's intrinsic size.
12541254
// We also need to properly estimate its grid area size, since it may affect to the baseline shims if such item participates in baseline alignment.
12551255
if (GridLayoutFunctions::isOrthogonalGridItem(*this, *gridItem)) {
1256-
updateGridAreaLogicalSize(*gridItem, algorithm.estimatedGridAreaBreadthForGridItem(*gridItem, Style::GridTrackSizingDirection::Columns), algorithm.estimatedGridAreaBreadthForGridItem(*gridItem, Style::GridTrackSizingDirection::Rows));
1256+
updateGridAreaWithEstimate(*gridItem, algorithm);
12571257
gridItem->layoutIfNeeded();
12581258
continue;
12591259
}
@@ -1265,7 +1265,7 @@ void RenderGrid::performPreLayoutForGridItems(const GridTrackSizingAlgorithm& al
12651265
if (isBaselineAlignmentForGridItem(*gridItem)) {
12661266
// FIXME: Hack to fix nested grid text size overflow during re-layouts.
12671267
if (shouldUpdateGridAreaLogicalSize == ShouldUpdateGridAreaLogicalSize::Yes)
1268-
updateGridAreaLogicalSize(*gridItem, algorithm.estimatedGridAreaBreadthForGridItem(*gridItem, Style::GridTrackSizingDirection::Columns), algorithm.estimatedGridAreaBreadthForGridItem(*gridItem, Style::GridTrackSizingDirection::Rows));
1268+
updateGridAreaWithEstimate(*gridItem, algorithm);
12691269
gridItem->layoutIfNeeded();
12701270
}
12711271
}
@@ -1543,6 +1543,28 @@ static bool hasRelativeBlockAxisSize(const RenderGrid& grid, const RenderBox& gr
15431543
return GridLayoutFunctions::isOrthogonalGridItem(grid, gridItem) ? gridItem.hasRelativeLogicalWidth() || gridItem.style().logicalWidth().isAuto() : gridItem.hasRelativeLogicalHeight();
15441544
}
15451545

1546+
void RenderGrid::updateGridAreaWithEstimate(RenderBox& gridItem, const GridTrackSizingAlgorithm& algorithm) const
1547+
{
1548+
auto logicalWidth = isMasonry(Style::GridTrackSizingDirection::Columns)
1549+
? contentBoxLogicalWidth()
1550+
: algorithm.estimatedGridAreaBreadthForGridItem(gridItem, Style::GridTrackSizingDirection::Columns);
1551+
auto logicalHeight = isMasonry(Style::GridTrackSizingDirection::Rows)
1552+
? availableLogicalHeightForContentBox()
1553+
: algorithm.estimatedGridAreaBreadthForGridItem(gridItem, Style::GridTrackSizingDirection::Rows);
1554+
updateGridAreaLogicalSize(gridItem, logicalWidth, logicalHeight);
1555+
}
1556+
1557+
void RenderGrid::updateGridAreaIncludingAlignment(RenderBox& gridItem) const
1558+
{
1559+
auto logicalWidth = isMasonry(Style::GridTrackSizingDirection::Columns)
1560+
? contentBoxLogicalWidth()
1561+
: gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Columns);
1562+
auto logicalHeight = isMasonry(Style::GridTrackSizingDirection::Rows)
1563+
? contentBoxLogicalHeight()
1564+
: gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Rows);
1565+
updateGridAreaLogicalSize(gridItem, logicalWidth, logicalHeight);
1566+
}
1567+
15461568
void RenderGrid::updateGridAreaLogicalSize(RenderBox& gridItem, std::optional<LayoutUnit> width, std::optional<LayoutUnit> height) const
15471569
{
15481570
// Because the grid area cannot be styled, we don't need to adjust
@@ -1562,7 +1584,7 @@ void RenderGrid::updateGridAreaForAspectRatioItems(const Vector<RenderBox*>& aut
15621584
populateGridPositionsForDirection(m_trackSizingAlgorithm, Style::GridTrackSizingDirection::Rows);
15631585

15641586
for (auto& autoGridItem : autoGridItems) {
1565-
updateGridAreaLogicalSize(*autoGridItem, gridAreaBreadthForGridItemIncludingAlignmentOffsets(*autoGridItem, Style::GridTrackSizingDirection::Columns), gridAreaBreadthForGridItemIncludingAlignmentOffsets(*autoGridItem, Style::GridTrackSizingDirection::Rows));
1587+
updateGridAreaIncludingAlignment(*autoGridItem);
15661588
// For an item with aspect-ratio, if it has stretch alignment that stretches to the definite row, we also need to transfer the size before laying out the grid item.
15671589
if (autoGridItem->hasStretchedLogicalHeight())
15681590
applyStretchAlignmentToGridItemIfNeeded(*autoGridItem, gridLayoutState);
@@ -1588,7 +1610,7 @@ void RenderGrid::layoutGridItems(GridLayoutState& gridLayoutState)
15881610
// Setting the definite grid area's sizes. It may imply that the
15891611
// item must perform a layout if its area differs from the one
15901612
// used during the track sizing algorithm.
1591-
updateGridAreaLogicalSize(gridItem, gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Columns), gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Rows));
1613+
updateGridAreaIncludingAlignment(gridItem);
15921614

15931615
LayoutRect oldGridItemRect = gridItem.frameRect();
15941616

@@ -1632,7 +1654,7 @@ void RenderGrid::layoutMasonryItems(GridLayoutState& gridLayoutState)
16321654
// Setting the definite grid area's sizes. It may imply that the
16331655
// item must perform a layout if its area differs from the one
16341656
// used during the track sizing algorithm.
1635-
updateGridAreaLogicalSize(gridItem, gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Columns), gridAreaBreadthForGridItemIncludingAlignmentOffsets(gridItem, Style::GridTrackSizingDirection::Rows));
1657+
updateGridAreaIncludingAlignment(gridItem);
16361658

16371659
LayoutRect oldGridItemRect = gridItem.frameRect();
16381660

@@ -1695,12 +1717,6 @@ void RenderGrid::layoutOutOfFlowBox(RenderBox& gridItem, RelayoutChildren relayo
16951717

16961718
LayoutUnit RenderGrid::gridAreaBreadthForGridItemIncludingAlignmentOffsets(const RenderBox& gridItem, Style::GridTrackSizingDirection direction) const
16971719
{
1698-
if (direction == Style::GridTrackSizingDirection::Rows) {
1699-
if (areMasonryRows())
1700-
return isHorizontalWritingMode() ? gridItem.height() + gridItem.verticalMarginExtent() : gridItem.width() + gridItem.horizontalMarginExtent();
1701-
} else if (areMasonryColumns())
1702-
return isHorizontalWritingMode() ? gridItem.width() + gridItem.horizontalMarginExtent() : gridItem.height() + gridItem.verticalMarginExtent();
1703-
17041720
// We need the cached value when available because Content Distribution alignment properties
17051721
// may have some influence in the final grid area breadth.
17061722
const auto& tracks = m_trackSizingAlgorithm.tracks(direction);

‎Source/WebCore/rendering/RenderGrid.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,9 @@ class RenderGrid final : public RenderBlock {
167167
friend class GridMasonryLayout;
168168
friend class PositionedLayoutConstraints;
169169

170+
inline void updateGridAreaWithEstimate(RenderBox& gridItem, const GridTrackSizingAlgorithm&) const;
171+
inline void updateGridAreaIncludingAlignment(RenderBox& gridItem) const;
172+
170173
void computeLayoutRequirementsForItemsBeforeLayout(GridLayoutState&) const;
171174
bool canSetColumnAxisStretchRequirementForItem(const RenderBox&) const;
172175

0 commit comments

Comments
 (0)