LibWeb: Prepare layout for result-carried fragment trees - #11035
Merged
kalenikaliaksandr merged 3 commits intoAug 7, 2026
Merged
Conversation
kalenikaliaksandr
commented
Aug 7, 2026
kalenikaliaksandr
commented
Member
- Resolve relpos percentage-inset definiteness on the owner side — compute_inset no longer climbs out of the running formatting context to read ancestor used values. parent computes the beyond-anonymous-root answer at spawn and carries it as a run input.
- Register anchor candidates instead of scanning all used values — anchor() resolution stops enumerating every used-values entry per lookup (O(all boxes) for a membership test). Boxes carrying anchor names register at used-values allocation, preserving the completion-order acceptability semantics in commit, measurement, and replay states.
- Shift inline containing block rects by relative insets — a rendering fix: an absolutely positioned box whose containing block is a relatively positioned inline (or nested in relpos inlines) didn't follow the inline's shifted fragments. The stored rect is now correct at production
compute_inset climbed the containing block chain past anonymous boxes to read has_definite_block_size() of the first non-anonymous ancestor whenever a position: relative box carries block-axis percentage insets. The climb escapes the running formatting context when its root is anonymous (anonymous flex or grid items, anonymous table wrappers), and then reads used values of boxes the running context does not own. The planned fragment-tree layout needs every formatting context run to be a function of its declared inputs: a run replayed from a result cache executes without its ancestors on the stack, and once working records become run-owned state that dies when the run returns, a box above the run root is not addressable at all. Reads that walk out of the run are exactly what forces the global used-values store and its freeze-time fix-ups to keep existing, so each one converted now is one less dependency the later series has to break.
anchor_lookup enumerated every used-values entry in the layout state and shipped all their shells across the FFI boundary on each anchor() resolution: the most global read in the layout engine, and O(all boxes) per lookup even though the C++ side only uses the array as a "has used values" membership test. A scan over the whole record store makes the store itself load-bearing as an index, which blocks two things the fragment-tree direction needs. Working records cannot become run-owned state that dies at freeze while anything still enumerates them globally, and a subtree skipped on a future result-cache hit allocates no records at all, so anchor candidacy has to be an explicit output a run can publish and a cached result can republish — never a side effect of iterating whatever happens to exist. Converting the scan into registration removes the last consumer that needs the store to be enumerable.
An absolutely positioned box whose containing block is a relatively positioned inline did not move with the inline's own inset, nor with insets of relative inline ancestors the inline is nested in, while the inline's fragments themselves were shifted at commit time — the stored containing block rectangle disagreed with the fragments it is generated from.
📝 WalkthroughWalkthrough
ChangesAnchor and inset layout
Estimated code review effort: 4 (Complex) | ~45 minutes 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Libraries/LibWeb/Rust/src/layout/formatting_context.rs (1)
1882-1906: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPass the replayed abspos context flag through anonymous formatting-context roots.
run_abspos_layout_passandrust_layout_replay_saved_abspos_layoutstart newFormattingContextRunobjects withtreat_block_axis_percentage_insets_as_auto_beyond_root = false, whilerust_layout_run_root_layoutpropagates this flag withtreat_block_axis_percentage_insets_as_auto_beyond_anonymous_child_root. If the abspos queue contains anonymous formatting-context roots, saved replay loses that propagation and treats percentage insets on those boxes as non-auto. Use the same propagation flag when building the replay run and enqueue it, and apply the same pattern inrun_abspos_layout_passif the queue can include anonymous roots.🤖 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/formatting_context.rs` around lines 1882 - 1906, Propagate treat_block_axis_percentage_insets_as_auto_beyond_anonymous_child_root through the abspos replay path: use the same flag when constructing the FormattingContextRun in rust_layout_replay_saved_abspos_layout and when enqueuing it, matching rust_layout_run_root_layout. Apply the corresponding change in run_abspos_layout_pass so anonymous formatting-context roots retain the flag and percentage insets are treated consistently.
🤖 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/abspos_engine.rs`:
- Line 1788: Preserve the percentage-inset definiteness policy when queuing
deferred absolute-positioned roots instead of hard-coding false in
FormattingContextRun::new. Store the policy with each queued root or recompute
it before construction, ensuring
resolve_block_axis_percentage_inset_basis_is_definite() treats anonymous roots
with an indefinite block-axis basis as indefinite; add coverage for a relative
descendant’s top/bottom percentage beneath that root.
---
Outside diff comments:
In `@Libraries/LibWeb/Rust/src/layout/formatting_context.rs`:
- Around line 1882-1906: Propagate
treat_block_axis_percentage_insets_as_auto_beyond_anonymous_child_root through
the abspos replay path: use the same flag when constructing the
FormattingContextRun in rust_layout_replay_saved_abspos_layout and when
enqueuing it, matching rust_layout_run_root_layout. Apply the corresponding
change in run_abspos_layout_pass so anonymous formatting-context roots retain
the flag and percentage insets are treated consistently.
🪄 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: fbf93c6c-e3c1-4e65-bd78-12d321e61748
📒 Files selected for processing (13)
Libraries/LibWeb/Layout/Node.cppLibraries/LibWeb/Rust/src/layout/abspos_engine.rsLibraries/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/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsLibraries/LibWeb/Rust/src/layout/node_data.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsTests/LibWeb/Text/expected/css/abspos-cb-inline-relative-inset-shift.txtTests/LibWeb/Text/input/css/abspos-cb-inline-relative-inset-shift.html
| } | ||
| let run = | ||
| crate::layout::FormattingContextRun::new(state, root, LayoutMode::Normal, callbacks, should_collect_devtools_layout_data); | ||
| crate::layout::FormattingContextRun::new(state, root, LayoutMode::Normal, callbacks, should_collect_devtools_layout_data, false); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Preserve the percentage-inset policy for deferred abspos layout.
Line 1788 hard-codes the policy to false. When root is an anonymous formatting-context root with an indefinite block-axis basis beyond that root, layout_inside_child() propagates this incorrect value. resolve_block_axis_percentage_inset_basis_is_definite() then treats the anonymous root as definite. A relative descendant resolves top or bottom percentages as lengths instead of auto.
Store this policy with each queued abspos root, or recompute it before constructing this run. Add coverage for an abspos descendant below an anonymous formatting-context root with an indefinite block size.
🤖 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/abspos_engine.rs` at line 1788, Preserve the
percentage-inset definiteness policy when queuing deferred absolute-positioned
roots instead of hard-coding false in FormattingContextRun::new. Store the
policy with each queued root or recompute it before construction, ensuring
resolve_block_axis_percentage_inset_basis_is_definite() treats anonymous roots
with an indefinite block-axis basis as indefinite; add coverage for a relative
descendant’s top/bottom percentage beneath that root.
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
1 participant
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.