Repository navigation
Grid: resolve named lines once (before the grid size estimate) - #1259
Merged
Merged
Conversation
|
I'll fix CI failures and address comments from users with write access that start with 'Devin'.
|
staging-devin-ai-integration
Bot
force-pushed
the
devin/1791249287-grid-size-estimate-named-lines
branch
6 times, most recently
from
October 8, 2026 02:31
5fe39ce to
201b768
Compare
staging-devin-ai-integration
Bot
force-pushed
the
devin/1791249287-grid-size-estimate-named-lines
branch
from
October 8, 2026 03:03
201b768 to
5bc0fde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Objective
compute_grid_size_estimatetreated every named-line placement asauto(into_origin_zero_ignoring_named), but its negative implicit track count is used as exact, and its positive count as a lower bound. Items placed with named lines therefore created implicit tracks that nothing occupies:grid-column: foo / 1(unknownfoo, resolves to1 / 4in a 2-column grid) was estimated asauto / 1, adding an implicit column before the grid.grid-row: span 5 / lastwas estimated as a fully auto-placed span of 5, adding implicit rows after the grid as well as the ones the item really needs before it.The estimate now works on each child's
grid-row/grid-columnwith named lines already resolved. To avoid resolving names twice (once for the estimate, once inplace_grid_items), name resolution is moved ahead of both:compute_grid_layoutresolves every in-flow child's named lines exactly once and hands the results to both the estimate andplace_grid_items.This fixes 2 WPT subtests in Blitz (
grid-placement-unknown-named-grid-line-001.html,grid-placement-negative-position-resolution-001.html), with no regressions incss/css-grid.Blitz PR: DioxusLabs/blitz#1072
Context
compute_grid_layoutbuildsitemsandplacements: Vec<ItemPlacement>(InBothAbsAxis<Line<OriginZeroGridPlacement>>— thegrid-row/grid-columnstyles with named lines resolved, in origin-zero coordinates; previously built insideplace_grid_itemsstep 0) in one walk over the in-flow children, then:compute_grid_size_estimate/get_known_child_positionstake&[ItemPlacement]instead of a style iterator (no generics, no resolver);child_min_line_max_line_spantakes theLine<OriginZeroGridPlacement>and is otherwise unchanged, so negative implicit tracks are still pre-sized by the estimate.place_grid_itemsloses step 0 and thechildren_iter/align_items/justify_items/named_line_resolverparameters; steps 1–4 are unchanged.ItemPlacementstays inplacement.rs, nowpub(super)and re-exported through the grid module forimplicit_grid.rs.util::test_helpersgainsresolve_named_lines(resolve test styles the same way) andgrid_items(build testGridItems), used by theimplicit_grid/placementunit tests.Memory is unchanged (
placementswas already allocated byplace_grid_items;itemsis reserved fromchild_countas before).mainwalked the children twice (estimate, then placement), callinggrid_row()/grid_column()— which clone the placement — on each walk; the single walk is measurably faster.Benchmarks vs the merge-base
d7cb4642(release, interleaved rounds pinned to one core, min of per-round Criterion medians; 15 rounds fornamed, 5 for the others;namedlayouts bit-identical):grid/named/lines63.8µs → 54.1µs (−15%),grid/named/areas66.6µs → 57.0µs (−14%),grid/named/unnamed28.4µs → 26.0µs (−8%); items extending before the explicit grid via negative integer lines −6%, via negative named lines (-2 foo) −7%;grid/wide31×31 / 100×100 / 316×316: −3% / −10% / −5%;grid/deep−4.5% / −8% / −5%;grid/superdeep−2% / +2%. Thenamedbenchmark (12×12 grid, 144 items placed by<integer> <ident>+span <ident>, by area name, or by negative lines) is not part of this PR.Tests: new generated fixtures
grid_implicit_tracks_named_line_placement(row flow) andgrid_implicit_tracks_named_line_placement_auto_flow_column(column flow, so the estimate's role in the flow axis is covered for both axes); both fail onmain.Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/2530fd358d4f4736bec3c4ac9accefdb
Open in Devin Desktop: https://dioxus.staging.devinenterprise.com/desktop/session/2530fd358d4f4736bec3c4ac9accefdb?variant=devin-insiders
Requested by: @nicoburns