Skip to content

Commit

Permalink
[GridNG] Second pass for orthogonal grid item contributions
Browse files Browse the repository at this point in the history
1. Introducing the second pass of the track sizing algorithm to
   `ComputeMinMaxSizes` (such pass is already implemented in `Layout`);
   this issue prevented many scenarios to correctly compute a grid
   container's 'auto' size due to the method only doing the first pass
   and ignoring whenever any item's inline size was dependent on the
   block size, which could be different between passes.

2. Refactoring the track sizing algorithm to avoid rebuilding the track
   collections when doing multiple runs of the track sizing algorithm.

Bug: 1045599
Change-Id: I98c48d6f54bde0c1276327841bbf3d911e64f19e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2935011
Commit-Queue: Ethan Jimenez <ethavar@microsoft.com>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#892435}
  • Loading branch information
ethanjv authored and Chromium LUCI CQ committed Jun 15, 2021
1 parent a2f157b commit e6560c2
Show file tree
Hide file tree
Showing 9 changed files with 282 additions and 214 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ class CORE_EXPORT NGGridLayoutAlgorithm
const NGBlockNode& node,
const NGGridData& grid_data,
const ComputedStyle& grid_style,
const WritingMode& container_writing_mode,
const WritingMode container_writing_mode,
const NGBoxStrut& borders,
const LogicalSize& border_box_size,
const LayoutUnit block_size);
Expand All @@ -324,7 +324,8 @@ class CORE_EXPORT NGGridLayoutAlgorithm
const GridItemData& grid_item,
GridTrackSizingDirection track_direction,
GridItemContributionType contribution_type,
bool* needs_additional_pass) const;
bool* needs_additional_pass,
bool* has_block_size_dependent_item) const;

wtf_size_t ComputeAutomaticRepetitions(
GridTrackSizingDirection track_direction) const;
Expand Down Expand Up @@ -356,17 +357,16 @@ class CORE_EXPORT NGGridLayoutAlgorithm

// Returns 'true' if it's possible to layout a grid item.
bool CanLayoutGridItem(const GridItemData& grid_item,
const GridTrackSizingDirection track_direction,
const NGConstraintSpace space,
const SizingConstraint sizing_constraint) const;
const NGConstraintSpace& space,
const GridTrackSizingDirection track_direction) const;

// Determines the major/minor alignment baselines for each row/column based on
// each item in |grid_items|, and stores the results in |grid_geometry|.
void CalculateAlignmentBaselines(GridTrackSizingDirection track_direction,
SizingConstraint sizing_constraint,
GridGeometry* grid_geometry,
GridItems* grid_items,
bool* needs_additional_pass = nullptr) const;
void CalculateAlignmentBaselines(
const GridTrackSizingDirection track_direction,
GridGeometry* grid_geometry,
GridItems* grid_items,
bool* needs_additional_pass) const;

// Initializes the given track collection, and returns the base set geometry.
SetGeometry InitializeTrackSizes(
Expand All @@ -378,15 +378,17 @@ class CORE_EXPORT NGGridLayoutAlgorithm
const GridGeometry& grid_geometry,
NGGridLayoutAlgorithmTrackCollection* track_collection,
GridItems* grid_items,
bool* needs_additional_pass) const;
bool* needs_additional_pass,
bool* has_block_size_dependent_item = nullptr) const;

// These methods implement the steps of the algorithm for intrinsic track size
// resolution defined in https://drafts.csswg.org/css-grid-2/#algo-content.
void ResolveIntrinsicTrackSizes(
const GridGeometry& grid_geometry,
NGGridLayoutAlgorithmTrackCollection* track_collection,
GridItems* grid_items,
bool* needs_additional_pass) const;
bool* needs_additional_pass,
bool* has_block_size_dependent_item) const;

void IncreaseTrackSizesToAccommodateGridItems(
const GridGeometry& grid_geometry,
Expand All @@ -395,22 +397,24 @@ class CORE_EXPORT NGGridLayoutAlgorithm
const bool is_group_spanning_flex_track,
GridItemContributionType contribution_type,
NGGridLayoutAlgorithmTrackCollection* track_collection,
bool* needs_additional_pass) const;
bool* needs_additional_pass,
bool* has_block_size_dependent_item) const;

