Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 77 additions & 1 deletion crates/rustmotion-cli/src/commands/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1182,6 +1182,7 @@ pub fn validate_geometry_animated(scenario: &ResolvedScenario) -> Vec<GeometryVi
&built.root.children,
&layouts,
&built.stagger_delays,
&built.time_params,
viewport,
vi,
si,
Expand All @@ -1206,6 +1207,7 @@ fn walk_anim(
boxes: &[BoxNode],
layouts: &LayoutResult,
stagger_delays: &[f64],
time_params: &[(f64, f64)],
viewport: (u32, u32),
vi: usize,
si: usize,
Expand Down Expand Up @@ -1257,8 +1259,26 @@ fn walk_anim(
// resolves them at absolute scene `time` — no re-timing by
// start_at, which would double-shift components that also
// declare a matching `animation` delay.
//
// `time` above is *global* scene time; the renderer never
// resolves effects at that raw value once a `time_scale`/
// `time_offset`-bearing container is in the ancestor chain —
// `build_child` remaps it first (`box_builder.rs`:
// `t_local = scale * t_global + shift`). Constat #3: this walker
// used to skip that remap entirely, resolving effects at a time
// that never occurs at render — false positives when the
// container speeds the subtree up past the checked sample, false
// negatives when it slows it down. `built.time_params` carries
// the exact same accumulated `(scale, shift)` the renderer used,
// indexed by the same `NodeId`, so applying it here keeps this
// walker and the renderer in lockstep.
let (scale, shift) = time_params
.get(box_node.id as usize)
.copied()
.unwrap_or((1.0, 0.0));
let local_time = scale * time + shift;
let props = match effective_effects(&child.component, stagger_delay) {
Some(effects) => resolve_props_for_effects(&effects, time, scene_duration),
Some(effects) => resolve_props_for_effects(&effects, local_time, scene_duration),
None => AnimatedProperties::default(),
};
let raw_bbox = bbox_of(layout);
Expand Down Expand Up @@ -1307,6 +1327,7 @@ fn walk_anim(
&box_node.children,
layouts,
stagger_delays,
time_params,
viewport,
vi,
si,
Expand Down Expand Up @@ -1899,6 +1920,61 @@ mod tests {
assert_eq!(v.unwrap().axis, Axis::X);
}

#[test]
fn strict_anim_respects_a_containers_time_offset_remap() {
// Constat #3: `walk_anim` used to resolve effects at raw *global*
// scene time, ignoring any `time_scale`/`time_offset` remap
// accumulated from ancestor containers — even though the renderer
// (`box_builder::build_child`) always resolves at the *local*
// remapped time (`t_local = scale * t_global + shift`).
//
// Here the shape's `slide_in_left` (delay=0, duration=1.0) sits
// inside a `flex` with `time_offset: -5.0`, which (per
// `rustmotion/src/tests.rs`'s time-remap tests) shifts local time to
// `t_local = t_global + 5.0`. Every sample in this 2s scene
// (t_global in [0, 2]) therefore resolves at local time in [5, 7] —
// 5-7s past the 1s animation window, fully settled at rest (x=100,
// well inside the 1920px-wide viewport). A walker that ignores the
// remap instead resolves at raw t_global in [0, 2], still inside the
// animation's own [0, 1] window for the first half of the scene,
// and reports a slide-in overflow that never actually happens at
// render time.
let json = r##"{
"video": { "width": 1920, "height": 1080 },
"scenes": [{
"duration": 2.0,
"children": [{
"type": "flex",
"time_offset": -5.0,
"style": { "width": "1920px", "height": "1080px" },
"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 overflow: Vec<_> = violations
.iter()
.filter(|v| v.kind == ViolationKind::AnimatedTextOverflow)
.collect();
assert!(
overflow.is_empty(),
"time_offset=-5.0 settles the slide-in 5-7s before any sampled instant; a \
walker that honours the remap must report zero overflows, got: {:?}",
overflow
);
}

#[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
Expand Down
61 changes: 61 additions & 0 deletions crates/rustmotion-cli/src/commands/validate_attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -351,4 +351,65 @@ mod tests {
assert_eq!(errors.len(), 1, "expected one error: {errors:?}");
assert!(errors[0].contains("counter"), "got: {}", errors[0]);
}

#[test]
fn typo_inside_style_animation_effect_is_reported() {
// Constat #8: `walk_component` only compares a component's own
// top-level keys, then recurses into `children` — it never looks
// inside `style`, let alone `style.animation[*]`. A typo'd field on
// an animation effect (`duratoin` instead of `duration`) used to
// deserialize silently (the effect config structs had no
// `deny_unknown_fields`), so the author got a scenario that "worked"
// but quietly ran the default 0.8s duration instead of theirs.
//
// The fix lives in `schema/video.rs` (adding `deny_unknown_fields` to
// every `AnimationEffect` payload struct) rather than here: an
// internally-tagged enum's tag field is excluded from what the
// variant's own `Deserialize` sees, so this rejects the typo without
// ever flagging the legitimate `name` tag as unknown. That routes the
// typo through the *existing* typed-parse-failure path in
// `check_component_attrs` (the same one that already catches, e.g.,
// a missing required field) — it surfaces as a blocking error, not a
// `walk_component` warning.
let s = resolved(serde_json::json!([
{
"type": "text", "content": "hi",
"style": { "animation": [{ "name": "fade_in_up", "duratoin": 0.6 }] }
}
]));
let (errors, _) = check_component_attrs(&s);
assert!(
errors.iter().any(|e| e.contains("duratoin")),
"expected the typo'd animation-effect field to be reported as an error: {errors:?}"
);
}

#[test]
fn well_formed_animation_effect_fields_are_not_flagged() {
// Sanity companion to the typo test: legitimate fields across a
// spread of effect kinds (preset timing, keyframes, wiggle, glow,
// motion_blur, tilt_in) must not trip the new deny_unknown_fields.
let s = resolved(serde_json::json!([
{
"type": "text", "content": "hi",
"style": { "animation": [
{ "name": "fade_in_up", "delay": 0.2, "duration": 0.6, "loop": false,
"overshoot": 0.1, "spring": { "damping": 12, "stiffness": 100, "mass": 1 } },
{ "name": "float_3d", "duration": 1.0, "amplitude": 20 },
{ "name": "tilt_in", "delay": 0.0, "duration": 0.4, "rotate_x": 10.0 },
{ "name": "wiggle", "property": "translate_y", "amplitude": 5, "frequency": 2, "seed": 3 },
{ "name": "glow", "color": "#ffffff", "radius": 10, "intensity": 1.0 },
{ "name": "motion_blur", "samples": 4, "shutter": 0.5 },
{ "name": "keyframes", "keyframes": [
{ "property": "opacity", "keyframes": [
{ "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 1.0 }
] }
], "delay": 0.0, "duration": 1.0, "loop": true }
] }
}
]));
let (errors, warnings) = check_component_attrs(&s);
assert!(errors.is_empty(), "unexpected errors: {errors:?}");
assert!(warnings.is_empty(), "unexpected warnings: {warnings:?}");
}
}
Loading
Loading