LibWeb: Various layout logic simplifications - #11057
Merged
kalenikaliaksandr merged 12 commits intoAug 9, 2026
Merged
Conversation
gmta
changed the title
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR refactors Rust layout sizing and formatting contexts. It removes unused 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 |
gmta
force-pushed
the
libweb-layout-simplifications
branch
from
August 9, 2026 15:09
a7ea61b to
256586e
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Libraries/LibWeb/Rust/src/layout/abspos_engine.rs (1)
1287-1304: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDead local variables kept only for a comment.
containing_block_inline_sizeandcontaining_block_block_sizeare computed and then discarded vialet _ = (...), with a comment explaining they document a basis distinction. Since neither value participates in any computation on this path, consider removing the variables and folding the note into a plain comment instead of keeping unused bindings alive.🤖 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` around lines 1287 - 1304, Remove the unused containing_block_inline_size and containing_block_block_size bindings and the let _ discard in the affected layout method. Preserve the basis-distinction note as a plain comment near the resolved solution values, without retaining dead locals.
🤖 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/abspos_engine.rs`:
- Around line 1287-1304: Remove the unused containing_block_inline_size and
containing_block_block_size bindings and the let _ discard in the affected
layout method. Preserve the basis-distinction note as a plain comment near the
resolved solution values, without retaining dead locals.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c49f1310-4167-467e-a508-3bfd47aff5ce
📒 Files selected for processing (9)
Libraries/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/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsLibraries/LibWeb/Rust/src/layout/svg_formatting_context.rsLibraries/LibWeb/Rust/src/layout/text_chunker.rs
💤 Files with no reviewable changes (1)
- Libraries/LibWeb/Rust/src/layout/inline_formatting_context.rs
gmta
force-pushed
the
libweb-layout-simplifications
branch
2 times, most recently
from
August 9, 2026 15:20
9ab40a6 to
2713875
Compare
Every one of `used_mut()`, `item_used_mut()` and `container_used_mut()` took `&self` and returned the same `&UsedValues` as its non-`_mut` counterpart, so the name promised a mutable borrow that Rust never handed out. They are leftovers from the C++ layout code, where the distinction was real. Remove the aliases and call the plain accessors instead.
Nearly every per-axis accessor the flex formatting context owns had to spell out a whole `if self.main_axis_is_horizontal()` block just to pick between an inline-axis and a block-axis value. Add `select_main()` and `select_cross()`, plus their `SizingAxis` variants, and route those accessors through them so each one states which pair of values it chooses between.
`solve_non_replaced_block_once()` kept the block axis unknowns in five separate locals, so its `solve_for` closure could not capture them and every call site had to re-pass all of them by hand. Collect them into a `BlockAxisSolution` that the closure takes by value, which also lets the caller thread one value around instead of destructuring a five-tuple three times.
`next_chunk()` commits chunks from eight different places, and each one had to restate the same `try_commit_chunk()` call in full because the chunk start, font, text type and pending tab break were all loose locals. Bundle them into a `PendingChunk` and add a helper that commits everything up to the cursor, leaving the varying `can_break_after` as the only thing a call site has to say.
Laying out a nested viewport and laying out a mask or clip resource both built the same `LayoutInput` literal. Move it into a `nested_layout_input()` helper.
Both places where an auto margin stops being honoured — an over-wide box, and a box with an automatic inline size — zeroed `margin_left` and `margin_right` and cleared their auto flags with the same code. Give that step a name and call it twice.
`place_float()` had two exits from its band search, and each one built the same `FloatPlacement` with the same nested left/right offset expression. Turn the search into a single `continue` and fall through to one construction site.
Computing a replaced element's inline size and computing its block size opened the same way, substituting `auto` for whichever of `width` and `height` has to be treated as automatic. Name that step and call it from both.
Resolving a used block size clamped it against `min-height` and `max-height` the same way whether or not the size is treated as automatic. Move the clamp into one helper.
The four min/max-content size calculators all ended the same way: consult the measurement cache, set up a `MeasurementState` under the matching size constraint, lay the box out and store the result. Move those tails into one helper per axis, keyed on the cache kind, and drop the hand-written `LayoutInput` literals in favour of `LayoutInput::new()` now that both agree on every field.
Adopting a parent grid area's names into a subgrid pushed the same `LineName` four times over, once per axis and edge, differing only in which raw name and which end it records. Loop over the two edges in a helper instead.
Almost everything the grid formatting context reads is stored once per axis, so it spelled out an `if axis.is_column()` block to pick between the two all over the file. Add `Axis::select()` along with `sizing_axis()` and `opposite()`, and give the repeatedly selected pairs — track lines, explicit line counts, content alignment and container padding — accessors of their own. `outer_edges()` now sums the two margin box edge helpers rather than restating them.
gmta
force-pushed
the
libweb-layout-simplifications
branch
from
August 9, 2026 15:24
2713875 to
cd3843a
Compare
gmta
enabled auto-merge (rebase)
August 9, 2026 15:28
kalenikaliaksandr
disabled auto-merge
August 9, 2026 18:41
kalenikaliaksandr
merged commit Aug 9, 2026
6bbff37
into
LadybirdBrowser:master
18 of 22 checks passed
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
2 participants
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.
No functional changes.