Stop eagerly resolving flex container block sizes - #11089
Conversation
Auto-height flex containers have their block size resolved eagerly by the parent through a full throwaway max-content measurement of the subtree, which the flex run then just echoes back as its automatic content block size. Begin moving that resolution into the run itself: a new root sizing directive marks eligible roots (fragment-building normal runs of standards-mode flex roots with an auto block size and auto min-height) and carries the parent's resolution space, and a run whose main axis is the block axis now resolves its own used block size from the flex fraction machinery the measurement bottoms out in, reproducing the measurement's sizing context: flex base sizes are determined under a max-content main-axis constraint and percentage main gaps resolve against a zeroed container block size. The eager resolution still runs for now, so the self-resolved value overwrites an identical one and the suite is unchanged.
The row-direction counterpart: when the cross axis is the block axis, a directed flex root now resolves its own used block size at the spec's used-cross-size step, from the sum of its flex line cross sizes plus cross gaps, resolving percentage cross gaps against a still-unresolved container block size exactly as the measurement it replaces did. Line placement and item inset resolution read the container's block size after this point, so they observe the same value as before, and exactly one of the two self-resolution points fires per container, chosen by which axis writes the block-size cell. The eager resolution still runs and is overwritten with an identical value, leaving the suite unchanged.
Two per-item sizing steps read the flex container's raw block-size cell before the self-resolution point: the percentage min-cross clamp transferred through an aspect ratio, and the stretch-fit hypothetical cross size of a replaced item whose only sizing information is a natural aspect ratio. Both observed the eagerly stored measurement, a value the measurement itself computed with that cell unset. Guard both on the container's cross size being definite, so the percentage clamp is skipped and the natural-ratio item transfers its used main size through the aspect ratio instead; the existing suite does not exercise either shape in an auto-height row container, and two new layout tests lock the guarded behavior.
With flex runs resolving their own automatic block sizes, skip the eager parent-side resolution at all three sites (block-level, float, and atomic inline roots) whenever the run carries the self-resolution directive; skipped-run paths, quirks documents, non-auto min-height roots, and measurement runs keep it. independent_root_automatic_block_size now returns the completed flex run's reported value instead of re-measuring, which also deletes the second throwaway measurement every auto-height inline-flex root paid in its finalize step. Every auto-height flex container was laid out at least twice; it is now laid out once, and the full suite is unchanged, with self-resolved values matching what the eager path would have produced.
📝 WalkthroughWalkthroughThe change adds flex self block-size resolution state to root sizing. Flex layout now resolves automatic block sizes from max-content main sizes and flex-line cross sizes. Root sizing propagates the resolution space through block, float, and atomic-inline layout. Aspect-ratio sizing applies definite-space conditions for minimum cross sizes and natural-ratio items. Two layout tests and expected snapshots cover the updated behavior. Sequence Diagram(s)sequenceDiagram
participant FormattingContext
participant FlexFormattingContext
participant BlockFormattingContext
FormattingContext->>FlexFormattingContext: resolve automatic block size
FlexFormattingContext->>FlexFormattingContext: calculate flex bases and align lines
FlexFormattingContext-->>FormattingContext: return automatic block-size result
FormattingContext->>BlockFormattingContext: pass flex self-resolution state
BlockFormattingContext-->>FormattingContext: complete root sizing
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs`:
- Around line 2750-2760: Preserve the zero-resolved cross-gap in the
self-resolution flow at
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs:2750-2760. Update
the later automatic block-size and line-alignment flow at
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs:3098-3101 so
align_all_flex_lines() does not recompute the cyclic gap against the newly
resolved block size; reuse the zero-based gap result when positioning lines.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ca06619-c4a5-416f-ab8f-64eca7e7def5
📒 Files selected for processing (9)
Libraries/LibWeb/Rust/src/layout/block_formatting_context.rsLibraries/LibWeb/Rust/src/layout/flex_formatting_context.rsLibraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/geometry.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsTests/LibWeb/Layout/expected/flex/auto-height-row-flex-with-natural-ratio-only-svg-item.txtTests/LibWeb/Layout/expected/flex/auto-height-row-flex-with-percentage-min-height-through-aspect-ratio.txtTests/LibWeb/Layout/input/flex/auto-height-row-flex-with-natural-ratio-only-svg-item.htmlTests/LibWeb/Layout/input/flex/auto-height-row-flex-with-percentage-min-height-through-aspect-ratio.html
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/Rust/src/layout/sizing_context.rs
| // https://drafts.csswg.org/css-align-3/#gap-percent | ||
| // In Flex Layout: Cyclic percentage sizes resolve against zero in all cases. | ||
| let style = self.style(self.flex_container); | ||
| let gap = if self.is_row_layout() { style.row_gap() } else { style.column_gap() }; | ||
| let cross_gap_resolved_against_zero = gap.to_px(CssPixels::default()); | ||
| let mut line_cross_size_sum = self | ||
| .flex_lines | ||
| .iter() | ||
| .fold(CssPixels::default(), |sum, line| sum + line.cross_size); | ||
| line_cross_size_sum += cross_gap_resolved_against_zero * self.flex_lines.len().saturating_sub(1); | ||
| self.resolve_own_auto_block_size(line_cross_size_sum, resolution_space); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Keep cyclic percentage cross gaps resolved against zero.
The helper resolves a cyclic percentage cross gap against zero. After line 3101 resolves the automatic block size, align_all_flex_lines() recalculates cross_gap() against that resolved size.
A wrapping auto-height row flex container with row-gap: 50% will size without the gap, then place lines with a nonzero gap. The lines can overflow the resolved block size.
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L2750-L2760: retain the zero-based cross-gap result for the self-resolution flow.Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L3098-L3101: ensure later line alignment does not resolve the cyclic gap against the newly resolved block size.
📍 Affects 1 file
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L2750-L2760(this comment)Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs#L3098-L3101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs` around lines
2750 - 2760, Preserve the zero-resolved cross-gap in the self-resolution flow at
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs:2750-2760. Update
the later automatic block-size and line-alignment flow at
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rs:3098-3101 so
align_all_flex_lines() does not recompute the cyclic gap against the newly
resolved block size; reuse the zero-based gap result when positioning lines.
Skip the eager parent-side resolution at all three sites whenever the run carries the self-resolution directive; skipped-run paths, quirks documents, non-auto min-height roots, and measurement runs keep it. independent_root_automatic_block_size now returns the completed flex
run's reported value instead of re-measuring, which also deletes the second throwaway measurement every auto-height inline-flex root paid in its finalize step.
Every auto-height flex container was laid out at least twice; it is now laid out once.