void MaximizeTracks(
SizingConstraint sizing_constraint,
NGGridLayoutAlgorithmTrackCollection* track_collection) const;

void StretchAutoTracks(SizingConstraint sizing_constraint,
NGGridLayoutAlgorithmTrackCollection* track_collection,
bool* needs_additional_pass) const;
void StretchAutoTracks(
SizingConstraint sizing_constraint,
NGGridLayoutAlgorithmTrackCollection* track_collection) const;

void ExpandFlexibleTracks(
SizingConstraint sizing_constraint,
const GridGeometry& grid_geometry,
NGGridLayoutAlgorithmTrackCollection* track_collection,
GridItems* grid_items,
bool* needs_additional_pass) const;
bool* needs_additional_pass,
bool* has_block_size_dependent_item) const;

SetGeometry ComputeSetGeometry(
const NGGridLayoutAlgorithmTrackCollection& track_collection) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,18 +443,14 @@ wtf_size_t NGGridBlockTrackCollection::RangeCount() const {
NGGridSet::NGGridSet(wtf_size_t track_count)
: track_count_(track_count),
track_size_(Length::Auto(), Length::Auto()),
growth_limit_(kIndefiniteSize),
fit_content_limit_(kIndefiniteSize),
is_infinitely_growable_(false) {}
fit_content_limit_(kIndefiniteSize) {}

NGGridSet::NGGridSet(wtf_size_t track_count,
const GridTrackSize& track_size,
bool is_available_size_indefinite)
: track_count_(track_count),
track_size_(track_size),
growth_limit_(kIndefiniteSize),
fit_content_limit_(kIndefiniteSize),
is_infinitely_growable_(false) {
fit_content_limit_(kIndefiniteSize) {
if (track_size_.IsFitContent()) {
DCHECK(track_size_.FitContentTrackBreadth().IsLength());

Expand Down Expand Up @@ -521,6 +517,12 @@ void NGGridSet::SetBaseSize(LayoutUnit base_size) {
EnsureGrowthLimitIsNotLessThanBaseSize();
}

void NGGridSet::InitBaseSize(LayoutUnit base_size) {
DCHECK_NE(base_size, kIndefiniteSize);
base_size_ = base_size;
EnsureGrowthLimitIsNotLessThanBaseSize();
}

LayoutUnit NGGridSet::GrowthLimit() const {
DCHECK(!IsGrowthLimitLessThanBaseSize());
return growth_limit_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,11 @@ class CORE_EXPORT NGGridSet {
bool IsInfinitelyGrowable() const { return is_infinitely_growable_; }

void SetBaseSize(LayoutUnit base_size);
void InitBaseSize(LayoutUnit base_size);
void SetGrowthLimit(LayoutUnit growth_limit);
void InitGrowthLimit(LayoutUnit growth_limit) {
growth_limit_ = growth_limit;
}
void SetPlannedIncrease(LayoutUnit planned_increase) {
planned_increase_ = planned_increase;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,11 @@ crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-content-distri
crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-001.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-002.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-003.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-intrinsic-size-with-orthogonal-items.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-025.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-026.html [ Failure ]
crbug.com/1045599 fast/css-grid-layout/grid-item-spanning-and-orthogonal-flows.html [ Failure ]
crbug.com/1045599 fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html [ Failure ]
crbug.com/1045599 fast/css-grid-layout/maximize-tracks-definite-indefinite-height.html [ Failure ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/abspos/grid-abspos-staticpos-align-self-safe-001.html [ Pass ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-4.html [ Pass ]
Expand All @@ -164,7 +167,6 @@ crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/gri
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-003.html [ Pass ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-004.html [ Pass ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-005.html [ Pass ]
crbug.com/1045599 virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-intrinsic-size-with-orthogonal-items.html [ Pass ]
crbug.com/1045599 http/tests/devtools/elements/highlight/highlight-css-grid-area.js [ Failure ]
crbug.com/1045599 http/tests/devtools/elements/highlight/highlight-css-grid-direction.js [ Failure ]
crbug.com/1045599 http/tests/devtools/elements/highlight/highlight-css-grid-huge.js [ Failure ]
Expand Down
10 changes: 5 additions & 5 deletions third_party/blink/web_tests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -3930,14 +3930,13 @@ crbug.com/1045599 external/wpt/css/css-grid/alignment/grid-item-no-aspect-ratio-
crbug.com/1045599 external/wpt/css/css-grid/alignment/grid-item-no-aspect-ratio-stretch-7.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/alignment/grid-row-axis-self-baseline-synthesized-004.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/alignment/grid-self-baseline-not-applied-if-sizing-cyclic-dependency-003.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-004.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-005.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/layout-algorithm/grid-intrinsic-size-with-orthogonal-items.html [ Failure ]
crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-columns-001.html [ Failure ]
crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-columns-interpolation.html [ Failure ]
crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-rows-001.html [ Failure ]
crbug.com/759665 external/wpt/css/css-grid/animation/grid-template-rows-interpolation.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-definition/grid-repeat-max-width-001.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-004.html [ Failure ]
crbug.com/1045599 external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-005.html [ Failure ]

### Tests failing with LayoutNGGrid disabled:
virtual/layout-ng-grid/external/wpt/css/css-grid/abspos/descendant-static-position-001.html [ Failure ]
Expand Down Expand Up @@ -3983,7 +3982,6 @@ virtual/layout-ng-grid/external/wpt/css/css-grid/grid-definition/grid-auto-repea
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-definition/grid-auto-repeat-dynamic-001.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-definition/grid-auto-repeat-dynamic-003.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/aspect-ratio-004.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-auto-margin-and-replaced-item-001.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-item-inline-contribution-002.tentative.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-items/grid-item-inline-contribution-003.tentative.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/grid-model/grid-areas-overflowing-grid-container-001.html [ Failure ]
Expand All @@ -3996,10 +3994,12 @@ virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-content-d
virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-001.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-002.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-flex-track-intrinsic-sizes-003.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-grid/layout-algorithm/grid-intrinsic-size-with-orthogonal-items.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-sizing/aspect-ratio/grid-aspect-ratio-018.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-sizing/aspect-ratio/grid-aspect-ratio-019.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-025.html [ Failure ]
virtual/layout-ng-grid/external/wpt/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-026.html [ Failure ]
virtual/layout-ng-grid/fast/css-grid-layout/grid-item-spanning-and-orthogonal-flows.html [ Failure ]
virtual/layout-ng-grid/fast/css-grid-layout/grid-track-sizing-with-orthogonal-flows.html [ Failure ]
virtual/layout-ng-grid/fast/css-grid-layout/maximize-tracks-definite-indefinite-height.html [ Failure ]

# Bad stretching of svgs without aspect-ratio.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,25 +33,25 @@

<pre>rows: auto</pre>

<div class="grid" data-expected-width="25" data-expected-height="150">
<div class="grid" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

<pre>rows: minmax(100px, 200px)</pre>

<div class="grid minmax-100-200" data-expected-width="50" data-expected-height="150">
<div class="grid minmax-100-200" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

<pre>rows: minmax(auto, 200px)</pre>

<div class="grid minmax-auto-200" data-expected-width="50" data-expected-height="150">
<div class="grid minmax-auto-200" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

<pre>rows: minmax(100px, auto)</pre>

<div class="grid minmax-100-auto" data-expected-width="25" data-expected-height="150">
<div class="grid minmax-100-auto" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

Expand All @@ -63,13 +63,13 @@

<pre>rows: minmax(100px, fit-content)</pre>

<div class="grid minmax-auto-fitcontent" data-expected-width="25" data-expected-height="150">
<div class="grid minmax-auto-fitcontent" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

<pre>rows: minmax(100px, 1fr)</pre>

<div class="grid minmax-auto-1fr" data-expected-width="25" data-expected-height="150">
<div class="grid minmax-auto-1fr" data-expected-width="75" data-expected-height="150">
<div class="item" data-expected-width="75" data-expected-height="150">XXX XX X XXX XX X</div>
</div>

Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
</div>

<p>Grid: <b>min-content</b> / <b>min-content min-content</b> | align: <b>stretch</b> | content-sized: <b>fit-content x auto</b> | font: <b>50px</b> | Blue(LR) - <b>row: 1 / 3</b> col: 1 - <b>XX X X</b> | Magenta - row: 1 col: 1 - <b>X X</b></p>
<div class="grid itemsStart oneMinContentColTwoMinContentRows fit-content" data-expected-width="50" data-expected-height="100">
<div class="grid itemsStart oneMinContentColTwoMinContentRows fit-content" data-expected-width="150" data-expected-height="100">
<div class="verticalLR" style="grid-row: 1 / 3; grid-column: 1" data-expected-width="150" data-expected-height="100">XX X X</div>
<div class="" style="grid-row: 1; grid-column: 1" data-expected-width="150" data-expected-height="50">X X</div>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@

<div class="container">
<p>Grid width under <b>min-content</b> constrain and <b>fixed</b> height.<br> All grid items sized with <b>min-{width, height} auto</b>.</p>
<div class="grid itemsStart contentStart min-content height200" data-expected-width="40" data-expected-height="200">
<div class="grid itemsStart contentStart min-content height200" data-expected-width="70" data-expected-height="200">
<div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="30" data-expected-height="50">XX XXX X XXX XX</div>
<div class="verticalLR firstRowSecondColumn" data-offset-x="30" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="verticalLR secondRowFirstColumn" data-offset-x="0" data-offset-y="105" data-expected-width="20" data-expected-height="95">XXXX XX X XX XXX</div>
Expand All @@ -58,18 +58,18 @@

<div class="container">
<p>Grid width under <b>max-content</b> constrain and <b>fixed</b> height.<br> All grid items sized with <b>min-{width, height} auto</b>.</p>
<div class="grid itemsStart contentStart max-content height200" data-expected-width="160" data-expected-height="200">
<div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="120" data-expected-height="20">XX XXX X XXX XX</div>
<div class="verticalLR firstRowSecondColumn" data-offset-x="120" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="grid itemsStart contentStart max-content height200" data-expected-width="190" data-expected-height="200">
<div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="150" data-expected-height="10">XX XXX X XXX XX</div>
<div class="verticalLR firstRowSecondColumn" data-offset-x="150" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="verticalLR secondRowFirstColumn" data-offset-x="0" data-offset-y="105" data-expected-width="20" data-expected-height="95">XXXX XX X XX XXX</div>
</div>
</div>

<div class="container">
<p>Grid width under <b>fit-content</b> constrain and <b>fixed</b> height.<br >All grid items sized with <b>min-{width, height} auto</b>.</p>
<div class="grid itemsStart contentStart fit-content height200" data-expected-width="160" data-expected-height="200">
<div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="120" data-expected-height="20">XX XXX X XXX XX</div>
<div class="verticalLR firstRowSecondColumn" data-offset-x="120" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="grid itemsStart contentStart fit-content height200" data-expected-width="190" data-expected-height="200">
<div class="firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="150" data-expected-height="10">XX XXX X XXX XX</div>
<div class="verticalLR firstRowSecondColumn" data-offset-x="150" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="verticalLR secondRowFirstColumn" data-offset-x="0" data-offset-y="105" data-expected-width="20" data-expected-height="95">XXXX XX X XX XXX</div>
</div>
</div>
Expand Down Expand Up @@ -123,9 +123,9 @@

<div class="container">
<p>Grid width under <b>max-content</b> constrain and <b>fixed</b> height.<br> All grid items sized with <b>min-width: 0px, min-height: auto</b>.</p>
<div class="grid itemsStart contentStart max-content height200" data-expected-width="160" data-expected-height="200">
<div class="minWidthZero firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="120" data-expected-height="20">XX XXX X XXX XX</div>
<div class="minWidthZero verticalLR firstRowSecondColumn" data-offset-x="120" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="grid itemsStart contentStart max-content height200" data-expected-width="190" data-expected-height="200">
<div class="minWidthZero firstRowFirstColumn" data-offset-x="0" data-offset-y="0" data-expected-width="150" data-expected-height="10">XX XXX X XXX XX</div>
<div class="minWidthZero verticalLR firstRowSecondColumn" data-offset-x="150" data-offset-y="0" data-expected-width="40" data-expected-height="105">X XXX XX XXXXX XX XXX X XXXX X XX</div>
<div class="minWidthZero verticalLR secondRowFirstColumn" data-offset-x="0" data-offset-y="105" data-expected-width="20" data-expected-height="95">XXXX XX X XX XXX</div>
</div>
</div>
Expand Down

0 comments on commit e6560c2

Please sign in to comment.