Carry resolved anchor() insets on the abspos layout inputs - #11078
Conversation
The calculated wrapper a bare anchor() inset resolves through was memoized in the pass-global anchor-inset store, which existed in part only to own the Arc those reads borrow. Build the wrapper eagerly when a fresh surround payload is created instead: equality excludes the wrappers (they are pure derivatives of the anchor-inset fields), so deduped payloads keep sharing, and inset reads now borrow the wrapper straight from the immutable payload.
Anchor resolution published its results through a pass-global side store on LayoutState that every later inset style read consulted, running as an implicit channel parallel to AbsposLayoutInputs, which already crosses the run boundary into the out-of-flow sizing passes and is persisted for replay. Make resolve_anchor_insets pure and thread its result explicitly: the abspos engine stores it on the layout inputs and hands it to the sizing chain, compute_inset consumes its own resolution locally, and replay now reuses the resolutions the original pass saved instead of reading unresolved anchor() values against an empty store. This removes the last pass-global mutable state from layout: LayoutState is now only the pass purpose.
📝 WalkthroughWalkthroughThe change adds computed wrapper handles for bare Sequence Diagram(s)sequenceDiagram
participant AbsposLayout as abspos_engine
participant StyleValues
participant ContainingBlock
participant LayoutInputs as AbsposLayoutInputs
AbsposLayout->>ContainingBlock: resolve anchor insets
AbsposLayout->>StyleValues: read resolved inset values
AbsposLayout->>LayoutInputs: save resolved_anchor_insets
LayoutInputs-->>AbsposLayout: provide data during replay
AbsposLayout->>StyleValues: apply inset overrides during sizing
Possibly related PRs
Suggested reviewers: 🚥 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.
🧹 Nitpick comments (1)
Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs (1)
3528-3530: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten the last clause of the comment.
The first two claims are accurate.
layout_pending_child()does resolve the insets later, atabspos_engine.rsLine 1728.The third claim, "an anchor-bearing inset is never auto either way", holds only before resolution. Before resolution the side is
InsetValue::BareAnchor, andis_auto()returns false. After resolution the side isInsetValue::Resolved, andis_auto()returnsresolved.is_auto, whichresolve_anchor_insets()sets to true when the anchor does not resolve. The recompute inbase_containing_block_info()covers that case, so the code is correct; only the stated invariant is too strong.📝 Proposed comment wording
- // Registration-time axis modes read raw style: anchor() - // insets resolve later in layout_pending_child, and an - // anchor-bearing inset is never auto either way. + // Registration-time axis modes read raw style: anchor() + // insets resolve later in layout_pending_child, where an + // unresolvable anchor becomes auto and the axis modes are + // recomputed from the resolutions.🤖 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/grid_formatting_context.rs` around lines 3528 - 3530, Update the comment near the axis-mode registration logic to limit the “anchor-bearing inset is never auto” statement to the pre-resolution/raw-style state. Mention that anchor insets may become auto after resolution when the anchor cannot be resolved, while preserving the accurate claims about later resolution in layout_pending_child().
🤖 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.
Nitpick comments:
In `@Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs`:
- Around line 3528-3530: Update the comment near the axis-mode registration
logic to limit the “anchor-bearing inset is never auto” statement to the
pre-resolution/raw-style state. Mention that anchor insets may become auto after
resolution when the anchor cannot be resolved, while preserving the accurate
claims about later resolution in layout_pending_child().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6708c445-adc5-4f8c-a27f-d0ab33cfcefa
📒 Files selected for processing (7)
Libraries/LibWeb/Rust/src/css/computed_value_types.rsLibraries/LibWeb/Rust/src/css/computed_values.rsLibraries/LibWeb/Rust/src/layout/abspos_engine.rsLibraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsLibraries/LibWeb/Rust/src/layout/style_facts.rs
0073974
into
LadybirdBrowser:master
Anchor resolution published its results through a pass-global side store
on LayoutState that every later inset style read consulted, running as
an implicit channel parallel to AbsposLayoutInputs, which already
crosses the run boundary into the out-of-flow sizing passes and is
persisted for replay. Make resolve_anchor_insets pure and thread its
result explicitly: the abspos engine stores it on the layout inputs and
hands it to the sizing chain, compute_inset consumes its own resolution
locally, and replay now reuses the resolutions the original pass saved
instead of reading unresolved anchor() values against an empty store.
This removes the last pass-global mutable state from layout:
LayoutState is now only the pass purpose.