From 067ad3a82237d582c67c708a8ac001bb5e69eee6 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 22 Sep 2026 10:46:10 +0200 Subject: [PATCH] fix(components): resolve default sizes against the node font-size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit font_size_px_or goes through Length::px() → px_or_warn, which returns 0.0 for any relative unit; style.rs:1601 asserts this explicitly (assert_eq!(s.font_size_px_or(48.0), 0.0) for "15.6vw"). Five live call sites remain in apply_intrinsic_overrides — List (1498), Callout (1643), Tooltip (1663), PillNav (1688), Marquee (1716) — while every corresponding painter was migrated to the context-aware resolver in the "lot B, wave S" pass (marquee.rs:82 self.style.font_size_px_ctx(&crate::intrinsic::font_size_ctx(...), self.font_size), list.rs:88 the same). So a marquee with "font-size": "2rem" and no explicit style.height gets apply_default_size(css, 800.0, 0.0) → the (false,false) branch writes css.height = Px(0.0) → the node is skipped at paint time and the marquee is invisible; list (1498-1503) collapses to height 0 the same way; callout/tooltip/pill_nav get boxes measured at font- size 0 while the painter draws at the real size, so the ink overflows its own box. intrinsic.rs:27-46 states this wave's goal was to replace font_size_px_or at every site; box_builder was missed, and it is also the one file that does not use the module's own measure_time_font_size_ctx helper (intrinsic.rs:87). Fix: Replace the five font_size_px_or(...) calls in apply_intrinsic_overrides with font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), ...), the same helper every *Intrinsic in intrinsic.rs already uses. Add a regression test asserting a marquee/list with "font-size": "2rem" and no explicit height lays out with positive height. Refs #220 --- .../rustmotion-components/src/box_builder.rs | 22 ++++-- .../rustmotion-components/tests/audit_ws_i.rs | 67 +++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 crates/rustmotion-components/tests/audit_ws_i.rs diff --git a/crates/rustmotion-components/src/box_builder.rs b/crates/rustmotion-components/src/box_builder.rs index 21d151ff..dee9b4bb 100644 --- a/crates/rustmotion-components/src/box_builder.rs +++ b/crates/rustmotion-components/src/box_builder.rs @@ -1495,7 +1495,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { css.width = Some(CSize::Length(CLP::Px(c.width))); } if css.height.is_none() { - let font_size = c.style.font_size_px_or(16.0); + let font_size = c + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 16.0); let line_height = font_size * 1.3; let n = c.items.len() as f32; let h = n * line_height + (n - 1.0).max(0.0) * c.gap; @@ -1640,7 +1642,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // box is fit exactly to the unwrapped text width, the painter's // own `wrap_text(text, font, Some(text_area_w))` never has a // reason to wrap, so painted output matches this box exactly. - let font_size = t.style.font_size_px_or(16.0); + let font_size = t + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 16.0); let family = t.style.font_family_or("Inter"); let text_w = measure_text_line_width(&t.text, font_size, family, false); let h_pad = 12.0; // callout.rs's own `let padding = 12.0;` @@ -1660,7 +1664,10 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // Same shape as Callout above; padding value borrowed from // callout.rs since tooltip.rs's own paint() centers text in the // body with no defined constant of its own. - let font_size = t.style.font_size_px_or(t.font_size); + let font_size = t.style.font_size_px_ctx( + &crate::intrinsic::measure_time_font_size_ctx(0.0), + t.font_size, + ); let family = t.style.font_family_or("Inter"); let text_w = measure_text_line_width(&t.text, font_size, family, false); let h_pad = 12.0; @@ -1685,7 +1692,9 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // formula (h_pad = font_size*1.2 per side, `gap` before/after/ // between every pill) using the same public fields and the same // `measure_text_with_fallback` call it makes internally. - let font_size = p.style.font_size_px_or(14.0); + let font_size = p + .style + .font_size_px_ctx(&crate::intrinsic::measure_time_font_size_ctx(0.0), 14.0); let family = p.style.font_family_or("Inter"); let h_pad = font_size * 1.2; let n = p.items.len() as f32; @@ -1713,7 +1722,10 @@ fn apply_intrinsic_overrides(component: &Component, css: &mut CssStyle) { // height ratio both of those same real usages share: // `font_size: 24` paired with `style.height: 48`, i.e. // `2 × font_size`. - let font_size = m.style.font_size_px_or(m.font_size); + let font_size = m.style.font_size_px_ctx( + &crate::intrinsic::measure_time_font_size_ctx(0.0), + m.font_size, + ); apply_default_size(css, 800.0, font_size * 2.0); } Stepper(s) => { diff --git a/crates/rustmotion-components/tests/audit_ws_i.rs b/crates/rustmotion-components/tests/audit_ws_i.rs new file mode 100644 index 00000000..1915ed02 --- /dev/null +++ b/crates/rustmotion-components/tests/audit_ws_i.rs @@ -0,0 +1,67 @@ +//! Regression tests for workstream I of the 2026-09 audit (components and +//! the CSS cascade). +//! +//! - `apply_intrinsic_overrides`'s default-size branches resolved +//! `font-size` through the context-free `font_size_px_or`, which returns +//! `0.0` for a relative unit (`rem`/`vw`/`vh`) instead of resolving it — +//! collapsing `marquee`/`list`/`callout`/`tooltip`/`pill_nav` to a 0px box. +//! - `cascade::inherit_from` computes inherited `color`/`font-*` but +//! nothing on the render path ever reads the result — every painter reads +//! its own component's un-cascaded `style` field instead. +//! - `TableIntrinsic` measures content-fitted per-column widths, but +//! the painter splits the box evenly across columns regardless. + +use rustmotion_components::box_builder::{build_scene_with_anim, BuildAnimationCtx}; +use rustmotion_components::{ChildComponent, Component, PositionMode}; +use rustmotion_core::css::taffy_bridge::ConversionContext; +use rustmotion_core::engine::layout_pass::run_layout; + +fn single_child_scene(json: serde_json::Value) -> ChildComponent { + let component: Component = serde_json::from_value(json).expect("deserialize component"); + ChildComponent { + component, + position: Some(PositionMode::Absolute { x: 0.0, y: 0.0 }), + x: None, + y: None, + z_index: None, + bleed: false, + } +} + +// ─── default sizes must resolve against the node's font-size ─────────────── + +#[test] +fn marquee_with_relative_font_size_gets_a_positive_height() { + // Reproduction: `font_size_px_or` returns 0.0 for any relative unit + // (`.px()` can't resolve `%`/`em`/`rem`/`vw`/`vh`). Five sites in + // `apply_intrinsic_overrides` still called it — Marquee among them — + // so a marquee declaring `"font-size": "2rem"` and no explicit height + // used to collapse `apply_default_size(css, 800.0, 0.0)` to a 0px-tall + // box, and `paint_pass`'s `height <= 0.0` guard then skipped it + // entirely: the marquee never appeared on screen. + let child = single_child_scene(serde_json::json!({ + "type": "marquee", + "content": "BREAKING NEWS", + "style": { "font-size": "2rem" } + })); + let children = vec![child]; + + let built = build_scene_with_anim( + &children, + (1920.0, 1080.0), + BuildAnimationCtx { + time: 0.0, + scenario_time: 0.0, + scene_duration: 1.0, + fps: 30, + }, + ); + let layout = run_layout(&built.root, (1920.0, 1080.0), &ConversionContext::default()); + let marquee_id = built.root.children[0].id; + let l = layout.get(marquee_id).expect("marquee laid out"); + assert!( + l.height > 0.0, + "marquee at font-size: 2rem must lay out with a positive height, got {}", + l.height + ); +}