Commit 7eeda924 authored by svillar's avatar svillar Committed by Commit bot

[css-grid] Remove the 2x computation of row sizes w/ indefinite heights

On crrev.com/358816, among other things, we added a second pass of the track
sizing algorithm for rows in order to properly compute row sizes when the
height was indefinite. We did that in order to have a symmetrical
implementation for columns and rows, but unfortunatelly that was not
correct.

Apart from issuing incorrect results in some cases it created a huge
performance issue in the case of nested grids because we were exponentially
increasing the amount of executions of the track sizing algorithm. The attached
performance test shows a 350% improvement with the patch.

BUG=619629

Review-Url: https://codereview.chromium.org/2339983002
Cr-Commit-Position: refs/heads/master@{#418892}
parent c08e4b27
...@@ -52,15 +52,15 @@ ...@@ -52,15 +52,15 @@
</div> </div>
<div class="grid max-content max-height-35" data-expected-width="40" data-expected-height="35"> <div class="grid max-content max-height-35" data-expected-width="40" data-expected-height="35">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="35">XX XXX XX XXX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX XX XXX</div>
</div> </div>
<div class="grid max-content max-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid max-content max-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX XXX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX X</div>
</div> </div>
<div class="grid max-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid max-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX XXX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX</div>
</div> </div>
<div class="grid max-content max-height-fill-available" data-expected-width="40" data-expected-height="100"> <div class="grid max-content max-height-fill-available" data-expected-width="40" data-expected-height="100">
...@@ -130,11 +130,11 @@ ...@@ -130,11 +130,11 @@
</div> </div>
<div class="grid itemsStart min-content" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX XX XX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XX XX</div>
</div> </div>
<div class="grid itemsStart min-content min-height-50" data-expected-width="40" data-expected-height="50"> <div class="grid itemsStart min-content min-height-50" data-expected-width="40" data-expected-height="50">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="50">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
<div class="grid itemsStart min-content min-height-fit-content" data-expected-width="40" data-expected-height="100"> <div class="grid itemsStart min-content min-height-fit-content" data-expected-width="40" data-expected-height="100">
...@@ -148,11 +148,11 @@ ...@@ -148,11 +148,11 @@
</div> </div>
<div class="grid itemsStart min-content min-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart min-content min-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX XXX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX</div>
</div> </div>
<div class="grid itemsStart min-content min-height-35" data-expected-width="40" data-expected-height="35"> <div class="grid itemsStart min-content min-height-35" data-expected-width="40" data-expected-height="35">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="35">XX XX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XX</div>
</div> </div>
<div class="grid itemsStart min-content min-height-max-content" data-expected-width="40" data-expected-height="100"> <div class="grid itemsStart min-content min-height-max-content" data-expected-width="40" data-expected-height="100">
...@@ -160,11 +160,11 @@ ...@@ -160,11 +160,11 @@
</div> </div>
<div class="grid itemsStart min-content min-height-50" data-expected-width="40" data-expected-height="50"> <div class="grid itemsStart min-content min-height-50" data-expected-width="40" data-expected-height="50">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="50">XXXX XXXX XXXX XXXX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XXXX XXXX XXXX XXXX</div>
</div> </div>
<div class="grid itemsStart min-content max-height-50" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart min-content max-height-50" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea min-height-fill-available" data-expected-width="40" data-expected-height="50">XXXX X X XXXX</div> <div class="sizedToGridArea min-height-fill-available" data-expected-width="40" data-expected-height="100">XXXX X X XXXX</div>
</div> </div>
<br> <br>
...@@ -207,7 +207,7 @@ ...@@ -207,7 +207,7 @@
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">X XXXX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">X XXXX X</div>
</div> </div>
<div class="grid itemsStart max-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart max-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX XX XX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XX XX</div>
</div> </div>
<div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="100"> <div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="100">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">X XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">X XX X</div>
...@@ -228,7 +228,7 @@ ...@@ -228,7 +228,7 @@
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XXXX X X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XXXX X X</div>
</div> </div>
<div class="grid itemsStart max-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart max-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">X XXX XX</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">X XXX XX</div>
</div> </div>
<div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="100"> <div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="100">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX XXX XX X</div>
...@@ -237,22 +237,22 @@ ...@@ -237,22 +237,22 @@
<div class="fit-content min-height-50" style="height: 75px;"> <div class="fit-content min-height-50" style="height: 75px;">
<div class="grid itemsStart fill-available" data-expected-width="40" data-expected-height="75"> <div class="grid itemsStart fill-available" data-expected-width="40" data-expected-height="75">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="75">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
</div> </div>
<div style="height: 25px;"> <div style="height: 25px;">
<div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="25"> <div class="grid itemsStart fit-content" data-expected-width="40" data-expected-height="25">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="25">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
<div class="grid itemsStart fill-available" data-expected-width="40" data-expected-height="25"> <div class="grid itemsStart fill-available" data-expected-width="40" data-expected-height="25">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="25">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
<div class="grid itemsStart fit-content min-height-35" data-expected-width="40" data-expected-height="35"> <div class="grid itemsStart fit-content min-height-35" data-expected-width="40" data-expected-height="35">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="35">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
<div class="grid itemsStart fit-content max-height-min-content" data-expected-width="40" data-expected-height="0"> <div class="grid itemsStart fit-content max-height-min-content" data-expected-width="40" data-expected-height="0">
<div class="sizedToGridArea" data-expected-width="40" data-expected-height="0">XX X</div> <div class="sizedToGridArea" data-expected-width="40" data-expected-height="100">XX X</div>
</div> </div>
</div> </div>
......
This diff is collapsed.
...@@ -484,16 +484,10 @@ void LayoutGrid::layoutBlock(bool relayoutChildren) ...@@ -484,16 +484,10 @@ void LayoutGrid::layoutBlock(bool relayoutChildren)
LayoutUnit oldClientAfterEdge = clientLogicalBottom(); LayoutUnit oldClientAfterEdge = clientLogicalBottom();
updateLogicalHeight(); updateLogicalHeight();
// The above call might have changed the grid's logical height depending on min|max height restrictions.
// Update the sizes of the rows whose size depends on the logical height (also on definite|indefinite sizes).
LayoutUnit availableSpaceForRows = contentLogicalHeight();
if (!cachedHasDefiniteLogicalHeight())
computeTrackSizesForDirection(ForRows, sizingData, availableSpaceForRows);
// 3- If the min-content contribution of any grid items have changed based on the row // 3- If the min-content contribution of any grid items have changed based on the row
// sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content // sizes calculated in step 2, steps 1 and 2 are repeated with the new min-content
// contribution (once only). // contribution (once only).
repeatTracksSizingIfNeeded(sizingData, availableSpaceForColumns, availableSpaceForRows); repeatTracksSizingIfNeeded(sizingData, availableSpaceForColumns, contentLogicalHeight());
// Grid container should have the minimum height of a line if it's editable. That doesn't affect track sizing though. // Grid container should have the minimum height of a line if it's editable. That doesn't affect track sizing though.
if (hasLineIfEmpty()) if (hasLineIfEmpty())
...@@ -623,6 +617,7 @@ void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo ...@@ -623,6 +617,7 @@ void LayoutGrid::computeIntrinsicLogicalWidths(LayoutUnit& minLogicalWidth, Layo
void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData) void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
{ {
DCHECK(sizingData.isValidTransition(ForRows));
ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData)); ASSERT(tracksAreWiderThanMinTrackBreadth(ForColumns, sizingData));
sizingData.setAvailableSpace(LayoutUnit()); sizingData.setAvailableSpace(LayoutUnit());
sizingData.freeSpace(ForRows) = LayoutUnit(); sizingData.freeSpace(ForRows) = LayoutUnit();
...@@ -634,6 +629,8 @@ void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData) ...@@ -634,6 +629,8 @@ void LayoutGrid::computeIntrinsicLogicalHeight(GridSizingData& sizingData)
m_maxContentHeight += totalGuttersSize; m_maxContentHeight += totalGuttersSize;
ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData)); ASSERT(tracksAreWiderThanMinTrackBreadth(ForRows, sizingData));
sizingData.nextState();
sizingData.sizingOperation = TrackSizing;
} }
LayoutUnit LayoutGrid::computeIntrinsicLogicalContentHeightUsing(const Length& logicalHeightLength, LayoutUnit intrinsicContentHeight, LayoutUnit borderAndPadding) const LayoutUnit LayoutGrid::computeIntrinsicLogicalContentHeightUsing(const Length& logicalHeightLength, LayoutUnit intrinsicContentHeight, LayoutUnit borderAndPadding) const
...@@ -1892,6 +1889,7 @@ void LayoutGrid::applyStretchAlignmentToTracksIfNeeded(GridTrackSizingDirection ...@@ -1892,6 +1889,7 @@ void LayoutGrid::applyStretchAlignmentToTracksIfNeeded(GridTrackSizingDirection
void LayoutGrid::layoutGridItems(GridSizingData& sizingData) void LayoutGrid::layoutGridItems(GridSizingData& sizingData)
{ {
DCHECK_EQ(sizingData.sizingOperation, TrackSizing);
populateGridPositionsForDirection(sizingData, ForColumns); populateGridPositionsForDirection(sizingData, ForColumns);
populateGridPositionsForDirection(sizingData, ForRows); populateGridPositionsForDirection(sizingData, ForRows);
m_gridItemsOverflowingGridArea.resize(0); m_gridItemsOverflowingGridArea.resize(0);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment