Replace the layout engine style decode layer with lazy CV views - #10961
Merged
kalenikaliaksandr merged 10 commits intoAug 1, 2026
Merged
Conversation
The Rust layout engine still reads every sizing-shaped property by eagerly re-encoding the Rust-native group payloads into FfiSizeValue, the flat wire format left over from the days when style reads crossed an FFI callback boundary into C++. The C++ side never needed such an intermediate: CSS::Size implements its methods directly on the stored ComputedSize representation and resolves lazily per query. Introduce the same shape on the Rust side. LengthPercentageRef is a borrowed view over a retained length, percentage or calculated style value, and ComputedSize, ComputedLengthPercentageOrAuto and ComputedGap gain the CSS::Size method surface (kind predicates, to_px, contains_percentage, fit-content argument access) implemented lazily on the stored payload types. The percentage truncation, length rounding and Percentage::as_fraction multiplication-order semantics carry over from the decode path unchanged, and new unit tests pin them, including the truncate-versus-round split between percentages and lengths. The shared px-resolution helpers (truncated_css_pixels and the px-basis calc resolution context) move from style_facts.rs into the css module so the old decode path and the new views resolve through one implementation while later changes migrate readers off FfiSizeValue group by group.
Width, height, the min/max sizes, flex-basis and column-width now come out of StyleValues as &'static ComputedSize payload references carrying the lazy CSS::Size-style method surface, instead of being eagerly re-encoded into FfiSizeValue on every read. The StyleValues reader is pinned to the 'static payload lifetime the arena already guarantees for a pass, so the references are as freely storable as the decoded values were, and the sizing arms of SizeField and direct_size are gone. Consumers change mechanically: contains_percentage becomes a method call, the FfiSizeKind comparisons become kind predicates on the stored kind, the table percentage-contribution paths read the stored percentage through as_fraction() with the same multiplication order, and the fit-content keyword-versus-argument distinction is now the presence of the argument's style value rather than a decoded flag. Substituted auto sizes in the treat-as-auto replaced sizing paths share one static auto value. The FfiSizeValue kind predicates lose their last consumers here and are deleted; the remaining decode users only need is_auto and to_px.
Margin and padding reads now hand out &'static ComputedLengthPercentageOrAuto references into the surround style group payload instead of decoding each side into FfiSizeValue, following the same shape as the sizing properties. The margin and padding arms of SizeField and direct_size are gone; only the four inset fields still route through the decode path, pending the anchor-inset rework. Every consumer already reads these values through is_auto() and to_px(), so call sites are unchanged apart from the absolutely positioned box solver, where the shared resolve_or_auto helper splits into a margin variant while insets still carry the decoded representation.
The four inset properties were the last style reads flowing through size_value: they consult the per-LayoutState anchor-inset store before the payload, so a plain payload reference cannot represent them. They now come out of StyleValues as InsetValue, a three-state view that makes the store's masking rules structural: FromStyle borrows the stored computed value, BareAnchor borrows the store-owned calculated wrapper for a bare anchor() inset, and Resolved carries the px-or-auto value anchor resolution wrote back, which by construction masks both anchor representations and answers contains_anchor_function() with false, preserving the abspos engine's early-out on re-entry. The anchor-inset store now stores that resolved value directly rather than an FfiSizeValue, indexed by a dedicated InsetField enum, and the abspos engine reads anchor-bearing calc trees through anchor_bearing_calculated() instead of a raw pointer field. This removes size_value, the inset arms of SizeField and direct_size, and the decode helper for length-percentage-or-auto values.
Row and column gaps now come out of StyleValues as &'static ComputedGap references, where normal answers is_normal() and resolves to zero exactly as the decoded auto placeholder did. The SVG x and y properties, text-indent and the vertical-align offset hand out borrowed LengthPercentageRef views over their retained style values. These were the last fields flowing through the SizeField dispatch, so the enum, direct_size and the accessor macro built around them are gone, and FfiSizeValue keeps only the constructors and to_px that the grid track decode still uses.
Grid track breadths and sizing functions were the last holders of FfiSizeValue: every stored track size was eagerly decoded into the flat struct, and synthesized fixed tracks (collapsed tracks, gap tracks, fixed subgrid tracks) forged one from a px value. GridTrackBreadth is now an enum borrowing the computed size straight from the grid style group payload, TrackSizingFunction::Fixed carries that payload reference, and a dedicated FixedPx variant models the synthesized tracks, which never contain percentages and resolve to their stored px size exactly as the forged values did. TrackListSource and the track-expansion helpers now state the 'static payload lifetime the grid style reader already guarantees. With the grid decode gone, FfiSizeValue, FfiSizeKind and the decode_length_percentage/decode_computed_size helpers have no users left and are deleted: the wire format that once carried style reads across the FFI callback boundary is fully replaced by lazy views over the Rust-native computed values. ComputedSize and its handle gain a Debug derive so the grid track types keep theirs.
Every layout style read now goes through the lazy payload views, so the dead-code allowance that covered readers landing ahead of their consumers is gone, along with the few predicate methods that ended up with no callers outside the unit tests.
With every style read handing out payload references, StyleReader had become a payload multiplexer that StyleValues merely wrapped, and each reader-only context (tree building, node facts) juggled a second type for the same node. StyleValues now owns the payload pointer and the group accessors directly, generic over the payload lifetime instead of pinned to 'static: within a pass the payloads still come in 'static and the anchor-inset store bounds the view; outside a pass, from_payloads builds a store-less reader whose inset accessors are the one unavailable surface, matching what StyleReader could not answer either. The reader-only fact methods (display facts, the block-formatting-context predicate) move along, and style_reader_if_styled becomes style_values_if_styled. The 'static claims that leaked into consumers shrink accordingly: flex's UsedFlexBasis carries the pass lifetime, and the grid and sizing helpers take plain references. Grid track expansion keeps its 'static payload references by reading the grid style group through a store-less reader over the pass-blessed payloads.
The property accessors on StyleValues — which field of which style group carries width, the margins, tab-size and the rest — are computed-values knowledge, the Rust twin of the accessor layer in the C++ ComputedValues.h, so they now live beside the payload types as ComputedValuesView in computed_value_views.rs. The view owns the group pointer array, the typed group getters and every pure read, including the display facts and the style-only block-formatting-context predicate, with the group indices moving into computed_value_types.rs where the C++ static asserts keep pinning them through the same generated header. StyleValues shrinks to the layout pass state the css module cannot know about — the anchor-inset store behind the four inset reads and the line builder's vertical-align keyword substitution — and derefs to the view for everything else, so call sites are unchanged. This also undoes the optional-store compromise from folding StyleReader away: contexts outside a pass (tree building, node facts) now hold a plain ComputedValuesView with no inset surface at all, and the pass view carries the store unconditionally again.
The scalar_accessors table already expresses renamed projections, so the hand-written boolean and scalar accessors that merely rename a group field (has_column_count, the containment and text-indent flags, the tab-size triple, the font metrics, flex_basis_is_content and the natural-aspect-ratio flag) join it instead of restating the same shape one method at a time. A reference_accessors sibling covers the by-reference projections — the six sizing properties, column-width, margins, paddings and gaps — whose returns borrow the group payload for the view's lifetime, which the scalar macro's unnamed-lifetime impl block cannot express. The accessors that remain hand-written all have real bodies: the computed predicates, the fit-content and flex-basis substitutions, the expect-carrying length-percentage chains and the unit conversions.
📝 WalkthroughWalkthroughThe PR adds borrowed, lazy computed-style views and typed CSS value readers. Rust layout code migrates from ChangesComputed style and layout migration
Estimated code review effort: 5 (Critical) | ~120 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: 5
🧹 Nitpick comments (1)
Libraries/LibWeb/Rust/src/layout/abspos_engine.rs (1)
1876-1886: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winResolve the inset values lazily.
resolve_opposingcallsto_pxon both values before it testsis_auto. For aBareAnchorvalue,to_pxruns a full calc resolution. Move eachto_pxcall into the branch that uses it. This removes wasted work and narrows the surface of theBareAnchorresolution concern raised instyle_facts.rs.♻️ Proposed lazy resolution
let resolve_opposing = |first: InsetValue, second: InsetValue, basis: CssPixels| { - let resolved_first = first.to_px(basis); - let resolved_second = second.to_px(basis); if first.is_auto() && second.is_auto() { (CssPixels::default(), CssPixels::default()) } else if first.is_auto() { - (-resolved_second, resolved_second) + let resolved_second = second.to_px(basis); + (-resolved_second, resolved_second) } else { - (resolved_first, -resolved_first) + let resolved_first = first.to_px(basis); + (resolved_first, -resolved_first) } };🤖 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 1876 - 1886, Update the resolve_opposing closure to resolve inset values lazily: check is_auto() first, then call to_px only within the branch that uses that value. Avoid resolving either value in the both-auto case, and preserve the existing returned inset pairs for all other cases.
🤖 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/css/computed_value_views.rs`:
- Around line 312-328: Update decode_css_preferred_aspect_ratio to reject the
converted denominator when fraction_nearest_values_for produces a zero raw
value, alongside the existing numerator check. Return no_usable_ratio for either
zero fixed-point component; preserve the current validation and successful pair
return otherwise.
- Around line 482-489: Update native_group to use a release-enabled assert! for
the payload non-null check instead of debug_assert!, preserving the existing
unsafe cast while ensuring null group payloads fail before creating a reference.
- Around line 127-140: Update ComputedStyleValueHandle::length_percentage so its
receiver is borrowed as &'a self, binding LengthPercentageRef<'a> to the handle
borrow and preventing callers from selecting an independent longer lifetime.
Preserve the existing null-pointer handling and returned view construction.
In `@Libraries/LibWeb/Rust/src/layout/abspos_engine.rs`:
- Around line 1585-1586: Update the vertical margin resolution calls in the
absolute-positioning layout flow to pass the containing block’s inline-size
baseline, using available_space.inline_size.to_px_or_zero(), for both
style.margin_top() and style.margin_bottom() instead of
containing_block_block_size; document the deviation only if retaining the
current behavior intentionally.
In `@Libraries/LibWeb/Rust/src/layout/style_facts.rs`:
- Around line 110-117: Update InsetValue::to_px so Self::BareAnchor resolves the
anchor() calculation through resolve_calc_with_external_resolutions using the
appropriate anchor resolver before converting to pixels. Ensure unresolved
results are handled safely rather than reaching the resolved-value assertion
when resolve_anchor_insets() returns early.
---
Nitpick comments:
In `@Libraries/LibWeb/Rust/src/layout/abspos_engine.rs`:
- Around line 1876-1886: Update the resolve_opposing closure to resolve inset
values lazily: check is_auto() first, then call to_px only within the branch
that uses that value. Avoid resolving either value in the both-auto case, and
preserve the existing returned inset pairs for all other cases.
🪄 Autofix (Beta)
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: 21423f2f-080c-4aad-b617-3e79cb26f2ae
📒 Files selected for processing (16)
Libraries/LibWeb/Rust/src/css/computed_value_types.rsLibraries/LibWeb/Rust/src/css/computed_value_views.rsLibraries/LibWeb/Rust/src/css/mod.rsLibraries/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/grid_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/layout_state.rsLibraries/LibWeb/Rust/src/layout/mod.rsLibraries/LibWeb/Rust/src/layout/node_facts.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsLibraries/LibWeb/Rust/src/layout/style_facts.rsLibraries/LibWeb/Rust/src/layout/table_formatting_context.rsLibraries/LibWeb/Rust/src/layout/tree_builder.rs
Comment on lines
+127
to
+140
| impl ComputedStyleValueHandle { | ||
| /// The lifetime is the caller's to choose: the referenced style value is | ||
| /// retained by whatever owns the handle, not by the handle borrow itself. | ||
| pub(crate) fn length_percentage<'a>(&self) -> Option<LengthPercentageRef<'a>> { | ||
| if self.pointer.is_null() { | ||
| return None; | ||
| } | ||
| // SAFETY: A non-null handle points at the retained style value owned | ||
| // by the node's style group payload, which outlives every reader. | ||
| Some(LengthPercentageRef { | ||
| value: unsafe { &*self.pointer.cast::<StyleValueData>() }, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# List every call site so the tightened lifetime can be checked for compilation.
rg -n -C3 '\.length_percentage\(\)' Libraries/LibWeb/Rust/srcRepository: LadybirdBrowser/ladybird
Length of output: 11774
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file outline =="
ast-grep outline Libraries/LibWeb/Rust/src/css/computed_value_views.rs --view expanded | sed -n '1,220p'
echo "== relevant computed_value_views.rs =="
sed -n '1,330p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs | cat -n
echo "== relevant LengthPercentageRef definition candidates =="
rg -n -C4 "struct LengthPercentageRef|impl LengthPercentageRef|enum StyleValueData|struct ComputedStyleValueHandle|fn length_percentage" Libraries/LibWeb/Rust/src/css/computed_value_views.rsRepository: LadybirdBrowser/ladybird
Length of output: 21508
Bind length_percentage’s return lifetime to &'a self.
length_percentage<'a>(&self) lets callers pick an independent lifetime for LengthPercentageRef<'a>, including 'static. Use &'a self so the returned style-value view cannot outlive the handle borrow itself.
🤖 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/css/computed_value_views.rs` around lines 127 -
140, Update ComputedStyleValueHandle::length_percentage so its receiver is
borrowed as &'a self, binding LengthPercentageRef<'a> to the handle borrow and
preventing callers from selecting an independent longer lifetime. Preserve the
existing null-pointer handling and returned view construction.
Comment on lines
+312
to
+328
| fn decode_css_preferred_aspect_ratio(ratio: &ComputedAspectRatio) -> (CssPixels, CssPixels) { | ||
| let no_usable_ratio = (CssPixels::default(), CssPixels::default()); | ||
| if !ratio.has_preferred_ratio { | ||
| return no_usable_ratio; | ||
| } | ||
| let numerator = ratio.preferred_ratio_numerator; | ||
| let denominator = ratio.preferred_ratio_denominator; | ||
| let is_degenerate = !numerator.is_finite() || numerator == 0.0 || !denominator.is_finite() || denominator == 0.0; | ||
| if is_degenerate { | ||
| return no_usable_ratio; | ||
| } | ||
| let (numerator, denominator) = CssPixels::fraction_nearest_values_for(numerator, denominator); | ||
| if numerator.raw_value() == 0 { | ||
| return no_usable_ratio; | ||
| } | ||
| (numerator, denominator) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Also reject a denominator that collapses to zero.
fraction_nearest_values_for converts both terms to fixed point. The code checks only numerator.raw_value() == 0 afterwards. A very small but finite denominator can round to raw zero while the numerator stays non-zero. The function then returns a non-sentinel pair with a zero denominator. A consumer that tests only the numerator divides by zero.
🐛 Proposed fix
let (numerator, denominator) = CssPixels::fraction_nearest_values_for(numerator, denominator);
- if numerator.raw_value() == 0 {
+ if numerator.raw_value() == 0 || denominator.raw_value() == 0 {
return no_usable_ratio;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn decode_css_preferred_aspect_ratio(ratio: &ComputedAspectRatio) -> (CssPixels, CssPixels) { | |
| let no_usable_ratio = (CssPixels::default(), CssPixels::default()); | |
| if !ratio.has_preferred_ratio { | |
| return no_usable_ratio; | |
| } | |
| let numerator = ratio.preferred_ratio_numerator; | |
| let denominator = ratio.preferred_ratio_denominator; | |
| let is_degenerate = !numerator.is_finite() || numerator == 0.0 || !denominator.is_finite() || denominator == 0.0; | |
| if is_degenerate { | |
| return no_usable_ratio; | |
| } | |
| let (numerator, denominator) = CssPixels::fraction_nearest_values_for(numerator, denominator); | |
| if numerator.raw_value() == 0 { | |
| return no_usable_ratio; | |
| } | |
| (numerator, denominator) | |
| } | |
| fn decode_css_preferred_aspect_ratio(ratio: &ComputedAspectRatio) -> (CssPixels, CssPixels) { | |
| let no_usable_ratio = (CssPixels::default(), CssPixels::default()); | |
| if !ratio.has_preferred_ratio { | |
| return no_usable_ratio; | |
| } | |
| let numerator = ratio.preferred_ratio_numerator; | |
| let denominator = ratio.preferred_ratio_denominator; | |
| let is_degenerate = !numerator.is_finite() || numerator == 0.0 || !denominator.is_finite() || denominator == 0.0; | |
| if is_degenerate { | |
| return no_usable_ratio; | |
| } | |
| let (numerator, denominator) = CssPixels::fraction_nearest_values_for(numerator, denominator); | |
| if numerator.raw_value() == 0 || denominator.raw_value() == 0 { | |
| return no_usable_ratio; | |
| } | |
| (numerator, denominator) | |
| } |
🤖 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/css/computed_value_views.rs` around lines 312 -
328, Update decode_css_preferred_aspect_ratio to reject the converted
denominator when fraction_nearest_values_for produces a zero raw value,
alongside the existing numerator check. Return no_usable_ratio for either zero
fixed-point component; preserve the current validation and successful pair
return otherwise.
Comment on lines
+482
to
+489
| fn native_group<T>(self, group_index: usize) -> &'a T { | ||
| let payload = self.groups[group_index]; | ||
| debug_assert!(!payload.is_null()); | ||
| // SAFETY: The payload is the Rust-defined group struct itself; C++ | ||
| // derives its mirror from the cbindgen twin of the same type, and the | ||
| // node's ComputedValues keep it alive while readers exist. | ||
| unsafe { &*payload.cast::<T>() } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Promote the null check to a release assertion.
native_group guards the payload with debug_assert!. In release builds a null or missing payload creates a reference to address zero, which is undefined behavior, and every later field read is unsound. The index also panics only through the slice bound, which gives no group name. Use assert! here. The cost is one branch per style read, and the branch is predictable.
🛡️ Proposed fix
#[inline]
fn native_group<T>(self, group_index: usize) -> &'a T {
let payload = self.groups[group_index];
- debug_assert!(!payload.is_null());
+ assert!(!payload.is_null(), "style group {group_index} has no payload");🤖 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/css/computed_value_views.rs` around lines 482 -
489, Update native_group to use a release-enabled assert! for the payload
non-null check instead of debug_assert!, preserving the existing unsafe cast
while ensuring null group payloads fail before creating a reference.
Comment on lines
+1585
to
+1586
| resolve_margin_or_auto(style.margin_top(), containing_block_block_size), | ||
| resolve_margin_or_auto(style.margin_bottom(), containing_block_block_size), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare with the C++ replaced-element block-axis margin resolution.
rg -nP -C10 'margin_top|margin_bottom' Libraries/LibWeb/Layout/AbsoluteFormattingContext.cpp 2>/dev/null
rg -rn --iglob '*.cpp' -nP -C8 'compute_height_for_absolutely_positioned_replaced|absolutely_positioned_replaced' Libraries/LibWeb/Layout | head -80Repository: LadybirdBrowser/ladybird
Length of output: 162
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## candidate file outline/lens"
wc -l Libraries/LibWeb/Rust/src/layout/abspos_engine.rs 2>/dev/null || true
sed -n '1530,1600p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs 2>/dev/null || true
sed -n '1260,1305p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs 2>/dev/null || true
echo "## references to resolve_margin_or_auto and available_space.inline_size in Rust layout"
rg -n "resolve_margin_or_auto|containing_block_(inline|block)_size|available_space\.inline_size|containing_block_inline_size" Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
echo "## C++ absolute layout search"
git ls-files 'Libraries/LibWeb/Layout/*' | rg '(AbsoluteFormattingContext|Absolute|abs(?:olute|olutely)|abspos)' | head -80
rg -n -C8 'margin_top|margin_bottom|percentage.*margin|inline_size|block_size' Libraries/LibWeb/Layout/AbsoluteFormattingContext.cpp 2>/dev/null || true
rg -n -C8 'compute_height_for_absolutely_positioned_replaced|absolutely_positioned_replaced|replaced' Libraries/LibWeb/Layout/AbsoluteFormattingContext.cpp 2>/dev/null || trueRepository: LadybirdBrowser/ladybird
Length of output: 9118
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## repository file names for abspos implementation"
git ls-files | rg '(^|/)(AbsoluteFormattingContext|abspos_engine|Absolute.*Layout|.*Layout.*Absolute).*' || true
echo "## broad C++ source search for abspos/replaced axis margin resolution"
rg -n -C6 'margin_top|margin_bottom|margin-left|margin-right|percentage.*margin|absolute|replaced|solve.*replaced|replaced.*axis' Libraries/LibWeb -g '*.cpp' -g '*.h' -g '*.hpp' | head -240
echo "## targeted CSS text in repo for margin percentages"
rg -n -C3 'margin.*percentage|percentage margin|inline size containing block|margin block' Libraries/LibWeb/Layout -g '*.cpp' -g '*.h' -g '*.hpp' | head -160
echo "## read-only parser behavior probe for percentage margin conversion from source text"
python3 - <<'PY'
from pathlib import Path
p = Path('Libraries/LibWeb/Rust/src/layout/abspos_engine.rs')
s = p.read_text()
def read_block(start, end):
return s[s.find("fn " + start):s.find("fn " + start, s.find("fn " + start)+1), s.find("fn " + start):s.find("fn " + start, s.find("fn " + start)+1)]
PY
sed -n '820,865p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
sed -n '1520,1592p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
sed -n '1280,1300p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rsRepository: LadybirdBrowser/ladybird
Length of output: 25784
🌐 Web query:
CSS Positioned Layout Module percentage margins containing block inline size margin-top margin-bottom
💡 Result:
In CSS, percentage values for margin properties (including margin-top, margin-right, margin-bottom, and margin-left) are calculated with respect to the inline size of the containing block [1][2][3]. The inline size typically corresponds to the width of the containing block in horizontal writing modes [1][2]. This behavior is consistent across all four sides of a box, meaning that even vertical margins (margin-top and margin-bottom) are computed based on the width (inline size) of the containing block, rather than its height (block size) [4][5][2]. This design choice—basing all percentage margins on the inline size—prevents circular dependencies during layout calculation [5][2]. Since the inline size of a block is generally determined by its parent's width, it remains stable regardless of the content's vertical height, allowing browsers to calculate layout without needing to resolve the element's height first [5][2]. For absolutely positioned elements, the containing block is defined by the nearest positioned ancestor (or the initial containing block for fixed positioning) [1][6]. The percentages for their margins, like static or relative elements, continue to be resolved against the inline size of that designated containing block [1][2]. While CSS 2.1 documentation often referred explicitly to the "width" of the containing block [4][6], modern specifications use the term "inline size" to correctly account for different writing modes (e.g., in vertical writing modes, the inline size is the height of the containing block) [2][3].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/CSS/Guides/Display/Containing_block
- 2: https://stackoverflow.com/questions/11003911/why-are-margin-padding-percentages-in-css-always-calculated-against-width
- 3: https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Properties/margin
- 4: https://www.w3.org/TR/CSS22/box.html
- 5: https://stackoverflow.com/questions/34706180/how-is-the-margin-top-percentage-calculated
- 6: https://www.w3.org/TR/CSS22/visudet.html
Resolve replaced-block vertical margins against the inline size.
margin_top and margin_bottom must use percentage baselines from the containing block inline size. These calls use containing_block_block_size, so pass available_space.inline_size.to_px_or_zero() here. If this is an intentional compatibility deviation, document it.
🤖 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 1585 - 1586,
Update the vertical margin resolution calls in the absolute-positioning layout
flow to pass the containing block’s inline-size baseline, using
available_space.inline_size.to_px_or_zero(), for both style.margin_top() and
style.margin_bottom() instead of containing_block_block_size; document the
deviation only if retaining the current behavior intentionally.
Comment on lines
+110
to
117
| pub(crate) fn to_px(self, reference: CssPixels) -> CssPixels { | ||
| match self { | ||
| Self::FromStyle(value) => value.to_px(reference), | ||
| Self::BareAnchor(wrapper) => resolve_calc_to_px(std::ptr::from_ref(wrapper).cast(), reference), | ||
| Self::Resolved(resolved) if resolved.is_auto => CssPixels::default(), | ||
| Self::Resolved(resolved) => resolved.px, | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect how rust_calc_resolve handles anchor() nodes without external resolutions.
rg -nP -C20 'fn\s+rust_calc_resolve' Libraries/LibWeb/Rust/src/css/calc.rs
rg -nP -C10 'NonMathFunction' Libraries/LibWeb/Rust/src/css/calc.rs | head -100
rg -nP -C6 'resolve_calc_to_px|resolve_calc_with_external_resolutions' Libraries/LibWeb/Rust/srcRepository: LadybirdBrowser/ladybird
Length of output: 14093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== style_facts InsetValue =="
sed -n '70,150p' Libraries/LibWeb/Rust/src/layout/style_facts.rs
sed -n '251,305p' Libraries/LibWeb/Rust/src/layout/style_facts.rs
echo "== abspos_engine relevant insets and early returns =="
sed -n '560,635p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
sed -n '1140,1215p' Libraries/LibWeb/Rust/src/layout/abspos_engine.rs
echo "== resolve_calc_with_external_resolutions implementation =="
sed -n '251,285p' Libraries/LibWeb/Rust/src/layout/style_facts.rs
echo "== rust_calc_resolve implementation sections =="
sed -n '3079,3165p' Libraries/LibWeb/Rust/src/css/calc.rs
sed -n '3165,3260p' Libraries/LibWeb/Rust/src/css/calc.rs
echo "== external resolution helper usages =="
rg -n "FfiCalcExternalResolution|external_resolutions|external_resolution_count|rust_calc_resolve\\(" Libraries/LibWeb/Rust/src/css Libraries/LibWeb/Rust/src/layout -g '*.rs'Repository: LadybirdBrowser/ladybird
Length of output: 24878
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== computed_value_views px context =="
sed -n '35,58p' Libraries/LibWeb/Rust/src/css/computed_value_views.rs
echo "== px calculation resolution context implementation =="
rg -n "fn px_calc_resolution_context|struct FfiCalcResolutionContext|impl FfiCalcResolutionContext|external_resolutions.*ptr::null|external_resolution_count.*0" Libraries/LibWeb/Rust/src/css Libraries/LibWeb/Rust/src/layout -g '*.rs'
echo "== calc evaluation external resolution resolution =="
sed -n '2450,2785p' Libraries/LibWeb/Rust/src/css/calc.rs
echo "== resolve_simplified_calculation around NonMathFunction =="
rg -n -C20 "NonMathFunction|non_math_function|external_resolutions" Libraries/LibWeb/Rust/src/css/calc.rsRepository: LadybirdBrowser/ladybird
Length of output: 50380
Resolve bare anchor insets before calling to_px.
InsetValue::to_px() passes the bare anchor() non-math function to resolve_calc_to_px(), and that call has no external resolution callback. It returns an unresolved calculation, so the result.resolved assertion can panic when resolve_anchor_insets() returns early. Use resolve_calc_with_external_resolutions() with the anchor resolver, or handle the unresolved result before the px conversion.
🤖 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/style_facts.rs` around lines 110 - 117,
Update InsetValue::to_px so Self::BareAnchor resolves the anchor() calculation
through resolve_calc_with_external_resolutions using the appropriate anchor
resolver before converting to pixels. Ensure unresolved results are handled
safely rather than reaching the resolved-value assertion when
resolve_anchor_insets() returns early.
kalenikaliaksandr
merged commit Aug 1, 2026
5aa0645
into
LadybirdBrowser:master
14 of 15 checks passed
This was referenced Aug 7, 2026
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.
The Rust layout engine still read every style property by eagerly re-encoding the Rust-native computed value payloads into FfiSizeValue — the flat wire format left over from when style reads crossed an FFI callback boundary into C++ — which dragged along a field-selector enum, a central direct_size() dispatcher, and per-read decode work that the C++ engine never needed, because CSS::Size implements its methods directly on the stored representation. This series gives the Rust side the same shape: the stored computed types (ComputedSize, ComputedLengthPercentageOrAuto, ComputedGap, and a borrowed LengthPercentageRef) carry the lazy CSS::Size-style method surface, style accessors become plain payload projections generated from a table, and the one genuinely stateful read — anchor-positioned insets — is modeled explicitly as a typed InsetValue backed by the per-pass store instead of being masked inside the decode. The end state is a ComputedValuesView living beside the computed-value types (the Rust twin of the ComputedValues.h accessor layer), with layout's StyleValues reduced to that view plus pass state, and the entire FfiSizeValue/SizeField/decode machinery deleted.