From 26156c41b4f5f00efc838c5dee34542ddb1b0ea1 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 11 Aug 2026 12:35:56 +0200 Subject: [PATCH] feat(animation): interpolate more properties, and name the ones that cannot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `style.transition` promised smoothing that two properties delivered. Every other property snapped at the step's `at`, and the field's own doc comment admitted it — but nothing told the author. `validate` never looked at `style.transition` or at the diffs between `timeline` states, so a transition on `width` or `border-radius` passed clean and rendered as a jump. The missing capability was not the problem. The silence was. **Diagnosing comes first.** `check_transition_smoothing` replays the timeline in the author's order, diffs each field against the running state, and classifies every touched property into four buckets, each with its own message. A layout property explains *why* it cannot interpolate and points at `transform: translate`/`scale` as the paint-time alternative; a discrete property says the snap is expected CSS behaviour, not a rustmotion limitation — so nobody chases a bug that is not one. The check only runs when `style.transition` is actually set: nothing was promised otherwise. **Then interpolation, for what can be done honestly.** `background` (solid colours) and `border-radius` (uniform, absolute px) now animate. They are resolved onto `CssStyle` before layout, where `paint_pass` already reads them for the static case — so nothing in the frozen paint or layout passes had to change, and the interpolation maths is the existing keyframe solver, not a second one. Layout properties are deliberately left snapping. Interpolating `width` without re-running layout would put the measured box and the painted pixels out of step and blind the geometry validator — the class of bug this repository already spent a chantier repairing. Signalled, not simulated. Mixed units are refused rather than guessed: `box_builder` runs before layout, so `%` and `em` have no trustworthy base yet. The diagnostic and the runtime share one predicate for what is resolvable, so they cannot drift into disagreeing about it. Also removed: a diagnostic I had written for unknown `Animation.property` values before discovering that `schema/video.rs` already rejects them at parse time with a did-you-mean. A test pins that behaviour instead, rather than shipping dead code that looks like a feature. No shipped example changes: none of the eight declares `timeline` on a component, verified by walking the JSON rather than by grep. --- .../src/commands/validate_schema.rs | 434 +++++++++++++++++- .../rustmotion-components/src/box_builder.rs | 206 +++++++++ .../tests/transition_interpolation.rs | 164 +++++++ crates/rustmotion-core/src/css/style.rs | 51 +- crates/rustmotion-core/src/engine/animator.rs | 35 +- 5 files changed, 886 insertions(+), 4 deletions(-) create mode 100644 crates/rustmotion-components/tests/transition_interpolation.rs diff --git a/crates/rustmotion-cli/src/commands/validate_schema.rs b/crates/rustmotion-cli/src/commands/validate_schema.rs index 3cb2985..a998c70 100644 --- a/crates/rustmotion-cli/src/commands/validate_schema.rs +++ b/crates/rustmotion-cli/src/commands/validate_schema.rs @@ -3,7 +3,7 @@ use rustmotion::components::{ChildComponent, Component}; use rustmotion::core::css::style::{ - Background, BackgroundLayer, Color, CssStyle, Display as CssDisplay, + Background, BackgroundLayer, BorderRadius, Color, CssStyle, Display as CssDisplay, }; use rustmotion::schema::{AnimationEffect, CharAnimationTiming, ResolvedScenario, SpringConfig}; @@ -99,6 +99,18 @@ fn validate_children( // catching it before anyone renders. check_style_colors(style, &p, errors); + // Generic interpolation (issue: "interpolation générique de + // n'importe quelle propriété"): `timeline` + `style.transition` + // accepts a transition on any CSS property and used to silently + // snap instead of animating it for everything except opacity and + // color (text/counter). This turns that silence into a named + // diagnostic — see the function's doc comment. (The sibling gap — + // an explicit `style.animation` keyframes effect naming an + // unrecognized `property` — turned out to already be closed at + // deserialize time by `schema/video.rs`'s + // `validate_motion_property`; verified, not reopened here.) + check_transition_smoothing(&child.component, &p, warnings); + if let Some(timed) = child.component.as_timed() { let (start, end) = timed.timing(); if let (Some(s), Some(e)) = (start, end) { @@ -372,6 +384,250 @@ fn check_color_str(s: &str, label: &str, path: &str, errors: &mut Vec) { } } +// ─── Generic interpolation of timeline/style.transition properties ──────── +// +// Two independent silent-gap classes existed before this workstream, both +// rooted in the same fact: `style.animation`/`timeline` accept a transition +// on *any* named CSS/animation property, but the engine only actually knows +// how to smoothly interpolate a handful of them. Everything else either (a) +// snaps at the step's `at` instead of animating (`style.transition` + +// `timeline` style states — see `box_builder.rs::apply_style_states`'s doc +// comment), or (b) has no effect whatsoever, not even a snap (an explicit +// `style.animation: [{ "name": "keyframes", "keyframes": [{ "property": +// "…" }] }]` targeting a name `animator::apply_property` doesn't recognize). +// `validate` used to say nothing about either. These two checks do. + +/// Classification of a CSS property with respect to `style.transition` + +/// `timeline` style-state smoothing (`check_transition_smoothing` below). +/// Mirrors real CSS's own interpolable/discrete split — see each variant. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TransitionPropertyKind { + /// Already smoothed: `opacity` (always, when `style.transition` is + /// set), `color` (text/counter), `background`/`border-radius` (solid + /// colour / uniform absolute px — see the finer shape check in the + /// caller for when a *specific* value isn't one of those shapes). + Smoothed, + /// CSS-spec-discrete (keyword/enum-valued) — snapping is the correct, + /// expected behaviour, exactly like real CSS `transition-property` + /// would do. Still diagnosed (reassuring wording, not an alarm): an + /// author who set `style.transition` and sees a hard cut on this + /// property deserves a line saying that's expected, not silence either + /// way — "sauter et le dire" per the workstream brief. + Discrete, + /// Numeric/continuous but affects the layout box (size/position/flow). + /// Interpolating it would require the value to reach `run_layout` on + /// every sampled frame — out of reach without changing the frozen + /// `layout_pass.rs`/`paint_pass.rs`, and the reason this workstream + /// draws a hard line between paint-time and layout-time properties + /// (see the workstream report's "piège" section). + Layout, + /// Numeric/continuous, paint-time, but not yet wired up to interpolate + /// (includes the empty/unrecognized-name fallback below — a future + /// `CssStyle` field this table hasn't been taught about yet fails loud, + /// not silent). + UnsupportedPaint, +} + +/// Classify a `CssStyle` field by its kebab-case JSON key (the wire name — +/// matches what `apply_style_states`'s own `serde_json` merge keys on, and +/// what an author actually typed under `style`/a `timeline[*].style`). +fn classify_transition_property(name: &str) -> TransitionPropertyKind { + use TransitionPropertyKind::*; + match name { + "opacity" | "color" | "background" | "border-radius" => Smoothed, + "display" + | "position" + | "box-sizing" + | "flex-direction" + | "flex-wrap" + | "justify-content" + | "align-items" + | "align-self" + | "align-content" + | "justify-items" + | "justify-self" + | "grid-auto-flow" + | "font-family" + | "font-weight" + | "font-style" + | "text-align" + | "white-space" + | "overflow-wrap" + | "text-overflow" + | "text-decoration" + | "text-autofit" + | "mix-blend-mode" + | "clip-path" + | "overflow" + | "overflow-x" + | "overflow-y" + | "visibility" + | "z-index" + | "order" + | "grid-template-columns" + | "grid-template-rows" + | "grid-column" + | "grid-row" => Discrete, + "top" | "right" | "bottom" | "left" | "width" | "height" | "min-width" | "min-height" + | "max-width" | "max-height" | "margin" | "padding" | "border" | "aspect-ratio" | "gap" + | "flex-grow" | "flex-shrink" | "flex-basis" | "font-size" | "line-height" + | "letter-spacing" => Layout, + // "animation"/"transition"/"audio-reactive" are config, not visual + // state, and are filtered out of the walk before this is ever + // called (see `check_transition_smoothing`) — they never reach + // this match. Everything else — box-shadow, text-shadow, + // gradient-border, filter, backdrop-filter, transform, + // transform-origin, perspective, perspective-origin, depth, + // backdrop-blur, inner-shadow, and any `CssStyle` field added later + // that this table hasn't been taught about — fails loud here by + // design: an unrecognized name is treated as "known not to smooth" + // rather than silently passed through. + _ => UnsupportedPaint, + } +} + +/// `style.transition` promises to smooth whichever CSS properties a +/// `timeline` step changes — but `box_builder.rs`'s +/// `apply_style_states`/`resolve_transition_css_overrides`/ +/// `transition_keyframes` only actually smooth `opacity`, `color` +/// (text/counter), `background` (solid colour), and `border-radius` +/// (uniform absolute px). Everything else still snaps at the step's `at`. +/// +/// This walks the declared `timeline` in author order — not tied to any +/// particular render time, unlike the runtime: a static check must catch +/// every step-to-step (and base-to-first-step) diff, not just whichever one +/// happens to be "due" at some sampled `t`. Only runs when `style.transition` +/// is actually set: if it isn't, nothing was ever promised, and every +/// property snapping is exactly the documented, expected behaviour (no +/// diagnostic needed). +fn check_transition_smoothing(component: &Component, path: &str, warnings: &mut Vec) { + let style = component.as_styled().style_config(); + if style.transition.is_none() { + return; + } + let Some(animatable) = component.as_animatable() else { + return; + }; + let mut relevant: Vec<&rustmotion::schema::TimelineStep> = animatable + .timeline_steps() + .iter() + .filter(|s| s.style.is_some()) + .collect(); + if relevant.is_empty() { + return; + } + relevant.sort_by(|a, b| a.at.total_cmp(&b.at)); + + let Ok(mut current) = serde_json::to_value(style) else { + return; + }; + let mut warned: std::collections::HashSet = std::collections::HashSet::new(); + + for step in relevant { + let step_style = step.style.as_deref().unwrap(); + let Ok(serde_json::Value::Object(state)) = serde_json::to_value(step_style) else { + continue; + }; + let serde_json::Value::Object(cur_obj) = ¤t else { + continue; + }; + let mut merged = cur_obj.clone(); + for (k, v) in &state { + if v.is_null() || k == "animation" || k == "transition" || k == "audio-reactive" { + continue; + } + let changed = merged.get(k) != Some(v); + if changed && !warned.contains(k.as_str()) { + match classify_transition_property(k) { + TransitionPropertyKind::Discrete => { + // Still named, not silent: `display`/`position`/etc. + // have no in-between value (real CSS can't animate + // them either), so this is expected, correct + // behaviour — not a gap. The wording deliberately + // reads as reassurance, not an alarm, but the point + // is that an author who set `style.transition` + // expecting *something* to smooth still gets a line + // telling them exactly which property didn't and + // why, instead of silence either way. + warnings.push(format!( + "{path}: style.transition is set and timeline changes `{k}`, but \ + `{k}` is a discrete CSS property (no value exists in between the two \ + states) — it always snaps at the step's `at`, exactly like real CSS. \ + This is expected, not a rustmotion limitation." + )); + warned.insert(k.clone()); + } + TransitionPropertyKind::Smoothed => { + // `background`/`border-radius` only actually smooth + // for a specific value shape (solid colour / uniform + // absolute px) — anything else in the recognized- + // property bucket must still be caught, or an + // author using per-corner radii or a gradient would + // get silence again, just one level deeper. + let resolves = match k.as_str() { + "border-radius" => { + let mut probe = merged.clone(); + probe.insert(k.clone(), v.clone()); + serde_json::from_value::(serde_json::Value::Object(probe)) + .ok() + .and_then(|s| s.border_radius) + .and_then(|br| BorderRadius::absolute_px(&br)) + .is_some() + } + "background" => { + let mut probe = merged.clone(); + probe.insert(k.clone(), v.clone()); + serde_json::from_value::(serde_json::Value::Object(probe)) + .ok() + .and_then(|s| s.background) + .and_then(|bg| Background::solid_hex(&bg)) + .is_some() + } + _ => true, + }; + if !resolves { + let hint = if k == "border-radius" { + "only a single uniform px/unitless radius is smoothed today — \ + per-corner radii and %/em/rem/vw/vh are not" + } else { + "only a solid colour is smoothed today — gradients and image \ + layers are not" + }; + warnings.push(format!( + "{path}: style.transition is set and timeline changes `{k}`, but \ + this value isn't a shape rustmotion can interpolate yet ({hint}) \ + — it will snap instead of animating." + )); + warned.insert(k.clone()); + } + } + TransitionPropertyKind::Layout => { + warnings.push(format!( + "{path}: style.transition is set and timeline changes `{k}`, but \ + `{k}` affects layout (box size/position/flow) — rustmotion cannot \ + interpolate a layout property without re-running layout on every \ + sampled frame, so it will snap instead of animating. Consider \ + approximating the motion with `transform: translate`/`scale` \ + instead, which is paint-time and does interpolate." + )); + warned.insert(k.clone()); + } + TransitionPropertyKind::UnsupportedPaint => { + warnings.push(format!( + "{path}: style.transition is set and timeline changes `{k}`, but \ + rustmotion does not yet know how to interpolate `{k}` — it will \ + snap instead of animating at the step's `at`." + )); + warned.insert(k.clone()); + } + } + } + merged.insert(k.clone(), v.clone()); + } + current = serde_json::Value::Object(merged); + } +} + /// Constat #6: reject `SpringConfig` values that would make /// `engine::animator::spring_value` produce NaN (`mass <= 0`, `stiffness <= /// 0`) or diverge instead of settle (`damping < 0`). The solver itself also @@ -1001,3 +1257,179 @@ mod color_validation_tests { assert!(errors.is_empty(), "unexpected errors: {errors:?}"); } } + +#[cfg(test)] +mod transition_smoothing_tests { + use super::*; + + fn warnings_for(json: serde_json::Value) -> Vec { + let child: ChildComponent = serde_json::from_value(json).unwrap(); + let mut errors = Vec::new(); + let mut warnings = Vec::new(); + validate_children(&[child], "test", 4.0, &mut errors, &mut warnings); + warnings + } + + #[test] + fn layout_property_change_under_transition_is_diagnosed() { + // `width` affects the layout box — smoothing it would require + // `run_layout` on every sampled frame, which this workstream leaves + // to a future one (see the "piège" in the workstream report). It + // must still snap, but `validate` must say so instead of staying + // silent about it, the way it always has until now. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "width": "100px", "transition": 0.5 }, + "timeline": [{ "at": 1.0, "style": { "width": "300px" } }] + })); + assert!( + warnings + .iter() + .any(|w| w.contains("`width`") && w.contains("layout") && w.contains("snap")), + "expected a layout-property diagnostic naming `width`: {warnings:?}" + ); + } + + #[test] + fn unsupported_paint_property_change_under_transition_is_diagnosed() { + // `transform` is paint-time but this workstream deliberately did not + // implement list-shaped interpolation for it (see workstream + // report) — it must be diagnosed, not silently accepted just + // because it's "only" paint, not layout. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "transition": 0.5 }, + "timeline": [{ "at": 1.0, "style": { "transform": [{ "fn": "scale", "x": 2, "y": 2 }] } }] + })); + assert!( + warnings + .iter() + .any(|w| w.contains("`transform`") && w.contains("snap")), + "expected an unsupported-paint diagnostic naming `transform`: {warnings:?}" + ); + } + + #[test] + fn discrete_property_change_under_transition_is_diagnosed_but_reassuringly() { + // `display` is CSS-spec-discrete — snapping is the correct, + // expected behaviour (real CSS can't animate it either) — but the + // brief is explicit that a discrete property must still "sauter et + // le dire", not sauter en silence: an author who set + // `style.transition` and sees `display` hard-cut deserves a line + // explaining that's expected, distinguishable from an actual gap. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "transition": 0.5, "display": "flex" }, + "timeline": [{ "at": 1.0, "style": { "display": "block" } }] + })); + assert!( + warnings + .iter() + .any(|w| w.contains("`display`") && w.contains("expected")), + "expected a reassuring discrete-property diagnostic naming `display`: {warnings:?}" + ); + // But it must read differently from an actual gap — never the + // "snap instead of animating" alarm wording the Layout/ + // UnsupportedPaint branches use. + assert!( + warnings + .iter() + .filter(|w| w.contains("`display`")) + .all(|w| !w.contains("snap instead of animating")), + "a discrete property's diagnostic must not read like a gap: {warnings:?}" + ); + } + + #[test] + fn newly_smoothed_properties_are_not_diagnosed() { + // `border-radius` (uniform, absolute px) and `background` (solid + // colour) are exactly the two properties this workstream taught + // `box_builder.rs` to interpolate — they must not warn. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "transition": 0.5, "border-radius": 0, "background": "#000000" }, + "timeline": [{ "at": 1.0, "style": { "border-radius": 40, "background": "#ffffff" } }] + })); + assert!( + warnings + .iter() + .all(|w| !w.contains("border-radius") && !w.contains("`background`")), + "newly-smoothed properties must not be diagnosed: {warnings:?}" + ); + } + + #[test] + fn per_corner_border_radius_is_diagnosed_as_an_unresolvable_shape() { + // The interpolable set is "border-radius" by name, but only a + // uniform, absolute-px value actually resolves + // (`BorderRadius::absolute_px`) — a per-corner shape must still be + // caught, not pass silently just because the property name matches. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "transition": 0.5, "border-radius": 0 }, + "timeline": [{ + "at": 1.0, + "style": { "border-radius": { "top-left": 10, "top-right": 0, "bottom-right": 0, "bottom-left": 0 } } + }] + })); + assert!( + warnings + .iter() + .any(|w| w.contains("border-radius") && w.contains("per-corner")), + "expected a shape-mismatch diagnostic for per-corner border-radius: {warnings:?}" + ); + } + + #[test] + fn no_transition_configured_means_no_diagnostic_at_all() { + // Without `style.transition`, nothing was ever promised — every + // property snapping (including `width`) is exactly the documented, + // expected behaviour. No diagnostic should fire. + let warnings = warnings_for(serde_json::json!({ + "type": "div", + "style": { "width": "100px" }, + "timeline": [{ "at": 1.0, "style": { "width": "300px" } }] + })); + assert!( + warnings.iter().all(|w| !w.contains("width")), + "no style.transition means no diagnostic is expected: {warnings:?}" + ); + } + + #[test] + fn unknown_explicit_keyframes_property_is_rejected_at_parse_time_not_validate_time() { + // A sibling silent-gap hypothesis this workstream investigated and + // found already closed: an explicit `style.animation` keyframes + // effect naming a `property` `animator::apply_property` doesn't + // recognize (e.g. a CSS-ish `"background-color"` instead of the + // solver's `"background"`/`"color"`) used to *look* like the same + // "known property but not wired up" gap `check_transition_smoothing` + // covers above — but it isn't reachable that far: `schema/video.rs`'s + // `deserialize_validated_keyframes`/`validate_motion_property` + // (constat #4, an earlier workstream) already rejects it during + // `Component` deserialization, with a did-you-mean suggestion, well + // before a scenario ever reaches `validate_scenario`. This test + // pins that down instead of re-diagnosing something `validate` + // structurally cannot ever see. + let err = serde_json::from_value::(serde_json::json!({ + "type": "div", + "style": { + "animation": [{ + "name": "keyframes", + "keyframes": [{ + "property": "background-color", + "keyframes": [ + { "time": 0.0, "value": "#000000" }, + { "time": 1.0, "value": "#ffffff" } + ] + }] + }] + } + })) + .expect_err("an unrecognized animation property must fail to deserialize"); + assert!( + err.to_string().contains("background-color"), + "expected the parse error to name the offending property: {err}" + ); + } +} diff --git a/crates/rustmotion-components/src/box_builder.rs b/crates/rustmotion-components/src/box_builder.rs index 2ec06c0..d0ffe02 100644 --- a/crates/rustmotion-components/src/box_builder.rs +++ b/crates/rustmotion-components/src/box_builder.rs @@ -285,6 +285,21 @@ fn build_ghosts<'a>( if steps.iter().any(|s| s.style.is_some()) { let skip_opacity = css.transition.is_some(); apply_style_states(&mut css, steps, ghost_time - stagger_delay, skip_opacity); + // Same `border-radius`/`background` smoothing as the + // principal path in `build_child`, sampled at `ghost_time` + // so a motion-blur/trail ghost mid-transition matches what + // the principal will look like at that same instant. + let overrides = resolve_transition_css_overrides( + child.component.as_styled().style_config(), + steps, + ghost_time - stagger_delay, + ); + if let Some(br) = overrides.border_radius { + css.border_radius = Some(br); + } + if let Some(bg) = overrides.background { + css.background = Some(bg); + } } } // Resolve animation props at the *ghost* time. @@ -478,6 +493,23 @@ fn build_child<'a>( let t = local_actx.map(|a| a.time).unwrap_or(0.0); let skip_opacity = css.transition.is_some(); apply_style_states(&mut css, steps, t - stagger_delay, skip_opacity); + // `border-radius`/`background` (solid colour, uniform absolute + // px only — see `resolve_transition_css_overrides`'s doc + // comment) smooth the same way opacity does above, but land + // directly on `css` instead of through the generic effects + // pipeline: no `AnimatedProperties` field for them is ever read + // by a painter, so that pipeline is a dead end for these two. + let overrides = resolve_transition_css_overrides( + child.component.as_styled().style_config(), + steps, + t - stagger_delay, + ); + if let Some(br) = overrides.border_radius { + css.border_radius = Some(br); + } + if let Some(bg) = overrides.background { + css.background = Some(bg); + } } } @@ -805,6 +837,180 @@ pub(crate) fn transition_keyframes( out } +/// Resolved `style.transition` smoothing for `border-radius`/`background`, +/// ready to be written straight onto a `CssStyle`. +pub(crate) struct TransitionCssOverrides { + pub border_radius: Option, + pub background: Option, +} + +/// CSS-native smoothing for `border-radius` (uniform, absolute-px only) and +/// `background` (solid colour only) timeline style-state changes. +/// +/// Unlike `opacity`/`color` in `transition_keyframes` above, neither +/// property has anywhere to land in `AnimatedProperties` that any painter or +/// the CSS bridge (`css/animation.rs::apply_animated_props`) actually reads +/// (see `KNOWN_ANIMATABLE_PROPERTIES`'s doc comment in `animator.rs`) — +/// every painter reads `css.border_radius`/`css.background` straight off +/// the node's own `CssStyle` (`paint_pass.rs`, frozen, already does this for +/// the static case). So instead of synthesizing an `AnimationEffect` for the +/// generic effects pipeline (a dead end for these two), this resolves the +/// interpolated value directly and returns it for the caller to write onto +/// the box's `CssStyle` by hand — reusing `animator::resolve_keyframe_track` +/// for the actual segment/easing/spring math rather than reinventing it. +/// +/// Gated on `style.transition` being set, mirroring `opacity`'s gate above +/// (not `color`'s forced near-instant ramp — nothing downstream *requires* +/// these two to smooth the way text painters require `color` to). Absent an +/// explicit `style.transition`, this returns an all-`None` result, so every +/// existing scenario without one renders byte-identical to before this +/// workstream. +/// +/// **"Unités mixtes" decision** (see workstream report): resolves only when +/// *both* the origin and the target value are the exact shape +/// `BorderRadius::absolute_px`/`Background::solid_hex` can resolve without a +/// `LengthContext` — uniform absolute px, solid colour. Anything else +/// (per-corner radii, `%`/`em`/`rem`/`vw`/`vh`, gradients, image layers) is +/// refused rather than guessed: that property just falls back to +/// `apply_style_states`'s existing snap. `validate_schema.rs` calls the +/// exact same two predicates so the diagnostic and the runtime can never +/// disagree about what's interpolable. +pub(crate) fn resolve_transition_css_overrides( + base: &CssStyle, + steps: &[rustmotion_core::schema::TimelineStep], + t: f64, +) -> TransitionCssOverrides { + use rustmotion_core::css::style::{Background, BorderRadius, Color}; + use rustmotion_core::css::units::LengthPercentage as CssLP; + use rustmotion_core::engine::animator::resolve_keyframe_track; + use rustmotion_core::schema::{Animation, Keyframe, KeyframeValue}; + + let mut out = TransitionCssOverrides { + border_radius: None, + background: None, + }; + let Some(tr) = base.transition.as_ref() else { + return out; + }; + if tr.duration() <= 0.0 { + return out; + } + let duration = tr.duration(); + let easing = tr.easing(); + + let mut sorted: Vec<&rustmotion_core::schema::TimelineStep> = + steps.iter().filter(|s| s.style.is_some()).collect(); + sorted.sort_by(|a, b| a.at.total_cmp(&b.at)); + + let mut prev_radius = base + .border_radius + .as_ref() + .and_then(BorderRadius::absolute_px); + let mut prev_bg = base.background.as_ref().and_then(Background::solid_hex); + let mut radius_kfs: Vec = Vec::new(); + let mut bg_kfs: Vec = Vec::new(); + + // Same ascending-time bookkeeping as `transition_keyframes`'s + // `push_pair` above (kept local — a shared closure can't easily borrow + // two different `Vec`s across both loops below without upsetting the + // borrow checker for no real benefit at this size). + let push_pair = |kfs: &mut Vec, at: f64, from: Keyframe, to: Keyframe| { + let floor = kfs.last().map(|k| k.time + 1e-6).unwrap_or(f64::MIN); + let start = at.max(floor); + let mut from = from; + let mut to = to; + from.time = start; + to.time = to.time.max(start + 1e-6); + kfs.push(from); + kfs.push(to); + }; + + for step in sorted { + let style = step.style.as_deref().unwrap(); + if let Some(br) = style.border_radius.as_ref() { + match br.absolute_px() { + Some(target) => { + if prev_radius != Some(target) { + if let Some(from) = prev_radius { + push_pair( + &mut radius_kfs, + step.at, + Keyframe { + time: step.at, + value: KeyframeValue::Number(from as f64), + easing: None, + }, + Keyframe { + time: step.at + duration, + value: KeyframeValue::Number(target as f64), + easing: None, + }, + ); + } + prev_radius = Some(target); + } + } + // Unresolvable shape (per-corner, %/em/rem/vw/vh) — lose the + // interpolation origin for *this* transition only; + // `validate_schema.rs` diagnoses this exact step, and a + // later resolvable value simply resumes smoothing from + // itself onward (see the doc comment above). + None => prev_radius = None, + } + } + if let Some(bg) = style.background.as_ref() { + match bg.solid_hex() { + Some(target) => { + if prev_bg.as_deref() != Some(target.as_str()) { + if let Some(from) = prev_bg.clone() { + push_pair( + &mut bg_kfs, + step.at, + Keyframe { + time: step.at, + value: KeyframeValue::Color(from), + easing: None, + }, + Keyframe { + time: step.at + duration, + value: KeyframeValue::Color(target.clone()), + easing: None, + }, + ); + } + prev_bg = Some(target); + } + } + None => prev_bg = None, + } + } + } + + if !radius_kfs.is_empty() { + let anim = Animation { + property: "border_radius".to_string(), + keyframes: radius_kfs, + easing: easing.clone(), + spring: None, + }; + if let KeyframeValue::Number(v) = resolve_keyframe_track(&anim, t) { + out.border_radius = Some(BorderRadius::Uniform(CssLP::Px(v as f32))); + } + } + if !bg_kfs.is_empty() { + let anim = Animation { + property: "background".to_string(), + keyframes: bg_kfs, + easing, + spring: None, + }; + if let KeyframeValue::Color(c) = resolve_keyframe_track(&anim, t) { + out.background = Some(Background::Color(Color::String(c))); + } + } + out +} + /// Quick gate: does this resolved `AnimatedProperties` carry any property /// that we know how to translate to CSS? Avoids allocating a transform Vec /// when there's nothing to apply. diff --git a/crates/rustmotion-components/tests/transition_interpolation.rs b/crates/rustmotion-components/tests/transition_interpolation.rs new file mode 100644 index 0000000..2d9d084 --- /dev/null +++ b/crates/rustmotion-components/tests/transition_interpolation.rs @@ -0,0 +1,164 @@ +//! Generic interpolation of `timeline`/`style.transition` (issue: "generic +//! interpolation of any property"). Routes JSON straight through +//! `box_builder` (no layout/paint needed — the interpolated value is fully +//! decided in `CssStyle` before layout ever runs) and inspects the +//! resulting `BoxNode.css` at a sampled mid-transition time. +//! +//! Before this workstream, `transition_keyframes`/`apply_style_states` +//! (`box_builder.rs`) only smoothed `opacity` and `color` (text/counter +//! only) — every other property snapped straight to the target value the +//! instant `t >= step.at`, `style.transition` or not. The first test below +//! (`red_*`) pins that snap down with a mid-transition sample: at exactly +//! half the transition duration, a snapping property already equals its +//! *end* value, not something in between. That's the signature of a jump, +//! not an animation — a bounds-only test (checking only t=0 and t=1) would +//! pass even on a hard cut. + +use rustmotion_components::box_builder::{build_scene_at_time, BuildAnimationCtx}; +use rustmotion_components::{ChildComponent, Component, PositionMode}; +use rustmotion_core::css::style::{Background, BorderRadius, CssStyle}; + +const SCENE_DURATION: f64 = 4.0; +const STEP_AT: f64 = 1.0; +const DURATION: f64 = 1.0; +const MIDPOINT: f64 = STEP_AT + DURATION / 2.0; // 1.5 + +fn div_with_transition( + from_radius: f64, + to_radius: f64, + from_bg: &str, + to_bg: &str, +) -> serde_json::Value { + serde_json::json!({ + "type": "div", + "style": { + "background": from_bg, + "border-radius": from_radius, + "transition": DURATION + }, + "timeline": [ + { "at": STEP_AT, "style": { "background": to_bg, "border-radius": to_radius } } + ] + }) +} + +fn css_at(json: serde_json::Value, time: f64) -> CssStyle { + let component: Component = serde_json::from_value(json).expect("deserialize component"); + let child = ChildComponent { + component, + position: Some(PositionMode::Absolute { x: 0.0, y: 0.0 }), + x: None, + y: None, + z_index: None, + bleed: false, + }; + let children = vec![child]; + let built = build_scene_at_time( + &children, + (800.0, 600.0), + CssStyle::default(), + BuildAnimationCtx { + time, + scene_duration: SCENE_DURATION, + fps: 30, + }, + ); + built.root.children[0].css.clone() +} + +fn radius_px(css: &CssStyle) -> f32 { + match css.border_radius.as_ref().expect("border-radius set") { + BorderRadius::Uniform(lp) => lp.px(), + other => panic!("expected uniform border-radius, got {other:?}"), + } +} + +fn bg_hex(css: &CssStyle) -> String { + match css.background.as_ref().expect("background set") { + Background::Color(c) => c.to_css_string(), + other => panic!("expected solid background, got {other:?}"), + } +} + +/// GREEN (after the fix): `border-radius` is a *paint-only* CSS property +/// (it decorates a box `paint_pass.rs` already laid out; it never feeds +/// `run_layout`), so it's safe to interpolate at `box_builder` time. +/// Mid-transition (t=1.5, halfway through a 1s ease-in-out-in transition +/// from 0 to 40) must land strictly between the two endpoints. +#[test] +fn border_radius_interpolates_at_midpoint() { + let json = div_with_transition(0.0, 40.0, "#000000", "#000000"); + + let before = radius_px(&css_at(json.clone(), STEP_AT - 0.01)); + let mid = radius_px(&css_at(json.clone(), MIDPOINT)); + let after = radius_px(&css_at(json.clone(), STEP_AT + DURATION + 0.01)); + + assert!((before - 0.0).abs() < 0.5, "pre-step radius: {before}"); + assert!( + (after - 40.0).abs() < 0.5, + "post-transition radius: {after}" + ); + assert!( + mid > 2.0 && mid < 38.0, + "expected a genuine mid-transition value strictly between 0 and 40, got {mid} \ + (a value pinned to 0 or 40 here means the property snapped instead of interpolating)" + ); +} + +/// Same proof for `background` (solid-colour only): the interpolated hex at +/// the midpoint must differ from both the `from` and `to` colours. +#[test] +fn background_color_interpolates_at_midpoint() { + let json = div_with_transition(0.0, 0.0, "#000000", "#ffffff"); + + let mid = bg_hex(&css_at(json.clone(), MIDPOINT)); + let after = bg_hex(&css_at(json.clone(), STEP_AT + DURATION + 0.01)); + + assert_eq!(after.to_lowercase(), "#ffffff"); + assert_ne!( + mid.to_lowercase(), + "#000000", + "background should have started moving away from its origin by the midpoint" + ); + assert_ne!( + mid.to_lowercase(), + "#ffffff", + "background reached its destination color before the transition finished — that's a jump, \ + got {mid} at the midpoint" + ); +} + +/// `width` is a *layout* property (it must reach `run_layout`, which +/// `box_builder` cannot do on its own — the piège this workstream's brief +/// calls out explicitly). It is deliberately NOT interpolated: it must keep +/// snapping exactly like every other unhandled property, `style.transition` +/// or not. This pins that "signal the gap, don't fake the fix" contract: +/// `validate` growing a diagnostic here is fine; the renderer silently +/// getting this "right" by some accident would not be — it would mean a +/// layout property was interpolated purely at paint time, decoupled from +/// the box the geometry validator actually measured. +#[test] +fn width_still_snaps_because_it_is_a_layout_property() { + let json = serde_json::json!({ + "type": "div", + "style": { + "width": "100px", + "transition": DURATION + }, + "timeline": [ + { "at": STEP_AT, "style": { "width": "300px" } } + ] + }); + + let mid = css_at(json.clone(), MIDPOINT).width.expect("width set"); + // Snap semantics: at the midpoint the value already equals the *target* + // (300px), not something in between (e.g. ~200px). + let px = match mid { + rustmotion_core::css::style::Size::Length(lp) => lp.px(), + other => panic!("expected a length, got {other:?}"), + }; + assert!( + (px - 300.0).abs() < 0.5, + "width is documented as still snapping (not yet layout-integrated); got {px}, expected 300 (the snapped target)" + ); +} diff --git a/crates/rustmotion-core/src/css/style.rs b/crates/rustmotion-core/src/css/style.rs index 160b367..01545ff 100644 --- a/crates/rustmotion-core/src/css/style.rs +++ b/crates/rustmotion-core/src/css/style.rs @@ -219,8 +219,15 @@ pub struct CssStyle { #[serde(default, deserialize_with = "deserialize_animation_effects")] pub animation: Vec, /// Smoothing for `timeline` style-state changes. Supported properties: - /// `opacity`, and `color` on text/counter; everything else snaps at the - /// step's `at`. + /// `opacity`; `color` on text/counter; `background` (solid colour only) + /// and `border-radius` (single uniform, absolute-px value only) — see + /// `box_builder.rs`'s `transition_keyframes`/ + /// `resolve_transition_css_overrides`. Everything else — including a + /// `background`/`border-radius` value outside the shape those two + /// support (gradients, per-corner radii, `%`/`em`/`rem`/`vw`/`vh`) — + /// still snaps at the step's `at`; `rustmotion validate`'s + /// `check_transition_smoothing` (`validate_schema.rs`) reports exactly + /// which property and why whenever it would otherwise snap silently. pub transition: Option, // ---- Audio reactive binding ---- @@ -728,6 +735,30 @@ pub enum BorderRadius { }, } +impl BorderRadius { + /// The single uniform radius as an absolute pixel value, or `None` when + /// this isn't a shape a context-free (pre-layout) resolver can safely + /// interpolate: per-corner radii (which corner "wins" a 2-point + /// interpolation is undefined), or a unit that needs a + /// [`crate::css::units::LengthContext`] the caller doesn't have yet + /// (`%`/`em`/`rem`/`vw`/`vh` — see the "unités mixtes" decision in + /// `box_builder.rs`'s `resolve_transition_overrides`: resolved only + /// where both endpoints are unambiguous, refused otherwise rather than + /// guessed). Used by `box_builder.rs` (`style.transition` smoothing) and + /// `validate_schema.rs` (the matching diagnostic) — both must agree on + /// exactly which shapes are interpolable, which is why this lives here + /// once instead of being reimplemented on each side. + pub fn absolute_px(&self) -> Option { + match self { + BorderRadius::Uniform(lp) => match lp.try_parse() { + Some(crate::css::units::ParsedLength::Px(v)) => Some(v), + _ => None, + }, + BorderRadius::Corners { .. } => None, + } + } +} + // ---- Flex ---- #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] @@ -1053,6 +1084,22 @@ pub enum Background { Single(BackgroundLayer), } +impl Background { + /// The background's hex/rgba string when it's a plain solid colour, or + /// `None` for anything else (gradients, image layers, multi-layer + /// stacks) — those need real paint-time compositing to interpolate + /// correctly, which is out of reach for a pre-layout `CssStyle` value. + /// Same shared-predicate rationale as [`BorderRadius::absolute_px`]: + /// `box_builder.rs`'s smoothing and `validate_schema.rs`'s diagnostic + /// both call this so they can never disagree about what's interpolable. + pub fn solid_hex(&self) -> Option { + match self { + Background::Color(c) => Some(c.to_css_string()), + Background::Layers(_) | Background::Single(_) => None, + } + } +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(tag = "kind", rename_all = "kebab-case")] pub enum BackgroundLayer { diff --git a/crates/rustmotion-core/src/engine/animator.rs b/crates/rustmotion-core/src/engine/animator.rs index 3fbf118..b20a02c 100644 --- a/crates/rustmotion-core/src/engine/animator.rs +++ b/crates/rustmotion-core/src/engine/animator.rs @@ -845,6 +845,28 @@ enum ResolvedValue { Color(String), } +/// Public wrapper around `resolve_animation_value_full` for callers outside +/// this module that want to reuse the exact segment/easing/spring +/// interpolation math (ordering, per-keyframe easing override, clamping at +/// the ends) on a synthetic `Animation` they built themselves, without +/// routing the result through `AnimatedProperties`/`apply_property`. +/// +/// This is how `box_builder.rs`'s `style.transition` smoothing for +/// `border-radius`/`background` is implemented: those two properties are +/// paint-time `CssStyle` fields that every painter already reads directly +/// (via `paint_pass.rs`, frozen) — there is no `AnimatedProperties` field +/// for them to land in that anything downstream would ever look at, so +/// resolving through the generic effects pipeline the way `opacity`/`color` +/// do would be a dead end. Calling this directly and writing the resolved +/// `CssStyle` field by hand instead reuses the proven interpolation math +/// while staying entirely inside `box_builder.rs`'s own file scope. +pub fn resolve_keyframe_track(anim: &Animation, time: f64) -> KeyframeValue { + match resolve_animation_value_full(anim, time) { + ResolvedValue::Number(n) => KeyframeValue::Number(n), + ResolvedValue::Color(c) => KeyframeValue::Color(c), + } +} + fn resolve_animation_value_full(anim: &Animation, time: f64) -> ResolvedValue { let keyframes = &anim.keyframes; if keyframes.is_empty() { @@ -920,7 +942,7 @@ fn parse_hex_components(hex: &str) -> (f64, f64, f64, f64) { } /// Interpolate between two hex colors -fn lerp_color(c1: &str, c2: &str, t: f64) -> String { +pub fn lerp_color(c1: &str, c2: &str, t: f64) -> String { let (r1, g1, b1, a1) = parse_hex_components(c1); let (r2, g2, b2, a2) = parse_hex_components(c2); let r = (r1 + (r2 - r1) * t).clamp(0.0, 255.0) as u8; @@ -968,6 +990,17 @@ fn apply_property(props: &mut AnimatedProperties, property: &str, value: f64) { } } +// Note: an earlier workstream (constat #4, `schema/video.rs`) already closed +// the "unrecognized `Animation.property` is a silent no-op" gap this +// function's catch-all (`_ => {}` above) would otherwise hide — +// `KeyframesConfig.keyframes` deserializes through +// `deserialize_validated_keyframes`/`validate_motion_property`, which +// rejects any `property` outside `KNOWN_MOTION_PROPERTIES` (with a +// did-you-mean suggestion) at parse time, before a scenario ever reaches +// `validate`/render. This workstream verified that gap is closed rather +// than reopening it with a second, redundant "known properties" list here; +// see the workstream report's "generic interpolation" write-up. + // ─── Wiggle resolution ────────────────────────────────────────────────────── /// Simple noise function based on sine waves with seed for pseudo-random behavior