Skip to content

Commit b0174fe

Browse files
aethanycaethanyc
committed
Bug 1966450 - Skip measuring reflow for grid items when grid container is computing its intrinsic inline contribution. r=dholbert
In Bug 1944423, we introduced logic to resolve row sizes when computing a grid container's intrinsic inline-size. This can cause grid items to be reflowed to measure their block-sizes as their content contributions. Since the grid container is still computing its intrinsic inline-size at this point, the reflow occurs under provisional inline constraints. As a result, the grid items' sizes are temporary and must be reflowed again later under the container's final inline-size. In `grid-container-as-flex-item-001.html`, when the outer grid container prepares a `ReflowInput` for the final reflow of its grid item (the flex container), it calls `GetMinISize()` [1], which recursively calls into the inner grid container's `GetMinISize()`, triggering the row size resolution and reflows the inner grid items as described above. However, if the inner grid container has already been reflowed during a previous content measurement by the flex container, its block-size might be cached. In this case, the flex container may skip reflowing the inner grid container, leading to incorrect layout results. This patch fixes the issue by skipping the measuring reflow for grid item entirely when the grid container is computing its intrinsic inline contribution. This avoid triggering an unnecessary grid item reflow that under provisional inline constraints. [1] `GetMinSize()` is called in `nsIFrame::ComputeSize()`: https://searchfox.org/mozilla-central/rev/fdb34ddfe30bd54aba991feb72b1476c77938e46/layout/generic/nsIFrame.cpp#6771-6773 Differential Revision: https://phabricator.services.mozilla.com/D254131
1 parent a57936c commit b0174fe

File tree

7 files changed

+187
-59
lines changed

7 files changed

+187
-59
lines changed

‎layout/generic/nsGridContainerFrame.cpp‎

