From 503a515e053de8b7d1f765dd35b0f1a6db600a2d Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 16:43:26 +0200 Subject: [PATCH 1/6] fix(css): parse every common colour and length form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Colours written `#fff`, `#FFF`, `white` or `rgb(255,255,255)` deserialised without error and then fell back to SColor::BLACK when painting. On the dark backgrounds this engine targets that is invisible text, and `validate` still reported "Valid scenario" — the worst possible failure mode. Only the 6-digit hex form actually worked. Add `parse_css_color` covering 3/4/6/8-digit hex, rgb/rgba, hsl/hsla and the CSS named-colour set, and route the paint pass through it. An unresolvable colour now warns on stderr and renders as opaque magenta rather than black: black is a legitimate colour choice and disappears into a dark frame, magenta never blends in. Lengths had the same shape of bug — an unparseable value silently became 0px. Add fallible `try_parse` counterparts so callers can detect it. Alpha now rounds in both notations, so `0.5` and `50%` agree on 128. The old parser truncated to 127; that was an artefact of the formula rather than a decision, and it made the two spellings of one value disagree. Refs #104 --- crates/rustmotion-core/src/css/units.rs | 89 ++- .../rustmotion-core/src/engine/paint_pass.rs | 134 ++-- .../src/engine/renderer/colors.rs | 694 +++++++++++++++++- 3 files changed, 834 insertions(+), 83 deletions(-) diff --git a/crates/rustmotion-core/src/css/units.rs b/crates/rustmotion-core/src/css/units.rs index 5398eeb..5cf9681 100644 --- a/crates/rustmotion-core/src/css/units.rs +++ b/crates/rustmotion-core/src/css/units.rs @@ -136,10 +136,28 @@ pub fn parse_length(s: &str) -> Option { } impl Length { + /// Parse into a resolved unit. Falls back to `Px(0.0)` on unparseable + /// input, for the many existing call sites across the codebase that + /// expect an infallible result — but unlike the previous behaviour, + /// this now logs a warning so the fallback is a detectable signal + /// instead of a silent, indistinguishable zero. Callers that can + /// surface the failure themselves (e.g. a validator) should prefer + /// [`Length::try_parse`], which returns `None` instead of guessing. pub fn parse(&self) -> ParsedLength { match self { Length::Px(v) => ParsedLength::Px(*v), - Length::String(s) => parse_length(s).unwrap_or(ParsedLength::Px(0.0)), + Length::String(s) => parse_length_or_warn(s), + } + } + + /// Fallible counterpart of [`Length::parse`]: `None` on unparseable + /// input rather than a silent `Px(0.0)`, and never logs. Prefer this + /// when the failure can be reported to the caller (e.g. schema + /// validation) instead of swallowed. + pub fn try_parse(&self) -> Option { + match self { + Length::Px(v) => Some(ParsedLength::Px(*v)), + Length::String(s) => parse_length(s), } } @@ -158,10 +176,19 @@ impl Length { } impl LengthPercentage { + /// See [`Length::parse`] — same infallible-with-a-warning contract. pub fn parse(&self) -> ParsedLength { match self { LengthPercentage::Px(v) => ParsedLength::Px(*v), - LengthPercentage::String(s) => parse_length(s).unwrap_or(ParsedLength::Px(0.0)), + LengthPercentage::String(s) => parse_length_or_warn(s), + } + } + + /// See [`Length::try_parse`] — fallible, silent, `None` on failure. + pub fn try_parse(&self) -> Option { + match self { + LengthPercentage::Px(v) => Some(ParsedLength::Px(*v)), + LengthPercentage::String(s) => parse_length(s), } } @@ -179,6 +206,25 @@ impl LengthPercentage { } } +/// Shared fallback used by both `Length::parse` and `LengthPercentage::parse`: +/// parse `s`, or log a warning and return `Px(0.0)` if it doesn't match any +/// known length form. This is what closes the "silently fold into 0px with +/// no signal" gap — the previous `unwrap_or(ParsedLength::Px(0.0))` produced +/// an indistinguishable, unreported zero for a typo'd unit exactly like a +/// deliberate `0px`. +fn parse_length_or_warn(s: &str) -> ParsedLength { + match parse_length(s) { + Some(p) => p, + None => { + eprintln!( + "Warning: unparseable length '{s}' — falling back to 0px instead of silently \ + resolving with no signal" + ); + ParsedLength::Px(0.0) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -268,4 +314,43 @@ mod tests { let l = Length::Px(10.0); assert_eq!(l.resolve(&LengthContext::default()), 10.0); } + + // ---- unparseable length: detectable signal, not a silent 0px ---- + + #[test] + fn length_try_parse_returns_none_for_garbage() { + let l = Length::String("not-a-length".into()); + assert_eq!(l.try_parse(), None); + } + + #[test] + fn length_percentage_try_parse_returns_none_for_garbage() { + let l = LengthPercentage::String("wat".into()); + assert_eq!(l.try_parse(), None); + } + + #[test] + fn length_try_parse_agrees_with_parse_on_valid_input() { + let l = Length::String("24px".into()); + assert_eq!(l.try_parse(), Some(ParsedLength::Px(24.0))); + assert_eq!(l.parse(), ParsedLength::Px(24.0)); + } + + #[test] + fn length_parse_still_infallible_fallback_for_garbage() { + // `.parse()` keeps its infallible signature (existing call sites + // across the codebase depend on it) — it still resolves to 0px for + // unparseable input, but `try_parse` above proves the failure is + // now independently detectable rather than indistinguishable from + // a deliberate `0px`. + let l = Length::String("not-a-length".into()); + assert_eq!(l.parse(), ParsedLength::Px(0.0)); + assert_eq!(l.resolve(&LengthContext::default()), 0.0); + } + + #[test] + fn length_percentage_parse_still_infallible_fallback_for_garbage() { + let l = LengthPercentage::String("wat".into()); + assert_eq!(l.parse(), ParsedLength::Px(0.0)); + } } diff --git a/crates/rustmotion-core/src/engine/paint_pass.rs b/crates/rustmotion-core/src/engine/paint_pass.rs index c306ee8..3b129e0 100644 --- a/crates/rustmotion-core/src/engine/paint_pass.rs +++ b/crates/rustmotion-core/src/engine/paint_pass.rs @@ -1130,7 +1130,7 @@ fn paint_gradient_border( let colors: Vec = gb .colors .iter() - .map(|c| parse_color_string(c).unwrap_or(SColor::BLACK)) + .map(|c| parse_color_string(c).unwrap_or_else(|| unresolved_color(c))) .collect(); let n = colors.len(); let positions: Vec = (0..n) @@ -1297,6 +1297,21 @@ fn paint_box_shadow( } // ---- Color parsing ---- +// +// Both functions below route through `renderer::parse_css_color` — the +// single frozen entry point (see `renderer/colors.rs`) — rather than +// duplicating hex/rgb/hsl/named-colour parsing here. That parser accepts +// every common CSS colour form (3/4/6/8-digit hex, rgb()/rgba(), hsl()/ +// hsla(), the full CSS named-colour set) and returns `None` for anything +// else. +// +// A `Color::String` that fails to parse is a real authoring bug — the +// previous behaviour (`unwrap_or(SColor::BLACK)`) rendered it as invisible +// text on this tool's dark-background target style. It now falls back to +// `renderer::UNRESOLVED_COLOR` (opaque magenta) and logs a warning instead, +// so the failure is visible on screen and in the render logs. Wiring a +// hard validation-time error for this is out of scope here (sibling +// workstream); this only stops the render path from lying. pub fn parse_color(c: &Color) -> SColor { match c { @@ -1304,77 +1319,23 @@ pub fn parse_color(c: &Color) -> SColor { let alpha = (a.clamp(0.0, 1.0) * 255.0) as u8; SColor::from_argb(alpha, *r, *g, *b) } - Color::String(s) => parse_color_string(s).unwrap_or(SColor::BLACK), + Color::String(s) => parse_color_string(s).unwrap_or_else(|| unresolved_color(s)), } } fn parse_color_string(s: &str) -> Option { - let s = s.trim(); - if let Some(hex) = s.strip_prefix('#') { - return parse_hex(hex); - } - if let Some(inner) = s.strip_prefix("rgb(").and_then(|s| s.strip_suffix(')')) { - let parts: Vec<_> = inner.split(',').map(|s| s.trim()).collect(); - if parts.len() == 3 { - let r = parts[0].parse::().ok()?; - let g = parts[1].parse::().ok()?; - let b = parts[2].parse::().ok()?; - return Some(SColor::from_argb(255, r, g, b)); - } - } - if let Some(inner) = s.strip_prefix("rgba(").and_then(|s| s.strip_suffix(')')) { - let parts: Vec<_> = inner.split(',').map(|s| s.trim()).collect(); - if parts.len() == 4 { - let r = parts[0].parse::().ok()?; - let g = parts[1].parse::().ok()?; - let b = parts[2].parse::().ok()?; - let a = parts[3].parse::().ok()?; - return Some(SColor::from_argb( - (a.clamp(0.0, 1.0) * 255.0) as u8, - r, - g, - b, - )); - } - } - match s.to_ascii_lowercase().as_str() { - "black" => Some(SColor::BLACK), - "white" => Some(SColor::WHITE), - "transparent" => Some(SColor::TRANSPARENT), - "red" => Some(SColor::RED), - "green" => Some(SColor::GREEN), - "blue" => Some(SColor::BLUE), - "yellow" => Some(SColor::YELLOW), - "magenta" | "fuchsia" => Some(SColor::MAGENTA), - "cyan" | "aqua" => Some(SColor::CYAN), - "gray" | "grey" => Some(SColor::GRAY), - _ => None, - } + crate::engine::renderer::parse_css_color(s).map(|(r, g, b, a)| SColor::from_argb(a, r, g, b)) } -fn parse_hex(hex: &str) -> Option { - match hex.len() { - 3 => { - let r = u8::from_str_radix(&hex[0..1], 16).ok()? * 17; - let g = u8::from_str_radix(&hex[1..2], 16).ok()? * 17; - let b = u8::from_str_radix(&hex[2..3], 16).ok()? * 17; - Some(SColor::from_argb(255, r, g, b)) - } - 6 => { - let r = u8::from_str_radix(&hex[0..2], 16).ok()?; - let g = u8::from_str_radix(&hex[2..4], 16).ok()?; - let b = u8::from_str_radix(&hex[4..6], 16).ok()?; - Some(SColor::from_argb(255, r, g, b)) - } - 8 => { - let r = u8::from_str_radix(&hex[0..2], 16).ok()?; - let g = u8::from_str_radix(&hex[2..4], 16).ok()?; - let b = u8::from_str_radix(&hex[4..6], 16).ok()?; - let a = u8::from_str_radix(&hex[6..8], 16).ok()?; - Some(SColor::from_argb(a, r, g, b)) - } - _ => None, - } +/// Loud, non-black fallback for a colour string that couldn't be resolved. +/// See the module note above `parse_color`. +fn unresolved_color(original: &str) -> SColor { + eprintln!( + "Warning: unrecognized color '{original}' — rendering as opaque magenta instead of \ + silently falling back to black" + ); + let (r, g, b, a) = crate::engine::renderer::UNRESOLVED_COLOR; + SColor::from_argb(a, r, g, b) } // Suppress unused import warnings for items only used in trait-bound paths. @@ -2462,7 +2423,7 @@ mod tests { fn parse_rgba_string() { let c = parse_color_string("rgba(10, 20, 30, 0.5)").unwrap(); assert_eq!(c.r(), 10); - assert_eq!(c.a(), 127); + assert_eq!(c.a(), 128); } #[test] @@ -2473,4 +2434,43 @@ mod tests { SColor::TRANSPARENT ); } + + #[test] + fn parse_white_forms_all_agree() { + // Regression test for the C2 audit finding: `#fff`, `#FFF`, `white` + // and `rgb(255,255,255)` must all resolve to the same opaque white, + // never to black. + let white = SColor::from_argb(255, 255, 255, 255); + assert_eq!(parse_color_string("#fff").unwrap(), white); + assert_eq!(parse_color_string("#FFF").unwrap(), white); + assert_eq!(parse_color_string("white").unwrap(), white); + assert_eq!(parse_color_string("WHITE").unwrap(), white); + assert_eq!(parse_color_string("rgb(255,255,255)").unwrap(), white); + assert_eq!(parse_color_string("rgb(255, 255, 255)").unwrap(), white); + } + + #[test] + fn parse_color_string_supports_extended_named_set() { + // Only 11 names were hardcoded before; spot-check a few outside + // that set to prove it now routes through the full CSS keyword + // table in `renderer::colors`. + assert!(parse_color_string("rebeccapurple").is_some()); + assert!(parse_color_string("cornflowerblue").is_some()); + assert!(parse_color_string("dodgerblue").is_some()); + } + + #[test] + fn parse_color_string_supports_hsl() { + assert_eq!( + parse_color_string("hsl(0, 100%, 50%)").unwrap(), + SColor::from_argb(255, 255, 0, 0) + ); + } + + #[test] + fn parse_color_unresolvable_string_is_not_black() { + let c = parse_color(&Color::String("not-a-color".to_string())); + assert_ne!(c, SColor::BLACK); + assert_eq!(c, SColor::from_argb(255, 255, 0, 255)); + } } diff --git a/crates/rustmotion-core/src/engine/renderer/colors.rs b/crates/rustmotion-core/src/engine/renderer/colors.rs index 9857fd6..07b2d79 100644 --- a/crates/rustmotion-core/src/engine/renderer/colors.rs +++ b/crates/rustmotion-core/src/engine/renderer/colors.rs @@ -1,20 +1,92 @@ +//! CSS colour parsing. +//! +//! [`parse_css_color`] is the single entry point every colour-consuming path +//! in the engine should route through. It accepts every colour form a CSS +//! author would reasonably write: +//! +//! - hex: `#rgb`, `#rgba`, `#rrggbb`, `#rrggbbaa` (case-insensitive) +//! - `rgb()` / `rgba()`, comma- or space-separated, integers or percentages, +//! alpha as a `0..1` float, a percentage, or after a `/` +//! - `hsl()` / `hsla()`, same separator/alpha flexibility +//! - the full CSS named-colour keyword set (147 names + `transparent`) +//! +//! It returns `None` for anything else so callers can detect the failure +//! instead of silently painting something wrong. +//! +//! [`parse_hex_color`] predates this module and keeps its historical, +//! infallible signature so every existing call site keeps compiling +//! untouched — it is now a thin wrapper around [`parse_css_color`]. Its name +//! is a bit of a misnomer at this point (it accepts any CSS colour form, not +//! just hex) but changing it would ripple across dozens of call sites in +//! sibling crates, which is out of scope here. +//! +//! Because `parse_hex_color` can't propagate a `None` (its signature is +//! frozen), unresolvable input logs a warning to stderr and returns +//! [`UNRESOLVED_COLOR`] — an unmistakable opaque magenta — instead of +//! quietly falling back to black. Black is a real, common colour choice; on +//! the dark backgrounds this tool targets, a black fallback for a failed +//! parse is *invisible*, which is exactly the silent-failure bug this module +//! exists to close. Magenta never blends in. + use skia_safe::{Color4f, Paint}; -/// Parse a hex color string (#RRGGBB or #RRGGBBAA) into RGBA components +/// Sentinel colour returned by [`parse_hex_color`] (and used by callers of +/// [`parse_css_color`] that choose to mirror this convention) when the input +/// string cannot be resolved as any known CSS colour form. Deliberately +/// jarring so a bad colour string is obvious on screen rather than +/// disappearing into a dark background. +pub const UNRESOLVED_COLOR: (u8, u8, u8, u8) = (255, 0, 255, 255); + +/// Parse any CSS colour string into `(r, g, b, a)` bytes. +/// +/// Returns `None` when `input` doesn't match any recognised form. Callers +/// that need an infallible result should decide their own fallback and +/// **should not** default to black without a very good reason — see the +/// module docs. +pub fn parse_css_color(input: &str) -> Option<(u8, u8, u8, u8)> { + let s = input.trim(); + if s.is_empty() { + return None; + } + + if let Some(hex) = s.strip_prefix('#') { + return parse_hex_digits(hex); + } + + let lower = s.to_ascii_lowercase(); + if let Some(rgba) = parse_rgb_fn(&lower) { + return Some(rgba); + } + if let Some(rgba) = parse_hsl_fn(&lower) { + return Some(rgba); + } + named_color(&lower) +} + +/// Parse a hex colour string (any CSS colour form, historically hex-only — +/// see module docs) into RGBA components. Infallible: unresolvable input is +/// reported to stderr and mapped to [`UNRESOLVED_COLOR`] rather than black. pub fn parse_hex_color(hex: &str) -> (u8, u8, u8, u8) { - let hex = hex.trim_start_matches('#'); - if hex.len() < 6 { - return (0, 0, 0, 255); - } - let r = u8::from_str_radix(&hex[0..2], 16).unwrap_or(0); - let g = u8::from_str_radix(&hex[2..4], 16).unwrap_or(0); - let b = u8::from_str_radix(&hex[4..6], 16).unwrap_or(0); - let a = if hex.len() >= 8 { - u8::from_str_radix(&hex[6..8], 16).unwrap_or(255) - } else { - 255 - }; - (r, g, b, a) + let s = hex.trim(); + + if let Some(rgba) = parse_css_color(s) { + return rgba; + } + + // Historical leniency: the original implementation stripped a leading + // '#' unconditionally, so callers could (and do) pass hex digits with no + // '#' at all. Retry with one prepended before giving up. + if !s.starts_with('#') { + if let Some(rgba) = parse_css_color(&format!("#{s}")) { + return rgba; + } + } + + eprintln!( + "Warning: unrecognized color '{hex}' — rendering as opaque magenta {UNRESOLVED_COLOR:?} \ + instead of silently falling back to black" + ); + UNRESOLVED_COLOR } pub fn color4f_from_hex(hex: &str) -> Color4f { @@ -32,3 +104,597 @@ pub fn paint_from_hex(hex: &str) -> Paint { paint.set_anti_alias(true); paint } + +// ---- hex ---- + +fn parse_hex_digits(hex: &str) -> Option<(u8, u8, u8, u8)> { + if hex.is_empty() || !hex.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + let nibble = |c: char| c.to_digit(16).unwrap() as u8; + match hex.len() { + 3 => { + let mut cs = hex.chars(); + let r = nibble(cs.next()?) * 17; + let g = nibble(cs.next()?) * 17; + let b = nibble(cs.next()?) * 17; + Some((r, g, b, 255)) + } + 4 => { + let mut cs = hex.chars(); + let r = nibble(cs.next()?) * 17; + let g = nibble(cs.next()?) * 17; + let b = nibble(cs.next()?) * 17; + let a = nibble(cs.next()?) * 17; + Some((r, g, b, a)) + } + 6 => { + let r = u8::from_str_radix(&hex[0..2], 16).ok()?; + let g = u8::from_str_radix(&hex[2..4], 16).ok()?; + let b = u8::from_str_radix(&hex[4..6], 16).ok()?; + Some((r, g, b, 255)) + } + 8 => { + let r = u8::from_str_radix(&hex[0..2], 16).ok()?; + let g = u8::from_str_radix(&hex[2..4], 16).ok()?; + let b = u8::from_str_radix(&hex[4..6], 16).ok()?; + let a = u8::from_str_radix(&hex[6..8], 16).ok()?; + Some((r, g, b, a)) + } + _ => None, + } +} + +// ---- functional notation: rgb()/rgba(), hsl()/hsla() ---- + +/// Split `name(inner)` into its lowercase function name and inner argument +/// string. `s` is expected to already be trimmed and lowercased. +fn split_function(s: &str) -> Option<(&str, &str)> { + let s = s.trim(); + let open = s.find('(')?; + if !s.ends_with(')') { + return None; + } + let name = s[..open].trim(); + let inner = &s[open + 1..s.len() - 1]; + Some((name, inner)) +} + +/// Tokenize a functional-notation argument list. Accepts the classic +/// comma-separated form (`10, 20, 30, 0.5`) and the modern space-separated +/// form with an optional `/` before alpha (`10 20 30 / 50%`). +fn split_components(inner: &str) -> Vec { + let inner = inner.trim(); + if inner.contains(',') { + inner + .split(',') + .map(|p| p.trim().to_string()) + .filter(|p| !p.is_empty()) + .collect() + } else { + inner + .split('/') + .flat_map(|part| part.split_whitespace()) + .map(|p| p.to_string()) + .collect() + } +} + +fn parse_rgb_fn(s: &str) -> Option<(u8, u8, u8, u8)> { + let (name, inner) = split_function(s)?; + if name != "rgb" && name != "rgba" { + return None; + } + let comps = split_components(inner); + match comps.len() { + 3 => { + let r = parse_channel_255(&comps[0])?; + let g = parse_channel_255(&comps[1])?; + let b = parse_channel_255(&comps[2])?; + Some((r, g, b, 255)) + } + 4 => { + let r = parse_channel_255(&comps[0])?; + let g = parse_channel_255(&comps[1])?; + let b = parse_channel_255(&comps[2])?; + let a = parse_alpha(&comps[3])?; + Some((r, g, b, a)) + } + _ => None, + } +} + +fn parse_hsl_fn(s: &str) -> Option<(u8, u8, u8, u8)> { + let (name, inner) = split_function(s)?; + if name != "hsl" && name != "hsla" { + return None; + } + let comps = split_components(inner); + if comps.len() != 3 && comps.len() != 4 { + return None; + } + let h = parse_hue(&comps[0])?; + let sat = parse_percent_unit(&comps[1])?; + let light = parse_percent_unit(&comps[2])?; + let a = if comps.len() == 4 { + parse_alpha(&comps[3])? + } else { + 255 + }; + let (r, g, b) = hsl_to_rgb(h, sat, light); + Some((r, g, b, a)) +} + +/// A single `rgb()`/`rgba()` colour channel: an integer/float 0-255, or a +/// percentage 0%-100%. +fn parse_channel_255(s: &str) -> Option { + let s = s.trim(); + if let Some(pct) = s.strip_suffix('%') { + let v: f32 = pct.trim().parse().ok()?; + return Some((v.clamp(0.0, 100.0) / 100.0 * 255.0).round() as u8); + } + let v: f32 = s.parse().ok()?; + Some(v.clamp(0.0, 255.0).round() as u8) +} + +/// Alpha channel: a float 0-1, or a percentage 0%-100%. +/// +/// Both branches round, so `0.5` and `50%` resolve to the same byte (128). +/// The old hand-rolled `rgba()` parser truncated (`as u8`), mapping `0.5` to +/// `127`; that was an artefact of the formula rather than a deliberate +/// choice, and it made the two notations for one value disagree. Rounding +/// matches the CSS spec and browsers. +fn parse_alpha(s: &str) -> Option { + let s = s.trim(); + if let Some(pct) = s.strip_suffix('%') { + let v: f32 = pct.trim().parse().ok()?; + return Some((v.clamp(0.0, 100.0) / 100.0 * 255.0).round() as u8); + } + let v: f32 = s.parse().ok()?; + Some((v.clamp(0.0, 1.0) * 255.0).round() as u8) +} + +/// Hue: a bare number or a number with a `deg` suffix, normalised into +/// `0..360`. +fn parse_hue(s: &str) -> Option { + let s = s.trim(); + let s = s.strip_suffix("deg").unwrap_or(s).trim(); + let v: f32 = s.parse().ok()?; + let mut h = v % 360.0; + if h < 0.0 { + h += 360.0; + } + Some(h) +} + +/// A required percentage (saturation/lightness), normalised to `0..1`. +fn parse_percent_unit(s: &str) -> Option { + let s = s.trim().strip_suffix('%')?; + let v: f32 = s.trim().parse().ok()?; + Some(v.clamp(0.0, 100.0) / 100.0) +} + +fn hsl_to_rgb(h: f32, s: f32, l: f32) -> (u8, u8, u8) { + if s <= 0.0 { + let v = (l.clamp(0.0, 1.0) * 255.0).round() as u8; + return (v, v, v); + } + let c = (1.0 - (2.0 * l - 1.0).abs()) * s; + let hp = h / 60.0; + let x = c * (1.0 - (hp % 2.0 - 1.0).abs()); + let (r1, g1, b1) = match hp as i32 { + 0 => (c, x, 0.0), + 1 => (x, c, 0.0), + 2 => (0.0, c, x), + 3 => (0.0, x, c), + 4 => (x, 0.0, c), + _ => (c, 0.0, x), + }; + let m = l - c / 2.0; + let to_u8 = |v: f32| ((v + m).clamp(0.0, 1.0) * 255.0).round() as u8; + (to_u8(r1), to_u8(g1), to_u8(b1)) +} + +// ---- named colours ---- + +/// The full CSS named-colour keyword set (CSS Color Module Level 4 extended +/// keywords, 147 names) plus `transparent`. `name` must already be +/// lowercased. +fn named_color(name: &str) -> Option<(u8, u8, u8, u8)> { + if name == "transparent" { + return Some((0, 0, 0, 0)); + } + let hex: u32 = match name { + "aliceblue" => 0xF0F8FF, + "antiquewhite" => 0xFAEBD7, + "aqua" => 0x00FFFF, + "aquamarine" => 0x7FFFD4, + "azure" => 0xF0FFFF, + "beige" => 0xF5F5DC, + "bisque" => 0xFFE4C4, + "black" => 0x000000, + "blanchedalmond" => 0xFFEBCD, + "blue" => 0x0000FF, + "blueviolet" => 0x8A2BE2, + "brown" => 0xA52A2A, + "burlywood" => 0xDEB887, + "cadetblue" => 0x5F9EA0, + "chartreuse" => 0x7FFF00, + "chocolate" => 0xD2691E, + "coral" => 0xFF7F50, + "cornflowerblue" => 0x6495ED, + "cornsilk" => 0xFFF8DC, + "crimson" => 0xDC143C, + "cyan" => 0x00FFFF, + "darkblue" => 0x00008B, + "darkcyan" => 0x008B8B, + "darkgoldenrod" => 0xB8860B, + "darkgray" => 0xA9A9A9, + "darkgreen" => 0x006400, + "darkgrey" => 0xA9A9A9, + "darkkhaki" => 0xBDB76B, + "darkmagenta" => 0x8B008B, + "darkolivegreen" => 0x556B2F, + "darkorange" => 0xFF8C00, + "darkorchid" => 0x9932CC, + "darkred" => 0x8B0000, + "darksalmon" => 0xE9967A, + "darkseagreen" => 0x8FBC8F, + "darkslateblue" => 0x483D8B, + "darkslategray" => 0x2F4F4F, + "darkslategrey" => 0x2F4F4F, + "darkturquoise" => 0x00CED1, + "darkviolet" => 0x9400D3, + "deeppink" => 0xFF1493, + "deepskyblue" => 0x00BFFF, + "dimgray" => 0x696969, + "dimgrey" => 0x696969, + "dodgerblue" => 0x1E90FF, + "firebrick" => 0xB22222, + "floralwhite" => 0xFFFAF0, + "forestgreen" => 0x228B22, + "fuchsia" => 0xFF00FF, + "gainsboro" => 0xDCDCDC, + "ghostwhite" => 0xF8F8FF, + "gold" => 0xFFD700, + "goldenrod" => 0xDAA520, + "gray" => 0x808080, + "green" => 0x008000, + "greenyellow" => 0xADFF2F, + "grey" => 0x808080, + "honeydew" => 0xF0FFF0, + "hotpink" => 0xFF69B4, + "indianred" => 0xCD5C5C, + "indigo" => 0x4B0082, + "ivory" => 0xFFFFF0, + "khaki" => 0xF0E68C, + "lavender" => 0xE6E6FA, + "lavenderblush" => 0xFFF0F5, + "lawngreen" => 0x7CFC00, + "lemonchiffon" => 0xFFFACD, + "lightblue" => 0xADD8E6, + "lightcoral" => 0xF08080, + "lightcyan" => 0xE0FFFF, + "lightgoldenrodyellow" => 0xFAFAD2, + "lightgray" => 0xD3D3D3, + "lightgreen" => 0x90EE90, + "lightgrey" => 0xD3D3D3, + "lightpink" => 0xFFB6C1, + "lightsalmon" => 0xFFA07A, + "lightseagreen" => 0x20B2AA, + "lightskyblue" => 0x87CEFA, + "lightslategray" => 0x778899, + "lightslategrey" => 0x778899, + "lightsteelblue" => 0xB0C4DE, + "lightyellow" => 0xFFFFE0, + "lime" => 0x00FF00, + "limegreen" => 0x32CD32, + "linen" => 0xFAF0E6, + "magenta" => 0xFF00FF, + "maroon" => 0x800000, + "mediumaquamarine" => 0x66CDAA, + "mediumblue" => 0x0000CD, + "mediumorchid" => 0xBA55D3, + "mediumpurple" => 0x9370DB, + "mediumseagreen" => 0x3CB371, + "mediumslateblue" => 0x7B68EE, + "mediumspringgreen" => 0x00FA9A, + "mediumturquoise" => 0x48D1CC, + "mediumvioletred" => 0xC71585, + "midnightblue" => 0x191970, + "mintcream" => 0xF5FFFA, + "mistyrose" => 0xFFE4E1, + "moccasin" => 0xFFE4B5, + "navajowhite" => 0xFFDEAD, + "navy" => 0x000080, + "oldlace" => 0xFDF5E6, + "olive" => 0x808000, + "olivedrab" => 0x6B8E23, + "orange" => 0xFFA500, + "orangered" => 0xFF4500, + "orchid" => 0xDA70D6, + "palegoldenrod" => 0xEEE8AA, + "palegreen" => 0x98FB98, + "paleturquoise" => 0xAFEEEE, + "palevioletred" => 0xDB7093, + "papayawhip" => 0xFFEFD5, + "peachpuff" => 0xFFDAB9, + "peru" => 0xCD853F, + "pink" => 0xFFC0CB, + "plum" => 0xDDA0DD, + "powderblue" => 0xB0E0E6, + "purple" => 0x800080, + "rebeccapurple" => 0x663399, + "red" => 0xFF0000, + "rosybrown" => 0xBC8F8F, + "royalblue" => 0x4169E1, + "saddlebrown" => 0x8B4513, + "salmon" => 0xFA8072, + "sandybrown" => 0xF4A460, + "seagreen" => 0x2E8B57, + "seashell" => 0xFFF5EE, + "sienna" => 0xA0522D, + "silver" => 0xC0C0C0, + "skyblue" => 0x87CEEB, + "slateblue" => 0x6A5ACD, + "slategray" => 0x708090, + "slategrey" => 0x708090, + "snow" => 0xFFFAFA, + "springgreen" => 0x00FF7F, + "steelblue" => 0x4682B4, + "tan" => 0xD2B48C, + "teal" => 0x008080, + "thistle" => 0xD8BFD8, + "tomato" => 0xFF6347, + "turquoise" => 0x40E0D0, + "violet" => 0xEE82EE, + "wheat" => 0xF5DEB3, + "white" => 0xFFFFFF, + "whitesmoke" => 0xF5F5F5, + "yellow" => 0xFFFF00, + "yellowgreen" => 0x9ACD32, + _ => return None, + }; + Some(( + ((hex >> 16) & 0xFF) as u8, + ((hex >> 8) & 0xFF) as u8, + (hex & 0xFF) as u8, + 255, + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + // ---- hex ---- + + #[test] + fn hex_3_digit_shorthand() { + assert_eq!(parse_css_color("#fff"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("#FFF"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("#f00"), Some((255, 0, 0, 255))); + } + + #[test] + fn hex_4_digit_shorthand_with_alpha() { + assert_eq!(parse_css_color("#ffff"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("#f008"), Some((255, 0, 0, 136))); + } + + #[test] + fn hex_6_digit() { + assert_eq!(parse_css_color("#ffffff"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("#102030"), Some((0x10, 0x20, 0x30, 255))); + } + + #[test] + fn hex_8_digit_with_alpha() { + assert_eq!(parse_css_color("#ffffffff"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("#80ffffff"), Some((0x80, 255, 255, 255))); + } + + #[test] + fn hex_case_insensitive() { + assert_eq!(parse_css_color("#AbCdEf"), parse_css_color("#abcdef")); + } + + // ---- rgb / rgba ---- + + #[test] + fn rgb_function_white() { + assert_eq!(parse_css_color("rgb(255,255,255)"), Some((255, 255, 255, 255))); + assert_eq!( + parse_css_color("rgb(255, 255, 255)"), + Some((255, 255, 255, 255)) + ); + } + + #[test] + fn rgb_function_space_syntax() { + assert_eq!(parse_css_color("rgb(255 255 255)"), Some((255, 255, 255, 255))); + } + + #[test] + fn rgba_function_with_float_alpha() { + // 0.5 * 255 = 127.5, rounded like the CSS spec and like browsers. + // Percentage and bare-float alpha resolve identically. + let (r, g, b, a) = parse_css_color("rgba(255,255,255,0.5)").unwrap(); + assert_eq!((r, g, b), (255, 255, 255)); + assert_eq!(a, 128); + assert_eq!(parse_css_color("rgba(255,255,255,50%)").unwrap().3, a); + } + + #[test] + fn rgba_function_space_slash_alpha() { + let (r, g, b, a) = parse_css_color("rgb(255 255 255 / 50%)").unwrap(); + assert_eq!((r, g, b), (255, 255, 255)); + assert_eq!(a, 128); + } + + #[test] + fn rgb_function_percent_channels() { + assert_eq!( + parse_css_color("rgb(100%, 100%, 100%)"), + Some((255, 255, 255, 255)) + ); + } + + // ---- hsl / hsla ---- + + #[test] + fn hsl_function_primary_colors() { + assert_eq!(parse_css_color("hsl(0, 100%, 50%)"), Some((255, 0, 0, 255))); + assert_eq!( + parse_css_color("hsl(120, 100%, 50%)"), + Some((0, 255, 0, 255)) + ); + assert_eq!( + parse_css_color("hsl(240, 100%, 50%)"), + Some((0, 0, 255, 255)) + ); + } + + #[test] + fn hsl_function_white_and_black() { + assert_eq!( + parse_css_color("hsl(0, 0%, 100%)"), + Some((255, 255, 255, 255)) + ); + assert_eq!(parse_css_color("hsl(0, 0%, 0%)"), Some((0, 0, 0, 255))); + } + + #[test] + fn hsla_function_with_alpha() { + let (r, g, b, a) = parse_css_color("hsla(0, 100%, 50%, 0.5)").unwrap(); + assert_eq!((r, g, b), (255, 0, 0)); + assert_eq!(a, 128); + } + + #[test] + fn hsl_function_space_syntax() { + assert_eq!( + parse_css_color("hsl(0 100% 50%)"), + Some((255, 0, 0, 255)) + ); + } + + // ---- named colors ---- + + #[test] + fn named_color_white_black() { + assert_eq!(parse_css_color("white"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("WHITE"), Some((255, 255, 255, 255))); + assert_eq!(parse_css_color("black"), Some((0, 0, 0, 255))); + } + + #[test] + fn named_color_transparent() { + assert_eq!(parse_css_color("transparent"), Some((0, 0, 0, 0))); + } + + #[test] + fn named_color_extended_set_sample() { + // Spot-check a handful outside the old 11-name table. + assert_eq!(parse_css_color("rebeccapurple"), Some((0x66, 0x33, 0x99, 255))); + assert_eq!(parse_css_color("cornflowerblue"), Some((0x64, 0x95, 0xED, 255))); + assert_eq!(parse_css_color("tomato"), Some((0xFF, 0x63, 0x47, 255))); + assert_eq!(parse_css_color("dodgerblue"), Some((0x1E, 0x90, 0xFF, 255))); + } + + #[test] + fn named_color_gray_grey_spelling() { + assert_eq!(parse_css_color("gray"), parse_css_color("grey")); + assert_eq!(parse_css_color("darkgray"), parse_css_color("darkgrey")); + } + + // ---- whitespace tolerance ---- + + #[test] + fn tolerates_surrounding_and_internal_whitespace() { + assert_eq!(parse_css_color(" #fff "), Some((255, 255, 255, 255))); + assert_eq!( + parse_css_color(" white "), + Some((255, 255, 255, 255)) + ); + assert_eq!( + parse_css_color("rgb( 10 , 20 , 30 )"), + Some((10, 20, 30, 255)) + ); + } + + // ---- rejected forms ---- + + #[test] + fn rejects_garbage_word() { + assert_eq!(parse_css_color("notacolor"), None); + } + + #[test] + fn rejects_malformed_hex_length() { + assert_eq!(parse_css_color("#ff"), None); + assert_eq!(parse_css_color("#fffff"), None); + } + + #[test] + fn rejects_malformed_hex_digits() { + assert_eq!(parse_css_color("#gggggg"), None); + } + + #[test] + fn rejects_incomplete_rgb() { + assert_eq!(parse_css_color("rgb(255, 255)"), None); + } + + #[test] + fn rejects_unclosed_function() { + assert_eq!(parse_css_color("rgb(255,255,255"), None); + } + + #[test] + fn rejects_empty_string() { + assert_eq!(parse_css_color(""), None); + assert_eq!(parse_css_color(" "), None); + } + + // ---- parse_hex_color (infallible wrapper) ---- + + #[test] + fn parse_hex_color_still_infallible_for_6_digit() { + assert_eq!(parse_hex_color("#ffffff"), (255, 255, 255, 255)); + } + + #[test] + fn parse_hex_color_now_handles_3_digit_shorthand() { + // Regression test for the exact silent-black bug: previously any hex + // string shorter than 6 chars (post `#`-strip) returned black. + assert_eq!(parse_hex_color("#fff"), (255, 255, 255, 255)); + assert_eq!(parse_hex_color("#FFF"), (255, 255, 255, 255)); + } + + #[test] + fn parse_hex_color_now_handles_named_and_functional_forms() { + // Callers across the codebase pass arbitrary CSS colour strings + // (not just hex) through this function via `paint_from_hex`. + assert_eq!(parse_hex_color("white"), (255, 255, 255, 255)); + assert_eq!(parse_hex_color("rgb(255,255,255)"), (255, 255, 255, 255)); + } + + #[test] + fn parse_hex_color_without_leading_hash_still_works() { + assert_eq!(parse_hex_color("ffffff"), (255, 255, 255, 255)); + assert_eq!(parse_hex_color("fff"), (255, 255, 255, 255)); + } + + #[test] + fn parse_hex_color_unresolvable_input_is_loud_not_black() { + assert_eq!(parse_hex_color("not-a-color"), UNRESOLVED_COLOR); + assert_ne!(UNRESOLVED_COLOR, (0, 0, 0, 255)); + } + + #[test] + fn color4f_from_hex_white_is_actually_white() { + let c = color4f_from_hex("#fff"); + assert_eq!((c.r, c.g, c.b, c.a), (1.0, 1.0, 1.0, 1.0)); + } +} From 43f449807f18c9b55edac6e4547b9f4bd2bf7e1b Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 16:43:26 +0200 Subject: [PATCH 2/6] feat(layout): wire grid track definitions through to taffy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grid component accepted `grid-template-columns`, the validator made it mandatory, and taffy was never told about it — the only grid reference in taffy_bridge was the Display::Grid mapping. Taffy therefore laid out a grid with no tracks and collapsed every child into one implicit column, so a three-column KPI row rendered as three stacked full-width rows while validation reported no problem. Translate grid-template-columns/rows, grid-column, grid-row and grid-auto-flow into their taffy equivalents. Also reorder GridTrack's untagged variants. Length's inner String arm matched any JSON string or number, so it swallowed bare numbers meant for Fr and keywords meant for Keyword, leaving both unreachable. That is why [1,1,1] used to mean three 1px tracks instead of three equal fractions. Single fr values use taffy's flex() helper (minmax(0, Nfr)) rather than fr(), so tracks stay evenly sized regardless of content — matching the flex-row equivalent byte for byte, which matters for a video renderer. Refs #105 --- crates/rustmotion-core/src/css/style.rs | 91 +++++- .../rustmotion-core/src/css/taffy_bridge.rs | 298 +++++++++++++++++- 2 files changed, 387 insertions(+), 2 deletions(-) diff --git a/crates/rustmotion-core/src/css/style.rs b/crates/rustmotion-core/src/css/style.rs index 3570daf..39b4827 100644 --- a/crates/rustmotion-core/src/css/style.rs +++ b/crates/rustmotion-core/src/css/style.rs @@ -563,12 +563,27 @@ pub enum Gap { // ---- Grid ---- +/// A single grid track (column or row) sizing function. +/// +/// Variant order matters here: this is `#[serde(untagged)]`, and serde tries +/// each variant in declaration order, keeping the first that deserializes +/// successfully. `Length(LengthPercentage)` accepts *any* JSON number or +/// string (its own `String` fallback variant is a catch-all), so it must be +/// tried last — otherwise it silently swallows bare numbers (meant to be +/// `Fr`, matching the `flex-grow` convention) and keyword strings like +/// `"auto"` (meant to be `Keyword`). #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(untagged)] pub enum GridTrack { - Length(LengthPercentage), + /// Bare JSON number, e.g. `1` — a flex fraction, same convention as + /// `flex-grow`. Equivalent to the string form `"1fr"`. Fr(f32), + /// `"auto"` / `"min-content"` / `"max-content"`. Keyword(GridTrackKeyword), + /// Any other length/percentage, including the explicit string form of a + /// flex fraction (`"1fr"`), which `LengthPercentage::parse()` resolves + /// to the same `ParsedLength::Fr` as the bare-number form above. + Length(LengthPercentage), /// `minmax(min, max)` Minmax { min: Box, @@ -1182,4 +1197,78 @@ mod tests { assert_eq!(parsed.opacity, Some(0.5)); assert_eq!(parsed.z_index, Some(10)); } + + // ---- Grid track deserialization (issue #105) ---- + // + // `GridTrack` is `#[serde(untagged)]`; these lock in which variant a + // given JSON shape resolves to, since that resolution previously + // silently swallowed both `Fr` and `Keyword` into `Length`. + + #[test] + fn grid_track_bare_number_is_fr() { + let json = r#"{ "grid-template-columns": [1, 1, 1] }"#; + let s: CssStyle = serde_json::from_str(json).unwrap(); + let tracks = s.grid_template_columns.expect("tracks set"); + assert_eq!(tracks.len(), 3); + for t in &tracks { + assert!(matches!(t, GridTrack::Fr(n) if (*n - 1.0).abs() < f32::EPSILON)); + } + } + + #[test] + fn grid_track_string_fr_is_length_parsed_as_fr() { + let json = r#"{ "grid-template-columns": ["1fr", "2fr"] }"#; + let s: CssStyle = serde_json::from_str(json).unwrap(); + let tracks = s.grid_template_columns.expect("tracks set"); + match &tracks[0] { + GridTrack::Length(lp) => { + assert_eq!(lp.parse(), crate::css::units::ParsedLength::Fr(1.0)) + } + other => panic!("expected Length(\"1fr\"), got {other:?}"), + } + match &tracks[1] { + GridTrack::Length(lp) => { + assert_eq!(lp.parse(), crate::css::units::ParsedLength::Fr(2.0)) + } + other => panic!("expected Length(\"2fr\"), got {other:?}"), + } + } + + #[test] + fn grid_track_keyword_strings() { + let json = r#"{ "grid-template-columns": ["auto", "min-content", "max-content"] }"#; + let s: CssStyle = serde_json::from_str(json).unwrap(); + let tracks = s.grid_template_columns.expect("tracks set"); + assert!(matches!( + tracks[0], + GridTrack::Keyword(GridTrackKeyword::Auto) + )); + assert!(matches!( + tracks[1], + GridTrack::Keyword(GridTrackKeyword::MinContent) + )); + assert!(matches!( + tracks[2], + GridTrack::Keyword(GridTrackKeyword::MaxContent) + )); + } + + #[test] + fn grid_track_px_string_is_length() { + let json = r#"{ "grid-template-columns": ["200px", "50%"] }"#; + let s: CssStyle = serde_json::from_str(json).unwrap(); + let tracks = s.grid_template_columns.expect("tracks set"); + match &tracks[0] { + GridTrack::Length(lp) => { + assert_eq!(lp.parse(), crate::css::units::ParsedLength::Px(200.0)) + } + other => panic!("expected Length(200px), got {other:?}"), + } + match &tracks[1] { + GridTrack::Length(lp) => { + assert_eq!(lp.parse(), crate::css::units::ParsedLength::Percent(50.0)) + } + other => panic!("expected Length(50%), got {other:?}"), + } + } } diff --git a/crates/rustmotion-core/src/css/taffy_bridge.rs b/crates/rustmotion-core/src/css/taffy_bridge.rs index 4dcdadc..f6be557 100644 --- a/crates/rustmotion-core/src/css/taffy_bridge.rs +++ b/crates/rustmotion-core/src/css/taffy_bridge.rs @@ -10,7 +10,8 @@ use taffy::prelude as tf; use super::style::{ AlignContent, AlignItems, AlignSelf, CssStyle, Display, Edges, FlexDirection, FlexWrap, Gap, - JustifyContent, Overflow, Position, Size, + GridAutoFlow, GridLine, GridLineEnd, GridTrack, GridTrackKeyword, JustifyContent, Overflow, + Position, Size, }; use super::units::{LengthContext, LengthPercentage, ParsedLength}; @@ -126,6 +127,34 @@ pub fn to_taffy_style(css: &CssStyle, ctx: &ConversionContext) -> tf::Style { }; } + // Grid + if let Some(tracks) = css.grid_template_columns.as_ref() { + style.grid_template_columns = tracks + .iter() + .map(|t| tf::GridTemplateComponent::Single(grid_track_sizing(t, ctx))) + .collect(); + } + if let Some(tracks) = css.grid_template_rows.as_ref() { + style.grid_template_rows = tracks + .iter() + .map(|t| tf::GridTemplateComponent::Single(grid_track_sizing(t, ctx))) + .collect(); + } + if let Some(flow) = css.grid_auto_flow { + style.grid_auto_flow = match flow { + GridAutoFlow::Row => tf::GridAutoFlow::Row, + GridAutoFlow::Column => tf::GridAutoFlow::Column, + GridAutoFlow::RowDense => tf::GridAutoFlow::RowDense, + GridAutoFlow::ColumnDense => tf::GridAutoFlow::ColumnDense, + }; + } + if let Some(gc) = css.grid_column.as_ref() { + style.grid_column = grid_placement_line(gc); + } + if let Some(gr) = css.grid_row.as_ref() { + style.grid_row = grid_placement_line(gr); + } + // Overflow if let Some(o) = css.overflow { let v = overflow_to_taffy(o); @@ -324,6 +353,132 @@ fn border_widths( } } +/// Convert a [`GridTrack`] into a taffy `TrackSizingFunction` (used for both +/// the min and max sizing function, except `fr` which uses taffy's `flex()` +/// helper — `minmax(0, Nfr)`. This gives *exactly* evenly-sized tracks +/// regardless of child content, matching the `flex-direction: row` control +/// (`flex-grow: 1` siblings) rather than CSS's stricter `minmax(auto, Nfr)` +/// default, which lets content push a track wider than its fair share. +fn grid_track_sizing(t: &GridTrack, ctx: &ConversionContext) -> tf::TrackSizingFunction { + match t { + GridTrack::Fr(n) => tf::flex(*n), + GridTrack::Keyword(k) => grid_keyword_sizing(*k), + GridTrack::Length(lp) => grid_length_sizing(lp, ctx), + GridTrack::Minmax { min, max } => { + tf::minmax(grid_track_min(min, ctx), grid_track_max(max, ctx)) + } + } +} + +fn grid_keyword_sizing(k: GridTrackKeyword) -> tf::TrackSizingFunction { + match k { + GridTrackKeyword::Auto => tf::auto(), + GridTrackKeyword::MinContent => tf::min_content(), + GridTrackKeyword::MaxContent => tf::max_content(), + } +} + +fn grid_length_sizing(lp: &LengthPercentage, ctx: &ConversionContext) -> tf::TrackSizingFunction { + match lp.parse() { + ParsedLength::Fr(n) => tf::flex(n), + ParsedLength::Auto => tf::auto(), + ParsedLength::Px(p) => tf::length(p), + ParsedLength::Percent(p) => tf::percent(p / 100.0), + ParsedLength::Em(em) => tf::length(em * ctx.length.font_size), + ParsedLength::Rem(r) => tf::length(r * ctx.length.root_font_size), + ParsedLength::Vw(p) => tf::length(p / 100.0 * ctx.length.viewport_width), + ParsedLength::Vh(p) => tf::length(p / 100.0 * ctx.length.viewport_height), + } +} + +/// `min` side of an explicit `minmax(min, max)`. `fr` is not a valid CSS +/// minimum sizing function, so it falls back to `auto` (matches taffy's own +/// `MaxTrackSizingFunction -> MinTrackSizingFunction` conversion for `fr`). +fn grid_track_min(t: &GridTrack, ctx: &ConversionContext) -> tf::MinTrackSizingFunction { + match t { + GridTrack::Fr(_) => tf::auto(), + GridTrack::Keyword(GridTrackKeyword::Auto) => tf::auto(), + GridTrack::Keyword(GridTrackKeyword::MinContent) => tf::min_content(), + GridTrack::Keyword(GridTrackKeyword::MaxContent) => tf::max_content(), + GridTrack::Length(lp) => grid_length_min(lp, ctx), + // A `minmax` nested inside a `minmax` isn't valid CSS; degrade + // gracefully by taking the inner track's own min side. + GridTrack::Minmax { min, .. } => grid_track_min(min, ctx), + } +} + +fn grid_length_min(lp: &LengthPercentage, ctx: &ConversionContext) -> tf::MinTrackSizingFunction { + match lp.parse() { + ParsedLength::Fr(_) | ParsedLength::Auto => tf::auto(), + ParsedLength::Px(p) => tf::length(p), + ParsedLength::Percent(p) => tf::percent(p / 100.0), + ParsedLength::Em(em) => tf::length(em * ctx.length.font_size), + ParsedLength::Rem(r) => tf::length(r * ctx.length.root_font_size), + ParsedLength::Vw(p) => tf::length(p / 100.0 * ctx.length.viewport_width), + ParsedLength::Vh(p) => tf::length(p / 100.0 * ctx.length.viewport_height), + } +} + +/// `max` side of an explicit `minmax(min, max)`. +fn grid_track_max(t: &GridTrack, ctx: &ConversionContext) -> tf::MaxTrackSizingFunction { + match t { + GridTrack::Fr(n) => tf::fr(*n), + GridTrack::Keyword(GridTrackKeyword::Auto) => tf::auto(), + GridTrack::Keyword(GridTrackKeyword::MinContent) => tf::min_content(), + GridTrack::Keyword(GridTrackKeyword::MaxContent) => tf::max_content(), + GridTrack::Length(lp) => grid_length_max(lp, ctx), + GridTrack::Minmax { max, .. } => grid_track_max(max, ctx), + } +} + +fn grid_length_max(lp: &LengthPercentage, ctx: &ConversionContext) -> tf::MaxTrackSizingFunction { + match lp.parse() { + ParsedLength::Fr(n) => tf::fr(n), + ParsedLength::Auto => tf::auto(), + ParsedLength::Px(p) => tf::length(p), + ParsedLength::Percent(p) => tf::percent(p / 100.0), + ParsedLength::Em(em) => tf::length(em * ctx.length.font_size), + ParsedLength::Rem(r) => tf::length(r * ctx.length.root_font_size), + ParsedLength::Vw(p) => tf::length(p / 100.0 * ctx.length.viewport_width), + ParsedLength::Vh(p) => tf::length(p / 100.0 * ctx.length.viewport_height), + } +} + +/// Convert a `grid-column` / `grid-row` placement (`{ start, end, span }`) +/// into a taffy `Line`. Named lines / grid areas are out of +/// scope (issue #105) — only numeric line indices and spans are supported. +fn grid_placement_line(g: &GridLine) -> tf::Line { + let start = g.start.map(|GridLineEnd::Index(i)| i as i16); + let end = g.end.map(|GridLineEnd::Index(i)| i as i16); + match (start, end, g.span) { + (Some(s), Some(e), _) => tf::Line { + start: tf::line(s), + end: tf::line(e), + }, + (Some(s), None, Some(n)) => tf::Line { + start: tf::line(s), + end: tf::span(n), + }, + (Some(s), None, None) => tf::Line { + start: tf::line(s), + end: tf::auto(), + }, + (None, Some(e), Some(n)) => tf::Line { + start: tf::span(n), + end: tf::line(e), + }, + (None, Some(e), None) => tf::Line { + start: tf::auto(), + end: tf::line(e), + }, + (None, None, Some(n)) => tf::Line { + start: tf::span(n), + end: tf::auto(), + }, + (None, None, None) => tf::Line::auto(), + } +} + #[cfg(test)] mod tests { use super::*; @@ -416,4 +571,145 @@ mod tests { assert_eq!(s.overflow.x, taffy::Overflow::Hidden); assert_eq!(s.overflow.y, taffy::Overflow::Hidden); } + + // ---- Grid (issue #105) ---- + + #[test] + fn grid_display_translated() { + let css = CssStyle { + display: Some(Display::Grid), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert_eq!(s.display, tf::Display::Grid); + } + + #[test] + fn grid_template_columns_fr_tracks_translated() { + let css = CssStyle { + grid_template_columns: Some(vec![GridTrack::Fr(1.0), GridTrack::Fr(2.0)]), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert_eq!(s.grid_template_columns.len(), 2); + } + + #[test] + fn grid_template_rows_string_fr_tracks_translated() { + // Same string-encoded form used by examples/mega-showcase.json. + let css = CssStyle { + grid_template_rows: Some(vec![ + GridTrack::Length(LengthPercentage::String("1fr".into())), + GridTrack::Length(LengthPercentage::String("1fr".into())), + ]), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert_eq!(s.grid_template_rows.len(), 2); + } + + #[test] + fn grid_auto_flow_column_translated() { + let css = CssStyle { + grid_auto_flow: Some(GridAutoFlow::Column), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert_eq!(s.grid_auto_flow, tf::GridAutoFlow::Column); + } + + #[test] + fn grid_column_start_and_span_translated() { + let css = CssStyle { + grid_column: Some(GridLine { + start: Some(GridLineEnd::Index(2)), + span: Some(3), + ..Default::default() + }), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert!(matches!(s.grid_column.start, tf::GridPlacement::Line(_))); + assert!(matches!(s.grid_column.end, tf::GridPlacement::Span(3))); + } + + #[test] + fn grid_row_start_end_translated() { + let css = CssStyle { + grid_row: Some(GridLine { + start: Some(GridLineEnd::Index(1)), + end: Some(GridLineEnd::Index(3)), + ..Default::default() + }), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert!(matches!(s.grid_row.start, tf::GridPlacement::Line(_))); + assert!(matches!(s.grid_row.end, tf::GridPlacement::Line(_))); + } + + #[test] + fn grid_gap_applies_to_both_axes() { + let css = CssStyle { + display: Some(Display::Grid), + gap: Some(Gap::Uniform(LengthPercentage::Px(24.0))), + ..Default::default() + }; + let s = to_taffy_style(&css, &ctx()); + assert_eq!(s.gap.width, tf::LengthPercentage::length(24.0)); + assert_eq!(s.gap.height, tf::LengthPercentage::length(24.0)); + } + + /// The audit's core repro: a grid with three `1fr` columns must lay its + /// children out side-by-side (distinct x-offsets), matching a + /// `flex-direction: row` control — NOT stacked as three full-width rows, + /// which is what the engine did before `grid-template-columns` was wired + /// through to taffy (grid fell back to taffy's single-implicit-column + /// default because it never received any track definitions). + #[test] + fn grid_three_fr_columns_produce_distinct_x_offsets() { + let root_css = CssStyle { + display: Some(Display::Grid), + // The grid container needs a definite size: an `auto`-sized root + // (the CssStyle default) has no intrinsic content of its own, so + // it collapses to 0×0 and every column ends up at x=0. + width: Some(Size::Length(LengthPercentage::Px(900.0))), + height: Some(Size::Length(LengthPercentage::Px(300.0))), + grid_template_columns: Some(vec![ + GridTrack::Fr(1.0), + GridTrack::Fr(1.0), + GridTrack::Fr(1.0), + ]), + ..Default::default() + }; + let root_style = to_taffy_style(&root_css, &ctx()); + let leaf_style = to_taffy_style(&CssStyle::default(), &ctx()); + + let mut tree: tf::TaffyTree = tf::TaffyTree::new(); + let c1 = tree.new_leaf(leaf_style.clone()).unwrap(); + let c2 = tree.new_leaf(leaf_style.clone()).unwrap(); + let c3 = tree.new_leaf(leaf_style).unwrap(); + let root = tree.new_with_children(root_style, &[c1, c2, c3]).unwrap(); + + tree.compute_layout( + root, + tf::Size { + width: tf::AvailableSpace::Definite(900.0), + height: tf::AvailableSpace::Definite(300.0), + }, + ) + .unwrap(); + + let x1 = tree.layout(c1).unwrap().location.x; + let x2 = tree.layout(c2).unwrap().location.x; + let x3 = tree.layout(c3).unwrap().location.x; + let w1 = tree.layout(c1).unwrap().size.width; + + assert_ne!(x1, x2, "columns 1 and 2 must not share an x-offset"); + assert_ne!(x2, x3, "columns 2 and 3 must not share an x-offset"); + assert!((x1 - 0.0).abs() < 0.5, "column 1 should start at x=0, got {x1}"); + assert!((x2 - 300.0).abs() < 1.0, "column 2 should start at ~300px, got {x2}"); + assert!((x3 - 600.0).abs() < 1.0, "column 3 should start at ~600px, got {x3}"); + assert!((w1 - 300.0).abs() < 1.0, "each 1fr column should be ~300px wide, got {w1}"); + } } From 525a2937e909c851cceb5fa1dfbb314667baa807 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 16:43:26 +0200 Subject: [PATCH 3/6] feat(schema): export the component schema and load fonts before validating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `rustmotion schema` described none of the 57 components: Scene.children is a Vec, so schemars emitted "items": true — literally any JSON. Constrained generation was therefore impossible, which is the structural reason the authoring surface needs tens of thousands of words of prose rules to be usable. The full schema already existed internally, built by validate_attrs to populate a warning allowlist and never surfaced. Merge schema_for!(Component) into the exported document and type Scene.children as a $ref to it. Definitions go from 38 to 178, with a 57-variant oneOf. Validation also measured text through the Helvetica/Arial fallback while the render used the declared face, because load_custom_fonts only ran on render paths. Call it in run_checks, before any measurement. Delete LayerStyle and its orphaned siblings (BlendMode, Overflow, CardDisplay, Size). LayerStyle is the pre-CSS style model: dead code, but still exported and still carrying wrap/motion-path/size, which kept that vocabulary alive in the documentation long after the code stopped accepting it. Refs #106 --- crates/rustmotion-cli/src/commands/schema.rs | 63 ++++- .../rustmotion-cli/src/commands/validation.rs | 70 +++++ crates/rustmotion-core/src/schema/style.rs | 263 +----------------- crates/rustmotion-core/src/schema/video.rs | 44 +-- 4 files changed, 134 insertions(+), 306 deletions(-) diff --git a/crates/rustmotion-cli/src/commands/schema.rs b/crates/rustmotion-cli/src/commands/schema.rs index 9501080..3b52995 100644 --- a/crates/rustmotion-cli/src/commands/schema.rs +++ b/crates/rustmotion-cli/src/commands/schema.rs @@ -1,8 +1,9 @@ +use rustmotion::components::Component; use rustmotion::error::Result; use rustmotion::schema; pub fn cmd_schema(output: Option<&std::path::Path>) -> Result<()> { - let schema = schema::generate_json_schema(); + let schema = build_schema(); let json = serde_json::to_string_pretty(&schema)?; if let Some(path) = output { @@ -14,3 +15,63 @@ pub fn cmd_schema(output: Option<&std::path::Path>) -> Result<()> { Ok(()) } + +/// Build the full scenario schema, with `Scene.children` typed against the +/// real `Component` variants instead of `serde_json::Value` ("items: true"). +/// +/// `Scene.children` is `Vec` in `rustmotion-core` (schema +/// can't depend on `rustmotion-components`, which depends on it), so +/// schemars types it as "any JSON". `Component` — with its full `oneOf` over +/// all 57 component variants — is already built for internal use at +/// `validate_attrs.rs:40`; here we merge it into the scenario schema so the +/// exported schema actually documents the authoring surface. +fn build_schema() -> serde_json::Value { + let mut scenario_schema = schema::generate_json_schema(); + let component_schema = serde_json::to_value(schemars::schema_for!(Component)) + .expect("Component schema serializes"); + + let Some(component_defs) = component_schema + .get("definitions") + .and_then(|d| d.as_object()) + else { + return scenario_schema; + }; + + let Some(scenario_obj) = scenario_schema.as_object_mut() else { + return scenario_schema; + }; + let defs = scenario_obj + .entry("definitions") + .or_insert_with(|| serde_json::Value::Object(Default::default())); + let Some(defs_obj) = defs.as_object_mut() else { + return scenario_schema; + }; + + // Merge Component's auxiliary type definitions (CssStyle, AnimationEffect, + // etc. — the same underlying Rust types the scenario schema may already + // reference elsewhere). + for (k, v) in component_defs { + defs_obj.entry(k.clone()).or_insert_with(|| v.clone()); + } + + // Register `Component` itself (its `oneOf` over every variant) as a + // named definition. + let mut component_root = component_schema.clone(); + if let Some(obj) = component_root.as_object_mut() { + obj.remove("definitions"); + obj.remove("$schema"); + } + defs_obj.insert("Component".to_string(), component_root); + + // Point `Scene.children` at `Component` instead of the untyped + // `serde_json::Value` ("items: true"). + if let Some(children) = scenario_schema.pointer_mut("/definitions/Scene/properties/children") + { + *children = serde_json::json!({ + "type": "array", + "items": { "$ref": "#/definitions/Component" } + }); + } + + scenario_schema +} diff --git a/crates/rustmotion-cli/src/commands/validation.rs b/crates/rustmotion-cli/src/commands/validation.rs index 5733b4b..9c7cad4 100644 --- a/crates/rustmotion-cli/src/commands/validation.rs +++ b/crates/rustmotion-cli/src/commands/validation.rs @@ -10,6 +10,7 @@ //! 6. Schema-level checks (file existence, dimensions, durations, etc.) //! 7. Geometry checks (viewport overflow, wrap, auto_scroll) +use rustmotion::engine; use rustmotion::error::{Result, RustmotionError}; use rustmotion::include::{self, IncludeSource}; use rustmotion::schema::{ResolvedScenario, Scenario}; @@ -151,7 +152,16 @@ pub fn load_with_vars( /// Run all validation checks against a loaded scenario. When `strict_anim` /// is true, also sample animation frames and check that no widget's /// transformed bbox leaves the viewport. +/// +/// Registers any custom/Google fonts declared on the scenario *before* +/// running geometry checks, so text is measured with the same typeface the +/// render path will use. Without this, geometry checks measure through the +/// Helvetica → Arial → OS fallback chain regardless of what the scenario +/// declares — a systematic false-negative/false-positive source (issue #106). pub fn run_checks(loaded: &LoadedScenario, strict_anim: bool) -> ValidationReport { + if !loaded.scenario.fonts.is_empty() { + engine::renderer::load_custom_fonts(&loaded.scenario.fonts); + } let mut geom_violations = validate_geometry(&loaded.scenario); if strict_anim { geom_violations.extend(validate_geometry_animated(&loaded.scenario)); @@ -349,3 +359,63 @@ mod misplaced_animation_tests { assert!(w[0].contains("style.animation")); } } + +#[cfg(test)] +mod font_loading_wiring_tests { + use super::*; + use serde_json::json; + + /// H5 (issue #106): `run_checks` must register the scenario's declared + /// fonts before running geometry checks — matching what the render path + /// already does at `render.rs:47` — so validation and render resolve + /// fonts through the same call order instead of validation always + /// measuring through the fallback chain. A missing/unreadable font path + /// only warns (see `engine::renderer::fonts::register_font_file`); it + /// must not turn into a blocking validation error. + #[test] + fn run_checks_does_not_block_on_a_declared_font() { + let json = json!({ + "video": { "width": 200, "height": 200 }, + "fonts": [ + { "family": "DoesNotExist", "path": "does/not/exist.ttf" } + ], + "scenes": [{ + "duration": 1.0, + "children": [ + { "type": "text", "content": "hi", "style": { "color": "#fff" } } + ] + }] + }) + .to_string(); + + let loaded = load(ValidationSource::Inline(&json)).expect("scenario loads"); + let report = run_checks(&loaded, false); + assert!( + report.schema_errors.is_empty(), + "declared fonts must not block validation: {:?}", + report.schema_errors + ); + assert!(!report.is_blocking(false)); + } + + /// A scenario with no `fonts` entries must not attempt any font I/O + /// (the `!loaded.scenario.fonts.is_empty()` guard in `run_checks`). + #[test] + fn run_checks_skips_font_loading_when_no_fonts_declared() { + let json = json!({ + "video": { "width": 200, "height": 200 }, + "scenes": [{ + "duration": 1.0, + "children": [ + { "type": "text", "content": "hi", "style": { "color": "#fff" } } + ] + }] + }) + .to_string(); + + let loaded = load(ValidationSource::Inline(&json)).expect("scenario loads"); + assert!(loaded.scenario.fonts.is_empty()); + let report = run_checks(&loaded, false); + assert!(report.schema_errors.is_empty(), "{:?}", report.schema_errors); + } +} diff --git a/crates/rustmotion-core/src/schema/style.rs b/crates/rustmotion-core/src/schema/style.rs index 582544f..da63738 100644 --- a/crates/rustmotion-core/src/schema/style.rs +++ b/crates/rustmotion-core/src/schema/style.rs @@ -1,10 +1,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use super::video::{ - AnimationEffect, BlendMode, CardBorder, CardShadow, DropShadow, Fill, FilterConfig, - GradientBorder, Stroke, TextBackground, TextGradient, TextShadow, -}; +use super::video::AnimationEffect; // --- Card types --- @@ -76,15 +73,6 @@ impl Spacing { } } -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -#[derive(Default)] -pub enum CardDisplay { - #[default] - Flex, - Grid, -} - #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum GridTrack { @@ -206,19 +194,6 @@ pub enum VerticalAlign { Bottom, } -/// CSS-like overflow: controls whether children that exceed this container's -/// bounds are clipped (`hidden`) or allowed to bleed out (`visible`, default). -/// Validation only fails when content escapes the *viewport*; escaping a -/// `visible` parent is legitimate (e.g. a badge sticking out of a card). -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -#[derive(Default)] -pub enum Overflow { - #[default] - Visible, - Hidden, -} - /// Size dimension: fixed px, "auto", or "50%" (percent of parent) #[derive(Debug, Clone, JsonSchema)] pub enum SizeDimension { @@ -310,239 +285,3 @@ where deserializer.deserialize_any(OneOrMany) } - -// --- Unified LayerStyle --- - -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct LayerStyle { - // Common visual - #[serde(default = "default_opacity")] - pub opacity: f32, - #[serde(default)] - pub padding: Option, - #[serde(default)] - pub margin: Option, - #[serde(default)] - pub background: Option, - #[serde(default, rename = "border-radius")] - pub border_radius: Option, - #[serde(default)] - pub border: Option, - #[serde(default, rename = "box-shadow")] - pub box_shadow: Option, - #[serde(default, rename = "text-shadow")] - pub text_shadow: Option, - // Typography - #[serde(default, rename = "font-size")] - pub font_size: Option, - #[serde(default)] - pub color: Option, - #[serde(default, rename = "font-family")] - pub font_family: Option, - #[serde(default, rename = "font-weight")] - pub font_weight: Option, - #[serde(default, rename = "font-style")] - pub font_style: Option, - #[serde(default, rename = "text-align")] - pub text_align: Option, - #[serde(default, rename = "letter-spacing")] - pub letter_spacing: Option, - #[serde(default, rename = "line-height")] - pub line_height: Option, - // SVG/Shape - #[serde(default)] - pub stroke: Option, - #[serde(default)] - pub fill: Option, - // Flex container - #[serde(default, rename = "flex-direction")] - pub flex_direction: Option, - #[serde(default)] - pub gap: Option, - #[serde(default, rename = "align-items")] - pub align_items: Option, - #[serde(default, rename = "justify-content")] - pub justify_content: Option, - #[serde(default, rename = "flex-wrap")] - pub flex_wrap: Option, - #[serde(default)] - pub display: Option, - // Grid container - #[serde(default, rename = "grid-template-columns")] - pub grid_template_columns: Option>, - #[serde(default, rename = "grid-template-rows")] - pub grid_template_rows: Option>, - // Per-child flex - #[serde(default, rename = "flex-grow")] - pub flex_grow: Option, - #[serde(default, rename = "flex-shrink")] - pub flex_shrink: Option, - #[serde(default, rename = "flex-basis")] - pub flex_basis: Option, - #[serde(default, rename = "align-self")] - pub align_self: Option, - // Per-child grid - #[serde(default, rename = "grid-column")] - pub grid_column: Option, - #[serde(default, rename = "grid-row")] - pub grid_row: Option, - // Text highlight background - #[serde(default, rename = "text-background")] - pub text_background: Option, - // Glassmorphism / advanced effects. - // `backdrop-blur` / `inner-shadow` were removed (issue #87): they were - // never consumed — the working equivalents are the CSS `backdrop-filter` - // and `box-shadow` with `inset: true`. `CssStyle` still accepts them for - // compat and the validator warns. - #[serde(default, rename = "gradient-border")] - pub gradient_border: Option, - // Visual effects - #[serde(default)] - pub filter: Option, - #[serde(default, rename = "drop-shadow")] - pub drop_shadow: Option, - #[serde(default, rename = "blend-mode")] - pub blend_mode: Option, - #[serde(default, rename = "clip-path")] - pub clip_path: Option, - #[serde(default, rename = "aspect-ratio")] - pub aspect_ratio: Option, - #[serde(default, rename = "text-gradient")] - pub text_gradient: Option, - // Motion path: SVG path string that elements follow - #[serde(default, rename = "motion-path")] - pub motion_path: Option, - /// Wrap text content at the available width. None = use sensible default - /// per component (true for paragraphs, false for atomic labels like counter, - /// kbd, badge). Explicit `false` makes the geometry validator fail if the - /// natural width exceeds the constraint. - #[serde(default)] - pub wrap: Option, - /// CSS-like overflow on containers: `visible` (default) lets children bleed - /// out, `hidden` clips them. Viewport overflow is always a validation error - /// regardless of this property. - #[serde(default)] - pub overflow: Option, - // Stagger: automatic delay offset per child in a container (seconds) - #[serde(default)] - pub stagger: Option, - // Animation effects (accepts single object or array) - #[serde(default, deserialize_with = "deserialize_animation_effects")] - pub animation: Vec, - // Timeline: sequence of animation steps triggered at specific times. - // Each step has an `at` time and its own animation effects. - // When present, animations are resolved per-step instead of all at once. - #[serde(default)] - pub timeline: Vec, -} - -impl Default for LayerStyle { - fn default() -> Self { - Self { - opacity: 1.0, - padding: None, - margin: None, - background: None, - border_radius: None, - border: None, - box_shadow: None, - text_shadow: None, - font_size: None, - color: None, - font_family: None, - font_weight: None, - font_style: None, - text_align: None, - letter_spacing: None, - line_height: None, - stroke: None, - fill: None, - flex_direction: None, - gap: None, - align_items: None, - justify_content: None, - flex_wrap: None, - display: None, - grid_template_columns: None, - grid_template_rows: None, - flex_grow: None, - flex_shrink: None, - flex_basis: None, - align_self: None, - grid_column: None, - grid_row: None, - text_background: None, - gradient_border: None, - filter: None, - drop_shadow: None, - blend_mode: None, - clip_path: None, - aspect_ratio: None, - text_gradient: None, - motion_path: None, - stagger: None, - animation: Vec::new(), - timeline: Vec::new(), - wrap: None, - overflow: None, - } - } -} - -impl LayerStyle { - pub fn font_size_or(&self, default: f32) -> f32 { - self.font_size.unwrap_or(default) - } - pub fn color_or<'a>(&'a self, default: &'a str) -> &'a str { - self.color.as_deref().unwrap_or(default) - } - pub fn font_family_or<'a>(&'a self, default: &'a str) -> &'a str { - self.font_family.as_deref().unwrap_or(default) - } - pub fn font_weight_or(&self, default: FontWeight) -> FontWeight { - self.font_weight.clone().unwrap_or(default) - } - pub fn font_style_or(&self, default: FontStyleType) -> FontStyleType { - self.font_style.clone().unwrap_or(default) - } - pub fn text_align_or(&self, default: TextAlign) -> TextAlign { - self.text_align.clone().unwrap_or(default) - } - pub fn border_radius_or(&self, default: f32) -> f32 { - self.border_radius.unwrap_or(default) - } - pub fn gap_or(&self, default: f32) -> f32 { - self.gap.unwrap_or(default) - } - pub fn flex_direction_or(&self, default: CardDirection) -> CardDirection { - self.flex_direction.clone().unwrap_or(default) - } - pub fn align_items_or(&self, default: CardAlign) -> CardAlign { - self.align_items.clone().unwrap_or(default) - } - pub fn justify_content_or(&self, default: CardJustify) -> CardJustify { - self.justify_content.clone().unwrap_or(default) - } - pub fn flex_wrap_or(&self, default: bool) -> bool { - self.flex_wrap.unwrap_or(default) - } - pub fn display_or(&self, default: CardDisplay) -> CardDisplay { - self.display.clone().unwrap_or(default) - } - pub fn padding_resolved(&self) -> (f32, f32, f32, f32) { - self.padding - .as_ref() - .map(|p| p.resolve()) - .unwrap_or((0.0, 0.0, 0.0, 0.0)) - } - pub fn margin_resolved(&self) -> (f32, f32, f32, f32) { - self.margin - .as_ref() - .map(|m| m.resolve()) - .unwrap_or((0.0, 0.0, 0.0, 0.0)) - } -} - -fn default_opacity() -> f32 { - 1.0 -} diff --git a/crates/rustmotion-core/src/schema/video.rs b/crates/rustmotion-core/src/schema/video.rs index 0687584..f1d1235 100644 --- a/crates/rustmotion-core/src/schema/video.rs +++ b/crates/rustmotion-core/src/schema/video.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use super::animation::{Animation, AnimationPreset, EasingType, PresetConfig, SpringConfig}; use super::style::{FontWeight, TextAlign, VerticalAlign}; -// --- Animation effects (nested inside LayerStyle as typed array) --- +// --- Animation effects (nested inside CssStyle as typed array) --- /// A single animation effect. Discriminated by `"type"` in JSON. /// Each preset name is a valid type, plus special types: glow, wiggle, keyframes, motion_blur. @@ -473,23 +473,6 @@ pub struct WiggleConfig { // --- Supporting types --- -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -pub struct Size { - #[serde(default = "default_size_dim")] - pub width: f32, - #[serde(default = "default_size_dim")] - pub height: f32, -} - -impl Default for Size { - fn default() -> Self { - Self { - width: 100.0, - height: 100.0, - } - } -} - #[derive(Debug, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum ShapeType { @@ -735,27 +718,6 @@ fn default_glow_intensity() -> f32 { 1.0 } -/// Blend mode for layer compositing -#[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub enum BlendMode { - Multiply, - Screen, - Overlay, - Darken, - Lighten, - ColorDodge, - ColorBurn, - HardLight, - SoftLight, - Difference, - Exclusion, - Hue, - Saturation, - Color, - Luminosity, -} - /// Gradient fill for text #[derive(Debug, Clone, Serialize, Deserialize, JsonSchema)] pub struct TextGradient { @@ -778,10 +740,6 @@ fn default_font_family() -> String { "Inter".to_string() } -fn default_size_dim() -> f32 { - 100.0 -} - fn default_stroke_width() -> f32 { 2.0 } From d4b75f14511db5d7a9d6cd1fc36968fd94ccb730 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 16:44:06 +0200 Subject: [PATCH 4/6] fix(validate): make --fix non-destructive and close geometry blind spots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --fix wrote style.wrap on an unwrappable_text_overflow. CssStyle is deny_unknown_fields and has no wrap field, so the component failed to deserialise and was dropped from the render — and because the geometry walker then saw nothing left to check, validation reported clean. The auto-fixer turned text that overflowed into text that was absent, and certified it. Remove style.white-space instead, which falls back to the wrapping default. Violation paths indexed the post-filter child vector while navigate() indexed raw JSON, so a single malformed sibling made --fix patch an unrelated component. Preserve the raw index. container_clips was computed and never read. Honouring it fixes both directions at once: content clipped by an ancestor is no longer a false positive, which is what made deliberate frame-bleeding typography — the signature of the reference style — impossible to author. Its old heuristic also treated a background as clipping, which would have suppressed real overflow on any card that had one. Add ContentOverflowsBox: wrapping text measured against the box it was actually assigned. A paragraph in a fixed-height card paints far outside it while both boxes stay inside the frame, so the viewport check never saw it. --strict-anim discarded t=0 through an opacity guard, and every entrance preset ramps opacity from zero — so the frame of maximum displacement was always the one skipped. It also forked the renderer's effect resolution and re-timed on start_at, which the renderer does not do. Reuse effective_effects and resolve_props_for_effects, and sample proportionally to scene duration. Static css.transform and non-keyframed scene.camera now contribute to bboxes; both were silently ignored. Camera folding is skipped when a scene uses per-plane depth, where the global formula would not apply. Refs #103 --- .../rustmotion-cli/src/commands/geometry.rs | 1158 +++++++++++++++-- .../rustmotion-cli/src/commands/validate.rs | 168 ++- 2 files changed, 1201 insertions(+), 125 deletions(-) diff --git a/crates/rustmotion-cli/src/commands/geometry.rs b/crates/rustmotion-cli/src/commands/geometry.rs index 5f515dd..328a08b 100644 --- a/crates/rustmotion-cli/src/commands/geometry.rs +++ b/crates/rustmotion-cli/src/commands/geometry.rs @@ -1,33 +1,43 @@ //! Geometry validator — walks the resolved box tree of every scene and //! reports nodes whose absolute bounding box leaves the device viewport. //! -//! Scope of v1: -//! * detect absolute positions placed past the viewport edge +//! Scope: +//! * detect absolute positions placed past the viewport edge, folding in +//! static `style.transform` and a static (non-keyframed) `scene.camera` +//! pan/zoom (H5, partial: rotation/skew/3D and keyframed camera motion +//! are not modeled) //! * detect text components whose unwrapped natural width exceeds the -//! allocated width when `wrap: false` is set +//! allocated width when `white-space: nowrap`/`pre` is set //! * detect terminal/codeblock content that overflows their box when //! `auto_scroll: false` //! * exempt `marquee` and `cursor` (designed to bleed) +//! * never report a node clipped by an `overflow: hidden`/`clip`/`scroll`/ +//! `auto` ancestor as a viewport overflow (H4) — the ancestor's own bbox +//! is still checked independently, at its own level //! //! Animation handling is layered: by default we only check the resting //! (untransformed) layout. With `--strict-anim`, we additionally sample -//! frames and reapply the same canvas transforms the renderer uses to verify -//! each frame stays inside the viewport. +//! frames — proportionally to scene duration — and reapply the *real* +//! renderer's animation resolution (`effective_effects` + +//! `resolve_props_for_effects`) plus the paint pass's start_at/end_at +//! visibility window, rather than a hand-rolled fork of that logic (H6). //! //! This walker runs the new CSS-engine pipeline (taffy + cosmic-text) so the //! geometry it checks matches what the renderer will actually paint. -use rustmotion::components::box_builder::build_scene_from_refs; +use std::collections::HashSet; + +use rustmotion::components::box_builder::{build_scene_from_refs, effective_effects}; use rustmotion::components::intrinsic::TextIntrinsic; use rustmotion::components::{ChildComponent, Component}; +use rustmotion::core::css::style::{CssStyle, TransformFn}; use rustmotion::core::css::taffy_bridge::ConversionContext; +use rustmotion::core::css::units::LengthContext; use rustmotion::core::engine::box_tree::{AvailableSpace, BoxNode, IntrinsicMeasure}; use rustmotion::core::engine::layout_pass::{run_layout, BoxLayout, LayoutResult}; -use rustmotion::engine::animator::{ - apply_orbits, apply_wiggles, extract_effects, resolve_animations, AnimatedProperties, -}; +use rustmotion::engine::animator::{resolve_props_for_effects, AnimatedProperties}; use rustmotion::engine::render; -use rustmotion::schema::ResolvedScenario; +use rustmotion::schema::{Camera, ResolvedScenario, Scene}; use serde::Serialize; /// One detected layout violation. @@ -63,10 +73,18 @@ pub enum Axis { pub enum ViolationKind { /// Component bbox crosses the viewport edge. ViewportOverflow, - /// `wrap: false` set but the natural width exceeds the allocated width. + /// `white-space: nowrap`/`pre` set but the natural width exceeds the + /// allocated width. UnwrappableTextOverflow, /// terminal/codeblock has `auto_scroll: false` but content > box. AutoScrollDisabledOverflow, + /// Wrapping text's content, measured at the width its own box was + /// actually assigned, needs more width (an unbreakable word/token) or + /// height (wrapped lines) than that box's `content_box()` — e.g. a + /// paragraph whose parent has a fixed `height` too small for it. Text + /// painters never clip themselves, so this paints outside its box + /// regardless of where that box sits relative to the viewport. + ContentOverflowsBox, /// Animated transform (scale/translate/wiggle/orbit) pushes the bbox out /// of the viewport at some sampled time. Only emitted with `--strict-anim`. AnimatedTextOverflow, @@ -85,7 +103,9 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec let mut violations = Vec::new(); for (vi, view) in scenario.views.iter().enumerate() { for (si, scene) in view.scenes.iter().enumerate() { - let children = render::deserialize_children(scene); + let indexed = deserialize_children_indexed(scene); + let raw_indices: Vec = indexed.iter().map(|(i, _)| *i).collect(); + let children: Vec = indexed.into_iter().map(|(_, c)| c).collect(); let viewport = (scenario.video.width, scenario.video.height); let viewport_f = (viewport.0 as f32, viewport.1 as f32); @@ -93,6 +113,8 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec let built = build_scene_from_refs(children.iter(), viewport_f, root_css, None); let layouts = run_layout(&built.root, viewport_f, &ConversionContext::default()); + let camera = scene.camera.as_ref().filter(|_| !scene_uses_depth(&children)); + let path_root = format!("views[{}].scenes[{}]", vi, si); walk( &children, @@ -102,7 +124,12 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec vi, si, &path_root, - /*parent_clips=*/ true, // scene root always clips + Some(&raw_indices), + // Nothing clips top-level scene children but the viewport + // frame itself — and that's exactly what check_viewport + // tests, so top level must not be pre-suppressed. + /*parent_clips=*/ false, + camera, &mut violations, ); } @@ -110,6 +137,42 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec violations } +/// Deserialize a scene's raw JSON children like `render::deserialize_children`, +/// but keep each survivor's index into the RAW `scene.children` array. +/// +/// `render::deserialize_children` filters failures out and returns a plain +/// `Vec`, so a plain `.enumerate()` over its output drifts from +/// `scene.children` as soon as an earlier sibling fails to deserialize. +/// `apply_fixes`/`navigate` in `validate.rs` walk the RAW JSON array, so a +/// path built from the drifted index patches the wrong sibling (H3). We +/// duplicate the (trivial) filter-and-skip here so violation paths always +/// resolve to the JSON node we actually measured. +fn deserialize_children_indexed(scene: &Scene) -> Vec<(usize, ChildComponent)> { + scene + .children + .iter() + .enumerate() + .filter_map(|(i, v)| { + serde_json::from_value::(v.clone()) + .ok() + .map(|c| (i, c)) + }) + .collect() +} + +/// True when any top-level child declares an explicit `style.depth` — the v1 +/// parallax-plane rule (mirrors the private `scene_uses_depth` in +/// `engine/render/scene.rs`, reimplemented here since that one isn't `pub`). +/// When depth planes are in play the renderer applies a per-plane, +/// depth-scaled camera instead of the single global transform +/// `fold_static_camera` models, so callers skip camera folding entirely in +/// that case rather than risk a wrong correction. +fn scene_uses_depth(children: &[ChildComponent]) -> bool { + children + .iter() + .any(|c| c.component.as_styled().style_config().depth.is_some()) +} + #[allow(clippy::too_many_arguments)] fn walk( children: &[ChildComponent], @@ -119,21 +182,43 @@ fn walk( vi: usize, si: usize, path: &str, - _parent_clips: bool, + // Maps loop position -> raw JSON array index. `None` at nested levels, + // where the container's own `Vec` field is strict serde + // (fails the whole parent rather than skipping one bad element), so + // loop position already matches the JSON array position. + path_indices: Option<&[usize]>, + parent_clips: bool, + camera: Option<&Camera>, out: &mut Vec, ) { + let viewport_f = (viewport.0 as f32, viewport.1 as f32); for (i, (child, box_node)) in children.iter().zip(boxes.iter()).enumerate() { - let child_path = format!("{}.children[{}]", path, i); + let json_idx = path_indices.map(|idxs| idxs[i]).unwrap_or(i); + let child_path = format!("{}.children[{}]", path, json_idx); let layout = match layouts.get(box_node.id) { Some(l) => l, None => continue, }; - let bbox = bbox_of(layout); + let raw_bbox = bbox_of(layout); if !is_exempted(&child.component) { - check_viewport(&child.component, &child_path, &bbox, viewport, vi, si, out); - check_unwrappable_text(&child.component, &child_path, &bbox, viewport, vi, si, out); - check_auto_scroll(&child.component, &child_path, &bbox, viewport, vi, si, out); + if !parent_clips { + let mut vbbox = apply_static_node_transform(&raw_bbox, &box_node.css, viewport_f); + if let Some(cam) = camera { + vbbox = fold_static_camera(&vbbox, cam, viewport_f); + } + check_viewport(&child.component, &child_path, &vbbox, viewport, vi, si, out); + } + check_unwrappable_text(&child.component, &child_path, &raw_bbox, viewport, vi, si, out); + check_auto_scroll(&child.component, &child_path, &raw_bbox, viewport, vi, si, out); + // Suppressed under a clipping ancestor (parent_clips) exactly + // like check_viewport, and when the node clips its own overflow + // (paint_pass applies a node's own `overflow: hidden`/clip/ + // scroll/auto BEFORE painting its own content, at step 4 — + // self-clipping is real, not just a container->children thing). + if !parent_clips && !container_clips(&child.component) { + check_content_overflows_box(&child.component, &child_path, layout, viewport, vi, si, out); + } } if let Some(grandchildren) = container_children(&child.component) { @@ -145,7 +230,9 @@ fn walk( vi, si, &child_path, - /*parent_clips=*/ container_clips(&child.component), + None, + parent_clips || container_clips(&child.component), + camera, out, ); } @@ -176,15 +263,104 @@ fn container_children(c: &Component) -> Option<&[ChildComponent]> { } } -/// Whether a container clips its children. Used for nested overflow checks -/// (out of scope for v1: we only validate against the viewport, never the -/// parent box, since CSS-style overflow:visible is the default). +/// Whether a container clips its children to its own box — CSS `overflow` +/// semantics (H4). A node inside a clipping ancestor is bounded by that +/// ancestor before it can ever reach the viewport edge, so `check_viewport` +/// skips it entirely; the ancestor's own bbox is still checked independently, +/// at its own level in `walk`/`walk_anim`. Mirrors exactly what the paint +/// pass clips on (`Overflow::Hidden | Clip | Scroll | Auto`) — a plain +/// `background` does NOT imply clipping in CSS, so it is deliberately not a +/// trigger here. fn container_clips(c: &Component) -> bool { let style = c.as_styled().style_config(); matches!( style.overflow, - Some(rustmotion::core::css::style::Overflow::Hidden) - ) || style.background.is_some() // card with background already clips + Some( + rustmotion::core::css::style::Overflow::Hidden + | rustmotion::core::css::style::Overflow::Clip + | rustmotion::core::css::style::Overflow::Scroll + | rustmotion::core::css::style::Overflow::Auto + ) + ) +} + +/// Static (build-time, non-animated) CSS `transform` fold (H5, partial) — +/// approximates the paint pass's `apply_transform` for the axis-aligned +/// subset (translate + scale around the bbox center). Rotation/skew/3D are +/// intentionally ignored: an exact AABB under rotation needs the four +/// transformed corners, out of scope for a partial fix. Animated +/// transform-producing presets are folded separately in `walk_anim`; this +/// only handles what a component declares directly in `style.transform`. +fn apply_static_node_transform(bbox: &BBox, css: &CssStyle, viewport: (f32, f32)) -> BBox { + let transform = match css.transform.as_deref() { + Some(t) if !t.is_empty() => t, + _ => return *bbox, + }; + let ctx = LengthContext { + viewport_width: viewport.0, + viewport_height: viewport.1, + parent_size: bbox.w.max(bbox.h), + font_size: 16.0, + root_font_size: 16.0, + }; + let mut tx = 0.0f32; + let mut ty = 0.0f32; + let mut sx = 1.0f32; + let mut sy = 1.0f32; + for f in transform { + match f { + TransformFn::Translate { x, y } => { + tx += x.resolve(&ctx); + ty += y.resolve(&ctx); + } + TransformFn::TranslateX { x } => tx += x.resolve(&ctx), + TransformFn::TranslateY { y } => ty += y.resolve(&ctx), + TransformFn::Scale { x, y } => { + sx *= x; + sy *= y; + } + TransformFn::ScaleX { x } => sx *= x, + TransformFn::ScaleY { y } => sy *= y, + // Rotate/Skew/3D/Z: doesn't fold cleanly into an axis-aligned + // bbox delta — left untransformed (H5 is explicitly partial). + _ => {} + } + } + let cx = bbox.x + bbox.w / 2.0; + let cy = bbox.y + bbox.h / 2.0; + let new_w = bbox.w * sx.abs(); + let new_h = bbox.h * sy.abs(); + BBox { + x: cx - new_w / 2.0 + tx, + y: cy - new_h / 2.0 + ty, + w: new_w, + h: new_h, + } +} + +/// Static (non-keyframed) global scene-camera fold (H5, partial) — mirrors +/// `apply_camera_transform` in `engine/render/scene.rs` for the +/// non-rotated case: `device = zoom*p + (1-zoom)*origin - zoom*pan`. +/// Rotation and keyframed camera motion are ignored. Callers only pass a +/// camera here when the scene isn't using per-plane depth parallax (see +/// `scene_uses_depth`) — that path applies a *different*, depth-scaled +/// camera per top-level plane, and folding the global formula there would +/// be wrong. +fn fold_static_camera(bbox: &BBox, camera: &Camera, viewport: (f32, f32)) -> BBox { + let zoom = camera.zoom; + let (cx, cy) = camera + .origin + .as_ref() + .map(|o| (o.x, o.y)) + .unwrap_or((viewport.0 / 2.0, viewport.1 / 2.0)); + let new_x = zoom * bbox.x + (1.0 - zoom) * cx - zoom * camera.x; + let new_y = zoom * bbox.y + (1.0 - zoom) * cy - zoom * camera.y; + BBox { + x: new_x, + y: new_y, + w: bbox.w * zoom, + h: bbox.h * zoom, + } } fn check_viewport( @@ -235,7 +411,8 @@ fn hint_for_viewport(component: &Component, axis: Axis, bbox: &BBox, vp: (u32, u let vh = vp.1 as f32; match component_kind(component) { "text" | "rich_text" | "gradient_text" | "caption" => { - "set wrap: true on the text or reduce font_size".to_string() + "allow the text to wrap (remove style.white-space: nowrap) or reduce style.font-size" + .to_string() } "counter" => format!( "card width must be ≥ {:.0}px (counter natural width)", @@ -298,7 +475,7 @@ fn check_unwrappable_text( bbox: *bbox, viewport, hint: format!( - "text natural width is {:.0}px but only {:.0}px available — set wrap: true or reduce font_size", + "text natural width is {:.0}px but only {:.0}px available — remove style.white-space: nowrap (or set it to normal) so it can wrap, or reduce style.font-size", natural_w, bbox.w ), }); @@ -306,6 +483,107 @@ fn check_unwrappable_text( } } +/// H4 (second half): content larger than its *own* content box, independent +/// of where that box sits relative to the viewport. `check_viewport` only +/// catches a box escaping the *frame*; it says nothing about a box whose +/// declared size is simply too small for what's inside it — e.g. a card with +/// a fixed `height` shorter than the paragraph it wraps. Text painters never +/// clip themselves and `overflow: visible` (the CSS default) applies no +/// clip in the paint pass, so that paragraph paints straight out of the +/// card with zero signal from any other check. +/// +/// Complementary to `check_unwrappable_text`, not overlapping with it: +/// that one covers `white-space: nowrap`/`pre` (single unwrapped line, width +/// only, measured at natural/unconstrained width). This one covers the +/// default wrapping case — measured at the width the box actually *has* +/// (`content_box().2`, unconstrained height) so it also catches a single +/// unbreakable word/token/URL that's wider than the box even though wrap is +/// on (wrapping can't break within a word), plus the width axis stays +/// consistent with what will actually be painted. +fn check_content_overflows_box( + component: &Component, + path: &str, + layout: &BoxLayout, + viewport: (u32, u32), + vi: usize, + si: usize, + out: &mut Vec, +) { + if let Component::Text(t) = component { + let nowrap = matches!( + t.style.white_space, + Some( + rustmotion::core::css::style::WhiteSpace::Nowrap + | rustmotion::core::css::style::WhiteSpace::Pre + ) + ); + // nowrap/pre is check_unwrappable_text's territory: re-measuring it + // here at a constrained width would wrap text that will actually + // paint as one (too-wide) line, producing a height number that + // doesn't correspond to anything that gets painted. + if nowrap { + return; + } + + let (cx, cy, cw, ch) = layout.content_box(); + if cw <= 0.0 || ch <= 0.0 { + return; + } + + let intrinsic = TextIntrinsic::from_text(t); + let (measured_w, measured_h) = intrinsic.measure( + (None, None), + (AvailableSpace::Definite(cw), AvailableSpace::MaxContent), + ); + + let eps = 0.5; + let x_over = measured_w > cw + eps; + let y_over = measured_h > ch + eps; + if !x_over && !y_over { + return; + } + let axis = match (x_over, y_over) { + (true, true) => Axis::Both, + (true, false) => Axis::X, + (false, true) => Axis::Y, + (false, false) => return, + }; + let content_bbox = BBox { + x: cx, + y: cy, + w: cw, + h: ch, + }; + let hint = if y_over && !x_over { + format!( + "text wraps to {:.0}px tall at this width but its box is only {:.0}px tall — increase style.height (or the parent's), reduce style.font-size, or shorten the content", + measured_h, ch + ) + } else if x_over && !y_over { + format!( + "an unbreakable word/token needs {:.0}px but the box is only {:.0}px wide — widen the box, reduce style.font-size, or insert a break (e.g. a space) in the long token", + measured_w, cw + ) + } else { + format!( + "content needs {:.0}×{:.0}px but its box is only {:.0}×{:.0}px — widen/heighten the box or reduce style.font-size", + measured_w, measured_h, cw, ch + ) + }; + out.push(GeometryViolation { + view_index: vi, + scene_index: si, + path: path.to_string(), + component: "text".to_string(), + axis, + kind: ViolationKind::ContentOverflowsBox, + bbox: content_bbox, + viewport, + hint, + }); + } +} + fn check_auto_scroll( component: &Component, path: &str, @@ -435,46 +713,75 @@ fn component_kind(c: &Component) -> &'static str { // ─── Animated overflow sampling (--strict-anim) ───────────────────────────── -/// Number of timestamps sampled per scene when `strict_anim` is enabled. We -/// sample at quarters of the scene duration plus the boundaries, which is -/// enough to catch the worst case for typical entrance/exit presets without -/// blowing up validation time. -const ANIM_SAMPLES: &[f64] = &[0.0, 0.15, 0.3, 0.5, 0.7, 0.85, 1.0]; +/// Samples per second of scene duration. ~8/s (125ms resolution) is dense +/// enough to land inside the high-risk window right after an entrance +/// preset's delay — where opacity has started ramping up but translate/scale +/// is still near its most extreme — without either a fixed sample count +/// (misses short bursts in long scenes) or a fixed interval (wastes cycles +/// on long, mostly-static scenes). +const ANIM_SAMPLES_PER_SECOND: f64 = 8.0; +const ANIM_MIN_SAMPLES: usize = 5; +const ANIM_MAX_SAMPLES: usize = 40; -/// Walk every scene at multiple sampled times, apply the resolved animation -/// transform to each widget's bbox, and report viewport overflows. Only -/// emits `AnimatedTextOverflow` violations: the resting-layout checks live in -/// `validate_geometry`. +/// Sample times (seconds, scene-relative) for `--strict-anim`, spaced evenly +/// across `[0, scene_duration]`. Count scales with `scene_duration` (H6) — +/// more samples for longer scenes — and is clamped to keep validation time +/// bounded. +fn anim_sample_times(scene_duration: f64) -> Vec { + if scene_duration <= 0.0 { + return vec![0.0]; + } + let raw = (scene_duration * ANIM_SAMPLES_PER_SECOND).ceil() as usize; + let n = raw.clamp(ANIM_MIN_SAMPLES, ANIM_MAX_SAMPLES); + if n <= 1 { + return vec![0.0]; + } + (0..n) + .map(|i| scene_duration * (i as f64) / ((n - 1) as f64)) + .collect() +} + +/// Walk every scene at multiple sampled times, apply the *real* renderer's +/// animation resolution to each widget's bbox, and report viewport +/// overflows. Only emits `AnimatedTextOverflow` violations: the +/// resting-layout checks live in `validate_geometry`. pub fn validate_geometry_animated(scenario: &ResolvedScenario) -> Vec { let mut violations = Vec::new(); - let mut seen: std::collections::HashSet<(usize, usize, String)> = - std::collections::HashSet::new(); + let mut seen: HashSet<(usize, usize, String)> = HashSet::new(); for (vi, view) in scenario.views.iter().enumerate() { for (si, scene) in view.scenes.iter().enumerate() { - let children = render::deserialize_children(scene); + let indexed = deserialize_children_indexed(scene); + let raw_indices: Vec = indexed.iter().map(|(i, _)| *i).collect(); + let children: Vec = indexed.into_iter().map(|(_, c)| c).collect(); let viewport = (scenario.video.width, scenario.video.height); let viewport_f = (viewport.0 as f32, viewport.1 as f32); // Use the resting layout as the base bbox. Animation transforms // (translate/scale/wiggle/orbit) are applied analytically per - // sample — they don't reflow taffy. + // sample — they don't reflow taffy, matching how the paint pass + // applies them after layout. let root_css = render::root_style(scene.layout.as_ref()); let built = build_scene_from_refs(children.iter(), viewport_f, root_css, None); let layouts = run_layout(&built.root, viewport_f, &ConversionContext::default()); + let camera = scene.camera.as_ref().filter(|_| !scene_uses_depth(&children)); + let path_root = format!("views[{}].scenes[{}]", vi, si); let scene_duration = scene.duration; - for ratio in ANIM_SAMPLES { - let time = scene_duration * *ratio; + for time in anim_sample_times(scene_duration) { walk_anim( &children, &built.root.children, &layouts, + &built.stagger_delays, viewport, vi, si, &path_root, + Some(&raw_indices), + /*parent_clips=*/ false, + camera, time, scene_duration, &mut seen, @@ -491,26 +798,67 @@ fn walk_anim( children: &[ChildComponent], boxes: &[BoxNode], layouts: &LayoutResult, + stagger_delays: &[f64], viewport: (u32, u32), vi: usize, si: usize, path: &str, + path_indices: Option<&[usize]>, + parent_clips: bool, + camera: Option<&Camera>, time: f64, scene_duration: f64, - seen: &mut std::collections::HashSet<(usize, usize, String)>, + seen: &mut HashSet<(usize, usize, String)>, out: &mut Vec, ) { + let viewport_f = (viewport.0 as f32, viewport.1 as f32); for (i, (child, box_node)) in children.iter().zip(boxes.iter()).enumerate() { - let child_path = format!("{}.children[{}]", path, i); + let json_idx = path_indices.map(|idxs| idxs[i]).unwrap_or(i); + let child_path = format!("{}.children[{}]", path, json_idx); let layout = match layouts.get(box_node.id) { Some(l) => l, None => continue, }; - let base_bbox = bbox_of(layout); - if !is_exempted(&child.component) && layout.width > 0.5 && layout.height > 0.5 { - let props = resolve_props_for_validation(&child.component, time, scene_duration); - if let Some(transformed) = transform_bbox(&base_bbox, &props) { + // Visibility gate: identical to what `paint_node` checks before + // painting — a start_at/end_at window, already resolved (with + // stagger folded in) at box-tree build time. No manual re-timing: + // per PR #27, start_at/end_at gate paint visibility only, and the + // effects resolved below run at absolute scene time exactly like the + // renderer does. A node outside its window is not painted — subtree + // included — so we skip it (and its descendants) entirely, just like + // `paint_node` does; nothing invisible can overflow. + let visible = box_node.window.as_ref().is_none_or(|w| w.contains(time)); + if !visible { + continue; + } + + if !is_exempted(&child.component) + && !parent_clips + && layout.width > 0.5 + && layout.height > 0.5 + { + let stagger_delay = stagger_delays + .get(box_node.id as usize) + .copied() + .unwrap_or(0.0); + // Reuse the renderer's own effect resolution instead of a + // divergent fork (H6): `effective_effects` merges timeline + // steps/synthesized transitions/stagger exactly like + // `legacy_dispatch.rs` does, and `resolve_props_for_effects` + // resolves them at absolute scene `time` — no re-timing by + // start_at, which would double-shift components that also + // declare a matching `animation` delay. + let props = match effective_effects(&child.component, stagger_delay) { + Some(effects) => resolve_props_for_effects(&effects, time, scene_duration), + None => AnimatedProperties::default(), + }; + let raw_bbox = bbox_of(layout); + let base_bbox = apply_static_node_transform(&raw_bbox, &box_node.css, viewport_f); + if let Some(mut transformed) = transform_bbox(&base_bbox, &props) { + if let Some(cam) = camera { + transformed = fold_static_camera(&transformed, cam, viewport_f); + } let vw = viewport.0 as f32; let vh = viewport.1 as f32; let eps = 0.5; @@ -550,10 +898,14 @@ fn walk_anim( grandchildren, &box_node.children, layouts, + stagger_delays, viewport, vi, si, &child_path, + None, + parent_clips || container_clips(&child.component), + camera, time, scene_duration, seen, @@ -563,77 +915,6 @@ fn walk_anim( } } -/// Resolve animation properties for a component at a specific time. This is a -/// validation-flavored copy of the renderer's logic in -/// `engine::render::render_component`: we deliberately ignore motion paths, -/// timeline steps, char animations, and rotation, which are too renderer-bound -/// to emulate cheaply. The returned props cover the cases that can shift a -/// widget's bbox out of the viewport (translate, scale, wiggle, orbit). -fn resolve_props_for_validation( - component: &Component, - time: f64, - scene_duration: f64, -) -> AnimatedProperties { - let effects = component - .as_animatable() - .map(|a| a.animation_effects()) - .unwrap_or(&[]); - if effects.is_empty() { - return AnimatedProperties::default(); - } - let extracted = extract_effects(effects); - - // Apply timing offset (start_at / end_at). We mirror the renderer rule: - // animations only run inside [start_at, end_at]; outside, props default. - let anim_time = if let Some(timed) = component.as_timed() { - let (start_at, end_at) = timed.timing(); - if let Some(start) = start_at { - if time < start { - return AnimatedProperties::default(); - } - } - if let Some(end) = end_at { - if time > end { - return AnimatedProperties::default(); - } - } - let base = if let Some(start) = start_at { - time - start - } else { - time - }; - base.max(0.0) - } else { - time - }; - - let mut props = AnimatedProperties::default(); - for (preset, preset_config) in &extracted.presets { - let p = resolve_animations( - &[], - Some(preset), - Some(preset_config), - anim_time, - scene_duration, - ); - props.merge(&p); - } - if !extracted.keyframes.is_empty() { - let kf: Vec<_> = extracted.keyframes.iter().copied().cloned().collect(); - let kp = resolve_animations(&kf, None, None, anim_time, scene_duration); - props.merge(&kp); - } - if !extracted.wiggles.is_empty() { - let wiggles: Vec<_> = extracted.wiggles.into_iter().cloned().collect(); - apply_wiggles(&mut props, &wiggles, time); - } - if !extracted.orbits.is_empty() { - let orbits: Vec<_> = extracted.orbits.into_iter().cloned().collect(); - apply_orbits(&mut props, &orbits, time); - } - props -} - /// Apply the canvas transforms the renderer applies (translate then scale /// around the bbox center) to a base bbox. Returns `None` if the resulting /// box is degenerate (fully transparent / zero size). @@ -702,6 +983,7 @@ pub fn format_violation(v: &GeometryViolation) -> String { ViolationKind::ViewportOverflow => "viewport overflow", ViolationKind::UnwrappableTextOverflow => "wrap=false but text too wide", ViolationKind::AutoScrollDisabledOverflow => "auto_scroll=false but content too tall", + ViolationKind::ContentOverflowsBox => "wrapped content exceeds its own box", ViolationKind::AnimatedTextOverflow => "animation pushes content outside viewport", }; format!( @@ -889,4 +1171,642 @@ mod tests { ); assert_eq!(v.unwrap().component, "codeblock"); } + + // ─── C1: remediation hints must never name the nonexistent `wrap` field ── + + #[test] + fn unwrappable_text_hint_does_not_recommend_the_nonexistent_wrap_field() { + // `wrap` is not a `CssStyle` field (the real property is + // `white-space`); recommending it in a hint is what used to drive + // the destructive `--fix` (C1). + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "card", + "x": 100, "y": 100, + "style": { "width": "200px", "height": "200px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "this string is too long to fit", + "style": { "color": "#ffffff", "font-size": "96px", "white-space": "nowrap" } + }] + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::UnwrappableTextOverflow) + .expect("violation"); + assert!( + !v.hint.contains("wrap: true"), + "hint still recommends the dropped `wrap` field: {}", + v.hint + ); + assert!( + !v.hint.contains(" font_size"), + "hint still names the non-existent bare `font_size` field: {}", + v.hint + ); + assert!( + v.hint.contains("white-space"), + "hint should point at the real property: {}", + v.hint + ); + } + + // ─── H3: violation path must reference the RAW JSON index ──────────────── + + #[test] + fn violation_path_skips_the_raw_json_index_of_a_dropped_sibling() { + // Child #0 fails to deserialize (unknown component type) and is + // dropped. Without the H3 fix, the walker would re-enumerate the + // *filtered* vector and report this violation as `children[0]`, + // which in the raw JSON is the broken sibling, not the card that + // actually overflows. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [ + { "type": "not_a_real_component_kind" }, + { + "type": "card", + "x": 100, "y": 100, + "style": { "width": "200px", "height": "200px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "this string is too long to fit", + "style": { "color": "#ffffff", "font-size": "96px", "white-space": "nowrap" } + }] + } + ] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::UnwrappableTextOverflow) + .expect("violation"); + assert_eq!( + v.path, "views[0].scenes[0].children[1].children[0]", + "path must reference the raw JSON index (1), not the post-filter index (0)" + ); + } + + // ─── H4: overflow:hidden ancestor suppresses viewport checks, visible doesn't ── + + fn oversized_card_with_shape(overflow: &str) -> String { + format!( + r##"{{ + "video": {{ "width": 1920, "height": 1080 }}, + "scenes": [{{ + "duration": 1.0, + "children": [{{ + "type": "card", + "position": "absolute", + "x": 1700, "y": 100, + "style": {{ "width": "400px", "height": "200px", "overflow": "{overflow}" }}, + "children": [{{ + "type": "shape", + "shape": "rect", + "style": {{ "width": "100%", "height": "100%" }}, + "fill": "#ff0000" + }}] + }}] + }}] + }}"## + ) + } + + #[test] + fn shape_overflow_suppressed_by_hidden_ancestor_but_not_by_visible_one() { + // Card at x=1700, width=400 → right edge 2100, past the 1920 + // viewport: the card itself is still reported either way. The shape + // fills the card (100%/100%), so its own bbox tracks the card's. + // With `overflow: hidden` the shape is fully inside a clipping + // ancestor and must not ALSO be reported (H4). With the CSS default + // `overflow: visible`, nothing suppresses it. + let hidden = parse(&oversized_card_with_shape("hidden")); + let visible = parse(&oversized_card_with_shape("visible")); + + let hidden_violations = validate_geometry(&hidden); + assert!( + hidden_violations + .iter() + .any(|v| v.component == "card" && v.kind == ViolationKind::ViewportOverflow), + "the card itself must still be reported: {:?}", + hidden_violations + ); + assert!( + hidden_violations.iter().all(|v| v.component != "shape"), + "shape clipped by an overflow:hidden ancestor must not be reported: {:?}", + hidden_violations + ); + + let visible_violations = validate_geometry(&visible); + assert!( + visible_violations + .iter() + .any(|v| v.component == "shape" && v.kind == ViolationKind::ViewportOverflow), + "overflow:visible (the CSS default) must not suppress the check: {:?}", + visible_violations + ); + } + + // ─── H7: deliberate frame-bleeding typography inside a clipping plane ──── + + #[test] + fn oversized_type_inside_full_frame_hidden_plane_is_clean() { + // A full-viewport container with `overflow: hidden` — the + // reference "1600-style brutalist" pattern of bleeding huge type off + // frame. The text's own box spans from x=-100 to x=1300 in a + // 1080-wide viewport (bleeds on both edges) but is clipped by the + // plane, so it must not be reported. + let json = r##"{ + "video": { "width": 1080, "height": 1920 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "container", + "style": { "width": "100%", "height": "100%", "overflow": "hidden" }, + "children": [{ + "type": "text", + "content": "BOLD", + "position": "absolute", + "x": -100, "y": 700, + "style": { "color": "#ffffff", "font-size": "420px", "width": "1400px" } + }] + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations.is_empty(), + "type bleeding off a full-frame overflow:hidden plane should validate clean: {:?}", + violations + ); + } + + // ─── H5: static css.transform and scene.camera fold into the bbox ──────── + + #[test] + fn static_css_transform_is_folded_into_the_viewport_check() { + // Shape sits safely inside the viewport at rest (right edge 1800 < + // 1920), but a static `transform: translateX(200px)` pushes it out. + // Before H5 this was a silent false negative: geometry only looked + // at the taffy layout box, never at `css.transform`. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 1700, "y": 100, + "style": { + "width": "100px", "height": "100px", + "transform": [{ "fn": "translate-x", "x": "200px" }] + }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .any(|v| v.component == "shape" && v.kind == ViolationKind::ViewportOverflow), + "static transform should push the shape out of the viewport: {:?}", + violations + ); + } + + #[test] + fn static_scene_camera_zoom_is_folded_into_the_viewport_check() { + // Shape sits safely inside the viewport at rest (right edge 1900 < + // 1920), but the scene's static 2x camera zoom (around the frame + // centre) pushes it out. Before H5, `scene.camera` was completely + // ignored by geometry. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "camera": { "zoom": 2.0 }, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 1800, "y": 100, + "style": { "width": "100px", "height": "100px" }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .any(|v| v.component == "shape" && v.kind == ViolationKind::ViewportOverflow), + "camera zoom should push the shape out of the viewport: {:?}", + violations + ); + } + + #[test] + fn camera_fold_is_skipped_when_scene_uses_per_plane_depth() { + // With a `style.depth` plane, the renderer applies a *different*, + // depth-scaled camera per plane instead of the single global + // transform `fold_static_camera` models. Applying the global formula + // there would be wrong, so geometry must not fold `scene.camera` in + // this mode. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "camera": { "zoom": 2.0 }, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 1800, "y": 100, + "style": { "width": "100px", "height": "100px", "depth": 1.0 }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations.iter().all(|v| v.component != "shape"), + "camera must not be folded in per-plane depth mode: {:?}", + violations + ); + } + + // ─── H6: --strict-anim reuses the renderer's effect resolution ─────────── + + #[test] + fn strict_anim_detects_slide_in_overflow() { + // slide_in_left eases position.x from -200 to 0; a shape resting at + // x=100 gets pushed to a strongly negative x during the first + // fraction of the preset, after opacity has started ramping up (its + // own keyframe window is only the first 30% of the duration). + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 2.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 100, "y": 100, + "style": { + "width": "100px", "height": "100px", + "animation": [{ "name": "slide_in_left", "delay": 0, "duration": 1.0 }] + }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry_animated(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::AnimatedTextOverflow); + assert!( + v.is_some(), + "slide_in_left should push the shape past the left edge mid-animation: {:?}", + violations + ); + assert_eq!(v.unwrap().axis, Axis::X); + } + + #[test] + fn strict_anim_start_at_and_effect_delay_do_not_double_shift_the_timeline() { + // A component with BOTH `start_at` (visibility gate) and a matching + // animation `delay` — a common authoring pattern ("appear and + // animate in at the same moment"). The old hand-rolled fork + // re-based time by subtracting start_at *again* before resolving + // the preset, which is already absolute-time-shifted by its own + // `delay`. That double shift pushed every sample right after + // start_at below the preset's first keyframe, resolving to the + // untouched opacity=0 state and silently discarding a genuine + // overflow through the opacity guard (H6). Reusing + // `resolve_props_for_effects` at raw (absolute) scene time — like + // the renderer does — must not reproduce that. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 2.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 100, "y": 100, + "start_at": 0.5, + "style": { + "width": "100px", "height": "100px", + "animation": [{ "name": "slide_in_left", "delay": 0.5, "duration": 1.0 }] + }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry_animated(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::AnimatedTextOverflow); + assert!( + v.is_some(), + "overflow shortly after start_at must be caught, not masked by the opacity guard: {:?}", + violations + ); + } + + #[test] + fn strict_anim_respects_start_at_gate_before_visibility() { + // A short slide-in (delay=0, duration=0.3s) settles well before + // start_at=1.5s makes the component visible in a 2s scene. Once + // visible, effects resolve at absolute time — by t=1.5 the preset + // finished at t=0.3, so props are already at rest. Nothing should + // be flagged. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 2.0, + "children": [{ + "type": "shape", + "shape": "rect", + "position": "absolute", + "x": 100, "y": 100, + "start_at": 1.5, + "style": { + "width": "100px", "height": "100px", + "animation": [{ "name": "slide_in_left", "delay": 0, "duration": 0.3 }] + }, + "fill": "#ff0000" + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry_animated(&scenario); + assert!( + violations.is_empty(), + "component settles to rest well before start_at; nothing should be flagged: {:?}", + violations + ); + } + + #[test] + fn anim_sample_times_scale_with_scene_duration() { + let short = anim_sample_times(0.5); + let long = anim_sample_times(4.0); + assert!( + long.len() > short.len(), + "longer scenes should get more samples: {} vs {}", + long.len(), + short.len() + ); + assert!( + short.first().copied().unwrap().abs() < 1e-9, + "must include t=0" + ); + assert!( + (short.last().copied().unwrap() - 0.5).abs() < 1e-9, + "must include the scene end" + ); + assert!(long.len() <= ANIM_MAX_SAMPLES, "sample count must stay bounded"); + } + + // ─── H4 (second half): content larger than its own content box ─────────── + // + // The first half of H4 (already fixed above) suppresses a *viewport* + // check when a clipping ancestor is in the way. This half is a different + // bug: a box can sit entirely inside the viewport and still have content + // that paints outside *itself*, because text painters never clip their + // own overflow and `overflow: visible` (the CSS default) applies no clip + // in the paint pass. `check_viewport` alone never sees this. + + #[test] + fn wrapped_text_taller_than_its_fixed_height_card_is_flagged() { + // Exact repro from the audit: a card comfortably inside a 960x540 + // frame (x=330,y=200,w=300,h=80 -> right/bottom edges 630/280, both + // well inside frame) with a paragraph that, wrapped at the card's + // ~300px content width, needs ~343px of height — but the card is + // fixed at 80px. No viewport check ever fires (card and text both + // report a resting bbox inside the frame); this is purely a + // content-vs-own-box mismatch. + let json = r##"{"video":{"width":960,"height":540,"fps":30,"background":"#0A0A12"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"card","position":"absolute","x":330,"y":200, + "style":{"width":300,"height":80,"background":"#1e2233","overflow":"visible"}, + "children":[{"type":"text", + "content":"Ce paragraphe est beaucoup plus grand que la carte de 80px qui le contient.", + "style":{"font-size":44,"color":"#ffffff"}}]}]}]}"##; + let scenario = parse(json); + + // Sanity check: this scenario must NOT already fail some other way + // (e.g. viewport overflow) — the whole point is that it looks clean + // to every other check. + let viewport_violations: Vec<_> = validate_geometry(&scenario) + .into_iter() + .filter(|v| v.kind == ViolationKind::ViewportOverflow) + .collect(); + assert!( + viewport_violations.is_empty(), + "fixture should be viewport-clean by construction: {:?}", + viewport_violations + ); + + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::ContentOverflowsBox) + .unwrap_or_else(|| { + panic!( + "expected ContentOverflowsBox for text taller than its fixed-height card, got: {:?}", + violations + ) + }); + assert_eq!(v.component, "text"); + assert_eq!( + v.axis, + Axis::Y, + "card is wide enough (paragraph wraps to ~294px < 300px) — only height should overflow: {:?}", + v + ); + assert!( + v.hint.contains("height"), + "hint should point at the height mismatch: {}", + v.hint + ); + } + + #[test] + fn unbreakable_long_token_wider_than_its_box_is_flagged_even_with_wrap_on() { + // A single unbroken run with no whitespace or punctuation break + // opportunities (a long hash/id, not a URL — cosmic-text's wrapper + // treats `/`/`-` as break points, which would defeat the test) + // can't be broken by word-wrap, so even with the CSS default + // `white-space: normal`, it paints wider than a too-narrow box. + // + // Note the explicit `width: 150px` directly on the *text*, not just + // the card: empirically, an auto-sized flex child's cross-axis width + // floors at its min-content size (the widest unbreakable run) and + // simply escapes a non-clipping parent instead of being clamped — + // correct CSS flex behavior (`min-width: auto` on flex items), and + // not a bug this check is meant to catch (an element escaping a + // *non-clipping* intermediate container, while staying inside the + // viewport, is ordinary CSS — check_viewport is the check for hard + // frame-edge violations). An *explicit* size is a hard author + // constraint that IS supposed to be honored exactly regardless of + // content — cosmic-text still overflows it, which is the real bug. + // Card is tall enough that height is not the issue — only width + // should fire. + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "card", + "position": "absolute", + "x": 100, "y": 100, + "style": { "width": "150px", "height": "600px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "style": { "color": "#ffffff", "font-size": "24px", "width": "150px" } + }] + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::ContentOverflowsBox) + .unwrap_or_else(|| panic!("expected ContentOverflowsBox for an unbreakable long token: {:?}", violations)); + assert_eq!(v.axis, Axis::X, "the 600px-tall box rules out a height overflow: {:?}", v); + } + + #[test] + fn wrapped_text_that_fits_its_box_is_not_flagged() { + // Passing-case guard: same paragraph as the failing test above, but + // the card is tall enough (400px) to hold the ~343px wrapped + // content — must NOT fire. Proves the check compares against the + // actual resolved box, not some fixed threshold. + let json = r##"{"video":{"width":960,"height":540,"fps":30,"background":"#0A0A12"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"card","position":"absolute","x":330,"y":80, + "style":{"width":300,"height":400,"background":"#1e2233","overflow":"visible"}, + "children":[{"type":"text", + "content":"Ce paragraphe est beaucoup plus grand que la carte de 80px qui le contient.", + "style":{"font-size":44,"color":"#ffffff"}}]}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .all(|v| v.kind != ViolationKind::ContentOverflowsBox), + "content that fits its box must not be flagged: {:?}", + violations + ); + } + + #[test] + fn content_overflow_is_suppressed_under_a_clipping_ancestor() { + // Same overflowing paragraph/80px-card fixture, but the card clips + // via `overflow: hidden` — the paragraph genuinely gets clipped at + // paint time, so reporting it would be a false positive, consistent + // with the parent_clips suppression already applied to + // check_viewport. + let json = r##"{"video":{"width":960,"height":540,"fps":30,"background":"#0A0A12"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"card","position":"absolute","x":330,"y":200, + "style":{"width":300,"height":80,"background":"#1e2233","overflow":"hidden"}, + "children":[{"type":"text", + "content":"Ce paragraphe est beaucoup plus grand que la carte de 80px qui le contient.", + "style":{"font-size":44,"color":"#ffffff"}}]}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .all(|v| v.kind != ViolationKind::ContentOverflowsBox), + "content clipped by an overflow:hidden ancestor must not be flagged: {:?}", + violations + ); + } + + #[test] + fn content_overflow_is_suppressed_when_the_node_clips_itself() { + // Same fixture, but this time the TEXT node itself (not the card) + // declares `overflow: hidden`. paint_pass applies a node's own + // overflow clip before painting its own content (step 4, before + // step 8's component-specific paint) — self-clipping is real, so + // this must not be flagged either. + let json = r##"{"video":{"width":960,"height":540,"fps":30,"background":"#0A0A12"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"card","position":"absolute","x":330,"y":200, + "style":{"width":300,"height":80,"background":"#1e2233","overflow":"visible"}, + "children":[{"type":"text", + "content":"Ce paragraphe est beaucoup plus grand que la carte de 80px qui le contient.", + "style":{"font-size":44,"color":"#ffffff","overflow":"hidden"}}]}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .all(|v| v.kind != ViolationKind::ContentOverflowsBox), + "content clipped by its own overflow:hidden must not be flagged: {:?}", + violations + ); + } + + #[test] + fn nowrap_text_is_not_double_reported_by_content_overflows_box() { + // The nowrap/pre case belongs entirely to check_unwrappable_text; + // this check must defer to it rather than also firing (which would + // both double-report the same real bug AND compute a height number + // that doesn't correspond to what nowrap actually paints — a single + // line, not a wrapped block). + let json = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "card", + "x": 100, "y": 100, + "style": { "width": "200px", "height": "200px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "this string is too long to fit", + "style": { "color": "#ffffff", "font-size": "96px", "white-space": "nowrap" } + }] + }] + }] + }"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .all(|v| v.kind != ViolationKind::ContentOverflowsBox), + "nowrap text must be reported only as UnwrappableTextOverflow, not also ContentOverflowsBox: {:?}", + violations + ); + assert!(violations + .iter() + .any(|v| v.kind == ViolationKind::UnwrappableTextOverflow)); + } } diff --git a/crates/rustmotion-cli/src/commands/validate.rs b/crates/rustmotion-cli/src/commands/validate.rs index f242102..6fec979 100644 --- a/crates/rustmotion-cli/src/commands/validate.rs +++ b/crates/rustmotion-cli/src/commands/validate.rs @@ -116,10 +116,14 @@ fn apply_fixes(root: &mut serde_json::Value, violations: &[GeometryViolation]) - }; match v.kind { ViolationKind::UnwrappableTextOverflow => { - if let Some(obj) = target.as_object_mut() { - let style = obj.entry("style").or_insert_with(|| serde_json::json!({})); - if let Some(style_obj) = style.as_object_mut() { - style_obj.insert("wrap".into(), serde_json::Value::Bool(true)); + // `wrap` is not a `CssStyle` field — `CssStyle` is + // `deny_unknown_fields`, so writing it silently drops the + // whole component at the next parse (C1). The real property + // is `white-space`; removing `nowrap`/`pre` falls back to + // the schema default (`normal`, i.e. wrapping), which is + // always a valid, non-destructive mutation. + if let Some(style_obj) = target.get_mut("style").and_then(|s| s.as_object_mut()) { + if style_obj.remove("white-space").is_some() { applied += 1; } } @@ -130,9 +134,14 @@ fn apply_fixes(root: &mut serde_json::Value, violations: &[GeometryViolation]) - applied += 1; } } - ViolationKind::ViewportOverflow | ViolationKind::AnimatedTextOverflow => { + ViolationKind::ViewportOverflow + | ViolationKind::AnimatedTextOverflow + | ViolationKind::ContentOverflowsBox => { // Position/size clamping is too risky to auto-fix without - // losing intent — leave it for the user. + // losing intent — leave it for the user. (ContentOverflowsBox + // specifically: growing the box, shrinking the font, or + // shortening the copy are all legitimate fixes with very + // different visual outcomes — not ours to pick.) } } } @@ -182,3 +191,150 @@ fn parse_segments(path: &str) -> Vec<(String, usize)> { } out } + +#[cfg(test)] +mod tests { + use super::*; + use super::super::geometry::{validate_geometry, Axis, BBox}; + use rustmotion::components::Component; + use rustmotion::engine::render; + use rustmotion::loader::load_scenario_from_source; + + const NARROW_CARD_JSON: &str = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "card", + "x": 100, "y": 100, + "style": { "width": "200px", "height": "200px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "this string is too long to fit", + "style": { "color": "#ffffff", "font-size": "96px", "white-space": "nowrap" } + }] + }] + }] + }"##; + + fn unwrappable_violation(path: &str) -> GeometryViolation { + GeometryViolation { + view_index: 0, + scene_index: 0, + path: path.to_string(), + component: "text".to_string(), + axis: Axis::X, + kind: ViolationKind::UnwrappableTextOverflow, + bbox: BBox { + x: 0.0, + y: 0.0, + w: 200.0, + h: 40.0, + }, + viewport: (1920, 1080), + hint: String::new(), + } + } + + /// C1: `apply_fixes` must never write `style.wrap` (not a `CssStyle` + /// field — writing it drops the whole component at the next parse + /// because `CssStyle` is `deny_unknown_fields`). It must instead remove + /// `white-space: nowrap`, and the fixed file must still parse with the + /// text component intact. + #[test] + fn fix_removes_white_space_and_never_writes_the_nonexistent_wrap_field() { + let mut json: serde_json::Value = serde_json::from_str(NARROW_CARD_JSON).unwrap(); + let violations = vec![unwrappable_violation( + "views[0].scenes[0].children[0].children[0]", + )]; + let applied = apply_fixes(&mut json, &violations); + assert_eq!(applied, 1); + + let style = &json["scenes"][0]["children"][0]["children"][0]["style"]; + assert!( + style.get("wrap").is_none(), + "must never write the nonexistent CssStyle::wrap field: {}", + style + ); + assert!( + style.get("white-space").is_none(), + "white-space: nowrap must be removed, not left in place: {}", + style + ); + + // The fixed file must still parse, and the text must still be + // present — a `deny_unknown_fields` rejection would have silently + // dropped it (C1's original failure mode). + let pretty = serde_json::to_string(&json).unwrap(); + let scenario = + load_scenario_from_source(None, Some(&pretty)).expect("fixed scenario still parses"); + let top_children = render::deserialize_children(&scenario.views[0].scenes[0]); + assert_eq!(top_children.len(), 1, "card must survive the fix"); + let text_survived = match &top_children[0].component { + Component::Card(c) => c.children.len() == 1, + _ => false, + }; + assert!(text_survived, "text child must survive the fix, not be dropped"); + + // The fix must also clear the geometry violation it targeted. + let after = validate_geometry(&scenario); + assert!( + after + .iter() + .all(|v| v.kind != ViolationKind::UnwrappableTextOverflow), + "fix must clear the violation: {:?}", + after + ); + } + + /// H3: the violation path (produced by geometry.rs's raw-index-preserving + /// walker) must reference the RAW JSON position of the offending node, + /// so `navigate()`/`apply_fixes` mutate the right sibling even when an + /// earlier child never became a `ChildComponent`. + #[test] + fn fix_patches_the_raw_json_sibling_even_when_an_earlier_child_failed_to_deserialize() { + let raw = r##"{ + "video": { "width": 1920, "height": 1080 }, + "scenes": [{ + "duration": 1.0, + "children": [ + { "type": "not_a_real_component_kind" }, + { + "type": "card", + "x": 100, "y": 100, + "style": { "width": "200px", "height": "200px", "background": "#222244" }, + "children": [{ + "type": "text", + "content": "this string is too long to fit", + "style": { "color": "#ffffff", "font-size": "96px", "white-space": "nowrap" } + }] + } + ] + }] + }"##; + let mut json: serde_json::Value = serde_json::from_str(raw).unwrap(); + let violations = vec![unwrappable_violation( + "views[0].scenes[0].children[1].children[0]", + )]; + let applied = apply_fixes(&mut json, &violations); + assert_eq!(applied, 1); + + // children[0] (the broken sibling) must be untouched. + assert_eq!( + json["scenes"][0]["children"][0]["type"], + "not_a_real_component_kind" + ); + // children[1] (the card) is the one that got fixed. + let style = &json["scenes"][0]["children"][1]["children"][0]["style"]; + assert!(style.get("white-space").is_none()); + assert!(style.get("wrap").is_none()); + } + + #[test] + fn navigate_resolves_implicit_single_slide_view_at_index_zero() { + let mut json: serde_json::Value = serde_json::from_str(NARROW_CARD_JSON).unwrap(); + let target = navigate(&mut json, "views[0].scenes[0].children[0].children[0]"); + assert!(target.is_some(), "must resolve into the implicit view 0"); + assert_eq!(target.unwrap()["type"], "text"); + } +} From 321a2636081cc388adfc672e3c353c1f720412ce Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 16:44:06 +0200 Subject: [PATCH 5/6] docs(skills): realign the authoring guide with the schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skills corpus is the de-facto schema an LLM generates from, and it had drifted onto the pre-CSS style model. Because CssStyle is deny_unknown_fields, every stale property silently drops the whole component — so the guide was teaching people to write invisible scenarios. Its canonical Example 1 produced a dropped shape. Move fill/stroke, timeline/stagger and text-background to component root; fix box-shadow to an array with kebab keys; flex-wrap to an enum; transform to an array of functions; reduce grid-template-columns to its one valid syntax; remove motion-path and style.size, which do not exist. Rewrite geometry-safety around white-space, document the fifth violation kind and the clipping exemptions, and correct --fix now that it is safe to use. Resolve the contradictions: position inside card, end_at on counters (the checklist forbade it and two examples used it), the preset counts (31/45/39, actually 40) and the easing count (11, actually 17 plus cubic_bezier). Component count 51 to 57, qrcode to qr_code, and document qr_code, waveform and audio_spectrum, which appeared nowhere. Link the orphaned rules and reference examples/, which is more accurate than the prose was: extracting the 217 fenced JSON blocks and validating each takes failures from 26 to 14, with the remainder being illustrative paths and deliberately-invalid teaching examples. Refs #107 --- .claude/skills/rustmotion/SKILL.md | 154 ++++++++++-------- .../skills/rustmotion/rules/color-palettes.md | 19 ++- .../skills/rustmotion/rules/depth-layering.md | 10 +- .../rustmotion/rules/easing-guidelines.md | 6 +- .../rustmotion/rules/geometry-safety.md | 35 ++-- .../skills/rustmotion/rules/glassmorphism.md | 7 +- .../rustmotion/rules/icon-sizing-hierarchy.md | 21 ++- .../rustmotion/rules/module-structure.md | 2 +- .../skills/rustmotion/rules/prefer-presets.md | 2 +- .../rules/responsive-device-sizing.md | 4 +- .../rustmotion/rules/text-background.md | 14 +- .../skills/rustmotion/rules/validate-json.md | 8 +- .../rustmotion/rules/wiggle-additive.md | 3 +- CLAUDE.md | 16 +- 14 files changed, 179 insertions(+), 122 deletions(-) diff --git a/.claude/skills/rustmotion/SKILL.md b/.claude/skills/rustmotion/SKILL.md index 9fd0636..2b6d758 100644 --- a/.claude/skills/rustmotion/SKILL.md +++ b/.claude/skills/rustmotion/SKILL.md @@ -216,7 +216,7 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con - [rules/html-css-mental-model.md](rules/html-css-mental-model.md) - **CRITICAL:** Think HTML/CSS — flow layout first, absolute only for decorative/overlay elements - [rules/validate-json.md](rules/validate-json.md) - Always validate generated JSON with `rustmotion validate` before presenting -- [rules/geometry-safety.md](rules/geometry-safety.md) - Keep all content inside the viewport: `wrap`, `auto_scroll`, `overflow` semantics + violation kinds +- [rules/geometry-safety.md](rules/geometry-safety.md) - Keep all content inside the viewport: `white-space`, `auto_scroll`, `overflow` semantics + violation kinds - [rules/even-dimensions.md](rules/even-dimensions.md) - Use even width/height for H.264 encoding - [rules/counter-standalone.md](rules/counter-standalone.md) - Counter must be standalone (no baseline correction inside cards) - [rules/vertical-align.md](rules/vertical-align.md) - Shape text vertical_align: use "top"/"middle"/"bottom" (NOT "center") @@ -228,7 +228,7 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con - [rules/icon-format.md](rules/icon-format.md) - Icons use Iconify (200k+ icons), format "prefix:name" (e.g. "lucide:home") - [rules/grid-card-height.md](rules/grid-card-height.md) - Grid containers need explicit height (not "auto") to prevent row stretching - [rules/wiggle-additive.md](rules/wiggle-additive.md) - Wiggle is additive on top of presets and keyframes -- [rules/prefer-presets.md](rules/prefer-presets.md) - Prefer presets over manual keyframes (39 built-in presets) +- [rules/prefer-presets.md](rules/prefer-presets.md) - Prefer presets over manual keyframes (40 built-in presets + 6 char-only) - [rules/hex-colors.md](rules/hex-colors.md) - Colors in hex format only (#RRGGBB or #RRGGBBAA) - [rules/easing-guidelines.md](rules/easing-guidelines.md) - Easing guidelines for motion design - [rules/text-background.md](rules/text-background.md) - text-background renders a colored rectangle behind text @@ -253,18 +253,33 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con - [rules/icon-sizing-hierarchy.md](rules/icon-sizing-hierarchy.md) - Icon sizing (hero/card/inline roles), card spacing minimums, row layout by device - [rules/depth-layering.md](rules/depth-layering.md) - **NEW:** Visual depth — 3 planes (bg/mid/fg), z-index, blur, shadow hierarchy, scale gradient, 3D tilt - [rules/dynamic-depth.md](rules/dynamic-depth.md) - **NEW:** Multi-element parallax — wiggle seeds, float_3d preset, camera zoom, orbit phases, frequency hierarchy -- [rules/component-field-placement.md](rules/component-field-placement.md) - **CRITICAL:** Field placement (root vs style) — `animation`, `fill`, `stroke` at root; `box-shadow` as array; silently-dropped component pitfalls +- [rules/component-field-placement.md](rules/component-field-placement.md) - **CRITICAL:** Field placement (root vs style) — `width`/`height`/`animation` inside `style`; `fill`/`stroke`/`timeline`/`stagger` at root; `box-shadow` as array; silently-dropped component pitfalls - [rules/badge-video-sizing.md](rules/badge-video-sizing.md) - Badge sizing for video resolution — `badge_size` sm/md/lg is too small at 1080px; use `style.font-size` to override (40px recommended for 1080×1920) +- [rules/glassmorphism.md](rules/glassmorphism.md) - Frosted-glass card recipe: `backdrop-filter: blur`, translucent background, subtle border, layered over a colorful background +- [rules/audio-reactive.md](rules/audio-reactive.md) - Bind `style.audio-reactive` to an `audio` track — drives `waveform`/`audio_spectrum` and reactive scale/opacity on any component +- [rules/captions-workflow.md](rules/captions-workflow.md) - Generating `caption` word timings from a transcript/audio track +- [rules/time-remapping.md](rules/time-remapping.md) - `time_scale`/`time_offset` on containers — slow-motion, freeze-frame, and time-shifted children ### Architecture (pour contribuer au code) - [rules/paint-context.md](rules/paint-context.md) - Painter trait API: paint_content(canvas, layout, props, ctx) — remplace l'ancien Widget -- [rules/module-structure.md](rules/module-structure.md) - Structure des crates: rustmotion-core (css/, engine/, traits/) + rustmotion-components (51 composants) +- [rules/module-structure.md](rules/module-structure.md) - Structure des crates: rustmotion-core (css/, engine/, traits/) + rustmotion-components (57 composants) --- ## Complete Examples +The two examples below are short excerpts. For full, validated, end-to-end scenarios to study or copy from, see the `examples/` directory at the repo root — these are ahead of this prose (they use `fill`/`stroke`/`timeline`/`stagger` at root, correct `grid-template-columns` syntax, etc.) and are re-validated on every change: + +| File | Resolution | Scenes | What it demonstrates | +|---|---|---|---| +| `examples/demo.json` | 1080×1920 | 6 | Minimal skeleton — just `video` + solid-color scenes | +| `examples/component-showcase.json` | 1920×1080 | 4 | Broad tour of basic + data-viz + UI components | +| `examples/dynamic-glass.json` | 1920×1080 | 3 | Glassmorphism, `backdrop-filter`, depth layering | +| `examples/rustmotion-promo.json` | 1920×1080 | 6 | Product promo pacing, stagger, char animations | +| `examples/ferriskey-presentation.json` | 1920×1080 | 6 | Slide-deck style presentation, heavy char/word stagger | +| `examples/mega-showcase.json` | 1920×1080 | 9 | Largest example — grid layout, timeline component, most component types in one file | + ### Example 1: Marketing Card (Portrait) ```json @@ -278,14 +293,14 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con { "type": "shape", "shape": "rounded_rect", + "fill": { + "type": "linear", + "colors": ["#6366f1", "#8b5cf6"], + "angle": 135 + }, "style": { "width": 900, "height": 520, - "fill": { - "type": "linear", - "colors": ["#6366f1", "#8b5cf6"], - "angle": 135 - }, "border-radius": 32, "animation": [{ "name": "scale_in", "duration": 0.6 }] } @@ -418,7 +433,6 @@ Read individual rule files for detailed explanations, GOOD/BAD examples, and con "separator": ",", "easing": "ease_out", "start_at": 0.3, - "end_at": 3.5, "style": { "font-size": 96, "color": "#38BDF8", "font-weight": "bold", "text-align": "center" } }, { @@ -722,7 +736,7 @@ All components are discriminated by `"type"`. Rendered in array order (first = b | Effect name | Fields | Description | | --- | --- | --- | -| *preset name* | `delay`, `duration`, `loop`, `overshoot` | Any of the 39 presets (e.g. `fade_in_up`, `scale_in`) | +| *preset name* | `delay`, `duration`, `loop`, `overshoot` | Any of the 40 presets (e.g. `fade_in_up`, `scale_in`) | | *char preset* | `delay`, `duration`, `stagger`, `granularity`, `easing`, `overshoot` | Per-char/word animation: `char_scale_in`, `char_fade_in`, `char_wave`, `char_bounce`, `char_rotate_in`, `char_slide_up` | | `glow` | `color`, `radius`, `intensity` | Luminous halo effect | | `wiggle` | `property`, `amplitude`, `frequency`, `mode`, `seed`, ... | Procedural noise animation | @@ -768,7 +782,15 @@ All components are discriminated by `"type"`. Rendered in array order (first = b } ``` -**Root fields:** `content` (required), `max_width` +**Root fields:** `content` (required), `max_width`, `stroke`, `text-shadow`, `text-background` + +| Root field | Type | Default | +| ------------------ | -------- | ---------- | +| `stroke` | object | `null` — `{ "color": "#000", "width": 2 }` (snake_case has no inner fields to worry about) | +| `text-shadow` | object | `null` — single shadow, snake_case keys: `{ "color": "#000", "offset_x": 2, "offset_y": 2, "blur": 4 }` | +| `text-background` | object | `null` — `{ "color": "#000", "padding": 4, "corner_radius": 4 }`. See [rules/text-background.md](rules/text-background.md). | + +`stroke`, `text-shadow`, and `text-background` are fields on the `text` component itself (siblings of `style`) — `CssStyle` doesn't have `stroke` or `text-background` at all, so nesting them inside `style` drops the whole component (`deny_unknown_fields`). Confusingly, `CssStyle` *does* separately define its own `text-shadow` — but as an **array** with **kebab-case** inner keys (`[{ "color": "#000", "offset-x": 2, "offset-y": 2, "blur": 4 }]`), for multi-layer shadows. Prefer the root single-shadow form shown above unless you need more than one shadow layer. | Style field | Type | Default | | ----------------- | -------- | ---------- | @@ -780,10 +802,7 @@ All components are discriminated by `"type"`. Rendered in array order (first = b | `text-align` | enum | `"left"` — `"left"`, `"center"`, `"right"` | | `line-height` | f32 | `null` | | `letter-spacing` | f32 | `null` | -| `text-shadow` | object | `null` — `{ "color": "#000", "offset_x": 2, "offset_y": 2, "blur": 4 }` | -| `stroke` | object | `null` — `{ "color": "#000", "width": 2 }` | -| `text-background` | object | `null` — `{ "color": "#000", "padding": 4, "corner_radius": 4 }` | -| `wrap` | bool | `true` — when `true`, text wraps to parent's max width or `max_width`, whichever is smaller. Set `false` only when a finite `max-width` + small enough `font-size` guarantee a single line. The validator emits `unwrappable_text_overflow` otherwise. See [rules/geometry-safety.md](rules/geometry-safety.md). | +| `white-space` | enum | unset (wraps) — set `"nowrap"`/`"pre"` for single-line text. There is no `wrap` field. The validator emits `unwrappable_text_overflow` if the natural width exceeds the box. See [rules/geometry-safety.md](rules/geometry-safety.md). | | `overflow` | enum | `"visible"` — CSS-like: `"visible"` (default, children may bleed) or `"hidden"` (clip at the box). Validator only checks the **viewport**, never a `visible` parent. | **Per-character / per-word animation (char animation presets):** @@ -831,29 +850,36 @@ Animates each character or word independently with staggered timing. Use `char_* { "type": "shape", "shape": "rounded_rect", + "fill": "#FF5733", + "stroke": { "color": "#FFFFFF", "width": 2 }, "style": { "width": 200, "height": 100, - "fill": "#FF5733", - "border-radius": 16, - "stroke": { "color": "#FFFFFF", "width": 2 } + "border-radius": 16 } } ``` -**Root fields:** `shape` (required), `text` +`fill` and `stroke` are **root fields**, not CSS — placing them inside `style` fails with `unknown field` (`CssStyle` is `deny_unknown_fields`) and the shape is silently dropped. See [rules/component-field-placement.md](rules/component-field-placement.md). -| Style field | Type | Default | +**Root fields:** `shape` (required), `text`, `fill`, `stroke` + +| Root field | Type | Default | | --------------- | ------------------ | ----------- | | `fill` | string or gradient | `null` | | `stroke` | `{color, width}` | `null` | + +| Style field | Type | Default | +| --------------- | ------------------ | ----------- | | `border-radius` | f32 | `null` | **Shape types:** `rect`, `circle`, `rounded_rect`, `ellipse`, `triangle`, `star` (with `points`, default 5), `polygon` (with `sides`, default 6), `path` (with `data` SVG path string) -**Gradient fill:** +**Gradient fill (root field):** ```json { + "type": "shape", + "shape": "circle", "fill": { "type": "linear", "colors": ["#FF0000", "#0000FF"], @@ -1025,11 +1051,12 @@ Animated number counter. See Rule 4: must be standalone. "suffix": "€", "easing": "ease_out", "start_at": 0.5, - "end_at": 2.5, "style": { "font-size": 72, "color": "#FFFFFF", "font-weight": "bold", "text-align": "center" } } ``` +`end_at` is a visibility toggle, not an animation-completion boundary — setting it on a `counter` makes the number **disappear** once that time passes, since the counter's own animation is driven by `ctx.time / scene_duration`, not by `start_at`/`end_at`. Use `start_at` only. + **Root fields:** `from`, `to`, `decimals`, `separator`, `prefix`, `suffix`, `easing` **Easing options:** `linear`, `ease_in`, `ease_out`, `ease_in_out`, `ease_in_quad`, `ease_out_quad`, `ease_in_cubic`, `ease_out_cubic`, `ease_in_expo`, `ease_out_expo`, `spring` @@ -1045,7 +1072,7 @@ To place children at fixed absolute coordinates, use a `card` with transparent b "type": "card", "style": { "width": 1920, "height": 1080, "background": "#00000000", "padding": 0 }, "children": [ - { "type": "shape", "shape": "rect", "position": { "x": 0, "y": 0 }, "style": { "width": 400, "height": 300, "fill": "#1E293B", "border-radius": 16 } }, + { "type": "shape", "shape": "rect", "fill": "#1E293B", "position": { "x": 0, "y": 0 }, "style": { "width": 400, "height": 300, "border-radius": 16 } }, { "type": "icon", "icon": "lucide:phone-off", "position": { "x": 170, "y": 120 }, "style": { "width": 64, "height": 64, "color": "#FFFFFF" } } ] } @@ -1063,14 +1090,14 @@ Each dimension (`width`/`height` in `style`) can be a number or `"auto"`. "type": "card", "style": { "width": 800, "height": 100, "flex-direction": "row", "gap": 16 }, "children": [ - { "type": "shape", "shape": "rect", "style": { "width": 100, "height": 100, "fill": "#FF0000" } }, - { "type": "shape", "shape": "rect", "style": { "width": 100, "height": 100, "fill": "#00FF00", "flex-grow": 1 } }, - { "type": "shape", "shape": "rect", "style": { "width": 100, "height": 100, "fill": "#0000FF" } } + { "type": "shape", "shape": "rect", "fill": "#FF0000", "style": { "width": 100, "height": 100 } }, + { "type": "shape", "shape": "rect", "fill": "#00FF00", "style": { "width": 100, "height": 100, "flex-grow": 1 } }, + { "type": "shape", "shape": "rect", "fill": "#0000FF", "style": { "width": 100, "height": 100 } } ] } ``` -**Grid example (2x2):** Note: grid containers need explicit `height` (not `"auto"`) — see [rules/grid-card-height.md](rules/grid-card-height.md). +**Grid example (2x2):** Note: grid containers need explicit `height` (not `"auto"`) — see [rules/grid-card-height.md](rules/grid-card-height.md). `grid-template-columns`/`grid-template-rows` is `Vec`, an **untagged** enum: a bare number means px, a quoted string like `"1fr"` carries the unit, `"auto"` is the keyword. The object forms `{"fr": N}` / `{"px": N}` shown in older docs do **not** match any variant and drop the whole component. ```json { "type": "card", @@ -1078,8 +1105,8 @@ Each dimension (`width`/`height` in `style`) can be a number or `"auto"`. "width": 600, "height": 400, "display": "grid", - "grid-template-columns": [{ "fr": 1 }, { "fr": 1 }], - "grid-template-rows": [{ "fr": 1 }, { "fr": 1 }], + "grid-template-columns": ["1fr", "1fr"], + "grid-template-rows": ["1fr", "1fr"], "gap": 16, "padding": 24, "background": "#1a1a2e" @@ -1101,14 +1128,14 @@ Each dimension (`width`/`height` in `style`) can be a number or `"auto"`. | `background` | string | `null` | | `border-radius` | f32 | `12.0` | | `border` | object | `null` — `{ "color": "#E5E7EB", "width": 1 }` | -| `box-shadow` | object | `null` — `{ "color": "#00000040", "offset_x": 0, "offset_y": 4, "blur": 12 }` | +| `box-shadow` | array | `null` — `[{ "color": "#00000040", "offset-x": 0, "offset-y": 4, "blur": 12 }]` (kebab-case keys, always an array — see [rules/component-field-placement.md](rules/component-field-placement.md)) | | `padding` | f32 or obj | `null` | -| `flex-direction` | enum | `"column"` — `"column"`, `"row"`, `"column_reverse"`, `"row_reverse"` | -| `flex-wrap` | bool | `false` | -| `align-items` | enum | `"start"` — `"start"`, `"center"`, `"end"`, `"stretch"` | -| `justify-content` | enum | `"start"` — `"start"`, `"center"`, `"end"`, `"space_between"`, `"space_around"`, `"space_evenly"` | +| `flex-direction` | enum | `"column"` — `"row"`, `"row-reverse"`, `"column"`, `"column-reverse"` (kebab-case) | +| `flex-wrap` | enum | `"nowrap"` — `"nowrap"`, `"wrap"`, `"wrap-reverse"` (NOT a bool) | +| `align-items` | enum | `"start"` — `"start"`, `"center"`, `"end"`, `"stretch"`, `"flex-start"`, `"flex-end"`, `"baseline"` | +| `justify-content` | enum | `"start"` — `"start"`, `"center"`, `"end"`, `"space-between"`, `"space-around"`, `"space-evenly"` (kebab-case) | | `gap` | f32 | `0` | -| `grid-template-columns` | array | `null` — `[{"px": N}, {"fr": N}, "auto"]` | +| `grid-template-columns` | array | `null` — `["1fr", 200, "auto"]` (bare number = px, quoted `"Nfr"` = fr, `"auto"` = keyword) | | `grid-template-rows` | array | `null` | **Per-child layout properties** (in child `"style"`): @@ -1119,7 +1146,7 @@ Each dimension (`width`/`height` in `style`) can be a number or `"auto"`. - `grid-column` (object) — `{ "start": 1, "span": 2 }` (1-indexed) - `grid-row` (object) — `{ "start": 1, "span": 2 }` (1-indexed) -`position` is only valid inside `positioned` containers. Card children are laid out using flex/grid style properties. +`position: "absolute"` (root field, sibling of `style`) works on a child of **any** container — `card`, `div`, `grid`, `positioned`, or the scene root — not just inside `positioned`. `positioned` is simply a semantic Stack-like container with no visual decoration; it does not unlock `position` — every container already supports it. Children without `position` are laid out using flex/grid style properties. ### 12. `div` @@ -1133,14 +1160,14 @@ Utiliser `div` quand il faut grouper des éléments sans décoration visuelle (e "style": { "flex-direction": "column", "align-items": "center", - "gap": 36, - "timeline": [ - { "at": 3.5, "animation": [{ "name": "keyframes", "keyframes": [ - { "property": "scale", "keyframes": [{ "time": 0, "value": 1 }, { "time": 0.8, "value": 4 }], "easing": "ease_in" }, - { "property": "opacity", "keyframes": [{ "time": 0, "value": 1 }, { "time": 0.7, "value": 0 }], "easing": "ease_in" } - ]}]} - ] + "gap": 36 }, + "timeline": [ + { "at": 3.5, "animation": [{ "name": "keyframes", "keyframes": [ + { "property": "scale", "keyframes": [{ "time": 0, "value": 1 }, { "time": 0.8, "value": 4 }], "easing": "ease_in" }, + { "property": "opacity", "keyframes": [{ "time": 0, "value": 1 }, { "time": 0.7, "value": 0 }], "easing": "ease_in" } + ]}]} + ], "children": [ { "type": "icon", "icon": "lucide:zap", "style": { "width": 80, "height": 80, "color": "#25D366" } }, { "type": "text", "content": "Grouped content", "style": { "font-size": 48, "color": "#FFFFFF" } } @@ -1148,7 +1175,7 @@ Utiliser `div` quand il faut grouper des éléments sans décoration visuelle (e } ``` -Supporte toutes les propriétés CSS flex/grid (`flex-direction`, `align-items`, `justify-content`, `gap`, `padding`, `display: "grid"`, `grid-template-columns`) et toutes les propriétés d'animation/timeline. Préférer `div` à `card` avec fond transparent pour tout layout sans styling visuel. +`timeline` and `stagger` are **root fields**, not `style` — `CssStyle` has no `timeline` key and `deny_unknown_fields` drops the whole component if you nest it there. Supporte toutes les propriétés CSS flex/grid (`flex-direction`, `align-items`, `justify-content`, `gap`, `padding`, `display: "grid"`, `grid-template-columns`) dans `style`, plus `timeline`/`stagger` au niveau racine. Préférer `div` à `card` avec fond transparent pour tout layout sans styling visuel. ### 12. `codeblock` @@ -2248,9 +2275,8 @@ New style fields available on all components: | Style field | Type | Default | Description | | ----------------- | ------ | ------- | -------------------------------------------------------- | | `gradient-border` | object | `null` | `{ "colors": ["#f00", "#00f"], "width": 2, "angle": 0 }` — gradient-colored border ring, border-radius aware, painted instead of `border` when both are set | -| `motion-path` | string | `null` | SVG path string that the element follows during animation | -| `stagger` | f32 | `null` | Auto-delay offset per child in a container (seconds) | -| `timeline` | array | `[]` | Intra-scene timeline steps — sequential animation phases | + +`stagger` and `timeline` are **root fields** (siblings of `style`), not style fields — see [rules/component-field-placement.md](rules/component-field-placement.md). `motion-path` does not exist in the current schema (leftover from the pre-CSS `LayerStyle` model) — there is no motion-path-following mechanism today. **Deprecated (accepted but never rendered — the validator warns):** @@ -2282,7 +2308,7 @@ Any component can be rendered with true 3D perspective using keyframe animations "border-radius": 24, "backdrop-filter": [{ "fn": "blur", "radius": 15 }], "border": { "color": "#FFFFFF14", "width": 1 }, - "box-shadow": { "color": "#00000060", "offset_x": 0, "offset_y": 20, "blur": 60 }, + "box-shadow": [{ "color": "#00000060", "offset-x": 0, "offset-y": 20, "blur": 60 }], "animation": [{ "name": "keyframes", "keyframes": [ @@ -2302,24 +2328,24 @@ Any component can be rendered with true 3D perspective using keyframe animations ### Timeline Sequencing -The `timeline` field on any component's style allows defining sequential animation phases within a single scene. Each step triggers at a specific time and applies its own animation effects relative to that time. +The `timeline` field — a **root field**, sibling of `style`, not nested inside it — allows defining sequential animation phases within a single scene. Each step triggers at a specific time and applies its own animation effects relative to that time. ```json { "type": "card", "style": { - "animation": [{ "name": "fade_in_up", "duration": 0.6 }], - "timeline": [ - { - "at": 2.0, - "animation": [{ "name": "shake", "duration": 0.5 }] - }, - { - "at": 4.0, - "animation": [{ "name": "fade_out", "duration": 0.8 }] - } - ] - } + "animation": [{ "name": "fade_in_up", "duration": 0.6 }] + }, + "timeline": [ + { + "at": 2.0, + "animation": [{ "name": "shake", "duration": 0.5 }] + }, + { + "at": 4.0, + "animation": [{ "name": "fade_out", "duration": 0.8 }] + } + ] } ``` @@ -2395,7 +2421,7 @@ See Rule 13 for usage guidance. } ``` -**39 presets:** +**40 presets (+ 6 char-only presets, text component only):** | Category | Presets | | ---------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | @@ -2521,6 +2547,6 @@ Before presenting a generated scenario to the user, verify: - [ ] All scenes have `"layout": {"align_items": "center", "justify_content": "center"}` for centered composition - [ ] `concentric_circles` animated-background on at least 4 scenes for visual depth - [ ] No `end_at` on counters (makes them disappear — use `start_at` only) -- [ ] No text uses `style.wrap: false` unless a finite `max-width` keeps it inside the viewport (use `marquee` for intentional bleeding) +- [ ] No text uses `style.white-space: "nowrap"`/`"pre"` unless a finite `max-width` keeps it inside the viewport (use `marquee` for intentional bleeding) — see [rules/geometry-safety.md](rules/geometry-safety.md) - [ ] Long codeblocks/terminals leave `auto_scroll` at its default (`true`) — never set `false` unless content is guaranteed to fit - [ ] `rustmotion validate -f scenario.json` passes (zero schema **and** geometry violations) before presenting diff --git a/.claude/skills/rustmotion/rules/color-palettes.md b/.claude/skills/rustmotion/rules/color-palettes.md index f6e8c4b..e75b9df 100644 --- a/.claude/skills/rustmotion/rules/color-palettes.md +++ b/.claude/skills/rustmotion/rules/color-palettes.md @@ -95,28 +95,29 @@ Pick the palette in Phase 2. Every scene, every card, every text element uses co ### Rule P2: Use $ref for backgrounds -If the same gradient background appears in multiple scenes, define it once and reference it: +If the same gradient background appears in multiple scenes, define it once and reference it. Entries under the top-level `backgrounds` map are stored as raw JSON and only get type-checked when a scene actually resolves the `$ref` — at that point they must deserialize as an `AnimatedBackground`, which **requires a `preset`** (`gradient_shift`, `grid_dots`, `concentric_circles`, `halo`, or `heropattern`) plus that preset's config nested under its own name key. A template shaped like `{ "type": "gradient", "gradient": {...} }` is not an `AnimatedBackground` at all — `rustmotion validate` does not catch this (the `backgrounds` map and `background.$ref` overrides are permissive `serde_json::Value`s at the schema level), so the file validates clean but **renders a flat, ungra­dient background** at render time: ```json { "backgrounds": { "dark_radial": { - "type": "gradient", - "gradient": { - "type": "radial", + "preset": "gradient_shift", + "gradient_shift": { "colors": ["#1e1b4b", "#0f172a"], - "center_x": 0.5, - "center_y": 0.4 - } + "gradient_type": "radial" + }, + "speed": 10 } }, "scenes": [ - { "background": { "$ref": "dark_radial" }, ... }, - { "background": { "$ref": "dark_radial" }, ... } + { "background": { "$ref": "dark_radial" }, "duration": 3.0, "children": [] }, + { "background": { "$ref": "dark_radial" }, "duration": 3.0, "children": [] } ] } ``` +`speed` defaults to `0` (static) if omitted — set it low for a near-static gradient, higher for a visibly rotating one (degrees/sec for `gradient_shift`). Per-scene overrides merge on top of the template (e.g. `{ "$ref": "dark_radial", "gradient_shift": { "colors": [...] } }` swaps just the colors) — everything except `$ref` and `transition` deep-merges into the referenced template. + ### Rule P3: Accent color as the single highlight Use the accent color for: CTAs, icon fills, badge backgrounds, glow effects, border highlights. diff --git a/.claude/skills/rustmotion/rules/depth-layering.md b/.claude/skills/rustmotion/rules/depth-layering.md index 9495662..62a8b48 100644 --- a/.claude/skills/rustmotion/rules/depth-layering.md +++ b/.claude/skills/rustmotion/rules/depth-layering.md @@ -102,16 +102,18 @@ Background elements feel farther away when slightly blurred. Use `backdrop-filte ## Scale + opacity gradient across siblings -When cards or icons are in a row, give back-row items a smaller scale and lower opacity to simulate perspective receding. +When cards or icons are in a row, give back-row items a smaller scale and lower opacity to simulate perspective receding. `transform` is an **array of tagged transform functions** (`{ "fn": "scale", "x": ..., "y": ... }`), never a CSS string like `"scale(0.82)"` — a string fails to deserialize and drops the component. ```json [ - { "type": "card", "style": { "opacity": 0.4, "transform": "scale(0.82)" }, "..." : "back" }, - { "type": "card", "style": { "opacity": 0.7, "transform": "scale(0.91)" }, "..." : "mid" }, - { "type": "card", "style": { "opacity": 1.0, "transform": "scale(1.00)" }, "..." : "front" } + { "type": "card", "style": { "opacity": 0.4, "transform": [{ "fn": "scale", "x": 0.82, "y": 0.82 }] } }, + { "type": "card", "style": { "opacity": 0.7, "transform": [{ "fn": "scale", "x": 0.91, "y": 0.91 }] } }, + { "type": "card", "style": { "opacity": 1.0, "transform": [{ "fn": "scale", "x": 1.00, "y": 1.00 }] } } ] ``` +(Back/mid/front comment above each entry omitted here for brevity — each object above is a separate sibling `card`, e.g. inside a `div` with `flex-direction: "row"`.) + Use `scale` steps of ~0.08–0.12 between planes. More than 3 planes starts looking mechanical. --- diff --git a/.claude/skills/rustmotion/rules/easing-guidelines.md b/.claude/skills/rustmotion/rules/easing-guidelines.md index a5f56d4..b07bfd2 100644 --- a/.claude/skills/rustmotion/rules/easing-guidelines.md +++ b/.claude/skills/rustmotion/rules/easing-guidelines.md @@ -14,7 +14,11 @@ Entrance presets already use appropriate easing internally. ## Available Easing Functions -`linear`, `ease_in`, `ease_out`, `ease_in_out`, `ease_in_quad`, `ease_out_quad`, `ease_in_cubic`, `ease_out_cubic`, `ease_in_expo`, `ease_out_expo`, `spring` +17 named easings, plus `cubic_bezier` for a custom curve — 18 total: + +`linear`, `ease_in`, `ease_out`, `ease_in_out`, `ease_in_quad`, `ease_out_quad`, `ease_in_cubic`, `ease_out_cubic`, `ease_in_expo`, `ease_out_expo`, `ease_in_out_quad`, `ease_in_out_expo`, `ease_in_back`, `ease_out_back`, `ease_out_elastic`, `bounce`, `spring` + +`cubic_bezier` takes a parameter object instead of a bare string — `{ "cubic_bezier": { "x1": 0.4, "y1": 0.0, "x2": 0.2, "y2": 1.0 } }` — anywhere an `easing` value is accepted (top-level `animation.easing`, per-keyframe `easing`). ### Spring Physics diff --git a/.claude/skills/rustmotion/rules/geometry-safety.md b/.claude/skills/rustmotion/rules/geometry-safety.md index 127ae36..83eb529 100644 --- a/.claude/skills/rustmotion/rules/geometry-safety.md +++ b/.claude/skills/rustmotion/rules/geometry-safety.md @@ -2,15 +2,15 @@ No textual content may bleed out of the device viewport. The renderer enforces this through three opt-in mechanisms, all checked by `rustmotion validate`. -## 1. Text wrapping (`style.wrap`) +## 1. Text wrapping (`style.white-space`) -`text` is constraint-aware: it wraps to its parent's allocated width by default. Set `style.wrap: false` only when you intentionally want the text to render on one line and you guarantee it fits (e.g. a marquee that bleeds, a ticker, a title with a fixed `max-width`). +`text` is constraint-aware: it wraps to its parent's allocated width by default. There is no `style.wrap` field — that boolean belonged to the pre-CSS `LayerStyle` model and no longer exists in `CssStyle` (`deny_unknown_fields` rejects it and silently drops the component). Wrapping is controlled by the standard CSS `white-space` property instead: -- Default: `wrap: true` (wraps at parent or `max_width`, whichever is smaller). -- `wrap: false` → validator fails with `unwrappable_text_overflow` if the natural width exceeds the box. +- Default: unset / `"normal"` (and `"pre-line"`, `"pre-wrap"`, `"break-spaces"`) — wraps at parent width or `max-width`, whichever is smaller. +- `white-space: "nowrap"` or `"pre"` → the text renders on one line. The validator measures its natural (unbounded) width and fails with `unwrappable_text_overflow` if that width exceeds the box. Only set this when you intentionally want a single line and a finite `max-width` + reasonable `font-size` guarantee it fits (e.g. a title, a ticker-like label — not a marquee, which has its own component). ```json -{ "type": "text", "content": "Long sentence...", "style": { "wrap": true, "max-width": 800 } } +{ "type": "text", "content": "Long sentence...", "style": { "white-space": "nowrap", "max-width": 800 } } ``` ## 2. Codeblock / Terminal `auto_scroll` @@ -39,13 +39,15 @@ CSS-like semantics: `visible` (default) lets children bleed; `hidden` clips at t ## What the validator catches -`rustmotion validate scenario.json` reports three geometry violation kinds: +`rustmotion validate scenario.json` reports five geometry violation kinds: - `viewport_overflow` — absolute bbox crosses the device edge -- `unwrappable_text_overflow` — `wrap: false` but natural width > available width +- `unwrappable_text_overflow` — `white-space: "nowrap"`/`"pre"` but natural width > available width +- `content_overflows_box` — wrapping text needs more room than the box it was actually assigned, typically a paragraph inside a card with a fixed `height` too small for it. Text painters never clip themselves, so this paints outside its box even when the box sits comfortably inside the frame — which is why the viewport check alone never caught it. - `auto_scroll_disabled_overflow` — `auto_scroll: false` but content > box +- `animated_text_overflow` — an animated transform (scale/translate/wiggle/orbit) pushes the bbox out of the viewport at some sampled time. Only checked with `--strict-anim` (default runs check the resting, untransformed layout only). -`marquee` and `cursor` are exempt (their job is to bleed). +`marquee` and `cursor` are exempt (their job is to bleed). A node is also exempt when it clips itself, or when any ancestor clips it — `overflow` set to anything other than `visible`. Deliberate bleed under a clipping parent is a composition technique, not a defect: that is how you get giant type running off the frame. ## CLI usage @@ -53,24 +55,25 @@ CSS-like semantics: `visible` (default) lets children bleed; `hidden` clips at t rustmotion validate scenario.json # human-readable rustmotion validate scenario.json --report report.json # JSON report rustmotion validate scenario.json --fix # safe auto-fixes -rustmotion validate scenario.json --strict-anim # per-frame check +rustmotion validate scenario.json --strict-anim # per-frame check, adds animated_text_overflow rustmotion validate scenario.json --lenient # warnings only ``` -`--fix` rewrites the file in place. It only applies *safe* mutations: -- sets `style.wrap: true` on text that overflowed because wrap was off -- sets `auto_scroll: true` on codeblock/terminal with overflow +`--fix` rewrites the file in place: +- `auto_scroll_disabled_overflow` → sets `auto_scroll: true`. Safe. +- `unwrappable_text_overflow` → removes `style.white-space`, so the text falls back to the `normal` default and wraps again. Non-destructive: it only ever deletes the property that caused the violation. If you want the line to stay unbroken, widen the box or lower `font-size` by hand instead of running `--fix`. -Position/size clamping is never auto-applied — fix those by hand. +Position/size clamping (`viewport_overflow`) is never auto-applied — fix those by hand too. ## When to use what | Symptom | Fix | |---|---| -| Long sentence cut at viewport edge | leave `wrap: true` (default) and ensure parent has finite width | -| Need a single-line title that must fit | set `max-width` and a small enough `font-size`, leave `wrap: true` | -| Marquee / ticker text that intentionally scrolls past edges | use `marquee` (exempt) — never `text` with `wrap: false` | +| Long sentence cut at viewport edge | leave `white-space` unset (default wraps) and ensure parent has finite width | +| Need a single-line title that must fit | set `max-width` and a small enough `font-size`, leave `white-space` unset | +| Marquee / ticker text that intentionally scrolls past edges | use `marquee` (exempt) — never `text` with `white-space: "nowrap"` | | Code listing taller than its box | leave `auto_scroll: true` (default) | | Terminal log streaming many lines | leave `auto_scroll: true` | | Badge protruding from a card on purpose | container has `overflow: visible` (default) — no change needed | | Hard-clip children to a card border | container `style.overflow: "hidden"` | +| Animated element (wiggle/orbit/keyframe scale) might drift off-screen | run `rustmotion validate --strict-anim` to sample frames, not just the resting layout | diff --git a/.claude/skills/rustmotion/rules/glassmorphism.md b/.claude/skills/rustmotion/rules/glassmorphism.md index 3579ec6..c1eea24 100644 --- a/.claude/skills/rustmotion/rules/glassmorphism.md +++ b/.claude/skills/rustmotion/rules/glassmorphism.md @@ -236,10 +236,11 @@ La bordure et l'ombre reprennent la teinte accent — le verre a une couleur. ```json { - "video": { "background": "#0f172a" }, + "video": { "width": 1080, "height": 1920, "fps": 30, "background": "#0f172a" }, "scenes": [{ + "duration": 3.0, "children": [ - { "type": "card", "style": { "backdrop-filter": [{ "fn": "blur", "radius": 24 }], "background": "#FFFFFF18" } } + { "type": "card", "style": { "width": 800, "height": 400, "backdrop-filter": [{ "fn": "blur", "radius": 24 }], "background": "#FFFFFF18" } } ] }] } @@ -252,6 +253,6 @@ Fond uni `#0f172a` + verre → on ne voit rien à travers le flou, la carte est `backdrop-filter` floute ce qui est **derrière le composant dans le viewport**, pas ce qui est dans le parent. Si appliqué à un `text` ou `icon` enfant d'une carte opaque, il n'a aucun effet visible. ```json -{ "type": "text", "style": { "backdrop-filter": [{ "fn": "blur", "radius": 20 }], "color": "#fff" } } +{ "type": "text", "content": "Label", "style": { "backdrop-filter": [{ "fn": "blur", "radius": 20 }], "color": "#fff" } } ``` Aucun effet — `backdrop-filter` est uniquement pertinent sur les éléments directement superposés à une texture/image/blob de fond. ✗ diff --git a/.claude/skills/rustmotion/rules/icon-sizing-hierarchy.md b/.claude/skills/rustmotion/rules/icon-sizing-hierarchy.md index 072013d..3c6dd10 100644 --- a/.claude/skills/rustmotion/rules/icon-sizing-hierarchy.md +++ b/.claude/skills/rustmotion/rules/icon-sizing-hierarchy.md @@ -100,19 +100,28 @@ Each card is 230px = 77px CSS → icon and text completely unreadable. ## GOOD: 2×2 grid on mobile +`grid-template-columns` takes an array (`["1fr", "1fr"]`), never a raw CSS shorthand string like `"1fr 1fr"` — a bare string fails to deserialize (`Vec` expected) and drops the card. Grid containers also need an explicit `height` (not `"auto"`) — see [rules/grid-card-height.md](grid-card-height.md). + ```json { "type": "card", - "style": { "width": 984, "display": "grid", "grid-template-columns": "1fr 1fr", "gap": 24 }, + "style": { + "width": 984, + "height": 640, + "display": "grid", + "grid-template-columns": ["1fr", "1fr"], + "grid-template-rows": ["1fr", "1fr"], + "gap": 24 + }, "children": [ - { "type": "card", "style": { "width": 480, "padding": 40 }, "children": [...] }, - { "type": "card", "style": { "width": 480, "padding": 40 }, "children": [...] }, - { "type": "card", "style": { "width": 480, "padding": 40 }, "children": [...] }, - { "type": "card", "style": { "width": 480, "padding": 40 }, "children": [...] } + { "type": "card", "style": { "padding": 40 }, "children": [] }, + { "type": "card", "style": { "padding": 40 }, "children": [] }, + { "type": "card", "style": { "padding": 40 }, "children": [] }, + { "type": "card", "style": { "padding": 40 }, "children": [] } ] } ``` -480px per card = 160px CSS → readable content. 24px gap. ✓ +Each cell fills its grid track (~480px wide here) = 160px CSS → readable content. 24px gap. ✓ ## BAD: Icon inconsistency across scenes diff --git a/.claude/skills/rustmotion/rules/module-structure.md b/.claude/skills/rustmotion/rules/module-structure.md index 14fa0cd..c72d631 100644 --- a/.claude/skills/rustmotion/rules/module-structure.md +++ b/.claude/skills/rustmotion/rules/module-structure.md @@ -7,7 +7,7 @@ The workspace is split into three crates: | Crate | Role | |---|---| | `rustmotion-core` | Engine, CSS model, schema types, Painter trait | -| `rustmotion-components` | 51 component structs + Painter impls + box builder | +| `rustmotion-components` | 57 component structs + Painter impls + box builder | | `rustmotion-cli` | CLI commands (render, validate, schema, info) | --- diff --git a/.claude/skills/rustmotion/rules/prefer-presets.md b/.claude/skills/rustmotion/rules/prefer-presets.md index 838d6b0..8c94eba 100644 --- a/.claude/skills/rustmotion/rules/prefer-presets.md +++ b/.claude/skills/rustmotion/rules/prefer-presets.md @@ -1,6 +1,6 @@ # Rule: Prefer Presets Over Manual Keyframes -Presets are simpler, less error-prone, and produce consistent motion design. Only use `keyframes` animation effects for custom behavior not covered by the 31 built-in presets. +Presets are simpler, less error-prone, and produce consistent motion design. Only use `keyframes` animation effects for custom behavior not covered by the 40 built-in presets (+ 6 char-only presets on `text`). **GOOD:** ```json diff --git a/.claude/skills/rustmotion/rules/responsive-device-sizing.md b/.claude/skills/rustmotion/rules/responsive-device-sizing.md index a6b577d..1d6d983 100644 --- a/.claude/skills/rustmotion/rules/responsive-device-sizing.md +++ b/.claude/skills/rustmotion/rules/responsive-device-sizing.md @@ -129,9 +129,9 @@ Reference: Tailwind CSS default spacing (4px base unit). Sizing rules above are guidelines — `rustmotion validate` is the source of truth. It refuses any scenario whose layout tree leaves the device viewport. See [geometry-safety.md](geometry-safety.md): -- `text` wraps automatically at the parent's max width — leave `style.wrap` at its default (`true`) unless you have a finite `max-width`. +- `text` wraps automatically at the parent's max width — leave `style.white-space` unset (default wraps) unless you have a finite `max-width`. - `codeblock` / `terminal` auto-scroll when content exceeds their `size` — leave `auto_scroll: true` (default). -- Long single-line content that should bleed must use `marquee`, never `text` with `wrap: false`. +- Long single-line content that should bleed must use `marquee`, never `text` with `white-space: "nowrap"`. ## BAD: Using desktop sizes on mobile diff --git a/.claude/skills/rustmotion/rules/text-background.md b/.claude/skills/rustmotion/rules/text-background.md index a001d14..fca36ce 100644 --- a/.claude/skills/rustmotion/rules/text-background.md +++ b/.claude/skills/rustmotion/rules/text-background.md @@ -2,20 +2,22 @@ `text-background` adds a colored rectangle behind text content. The background is positioned to tightly wrap the text glyphs with configurable padding and corner radius. +`text-background` is a **root field** on `text` — a sibling of `style`, not nested inside it. `CssStyle` has no `text-background` key (`deny_unknown_fields`); putting it in `style` silently drops the whole `text` component. + **GOOD:** ```json { "type": "text", "content": " Highlighted text ", + "text-background": { + "color": "#6366F1", + "padding": 12, + "corner_radius": 8 + }, "style": { "font-size": 36, "color": "#FFFFFF", - "text-align": "center", - "text-background": { - "color": "#6366F1", - "padding": 12, - "corner_radius": 8 - } + "text-align": "center" } } ``` diff --git a/.claude/skills/rustmotion/rules/validate-json.md b/.claude/skills/rustmotion/rules/validate-json.md index 402ad01..3df831f 100644 --- a/.claude/skills/rustmotion/rules/validate-json.md +++ b/.claude/skills/rustmotion/rules/validate-json.md @@ -9,10 +9,12 @@ Every generated JSON scenario MUST be validated with `rustmotion validate` befor ## Geometry violations -The validator detects three viewport-overflow conditions: +The validator detects five overflow conditions: - `viewport_overflow` — absolute bbox crosses the device edge -- `unwrappable_text_overflow` — `style.wrap: false` but natural width > box +- `unwrappable_text_overflow` — `style.white-space: "nowrap"`/`"pre"` but natural width > box (there is no `wrap` field) +- `content_overflows_box` — wrapping text needs more room than its own box, e.g. a paragraph in a fixed-height card. Fires even when the box stays inside the frame - `auto_scroll_disabled_overflow` — `auto_scroll: false` on a codeblock/terminal with content > box +- `animated_text_overflow` — an animated transform pushes the bbox out of the viewport at some sampled time (`--strict-anim` only) See [rules/geometry-safety.md](geometry-safety.md) for the underlying mechanisms. @@ -25,6 +27,6 @@ rustmotion validate -f /tmp/scenario.json --strict-anim # per-frame checks rustmotion validate -f /tmp/scenario.json --lenient # warnings only ``` -`--fix` may set `wrap: true` and `auto_scroll: true` automatically. Position/size issues are never auto-fixed. +`--fix` handles the two mechanical cases: it sets `auto_scroll: true` on `auto_scroll_disabled_overflow`, and removes `style.white-space` on `unwrappable_text_overflow` (falling back to the `normal` default, i.e. wrapping). Viewport and content-box overflows are never auto-fixed — they need a layout decision only you can make. **FORBIDDEN:** Presenting JSON that has not been validated by `rustmotion validate`. diff --git a/.claude/skills/rustmotion/rules/wiggle-additive.md b/.claude/skills/rustmotion/rules/wiggle-additive.md index b1e2dd1..2c0a093 100644 --- a/.claude/skills/rustmotion/rules/wiggle-additive.md +++ b/.claude/skills/rustmotion/rules/wiggle-additive.md @@ -24,7 +24,8 @@ Wiggle offsets apply additively on top of keyframe animations and presets. Combi "type": "icon", "icon": "lucide:phone-off", "style": { - "size": 64, + "width": 64, + "height": 64, "color": "#FFFFFF", "animation": [ { "name": "scale_in" }, diff --git a/CLAUDE.md b/CLAUDE.md index 8e39d90..4d27ac1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,7 +11,7 @@ Tout JSON de scénario généré doit être validé avec `rustmotion validate` a Aucun contenu textuel ne doit dépasser du device. Trois propriétés contrôlent ce comportement : -- `style.wrap` (default `true`) sur `text` : laisse-le à `true` pour wrapper sur la largeur du parent. `wrap: false` est légitime uniquement si un `max-width` finit + `font-size` raisonnable garantissent que la ligne tient. Le validateur émet `unwrappable_text_overflow` sinon. +- `style.white-space` (default `normal`, donc wrap actif) sur `text` : le texte wrap sur la largeur du parent par défaut. `white-space: "nowrap"` (ou `"pre"`) est légitime uniquement si un `max-width` fini + `font-size` raisonnable garantissent que la ligne tient. Le validateur émet `unwrappable_text_overflow` sinon. Il n'existe pas de champ `style.wrap` — c'est un vocabulaire hérité de l'ancien modèle de style, supprimé de `CssStyle`. Voir [rules/geometry-safety.md](.claude/skills/rustmotion/rules/geometry-safety.md). - `auto_scroll` (default `true`) sur `codeblock` et `terminal` : quand le contenu dépasse la hauteur du `size`, le moteur scrolle (clip + translate) sans réduire la `font-size`. `auto_scroll: false` → `auto_scroll_disabled_overflow`. - `style.overflow` (default `visible`) sur les conteneurs : sémantique CSS. `hidden` clippe au bord du parent. Le validateur ne se plaint que si le contenu sort du **viewport**, pas d'un parent `visible`. @@ -19,9 +19,9 @@ Aucun contenu textuel ne doit dépasser du device. Trois propriétés contrôlen CLI : - `rustmotion validate -f file.json` — schema + geometry -- `--fix` — auto-fix sûr (`wrap: true`, `auto_scroll: true`) +- `--fix` — auto-fix sûr : `auto_scroll: true` sur `auto_scroll_disabled_overflow`, et retrait de `style.white-space` sur `unwrappable_text_overflow` (retour au défaut `normal`, donc au wrapping). Les débordements de viewport et de boîte ne sont jamais corrigés automatiquement : ils demandent un arbitrage de mise en page. - `--report r.json` — rapport JSON -- `--strict-anim` — vérification frame par frame +- `--strict-anim` — vérification frame par frame ; ajoute la détection `animated_text_overflow` (transform animé qui sort du viewport à un instant échantillonné) - `--strict-attrs` — promeut en erreurs les attributs inconnus (détection schéma + did-you-mean, activée par défaut en warnings) - `--lenient` — warnings au lieu d'errors @@ -31,7 +31,7 @@ CLI : - Sans ffmpeg, le fallback openh264 intégré encode en 8-bit - Pour les vidéos avec des gradients sombres, recommander `--codec prores` pour une qualité maximale -## Composants disponibles (51) +## Composants disponibles (57) ### Basiques `text`, `shape`, `image`, `icon`, `svg`, `video`, `gif`, `caption`, `rich_text`, `gradient_text` @@ -81,7 +81,13 @@ CLI : `arrow`, `connector`, `timeline`, `line` ### Média -`mockup`, `lottie`, `cursor`, `particle`, `qrcode` +`mockup`, `lottie`, `cursor`, `particle`, `qr_code` + +### Audio +- `waveform` — visualisation d'onde audio réactive au volume de la piste +- `audio_spectrum` — barres de spectre audio réactives (FFT) + +> Voir [rules/audio-reactive.md](.claude/skills/rustmotion/rules/audio-reactive.md) pour lier un composant à une piste `audio` via `style.audio-reactive`. ## Architecture From 0610a83c23602e5852634540df8085128fe18394 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sat, 1 Aug 2026 17:06:13 +0200 Subject: [PATCH 6/6] chore(fmt): apply rustfmt across the wave-1 changes --- .../rustmotion-cli/src/commands/geometry.rs | 62 ++++++++++++++++--- crates/rustmotion-cli/src/commands/schema.rs | 3 +- .../rustmotion-cli/src/commands/validate.rs | 7 ++- .../rustmotion-cli/src/commands/validation.rs | 6 +- .../rustmotion-core/src/css/taffy_bridge.rs | 20 ++++-- .../src/engine/renderer/colors.rs | 30 +++++---- 6 files changed, 98 insertions(+), 30 deletions(-) diff --git a/crates/rustmotion-cli/src/commands/geometry.rs b/crates/rustmotion-cli/src/commands/geometry.rs index 328a08b..81c814a 100644 --- a/crates/rustmotion-cli/src/commands/geometry.rs +++ b/crates/rustmotion-cli/src/commands/geometry.rs @@ -113,7 +113,10 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec let built = build_scene_from_refs(children.iter(), viewport_f, root_css, None); let layouts = run_layout(&built.root, viewport_f, &ConversionContext::default()); - let camera = scene.camera.as_ref().filter(|_| !scene_uses_depth(&children)); + let camera = scene + .camera + .as_ref() + .filter(|_| !scene_uses_depth(&children)); let path_root = format!("views[{}].scenes[{}]", vi, si); walk( @@ -128,7 +131,8 @@ pub fn validate_geometry(scenario: &ResolvedScenario) -> Vec // Nothing clips top-level scene children but the viewport // frame itself — and that's exactly what check_viewport // tests, so top level must not be pre-suppressed. - /*parent_clips=*/ false, + /*parent_clips=*/ + false, camera, &mut violations, ); @@ -209,15 +213,39 @@ fn walk( } check_viewport(&child.component, &child_path, &vbbox, viewport, vi, si, out); } - check_unwrappable_text(&child.component, &child_path, &raw_bbox, viewport, vi, si, out); - check_auto_scroll(&child.component, &child_path, &raw_bbox, viewport, vi, si, out); + check_unwrappable_text( + &child.component, + &child_path, + &raw_bbox, + viewport, + vi, + si, + out, + ); + check_auto_scroll( + &child.component, + &child_path, + &raw_bbox, + viewport, + vi, + si, + out, + ); // Suppressed under a clipping ancestor (parent_clips) exactly // like check_viewport, and when the node clips its own overflow // (paint_pass applies a node's own `overflow: hidden`/clip/ // scroll/auto BEFORE painting its own content, at step 4 — // self-clipping is real, not just a container->children thing). if !parent_clips && !container_clips(&child.component) { - check_content_overflows_box(&child.component, &child_path, layout, viewport, vi, si, out); + check_content_overflows_box( + &child.component, + &child_path, + layout, + viewport, + vi, + si, + out, + ); } } @@ -764,7 +792,10 @@ pub fn validate_geometry_animated(scenario: &ResolvedScenario) -> Vec serde_json::Value { // Point `Scene.children` at `Component` instead of the untyped // `serde_json::Value` ("items: true"). - if let Some(children) = scenario_schema.pointer_mut("/definitions/Scene/properties/children") - { + if let Some(children) = scenario_schema.pointer_mut("/definitions/Scene/properties/children") { *children = serde_json::json!({ "type": "array", "items": { "$ref": "#/definitions/Component" } diff --git a/crates/rustmotion-cli/src/commands/validate.rs b/crates/rustmotion-cli/src/commands/validate.rs index 6fec979..3ec4a87 100644 --- a/crates/rustmotion-cli/src/commands/validate.rs +++ b/crates/rustmotion-cli/src/commands/validate.rs @@ -194,8 +194,8 @@ fn parse_segments(path: &str) -> Vec<(String, usize)> { #[cfg(test)] mod tests { - use super::*; use super::super::geometry::{validate_geometry, Axis, BBox}; + use super::*; use rustmotion::components::Component; use rustmotion::engine::render; use rustmotion::loader::load_scenario_from_source; @@ -274,7 +274,10 @@ mod tests { Component::Card(c) => c.children.len() == 1, _ => false, }; - assert!(text_survived, "text child must survive the fix, not be dropped"); + assert!( + text_survived, + "text child must survive the fix, not be dropped" + ); // The fix must also clear the geometry violation it targeted. let after = validate_geometry(&scenario); diff --git a/crates/rustmotion-cli/src/commands/validation.rs b/crates/rustmotion-cli/src/commands/validation.rs index 9c7cad4..db87aab 100644 --- a/crates/rustmotion-cli/src/commands/validation.rs +++ b/crates/rustmotion-cli/src/commands/validation.rs @@ -416,6 +416,10 @@ mod font_loading_wiring_tests { let loaded = load(ValidationSource::Inline(&json)).expect("scenario loads"); assert!(loaded.scenario.fonts.is_empty()); let report = run_checks(&loaded, false); - assert!(report.schema_errors.is_empty(), "{:?}", report.schema_errors); + assert!( + report.schema_errors.is_empty(), + "{:?}", + report.schema_errors + ); } } diff --git a/crates/rustmotion-core/src/css/taffy_bridge.rs b/crates/rustmotion-core/src/css/taffy_bridge.rs index f6be557..c7a27b9 100644 --- a/crates/rustmotion-core/src/css/taffy_bridge.rs +++ b/crates/rustmotion-core/src/css/taffy_bridge.rs @@ -707,9 +707,21 @@ mod tests { assert_ne!(x1, x2, "columns 1 and 2 must not share an x-offset"); assert_ne!(x2, x3, "columns 2 and 3 must not share an x-offset"); - assert!((x1 - 0.0).abs() < 0.5, "column 1 should start at x=0, got {x1}"); - assert!((x2 - 300.0).abs() < 1.0, "column 2 should start at ~300px, got {x2}"); - assert!((x3 - 600.0).abs() < 1.0, "column 3 should start at ~600px, got {x3}"); - assert!((w1 - 300.0).abs() < 1.0, "each 1fr column should be ~300px wide, got {w1}"); + assert!( + (x1 - 0.0).abs() < 0.5, + "column 1 should start at x=0, got {x1}" + ); + assert!( + (x2 - 300.0).abs() < 1.0, + "column 2 should start at ~300px, got {x2}" + ); + assert!( + (x3 - 600.0).abs() < 1.0, + "column 3 should start at ~600px, got {x3}" + ); + assert!( + (w1 - 300.0).abs() < 1.0, + "each 1fr column should be ~300px wide, got {w1}" + ); } } diff --git a/crates/rustmotion-core/src/engine/renderer/colors.rs b/crates/rustmotion-core/src/engine/renderer/colors.rs index 07b2d79..5d4ffe4 100644 --- a/crates/rustmotion-core/src/engine/renderer/colors.rs +++ b/crates/rustmotion-core/src/engine/renderer/colors.rs @@ -503,7 +503,10 @@ mod tests { #[test] fn rgb_function_white() { - assert_eq!(parse_css_color("rgb(255,255,255)"), Some((255, 255, 255, 255))); + assert_eq!( + parse_css_color("rgb(255,255,255)"), + Some((255, 255, 255, 255)) + ); assert_eq!( parse_css_color("rgb(255, 255, 255)"), Some((255, 255, 255, 255)) @@ -512,7 +515,10 @@ mod tests { #[test] fn rgb_function_space_syntax() { - assert_eq!(parse_css_color("rgb(255 255 255)"), Some((255, 255, 255, 255))); + assert_eq!( + parse_css_color("rgb(255 255 255)"), + Some((255, 255, 255, 255)) + ); } #[test] @@ -573,10 +579,7 @@ mod tests { #[test] fn hsl_function_space_syntax() { - assert_eq!( - parse_css_color("hsl(0 100% 50%)"), - Some((255, 0, 0, 255)) - ); + assert_eq!(parse_css_color("hsl(0 100% 50%)"), Some((255, 0, 0, 255))); } // ---- named colors ---- @@ -596,8 +599,14 @@ mod tests { #[test] fn named_color_extended_set_sample() { // Spot-check a handful outside the old 11-name table. - assert_eq!(parse_css_color("rebeccapurple"), Some((0x66, 0x33, 0x99, 255))); - assert_eq!(parse_css_color("cornflowerblue"), Some((0x64, 0x95, 0xED, 255))); + assert_eq!( + parse_css_color("rebeccapurple"), + Some((0x66, 0x33, 0x99, 255)) + ); + assert_eq!( + parse_css_color("cornflowerblue"), + Some((0x64, 0x95, 0xED, 255)) + ); assert_eq!(parse_css_color("tomato"), Some((0xFF, 0x63, 0x47, 255))); assert_eq!(parse_css_color("dodgerblue"), Some((0x1E, 0x90, 0xFF, 255))); } @@ -613,10 +622,7 @@ mod tests { #[test] fn tolerates_surrounding_and_internal_whitespace() { assert_eq!(parse_css_color(" #fff "), Some((255, 255, 255, 255))); - assert_eq!( - parse_css_color(" white "), - Some((255, 255, 255, 255)) - ); + assert_eq!(parse_css_color(" white "), Some((255, 255, 255, 255))); assert_eq!( parse_css_color("rgb( 10 , 20 , 30 )"), Some((10, 20, 30, 255))