Make derived baselines an output of formatting context runs - #11050
Merged
kalenikaliaksandr merged 1 commit intoAug 9, 2026
Merged
Conversation
Baselines derived by a formatting context run were stored only on the run root's box record, and consumers read them back out of it after the run returned: line building for vertical-align on atomic inlines, flex baseline alignment, table row sizing, and measure_cell, which re-derived what the cell's run had already computed. ChildLayoutResult now carries the derived first and last baselines, and those consumers read the result of the run they just invoked; boxes whose run was skipped or that are laid out by the enclosing run keep the stored-value path. The box_baseline helpers take the box's record as a parameter, so which box's record is read is visible at the call site. A child's baselines are computed by its own run, so the result is their natural channel. This is groundwork for making a completed run's boxes private to that run: a run's result becomes a self-contained description of its subtree, which caching a run also requires, since on a cache hit the child's interior records never exist.
📝 WalkthroughWalkthroughThe layout engine now propagates ChangesDerived baseline propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FormattingContext
participant InlineFormattingContext
participant LineBuilder
participant FlexFormattingContext
participant TableFormattingContext
FormattingContext->>InlineFormattingContext: request child layout
InlineFormattingContext->>LineBuilder: pass DerivedBaselines
LineBuilder->>FormattingContext: use stored baselines
FormattingContext->>FlexFormattingContext: provide child layout baselines
FlexFormattingContext->>FormattingContext: compute baseline alignment
FormattingContext->>TableFormattingContext: provide measured or committed baselines
TableFormattingContext->>FormattingContext: resolve cell baseline
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
🤖 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/replaced_with_children_formatting_context.rs`:
- Around line 69-73: Update layout_replaced_with_children and the
ReplacedWithChildren path in run_formatting_context to derive and store the
wrapper child’s baselines instead of returning DerivedBaselines::default().
Ensure the derived baselines are forwarded or explicitly passed to
store_derived_baselines so input elements with block shadow-tree children retain
the wrapper baseline.
🪄 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: 83f8082b-4363-4151-abde-27a40ba55099
📒 Files selected for processing (10)
Libraries/LibWeb/Rust/src/layout/flex_formatting_context.rsLibraries/LibWeb/Rust/src/layout/formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_formatting_context.rsLibraries/LibWeb/Rust/src/layout/inline_level_iterator.rsLibraries/LibWeb/Rust/src/layout/line_box_fragment.rsLibraries/LibWeb/Rust/src/layout/line_builder.rsLibraries/LibWeb/Rust/src/layout/replaced_with_children_formatting_context.rsLibraries/LibWeb/Rust/src/layout/sizing_context.rsLibraries/LibWeb/Rust/src/layout/table_formatting_context.rsLibraries/LibWeb/Rust/src/layout/used_values.rs
Comment on lines
69
to
73
| ChildLayoutResult { | ||
| automatic_content_inline_size: content_inline_size, | ||
| automatic_content_block_size: wrapper_layout.automatic_content_block_size, | ||
| baselines: DerivedBaselines::default(), | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Trace whether ReplacedWithChildren roots ever get derived baselines stored
# through a path other than layout_replaced_with_children.
set -euo pipefail
rg -n 'ReplacedWithChildren' Libraries/LibWeb/Rust/src/layout -g '*.rs' -C5
echo '--- is_html_input_element / input_derives_from_children usage ---'
rg -n 'is_html_input_element|input_derives_from_children' Libraries/LibWeb/Rust/src/layout -g '*.rs' -C5
echo '--- finalize_block_level_root definition ---'
rg -n 'fn finalize_block_level_root' Libraries/LibWeb/Rust/src/layout -g '*.rs' -A40
echo '--- store_derived_baselines call sites ---'
rg -n 'store_derived_baselines\(' Libraries/LibWeb/Rust/src/layout -g '*.rs' -B3 -A1Repository: LadybirdBrowser/ladybird
Length of output: 19900
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- formatting_context.rs baseline functions ---'
sed -n '540,610p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs
echo '--- run_formatting_context relevant section ---'
sed -n '1500,1630p' Libraries/LibWeb/Rust/src/layout/formatting_context.rs
echo '--- replaced_with_children_formatting_context.rs ---'
sed -n '1,130p' Libraries/LibWeb/Rust/src/layout/replaced_with_children_formatting_context.rs
echo '--- all DerivedBaselines default/store/receive occurrences ---'
rg -n 'DerivedBaselines::default|derive_baselines|derived_baselines_of_root_box|content_baselines_from_cells|has_first_baseline|first_baseline|baselines:' Libraries/LibWeb/Rust/src/layout -g '*.rs' -C2Repository: LadybirdBrowser/ladybird
Length of output: 50380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("Libraries/LibWeb/Rust/src/layout")
def text(path):
return path.read_text()
ctx = text(root / "formatting_context.rs")
repr = text(root / "used_values.rs")
bfc = text(root / "block_formatting_context.rs")
replaced = text(root / "replaced_with_children_formatting_context.rs")
inline = text(root / "inline_formatting_context.rs")
flex = text(root / "flex_formatting_context.rs")
grid = text(root / "grid_formatting_context.rs")
table = text(root / "table_formatting_context.rs")
for name, pattern, file in [
("derive_baselines", r"pub\(crate\) fn derive_baselines", root/"formatting_context.rs"),
("FinalizeAtomicRootBlock", r"fn finalize_atomic_root_block_size", root/"formatting_context.rs"),
("compute_and_store_baselines Call", r"self\.compute_and_store_baselines\s*\(", root/"block_formatting_context.rs"),
("DeriveBaselines Call", r"crate::layout::derive_baselines\s*\(", file),
]:
idx = file.find(pattern)
print(f"--- {name}: {file} ---")
if idx == -1:
print("not found")
continue
start = file.rfind("\n", 0, max(0, idx - 1000)) + 1
end = min(idx + 2500, len(file))
print(file[start:end])
# Read-only invariants focused on the baseline storage data flow.
print("--- invariants ---")
# In run_formatting_context ReplacedWithChildren branch, the result is returned directly and
# the only post-run block-level finalize is finalize_block_level_root().
rf = ctx.find("FormattingContextImplementation::ReplacedWithChildren => layout_replaced_with_children(run, body_input),")
final = ctx.find("fn finalize_block_level_root(", rf)
print(f"run_formatting_context ReplacedWithChildren returns before finalize_block_level_root after child layout: {0 < rf < final}")
# Locate finalize_block_level_root and check it does not call store_derived_baselines.
start = ctx.find("fn finalize_block_level_root(")
end = ctx.find("\nfn size_skipped_independent_root", start)
frag = ctx[start:end]
print(f"finalize_block_level_root calls store_derived_baselines: {'store_derived_baselines(' in frag}")
print(f"finalize_block_level_root records_replaced_block_size: {'record_replaced_block_size(' in frag}")
# Locate derive_baselines and check ReplacedWithChildren children are never treated as derived baselines.
start = ctx.find("// pub(crate) fn derive_baselines", None if "pub(crate) fn derive_baselines" not in ctx else None)
start = ctx.find("pub(crate) fn derive_baselines")
end = ctx.find("pub(crate) struct ChildLayoutResult", start)
ds = ctx[start:end]
print(f"derive_baselines handles child formatting-context boxes: {'is_flex_container' in ds or 'GridFormattingContext' in ds or 'TableFormattingContext' in ds or 'block_container' in ds}")
print(f"derivation first baseline source line count around child-derived baselines:")
for m in re.finditer(r"(has_first_baseline|first_baseline|children_are_inline|BlockFormattingContext)", ds):
line = ds[:m.start()].count("\n") + 1
print(f" line {line}: {m.group()}")
PYRepository: LadybirdBrowser/ladybird
Length of output: 274
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
root = Path("Libraries/LibWeb/Rust/src/layout")
def text(path):
return path.read_text()
ctx = text(root / "formatting_context.rs")
repr = text(root / "used_values.rs")
bfc = text(root / "block_formatting_context.rs")
replaced = text(root / "replaced_with_children_formatting_context.rs")
inline = text(root / "inline_formatting_context.rs")
flex = text(root / "flex_formatting_context.rs")
grid = text(root / "grid_formatting_context.rs")
table = text(root / "table_formatting_context.rs")
fragments = [
("derive_baselines", root / "formatting_context.rs"),
("finalize_atomic_root_block_size", root / "formatting_context.rs"),
("compute_and_store_baselines in block FC", root / "block_formatting_context.rs"),
("derive_baselines calls", root / "block_formatting_context.rs"),
("box_baseline_with_content_baselines", root / "formatting_context.rs"),
("input_derives_from_children handling", root / "formatting_context.rs"),
("derive_baselines implementation", None),
]
for (name, file) in fragments:
print(f"--- {name}: {file or 'formatting_context.rs'} ---")
target = file.read_text() if file else ctx
if name == "derive_baselines implementation":
start = target.find("pub(crate) fn derive_baselines")
end = target.find("pub(crate) struct ChildLayoutResult", start)
else:
start = target.find("pub(crate) fn " + name)
if start == -1:
start = target.find("fn " + name)
if start == -1:
print("not found")
continue
if name == "derive_baselines implementation":
end = target.find("pub(crate) struct ChildLayoutResult", start)
else:
next_new = target.find("\nfn ", start + 1)
end = target.find("\n}", target.rfind("{", start, next_new if next_new != -1 else len(target))) + 2 if next_new != -1 else len(target)
if target.find("\n)", end) != -1:
end = target.find("\n)", end) + 2
end = min(end, target.find("\n}", end) + 2)
print(target[start:end])
print("--- invariants ---")
rf = ctx.find("FormattingContextImplementation::ReplacedWithChildren => layout_replaced_with_children(run, body_input),")
final_start = ctx.find("fn finalize_block_level_root(", rf)
print(f"run_formatting_context ReplacedWithChildren branch exists: {rf != -1}")
print(f"finalize_block_level_root exists after ReplacedWithChildren case: {final_start != -1}")
print(f"finalize_block_level_root contains store_derived_baselines: {'store_derived_baselines(' in ctx[final_start:ctx.find('fn size_skipped_independent_root', final_start)]}")
print(f"finalize_block_level_root contains record_replaced_block_size: {'record_replaced_block_size(' in ctx[final_start:ctx.find('fn size_skipped_independent_root', final_start)]}")
ds_start = ctx.find("pub(crate) fn derive_baselines")
ds_end = ctx.find("pub(crate) struct ChildLayoutResult", ds_start)
ds = ctx[ds_start:ds_end]
print(f"derive_baselines handles block container children: {'child_state.content_baselines_from_cells' in ds}")
print(f"derive_baselines handles flex container children: {'fctx.derived_baselines_of_root_box' in ds}")
print(f"derive_baselines handles grid container children: {'gctx.derived_baselines_of_root_box' in ds}")
print(f"derive_baselines handles table formatting context: {'TableFormattingContext' in ds}")
stored_calls = []
for p in [root / "block_formatting_context.rs", root / "inline_formatting_context.rs", root / "flex_formatting_context.rs", root / "grid_formatting_context.rs", root / "table_formatting_context.rs"]:
pc = p.read_text()
if "crate::layout::store_derived_baselines(" in pc:
for m in re.finditer(r"crate::layout::store_derived_baselines\s*\(", pc):
line = pc[:m.start()].count("\n") + 1
stored_calls.append((p, line, pc[pc.rfind("\n", 0, m.start()-1):pc.find("\n", m.start()) + 1].strip()))
print("store_derived_baselines call sites:")
for p,line,snippet in stored_calls:
print(f" {p}:{line} {snippet}")
PYRepository: LadybirdBrowser/ladybird
Length of output: 50380
Store baselines for ReplacedWithChildren roots.
layout_replaced_with_children always returns DerivedBaselines::default(), and run_formatting_context does not call store_derived_baselines for ReplacedWithChildren roots. Derive and forward the wrapper baseline result from layout_replaced_with_children, or explicitly derive/store baselines from the wrapper child instead of relying on finalize_block_level_root, so <input> elements with block shadow-tree children do not collapse to the margin-box bottom edge.
🤖 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/replaced_with_children_formatting_context.rs`
around lines 69 - 73, Update layout_replaced_with_children and the
ReplacedWithChildren path in run_formatting_context to derive and store the
wrapper child’s baselines instead of returning DerivedBaselines::default().
Ensure the derived baselines are forwarded or explicitly passed to
store_derived_baselines so input elements with block shadow-tree children retain
the wrapper baseline.
kalenikaliaksandr
merged commit Aug 9, 2026
825fe44
into
LadybirdBrowser:master
13 of 15 checks passed
This was referenced Aug 9, 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.
Baselines derived by a formatting context run were stored only on the run root's box record, and consumers read them back out of it after the run returned: line building for vertical-align on atomic inlines, flex baseline alignment, table row sizing, and measure_cell, which re-derived what the cell's run had already computed.
ChildLayoutResult now carries the derived first and last baselines, and those consumers read the result of the run they just invoked; boxes whose run was skipped or that are laid out by the enclosing run keep the stored-value path. The box_baseline helpers take the box's record as a parameter, so which box's record is read is visible at the call site.
A child's baselines are computed by its own run, so the result is their natural channel. This is groundwork for making a completed run's boxes private to that run: a run's result becomes a self-contained description of its subtree, which caching a run also requires, since on a cache hit the child's interior records never exist.