Lines changed: 73 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -5958,46 +5958,75 @@ static nscoord ContentContribution(const GridItemInfo& aGridItem,
59585958
const bool isOrthogonal = childWM.IsOrthogonalTo(gridWM);
59595959
auto childAxis = isOrthogonal ? GetOrthogonalAxis(aAxis) : aAxis;
59605960
if (size == NS_INTRINSIC_ISIZE_UNKNOWN && childAxis == LogicalAxis::Block) {
5961-
// We need to reflow the child to find its BSize contribution.
5962-
// XXX this will give mostly correct results for now (until bug 1300366).
5963-
nscoord availISize = INFINITE_ISIZE_COORD;
5964-
nscoord availBSize = NS_UNCONSTRAINEDSIZE;
5965-
// The next two variables are MinSizeClamp values in the child's axes.
5966-
nscoord iMinSizeClamp = NS_MAXSIZE;
5967-
nscoord bMinSizeClamp = NS_MAXSIZE;
5968-
LogicalSize cbSize(childWM, 0, NS_UNCONSTRAINEDSIZE);
5969-
// Below, we try to resolve the child's grid-area size in its inline-axis
5970-
// to use as the CB/Available size in the MeasuringReflow that follows.
5971-
if (child->GetParent() != aGridRI.mFrame) {
5972-
// This item is a child of a subgrid descendant.
5973-
auto* subgridFrame =
5974-
static_cast<nsGridContainerFrame*>(child->GetParent());
5975-
MOZ_ASSERT(subgridFrame->IsGridContainerFrame());
5976-
auto* uts = subgridFrame->GetProperty(UsedTrackSizes::Prop());
5977-
if (!uts) {
5978-
uts = new UsedTrackSizes();
5979-
subgridFrame->SetProperty(UsedTrackSizes::Prop(), uts);
5980-
}
5981-
// The grid-item's inline-axis as expressed in the subgrid's WM.
5982-
auto subgridAxis = childWM.IsOrthogonalTo(subgridFrame->GetWritingMode())
5983-
? LogicalAxis::Block
5984-
: LogicalAxis::Inline;
5985-
uts->ResolveTrackSizesForAxis(subgridFrame, subgridAxis, *rc);
5986-
if (uts->mCanResolveLineRangeSize[subgridAxis]) {
5987-
auto* subgrid =
5988-
subgridFrame->GetProperty(nsGridContainerFrame::Subgrid::Prop());
5989-
const GridItemInfo* originalItem = nullptr;
5990-
for (const auto& item : subgrid->mGridItems) {
5991-
if (item.mFrame == child) {
5992-
originalItem = &item;
5993-
break;
5961+
if (aGridRI.mIsGridIntrinsicSizing && aAxis == LogicalAxis::Block) {
5962+
// We may reach here while computing the grid container's min-content
5963+
// contribution in ComputeIntrinsicISize(), potentially during row size
5964+
// resolution. In this context, the main reason for computing row sizes is
5965+
// to transfer the child's block-size to the inline-axis via aspect-ratio,
5966+
// contributing to the grid container's intrinsic inline-size in a later
5967+
// column size resolution. Since an indefinite block-size cannot be
5968+
// transferred in this way, we can safely skip MeasuringReflow() and
5969+
// simply use zero as a dummy value because the value does not affect the
5970+
// result.
5971+
size = 0;
5972+
} else {
5973+
// We need to reflow the child to find its BSize contribution.
5974+
// XXX this will give mostly correct results for now (until bug 1300366).
5975+
nscoord availISize = INFINITE_ISIZE_COORD;
5976+
nscoord availBSize = NS_UNCONSTRAINEDSIZE;
5977+
// The next two variables are MinSizeClamp values in the child's axes.
5978+
nscoord iMinSizeClamp = NS_MAXSIZE;
5979+
nscoord bMinSizeClamp = NS_MAXSIZE;
5980+
LogicalSize cbSize(childWM, 0, NS_UNCONSTRAINEDSIZE);
5981+
// Below, we try to resolve the child's grid-area size in its inline-axis
5982+
// to use as the CB/Available size in the MeasuringReflow that follows.
5983+
if (child->GetParent() != aGridRI.mFrame) {
5984+
// This item is a child of a subgrid descendant.
5985+
auto* subgridFrame =
5986+
static_cast<nsGridContainerFrame*>(child->GetParent());
5987+
MOZ_ASSERT(subgridFrame->IsGridContainerFrame());
5988+
auto* uts = subgridFrame->GetProperty(UsedTrackSizes::Prop());
5989+
if (!uts) {
5990+
uts = new UsedTrackSizes();
5991+
subgridFrame->SetProperty(UsedTrackSizes::Prop(), uts);
5992+
}
5993+
// The grid-item's inline-axis as expressed in the subgrid's WM.
5994+
auto subgridAxis =
5995+
childWM.IsOrthogonalTo(subgridFrame->GetWritingMode())
5996+
? LogicalAxis::Block
5997+
: LogicalAxis::Inline;
5998+
uts->ResolveTrackSizesForAxis(subgridFrame, subgridAxis, *rc);
5999+
if (uts->mCanResolveLineRangeSize[subgridAxis]) {
6000+
auto* subgrid =
6001+
subgridFrame->GetProperty(nsGridContainerFrame::Subgrid::Prop());
6002+
const GridItemInfo* originalItem = nullptr;
6003+
for (const auto& item : subgrid->mGridItems) {
6004+
if (item.mFrame == child) {
6005+
originalItem = &item;
6006+
break;
6007+
}
6008+
}
6009+
MOZ_ASSERT(originalItem, "huh?");
6010+
const auto& range = originalItem->mArea.LineRangeForAxis(subgridAxis);
6011+
nscoord pos, sz;
6012+
range.ToPositionAndLength(uts->mSizes[subgridAxis], &pos, &sz);
6013+
if (childWM.IsOrthogonalTo(subgridFrame->GetWritingMode())) {
6014+
availBSize = sz;
6015+
cbSize.BSize(childWM) = sz;
6016+
if (aGridItem.mState[aAxis] & ItemState::eClampMarginBoxMinSize) {
6017+
bMinSizeClamp = sz;
6018+
}
6019+
} else {
6020+
availISize = sz;
6021+
cbSize.ISize(childWM) = sz;
6022+
if (aGridItem.mState[aAxis] & ItemState::eClampMarginBoxMinSize) {
6023+
iMinSizeClamp = sz;
6024+
}
59946025
}
59956026
}
5996-
MOZ_ASSERT(originalItem, "huh?");
5997-
const auto& range = originalItem->mArea.LineRangeForAxis(subgridAxis);
5998-
nscoord pos, sz;
5999-
range.ToPositionAndLength(uts->mSizes[subgridAxis], &pos, &sz);
6000-
if (childWM.IsOrthogonalTo(subgridFrame->GetWritingMode())) {
6027+
} else if (aGridRI.mCols.mCanResolveLineRangeSize) {
6028+
nscoord sz = aGridRI.mCols.ResolveSize(aGridItem.mArea.mCols);
6029+
if (isOrthogonal) {
60016030
availBSize = sz;
60026031
cbSize.BSize(childWM) = sz;
60036032
if (aGridItem.mState[aAxis] & ItemState::eClampMarginBoxMinSize) {
@@ -6011,30 +6040,15 @@ static nscoord ContentContribution(const GridItemInfo& aGridItem,
60116040
}
60126041
}
60136042
}
6014-
} else if (aGridRI.mCols.mCanResolveLineRangeSize) {
6015-
nscoord sz = aGridRI.mCols.ResolveSize(aGridItem.mArea.mCols);
6016-
if (isOrthogonal) {
6017-
availBSize = sz;
6018-
cbSize.BSize(childWM) = sz;
6019-
if (aGridItem.mState[aAxis] & ItemState::eClampMarginBoxMinSize) {
6020-
bMinSizeClamp = sz;
6021-
}
6043+
if (isOrthogonal == (aAxis == LogicalAxis::Inline)) {
6044+
bMinSizeClamp = aMinSizeClamp;
60226045
} else {
6023-
availISize = sz;
6024-
cbSize.ISize(childWM) = sz;
6025-
if (aGridItem.mState[aAxis] & ItemState::eClampMarginBoxMinSize) {
6026-
iMinSizeClamp = sz;
6027-
}
6046+
iMinSizeClamp = aMinSizeClamp;
60286047
}
6048+
LogicalSize availableSize(childWM, availISize, availBSize);
6049+
size = ::MeasuringReflow(child, aGridRI.mReflowInput, rc, availableSize,
6050+
cbSize, iMinSizeClamp, bMinSizeClamp);
60296051
}
6030-
if (isOrthogonal == (aAxis == LogicalAxis::Inline)) {
6031-
bMinSizeClamp = aMinSizeClamp;
6032-
} else {
6033-
iMinSizeClamp = aMinSizeClamp;
6034-
}
6035-
LogicalSize availableSize(childWM, availISize, availBSize);
6036-
size = ::MeasuringReflow(child, aGridRI.mReflowInput, rc, availableSize,
6037-
cbSize, iMinSizeClamp, bMinSizeClamp);
60386052
size += child->GetLogicalUsedMargin(childWM).BStartEnd(childWM);
60396053
nscoord overflow = size - aMinSizeClamp;
60406054
if (MOZ_UNLIKELY(overflow > 0)) {

‎testing/web-platform/meta/css/css-grid/grid-container-as-flex-item-001.html.ini‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[grid-container-as-flex-item-001.html]
2+
prefs: [layout.css.grid-multi-pass-track-sizing.enabled:true]

‎testing/web-platform/meta/css/css-grid/grid-container-as-flex-item-002.html.ini‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[grid-container-as-flex-item-002.html]
2+
prefs: [layout.css.grid-multi-pass-track-sizing.enabled:true]

‎testing/web-platform/meta/css/css-grid/grid-container-as-flex-item-003.html.ini‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
[grid-container-as-flex-item-003.html]
2+
prefs: [layout.css.grid-multi-pass-track-sizing.enabled:true]

‎testing/web-platform/tests/css/css-grid/grid-container-as-flex-item-001.html‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
3+
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
4+
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1966450">
5+
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
6+
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
7+
<meta name="assert" content="Tests that the inner grid container as a flex item is getting a reflow against its final size.">
8+
9+
<style>
10+
.inner-grid {
11+
display: grid;
12+
flex: 1 1 0%;
13+
width: 100%;
14+
background: red;
15+
}
16+
17+
.inner-grid-item {
18+
font: 50px/1 Ahem;
19+
/* Disable automatic minimum size in inline axis. This is essential to
20+
reproduce this bug. 'min-width:auto' doesn't reproduce. */
21+
min-width: 0;
22+
width: 100%;
23+
color: transparent;
24+
background: green;
25+
}
26+
</style>
27+
28+
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
29+
<div style="display: grid">
30+
<div style="display: flex; flex-direction: column; width: 100px">
31+
<div class="inner-grid">
32+
<div class="inner-grid-item">XX XX</div>
33+
</div>
34+
</div>
35+
</div>

‎testing/web-platform/tests/css/css-grid/grid-container-as-flex-item-002.html‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
3+
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
4+
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1966450">
5+
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
6+
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
7+
<meta name="assert" content="Tests that the inner grid container as a flex item is getting a reflow against its final size.">
8+
9+
<style>
10+
.inner-grid {
11+
display: grid;
12+
flex: 1 1 0%;
13+
width: 100%;
14+
min-height: 0; /* Disable automatic minimum size in the main axis */
15+
background: red;
16+
}
17+
18+
.inner-grid-item {
19+
font: 50px/1 Ahem;
20+
/* Disable automatic minimum size in inline axis. This is essential to
21+
reproduce this bug. 'min-width:auto' doesn't reproduce. */
22+
min-width: 0;
23+
width: 100%;
24+
color: transparent;
25+
background: green;
26+
}
27+
</style>
28+
29+
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
30+
<div style="display: grid">
31+
<div style="display: flex; flex-direction: column; width: 100px">
32+
<div class="inner-grid">
33+
<div class="inner-grid-item">XX XX</div>
34+
</div>
35+
</div>
36+
</div>

‎testing/web-platform/tests/css/css-grid/grid-container-as-flex-item-003.html‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Ting-Yu Lin" href="mailto:tlin@mozilla.com">
3+
<link rel="author" title="Mozilla" href="https://www.mozilla.org/">
4+
<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1966450">
5+
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
6+
<link rel="stylesheet" type="text/css" href="/fonts/ahem.css">
7+
<meta name="assert" content="Tests that the inner grid container as a flex item is getting a reflow against its final size.">
8+
9+
<style>
10+
.inner-grid {
11+
display: grid;
12+
flex: none; /* Making inner grid container inflexible */
13+
width: 100%;
14+
height: 100px;
15+
min-height: 0; /* Disable automatic minimum size in the main axis */
16+
background: red;
17+
}
18+
19+
.inner-grid-item {
20+
font: 50px/1 Ahem;
21+
/* Disable automatic minimum size in inline axis. This is essential to
22+
reproduce this bug. 'min-width:auto' doesn't reproduce. */
23+
min-width: 0;
24+
width: 100%;
25+
color: transparent;
26+
background: green;
27+
}
28+
</style>
29+
30+
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
31+
<div style="display: grid">
32+
<div style="display: flex; flex-direction: column; width: 100px">
33+
<div class="inner-grid">
34+
<div class="inner-grid-item">XX XX</div>
35+
</div>
36+
</div>
37+
</div>

0 commit comments

Comments
 (0)