From 39c771f75ee55406ee75eb3fc60f28d48eb28898 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 8 Sep 2026 14:24:12 +0800 Subject: [PATCH] fix(layout): resolve empty inline fragment box geometry once --- moli-layout/src/inline.rs | 129 ++++++++++++++++-- moli-layout/src/layout_tree/model.rs | 19 +++ moli-layout/src/overflow.rs | 2 +- moli-layout/src/paint/inline_boxes.rs | 58 ++------ moli-layout/src/projection.rs | 124 +---------------- moli-layout/src/taffy_tree.rs | 11 +- moli-renderer-v8/src/runtime/phase_one/mod.rs | 44 ++++++ .../fixtures/empty-inline-fragments.html | 55 ++++++++ 8 files changed, 257 insertions(+), 185 deletions(-) create mode 100644 moli-renderer-v8/tests/fixtures/empty-inline-fragments.html diff --git a/moli-layout/src/inline.rs b/moli-layout/src/inline.rs index 9e17c7fcdb..d4166e611b 100644 --- a/moli-layout/src/inline.rs +++ b/moli-layout/src/inline.rs @@ -15,10 +15,12 @@ use std::{ }; use parley::{BreakReason, InlineBox, InlineBoxKind, Layout, PositionedLayoutItem, TextStyle}; -use taffy::{MaybeResolve as _, Point, Size}; +use taffy::{MaybeResolve as _, Point, ResolveOrZero as _, Size}; use crate::{ - LayoutBoxId, LayoutBoxKind, LayoutWorld, PaintColor, PaintRect, + LayoutBox, LayoutBoxId, LayoutBoxKind, LayoutFragmentBoxModel, LayoutWorld, PaintColor, + PaintRect, ResolvedLayoutStyle, + overflow::{inset_rect, outset_rect}, style::{ InlineDirection, InlineTextTransform, InlineUnicodeBidi, InlineVerticalAlign, InlineWhiteSpaceCollapse, LayoutInlineAlignment, @@ -379,6 +381,7 @@ pub(crate) struct InlineLineFragment { /// CSSOM line geometry continues to use `rect`. pub(crate) paint_bounds: InlinePaintBounds, pub(crate) baseline: f32, + pub(crate) phantom: bool, } #[derive(Clone, Copy, Debug, Default, PartialEq)] @@ -413,15 +416,19 @@ pub(crate) struct InlineSourceFragment { pub(crate) struct InlineBoxFragment { pub(crate) line_index: usize, pub(crate) box_id: LayoutBoxId, - pub(crate) rect: PaintRect, + /// Resolved once during final line layout, in the IFC's content space. + /// Paint, CSSOM, overflow and positioned layout consume the same boxes. + pub(crate) box_model: LayoutFragmentBoxModel, pub(crate) has_start_edge: bool, pub(crate) has_end_edge: bool, } -pub(crate) fn build_inline_fragments( +pub(crate) fn build_inline_fragments( context: &InlineFormattingContext, layout: &Layout, line_placements: &[InlineLinePlacement], + boxes: &[LayoutBox], + containing_width: f32, ) -> InlineFragments { // Binary overlap lookup relies on both endpoints being monotonic. Validate // each immutable normalization product once, rather than rescanning the @@ -463,6 +470,7 @@ pub(crate) fn build_inline_fragments( InlinePaintBounds::Bounded(line_rect) }), baseline: placement.map_or(metrics.baseline, |placement| placement.baseline), + phantom: placement.is_some_and(|placement| placement.phantom), }); if let Some(placement) = placement { for box_placement in &placement.box_block_placements { @@ -597,11 +605,15 @@ pub(crate) fn build_inline_fragments( fragments.boxes = box_fragments .into_iter() .filter_map(|((box_index, line_index), accumulator)| { - let line_rect = fragments.lines.get(line_index)?.rect; + let line = fragments.lines.get(line_index)?; Some(InlineBoxFragment { line_index, box_id: LayoutBoxId::from_index(box_index), - rect: accumulator.rect(line_rect)?, + box_model: accumulator.box_model( + &boxes[box_index].style, + line, + containing_width, + )?, has_start_edge: accumulator.has_start_edge, has_end_edge: accumulator.has_end_edge, }) @@ -949,10 +961,13 @@ fn resolve_inline_lines( .map_or(fallback_root_bounds, InlineVerticalBounds::from_strut) }); for state in &mut states { - state.metrics = (!phantom) - .then_some(state.strut) - .flatten() - .map(InlineVerticalBounds::from_strut); + state.metrics = if phantom { + // Empty inline fragments still participate in vertical-align, + // but their font struts must not create block-axis geometry. + Some(InlineVerticalBounds::ZERO) + } else { + state.strut.map(InlineVerticalBounds::from_strut) + }; } // One pending list per structural target plus one for the root line @@ -1099,12 +1114,20 @@ fn resolve_inline_lines( let box_block_placements = states .iter() .filter_map(|state| { - let strut = state.strut?; let baseline = root_baseline + state.global_offset; + let (top, height) = if phantom { + (baseline, 0.0) + } else { + let strut = state.strut?; + ( + baseline - strut.text_ascent, + (strut.text_ascent + strut.text_descent).max(0.0), + ) + }; Some(InlineBoxBlockPlacement { box_id: state.box_id, - top: baseline - strut.text_ascent, - height: (strut.text_ascent + strut.text_descent).max(0.0), + top, + height, }) }) .collect(); @@ -1461,6 +1484,86 @@ struct FragmentAccumulator { } impl FragmentAccumulator { + fn box_model( + self, + style: &ResolvedLayoutStyle, + line: &InlineLineFragment, + containing_width: f32, + ) -> Option { + let rect = self.rect(line.rect)?; + let resolve = crate::style::resolve_stylo_calc_value; + let padding = style + .taffy + .padding + .resolve_or_zero(Some(containing_width), resolve); + let border = style + .taffy + .border + .resolve_or_zero(Some(containing_width), resolve); + let margin = style + .taffy + .margin + .resolve_or_zero(Some(containing_width), resolve); + let (has_left_edge, has_right_edge) = if style.direction() == InlineDirection::Ltr { + (self.has_start_edge, self.has_end_edge) + } else { + (self.has_end_edge, self.has_start_edge) + }; + let left_margin = if has_left_edge { + margin.left.max(0.0) + } else { + 0.0 + }; + let right_margin = if has_right_edge { + margin.right.max(0.0) + } else { + 0.0 + }; + // Blink's AddBoxFragmentPlaceholder gives an empty line's inline box + // zero block offset/size, including when it has block-axis padding or + // borders. Only an actual line gets the font box and those decorations. + let (top, height) = if line.phantom { + (rect.y, 0.0) + } else { + ( + rect.y - padding.top - border.top, + rect.height + padding.top + padding.bottom + border.top + border.bottom, + ) + }; + let border_box = PaintRect::new( + rect.x + left_margin, + top, + (rect.width - left_margin - right_margin).max(0.0), + height, + ); + let padding_box = inset_rect( + border_box, + border.top, + if has_right_edge { border.right } else { 0.0 }, + border.bottom, + if has_left_edge { border.left } else { 0.0 }, + ); + let content_box = inset_rect( + padding_box, + padding.top, + if has_right_edge { padding.right } else { 0.0 }, + padding.bottom, + if has_left_edge { padding.left } else { 0.0 }, + ); + Some(LayoutFragmentBoxModel { + content: content_box, + padding: padding_box, + border: border_box, + margin: outset_rect( + border_box, + margin.top, + right_margin, + margin.bottom, + left_margin, + ), + }) + } + fn include(&mut self, rect: PaintRect) { self.include_inline_axis(rect.x, rect.width); self.min_y = Some(self.min_y.map_or(rect.y, |value| value.min(rect.y))); diff --git a/moli-layout/src/layout_tree/model.rs b/moli-layout/src/layout_tree/model.rs index fbdb0c5899..e1673699eb 100644 --- a/moli-layout/src/layout_tree/model.rs +++ b/moli-layout/src/layout_tree/model.rs @@ -410,6 +410,25 @@ pub struct LayoutFragmentBoxModel { pub margin: LayoutRect, } +impl LayoutFragmentBoxModel { + pub(crate) fn translated(self, origin: LayoutPoint) -> Self { + let translate = |rect: LayoutRect| { + LayoutRect::new( + rect.x + origin.x, + rect.y + origin.y, + rect.width, + rect.height, + ) + }; + Self { + content: translate(self.content), + padding: translate(self.padding), + border: translate(self.border), + margin: translate(self.margin), + } + } +} + /// A geometry fragment kind. IDs contained here are valid only in the same /// [`crate::FrozenLayoutTree`]. #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/moli-layout/src/overflow.rs b/moli-layout/src/overflow.rs index 9ed1897b95..700d152fed 100644 --- a/moli-layout/src/overflow.rs +++ b/moli-layout/src/overflow.rs @@ -364,7 +364,7 @@ where local_overflow = local_overflow.union(offset_rect(fragment.rect, origin)); } for fragment in &context.fragments.boxes { - local_overflow = local_overflow.union(offset_rect(fragment.rect, origin)); + local_overflow = local_overflow.union(offset_rect(fragment.box_model.border, origin)); } } OverflowBoxGeometry { diff --git a/moli-layout/src/paint/inline_boxes.rs b/moli-layout/src/paint/inline_boxes.rs index 58f6c2b41a..8073d6962f 100644 --- a/moli-layout/src/paint/inline_boxes.rs +++ b/moli-layout/src/paint/inline_boxes.rs @@ -16,7 +16,9 @@ use super::{ geometry::{BoxAreas, inset_radii}, text::TextClipMaskScope, }; -use crate::{LayoutBox, LayoutRect, LayoutWorld, PaintEdgeSizes, PaintFragment, PaintSnapshot}; +use crate::{ + LayoutBox, LayoutPoint, LayoutRect, LayoutWorld, PaintEdgeSizes, PaintFragment, PaintSnapshot, +}; pub(super) fn project_inline_box_fragments( world: &LayoutWorld, @@ -33,8 +35,10 @@ pub(super) fn project_inline_box_fragments( return; }; let owner_layout = owner.final_layout; - let origin_x = owner_layout.border.left + owner_layout.padding.left; - let origin_y = owner_layout.border.top + owner_layout.padding.top; + let origin = LayoutPoint::new( + owner_layout.border.left + owner_layout.padding.left, + owner_layout.border.top + owner_layout.padding.top, + ); let containing_width = (owner_layout.size.width - owner_layout.border.left - owner_layout.border.right @@ -58,10 +62,6 @@ pub(super) fn project_inline_box_fragments( Some(containing_width), crate::style::resolve_stylo_calc_value, ); - let margin = style.taffy.margin.resolve_or_zero( - Some(containing_width), - crate::style::resolve_stylo_calc_value, - ); let ltr = style.direction() == crate::style::InlineDirection::Ltr; let has_left_edge = if ltr { fragment.has_start_edge @@ -73,22 +73,8 @@ pub(super) fn project_inline_box_fragments( } else { fragment.has_start_edge }; - let left_margin = if has_left_edge { - margin.left.max(0.0) - } else { - 0.0 - }; - let right_margin = if has_right_edge { - margin.right.max(0.0) - } else { - 0.0 - }; - let rect = LayoutRect::new( - origin_x + fragment.rect.x + left_margin, - origin_y + fragment.rect.y - padding.top - border.top, - (fragment.rect.width - left_margin - right_margin).max(0.0), - fragment.rect.height + padding.top + padding.bottom + border.top + border.bottom, - ); + let box_model = fragment.box_model.translated(origin); + let rect = box_model.border; if rect.width <= 0.0 || rect.height <= 0.0 { continue; } @@ -109,18 +95,11 @@ pub(super) fn project_inline_box_fragments( padding.bottom, if has_left_edge { padding.left } else { 0.0 }, ); - let padding_rect = inset_rect(rect, widths); - let content_rect = inset_rect(padding_rect, padding_widths); let areas = BoxAreas { - margin_rect: LayoutRect::new( - rect.x - left_margin, - rect.y - margin.top, - (rect.width + left_margin + right_margin).max(0.0), - (rect.height + margin.top + margin.bottom).max(0.0), - ), + margin_rect: box_model.margin, border_rect: rect, - padding_rect, - content_rect, + padding_rect: box_model.padding, + content_rect: box_model.content, border_radii: radii, padding_radii: inset_radii(radii, widths), content_radii: inset_radii( @@ -170,16 +149,3 @@ pub(super) fn project_inline_box_fragments( } } } - -fn inset_rect(rect: LayoutRect, widths: PaintEdgeSizes) -> LayoutRect { - let top = widths.top.max(0.0); - let right = widths.right.max(0.0); - let bottom = widths.bottom.max(0.0); - let left = widths.left.max(0.0); - LayoutRect::new( - rect.x + left, - rect.y + top, - (rect.width - left - right).max(0.0), - (rect.height - top - bottom).max(0.0), - ) -} diff --git a/moli-layout/src/projection.rs b/moli-layout/src/projection.rs index ded04acbe5..1ca62aabdb 100644 --- a/moli-layout/src/projection.rs +++ b/moli-layout/src/projection.rs @@ -1,9 +1,7 @@ use std::{collections::HashMap, fmt::Debug, hash::Hash, time::Instant}; -use taffy::ResolveOrZero; - use crate::layout_tree::{CssSizing, CssSizingBox, LayoutCoordinateSpace}; -use crate::overflow::{OverflowProjection, inset_rect, offset_rect, outset_rect}; +use crate::overflow::{OverflowProjection, offset_rect}; use crate::stacking::{PaintOrderEvent, build_paint_order}; use crate::style::ResolvedLayoutTransform; use crate::{ @@ -710,7 +708,7 @@ where } for inline in &context.fragments.boxes { let target = inline.box_id.index(); - let box_model = inline_fragment_box_model(self.world, index, inline); + let box_model = inline.box_model.translated(content_origin); let fragment = self.push_fragment(LayoutFragment { id: LayoutFragmentId::from_index(0), kind: LayoutFragmentKind::InlineBox { @@ -1237,124 +1235,6 @@ fn finite_point(point: LayoutPoint) -> LayoutPoint { ) } -fn inline_fragment_box_model( - world: &LayoutWorld, - owner_index: usize, - fragment: &crate::inline::InlineBoxFragment, -) -> LayoutFragmentBoxModel -where - N: Copy + Debug + Eq + Hash, -{ - let owner = &world.boxes[owner_index]; - let owner_id = LayoutBoxId::from_index(owner_index); - let owner_layout = owner.final_layout; - let vertical_leading_gutter = - if owner_id == world.root || world.is_viewport_defining_body(owner_id) { - 0.0 - } else { - owner - .style - .scrollbar_leading_gutter_thickness(LayoutScrollbarAxis::Vertical, false) - }; - let horizontal_leading_gutter = - if owner_id == world.root || world.is_viewport_defining_body(owner_id) { - 0.0 - } else { - owner - .style - .scrollbar_leading_gutter_thickness(LayoutScrollbarAxis::Horizontal, false) - }; - let content_origin = LayoutPoint::new( - owner_layout.border.left + owner_layout.padding.left + vertical_leading_gutter, - owner_layout.border.top + owner_layout.padding.top + horizontal_leading_gutter, - ); - let containing_width = (owner_layout.size.width - - owner_layout.border.left - - owner_layout.border.right - - owner_layout.padding.left - - owner_layout.padding.right) - - if owner_id == world.root || world.is_viewport_defining_body(owner_id) { - 0.0 - } else { - owner - .style - .scrollbar_gutter_thickness(LayoutScrollbarAxis::Vertical) - }; - let containing_width = containing_width.max(0.0); - let inline_box = &world.boxes[fragment.box_id.index()]; - let style = &inline_box.style; - let padding = style.taffy.padding.resolve_or_zero( - Some(containing_width), - crate::style::resolve_stylo_calc_value, - ); - let border = style.taffy.border.resolve_or_zero( - Some(containing_width), - crate::style::resolve_stylo_calc_value, - ); - let margin = style.taffy.margin.resolve_or_zero( - Some(containing_width), - crate::style::resolve_stylo_calc_value, - ); - let ltr = style.direction() == crate::style::InlineDirection::Ltr; - let has_left_edge = if ltr { - fragment.has_start_edge - } else { - fragment.has_end_edge - }; - let has_right_edge = if ltr { - fragment.has_end_edge - } else { - fragment.has_start_edge - }; - let left_margin = if has_left_edge { - margin.left.max(0.0) - } else { - 0.0 - }; - let right_margin = if has_right_edge { - margin.right.max(0.0) - } else { - 0.0 - }; - let left_padding = if has_left_edge { padding.left } else { 0.0 }; - let right_padding = if has_right_edge { padding.right } else { 0.0 }; - let left_border = if has_left_edge { border.left } else { 0.0 }; - let right_border = if has_right_edge { border.right } else { 0.0 }; - let border_box = LayoutRect::new( - content_origin.x + fragment.rect.x + left_margin, - content_origin.y + fragment.rect.y - padding.top - border.top, - (fragment.rect.width - left_margin - right_margin).max(0.0), - fragment.rect.height + padding.top + padding.bottom + border.top + border.bottom, - ); - let padding_box = inset_rect( - border_box, - border.top, - right_border, - border.bottom, - left_border, - ); - let content_box = inset_rect( - padding_box, - padding.top, - right_padding, - padding.bottom, - left_padding, - ); - let margin_box = outset_rect( - border_box, - margin.top, - right_margin, - margin.bottom, - left_margin, - ); - LayoutFragmentBoxModel { - content: content_box, - padding: padding_box, - border: border_box, - margin: margin_box, - } -} - #[cfg(test)] mod paint_space_tests { use super::*; diff --git a/moli-layout/src/taffy_tree.rs b/moli-layout/src/taffy_tree.rs index 5591965242..18879a8ebf 100644 --- a/moli-layout/src/taffy_tree.rs +++ b/moli-layout/src/taffy_tree.rs @@ -1052,7 +1052,7 @@ where .boxes .iter() .filter(|fragment| fragment.box_id == containing_block) - .map(|fragment| fragment.rect) + .map(|fragment| fragment.box_model.padding) .reduce(union_paint_rect) } @@ -2009,8 +2009,13 @@ where .line_placements .as_ref() .expect("final inline layout must retain line placements"); - let fragments = - build_inline_fragments(&inline_context, text_layout, line_placements); + let fragments = build_inline_fragments( + &inline_context, + text_layout, + line_placements, + &self.boxes, + content_box_size.width, + ); self.position_inline_objects( &inline_context, text_layout, diff --git a/moli-renderer-v8/src/runtime/phase_one/mod.rs b/moli-renderer-v8/src/runtime/phase_one/mod.rs index 4ad3a19081..f2263afca8 100644 --- a/moli-renderer-v8/src/runtime/phase_one/mod.rs +++ b/moli-renderer-v8/src/runtime/phase_one/mod.rs @@ -832,6 +832,50 @@ html, body { display: block; margin: 0; padding: 0 } })); } + #[test] + fn layout_renderer_preserves_empty_inline_fragment_geometry() { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("current-thread runtime should build"); + runtime.block_on(tokio::task::LocalSet::new().run_until(async move { + let mut page = parse_phase_one_html_into_page_vm_for_test_with_env( + include_str!("../../../tests/fixtures/empty-inline-fragments.html"), + default_test_page_vm_env_config_with(|env| { + env.layout_policy = moli_page_types::LayoutPolicy::OnDemand; + }), + ) + .await; + for font_size in [16, 64, 4] { + page.vm_mut() + .eval(&format!( + "document.getElementById('dynamic-font').style.fontSize = '{font_size}px'" + )) + .expect("update the empty inline's font size"); + page.vm_mut().sync_live_document_style_sources(); + page.vm_mut() + .screenshot_layout_snapshot(moli_layout::PaintViewport::new(800, 600, 1.0)) + .expect("empty inline fragment layout should succeed") + .expect("fixture should have a document element"); + let geometry = page + .vm_mut() + .eval("JSON.stringify(collectInlineFragmentChecks())") + .expect("read explicitly published inline geometry"); + let checks: serde_json::Value = + serde_json::from_str(&geometry).expect("geometry JSON"); + let checks = checks.as_array().expect("fragment checks"); + assert_eq!(checks.len(), 37); + for check in checks { + assert_eq!( + check["actual"], check["expected"], + "{} with dynamic font size {font_size}", + check["id"], + ); + } + } + })); + } + #[test] fn layout_renderer_preserves_calc_min_width_in_float_intrinsic_contribution() { let runtime = tokio::runtime::Builder::new_current_thread() diff --git a/moli-renderer-v8/tests/fixtures/empty-inline-fragments.html b/moli-renderer-v8/tests/fixtures/empty-inline-fragments.html new file mode 100644 index 0000000000..88a77d5543 --- /dev/null +++ b/moli-renderer-v8/tests/fixtures/empty-inline-fragments.html @@ -0,0 +1,55 @@ + + +Empty-line inline fragments retain positions, not font or decoration extents + +
+
+
+
+ +
+
+
+
+
+
+
+ +
+
+
+
+
+

+
+
+
X
+
+
+
+