[Masonry] Update intrinsic auto repeat heurisitic [chromium/src : main]
0 views
Skip to first unread message
Alison Maher (Gerrit)
Feb 3, 2026, 8:55:03 PM (9 days ago) Feb 3
to Ian Kilpatrick, Chromium LUCI CQ, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Ian Kilpatrick
Alison Maher added 1 comment![]( "Open in Gerrit")
File third_party/blink/renderer/core/style/grid_track_size.h
Line 275, Patchset 3:
static GridTrackSize EmptyValue() {
// Use kDeviceHeight length type as a sentinel as it is not valid for grid
// track sizes.
return GridTrackSize(Length(Length::Type::kDeviceHeight));
}
static GridTrackSize DeletedValue() {
// Use kDeviceWidth length type as another sentinel as it is not valid for
// grid track sizes.
return GridTrackSize(Length(Length::Type::kDeviceWidth));
}Alison Maher . unresolved
Unfortunately, `kSafeToCompareToEmptyOrDeleted = false;` didn't allow me to avoid adding these two methods. Curious if you think these are "ok" sententials in this case, or if I should add something more explicit to GridTrackSize to distinguish empty/deleted from other instances?
Related details
Attention is currently required from:
- Ian Kilpatrick
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Kilpatrick (Gerrit)
Feb 6, 2026, 4:28:27 AM (7 days ago) Feb 6
to Alison Maher, Chromium LUCI CQ, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Attention needed from Alison Maher
Ian Kilpatrick voted and added 2 comments![]( "Open in Gerrit")
Votes added by Ian Kilpatrick
| Code-Review | +1 |
2 comments
File third_party/blink/renderer/core/layout/grid/grid_item.h
Line 283, Patchset 4 (Latest):
contribution_sizes = VirtualItemContributions();Ian Kilpatrick . unresolved
So you want to clear or reset?
(e.g. should this be: contribution_size = std::nullopt ?).
File third_party/blink/renderer/core/style/grid_track_size.h
Line 275, Patchset 3:
static GridTrackSize EmptyValue() {
// Use kDeviceHeight length type as a sentinel as it is not valid for grid
// track sizes.
return GridTrackSize(Length(Length::Type::kDeviceHeight));
}
static GridTrackSize DeletedValue() {
// Use kDeviceWidth length type as another sentinel as it is not valid for
// grid track sizes.
return GridTrackSize(Length(Length::Type::kDeviceWidth));
}Alison Maher . unresolved
Unfortunately, `kSafeToCompareToEmptyOrDeleted = false;` didn't allow me to avoid adding these two methods. Curious if you think these are "ok" sententials in this case, or if I should add something more explicit to GridTrackSize to distinguish empty/deleted from other instances?
Ian Kilpatrick
I would moderately prefer adding something explicit to GridTrackSize instead of this if that's ok.
E.g. a is_deleted_ member.
Related details
Attention is currently required from:
- Alison Maher
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Maher (Gerrit)
Feb 6, 2026, 1:49:54 PM (7 days ago) Feb 6
to Ian Kilpatrick, Chromium LUCI CQ, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Alison Maher voted and added 2 comments![]( "Open in Gerrit")
Votes added by Alison Maher
| Commit-Queue | +1 |
2 comments
File third_party/blink/renderer/core/layout/grid/grid_item.h
So you want to clear or reset?
(e.g. should this be: contribution_size = std::nullopt ?).
Alison Maher
We rely on there always being a contribution_size on a virtual item later on. We could remove that expectation, and just return O as the contribution size where needed, but that could make it easier to miss bugs with that in the future.
Updated the comment to say "clear" instead of "reset" so it's hopefully less confusing
File third_party/blink/renderer/core/style/grid_track_size.h
Line 275, Patchset 3:
static GridTrackSize EmptyValue() {
// Use kDeviceHeight length type as a sentinel as it is not valid for grid
// track sizes.
return GridTrackSize(Length(Length::Type::kDeviceHeight));
}
static GridTrackSize DeletedValue() {
// Use kDeviceWidth length type as another sentinel as it is not valid for
// grid track sizes.
return GridTrackSize(Length(Length::Type::kDeviceWidth));
}Alison Maher . resolved
Ian KilpatrickUnfortunately, `kSafeToCompareToEmptyOrDeleted = false;` didn't allow me to avoid adding these two methods. Curious if you think these are "ok" sententials in this case, or if I should add something more explicit to GridTrackSize to distinguish empty/deleted from other instances?
I would moderately prefer adding something explicit to GridTrackSize instead of this if that's ok.
E.g. a is_deleted_ member.
Alison Maher
That sounds good to me - definitely feels a bit less hacky that way. Updated to add is_deleted_ and is_empty_ members.
Lmk if this isn't quite what you had in mind, though.
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Feb 6, 2026, 2:03:18 PM (7 days ago) Feb 6
to Alison Maher, Ian Kilpatrick, Chromium LUCI CQ, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
Exportable changes to web-platform-tests were detected in this CL and a pull request in the upstream repo has been made: https://github.com/web-platform-tests/wpt/pull/57624.
When this CL lands, the bot will automatically merge the PR on GitHub if the required GitHub checks pass; otherwise, ecosystem-infra@ team will triage the failures and may contact you.
WPT Export docs:
https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md#Automatic-export-process
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Maher (Gerrit)
Feb 9, 2026, 10:46:22 AM (4 days ago) Feb 9
to Blink W3C Test Autoroller, Ian Kilpatrick, Chromium LUCI CQ, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Alison Maher voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Feb 9, 2026, 12:33:46 PM (4 days ago) Feb 9
to Alison Maher, Blink W3C Test Autoroller, Ian Kilpatrick, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/style/grid_track_size.h
Insertions: 21, Deletions: 10.
@@ -108,6 +108,21 @@
CacheMinMaxTrackBreadthTypes();
}
+ static GridTrackSize DeletedTrackSize() {
+ GridTrackSize size(Length::Auto());
+ size.is_deleted_ = true;
+ return size;
+ }
+
+ static GridTrackSize EmptyTrackSize() {
+ GridTrackSize size(Length::Auto());
+ size.is_empty_ = true;
+ return size;
+ }
+
+ bool IsDeletedValue() const { return is_deleted_; }
+ bool IsEmptyValue() const { return is_empty_; }
+
const Length& FitContentTrackBreadth() const {
DCHECK(type_ == kFitContentTrackSizing);
return fit_content_track_breadth_;
@@ -151,7 +166,8 @@
return type_ == other.type_ &&
min_track_breadth_ == other.min_track_breadth_ &&
max_track_breadth_ == other.max_track_breadth_ &&
- fit_content_track_breadth_ == other.fit_content_track_breadth_;
+ fit_content_track_breadth_ == other.fit_content_track_breadth_ &&
+ is_deleted_ == other.is_deleted_ && is_empty_ == other.is_empty_;
}
void CacheMinMaxTrackBreadthTypes() {
@@ -249,6 +265,8 @@
bool min_track_breadth_is_min_content_ : 1;
bool max_track_breadth_is_min_content_ : 1;
bool track_size_definition_is_intrinsic_ : 1;
+ bool is_deleted_ : 1 = false;
+ bool is_empty_ : 1 = false;
};
template <>
@@ -270,18 +288,11 @@
}
static constexpr bool kEmptyValueIsZero = false;
- static constexpr bool kSafeToCompareToEmptyOrDeleted = false;
- static GridTrackSize EmptyValue() {
- // Use kDeviceHeight length type as a sentinel as it is not valid for grid
- // track sizes.
- return GridTrackSize(Length(Length::Type::kDeviceHeight));
- }
+ static GridTrackSize EmptyValue() { return GridTrackSize::EmptyTrackSize(); }
static GridTrackSize DeletedValue() {
- // Use kDeviceWidth length type as another sentinel as it is not valid for
- // grid track sizes.
- return GridTrackSize(Length(Length::Type::kDeviceWidth));
+ return GridTrackSize::DeletedTrackSize();
}
};
```
```
The name of the file: third_party/blink/renderer/core/layout/grid/grid_item.h
Insertions: 2, Deletions: 1.
@@ -278,7 +278,8 @@
std::max(contribution_sizes->min_clamp_size, min_clamp_size);
}
- // Reset all contribution sizes stored on a virtual item.
+ // Clear all contribution sizes stored on a virtual item so that they are set
+ // back to their default values.
void ClearContributionSizes() {
contribution_sizes = VirtualItemContributions();
}
```
Change information
Commit message:
[Masonry] Update intrinsic auto repeat heurisitic
Per recent CSSWG resolution [1], to avoid overflow when mixing fixed
and intrinsic track sizes in an auto repeat, the heuristic was updated
to avoid dividing by item spans, and expand the auto repeat as much
as needed to get every combination of item contributions based on the
largest item span size.
If an item span size is dependent on the total number of auto
repetitions, the calcululation in this CL may be inaccurate, because
that would be cyclic. Instead, create a grid line resolver with one
auto repeat, and use that to determine the largest span size.
Given the new heuristic, you can also end up with the same track
definition in the auto repeat evaluating to a different size. Update
to use a map so that we can keep track of the largest size per track
definition to determine the final number of intrinsic auto repeats.
[1] https://github.com/w3c/csswg-drafts/issues/12899#issuecomment-3525524685
Bug: 460411056, 343257585
Change-Id: Ie607b2f94649ef38bbcd3bd7a644023d32ee8fde
Reviewed-by: Ian Kilpatrick <ikilp...@chromium.org>
Commit-Queue: Alison Maher <alm...@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#1581819}
Files:
- M third_party/blink/renderer/core/layout/grid/grid_item.h
- M third_party/blink/renderer/core/layout/grid/grid_layout_utils.cc
- M third_party/blink/renderer/core/layout/grid/grid_layout_utils.h
- M third_party/blink/renderer/core/layout/grid/grid_track_collection.cc
- M third_party/blink/renderer/core/layout/grid/grid_track_collection.h
- M third_party/blink/renderer/core/layout/grid_lanes/grid_lanes_layout_algorithm.cc
- M third_party/blink/renderer/core/layout/grid_lanes/grid_lanes_layout_algorithm.h
- M third_party/blink/renderer/core/layout/grid_lanes/grid_lanes_layout_algorithm_test.cc
- M third_party/blink/renderer/core/layout/grid_lanes/grid_lanes_node.cc
- M third_party/blink/renderer/core/layout/grid_lanes/grid_lanes_node.h
- M third_party/blink/renderer/core/style/grid_track_size.h
- M third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-029-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-030-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-030.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-031-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-031.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-032-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-032.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/column-auto-repeat-auto-033.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-029-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-029.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-030-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-030.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-031-ref.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-031.html
- A third_party/blink/web_tests/external/wpt/css/css-grid/grid-lanes/tentative/track-sizing/auto-repeat/intrinsic-auto-repeat/row-auto-repeat-auto-032.html
Change size: L
Delta: 26 files changed, 771 insertions(+), 138 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Ian Kilpatrick
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Feb 9, 2026, 1:16:55 PM (4 days ago) Feb 9
to Chromium LUCI CQ, Alison Maher, Ian Kilpatrick, chromium...@chromium.org, Javier Fernandez, blink-revi...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org, zol...@webkit.org
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/57624
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages