diff --git a/crates/rustmotion-components/src/svg.rs b/crates/rustmotion-components/src/svg.rs index fe9c149..178052f 100644 --- a/crates/rustmotion-components/src/svg.rs +++ b/crates/rustmotion-components/src/svg.rs @@ -41,7 +41,10 @@ pub struct Svg { pub timeline: Vec, #[serde(default)] pub stagger: Option, - /// Force draw-on mode even when draw_progress is 1.0 (static draw trace view, no animation needed). + /// Take the draw-on path even outside an animation's own window. It needs a driver: + /// a `draw_in`/`stroke_reveal` preset or keyframes on `draw_progress`. On its own it + /// leaves `draw_progress` at rest, which paints the finished mark — `validate` rejects + /// that rather than let the flag look as though it did something. #[serde(default)] pub draw: bool, /// Stroke width used when tracing fill-only paths (no stroke in the SVG). @@ -1049,4 +1052,41 @@ mod tests { must trigger the stderr warning" ); } + #[test] + fn draw_with_nothing_driving_progress_is_pixel_identical_to_no_draw() { + fn render(draw: bool) -> Vec { + let mut svg = filled_square_svg(); + svg.draw = draw; + let mut surface = skia_safe::surfaces::raster_n32_premul((W, H)).unwrap(); + svg.paint_content( + surface.canvas(), + &test_layout(), + &AnimatedProperties::default(), + &test_ctx(), + ); + let snapshot = surface.image_snapshot(); + let info = skia_safe::ImageInfo::new( + (W, H), + skia_safe::ColorType::RGBA8888, + skia_safe::AlphaType::Unpremul, + None, + ); + let mut buf = vec![0u8; (W * H * 4) as usize]; + snapshot.read_pixels( + &info, + &mut buf, + (W * 4) as usize, + skia_safe::IPoint::new(0, 0), + skia_safe::image::CachingHint::Disallow, + ); + buf + } + + assert_eq!( + render(true), + render(false), + "with draw_progress at rest the draw branch short-circuits to the finished mark, \ + so the flag changes nothing — which is why validate now refuses it" + ); + } } diff --git a/crates/rustmotion/skills/rules/draw-progress-stroke.md b/crates/rustmotion/skills/rules/draw-progress-stroke.md index 439ec4d..acaef48 100644 --- a/crates/rustmotion/skills/rules/draw-progress-stroke.md +++ b/crates/rustmotion/skills/rules/draw-progress-stroke.md @@ -1,8 +1,30 @@ # `draw_progress` : rien à 0, et un trait qui ne change pas d'apparence en finissant -`draw_progress` (via l'animation `keyframes` sur la propriété du même nom, ou -`draw: true` sur `svg`) révèle un trait progressivement. Deux pièges à -connaître sur `line` et `svg` (`reveal: "stroke"`, celui par défaut). +`draw_progress` révèle un trait progressivement. On le pilote par un preset +`draw_in`/`stroke_reveal`, ou par des `keyframes` sur la propriété du même nom. +Trois pièges à connaître sur `line` et `svg` (`reveal: "stroke"`, celui par +défaut). + +## `svg` : `draw: true` n'est pas un pilote + +`draw: true` force le **chemin de rendu** « draw-on ». Il ne fait pas avancer +`draw_progress`. Sans pilote, la propriété reste à sa valeur au repos, le +peintre prend la branche « fini » (`progress >= 1.0`, qui délègue simplement à +resvg) et la marque se rend **exactement comme avec `draw: false`** — vérifié +octet pour octet sur deux PNG. + +Le validateur refuse donc `draw: true` sans pilote, plutôt que de laisser le +drapeau avoir l'air de faire quelque chose : + +``` +draw: true but nothing animates draw_progress — the mark renders finished, +pixel-identical to draw: false. Add a 'draw_in' or 'stroke_reveal' preset, +or keyframes on 'draw_progress'. +``` + +En pratique on n'a d'ailleurs pas besoin de `draw: true` : un preset `draw_in` +suffit à lui seul, puisque le peintre bascule dès que `draw_progress` est dans +`[0, 1[`. ## `line` : `draw_progress: 0` ne doit rien peindre diff --git a/crates/rustmotion/src/cli/commands/validate_schema.rs b/crates/rustmotion/src/cli/commands/validate_schema.rs index 2f68daf..1ee3c05 100644 --- a/crates/rustmotion/src/cli/commands/validate_schema.rs +++ b/crates/rustmotion/src/cli/commands/validate_schema.rs @@ -184,6 +184,14 @@ fn validate_children( errors.push(format!("{}.src: file not found '{}'", p, src)); } } + if svg.draw && !drives_draw_progress(&svg.style) { + errors.push(format!( + "{}: draw: true but nothing animates draw_progress — the mark renders \ + finished, pixel-identical to draw: false. Add a 'draw_in' or \ + 'stroke_reveal' preset, or keyframes on 'draw_progress'.", + p + )); + } } Component::Icon(icon) => { if let Some((prefix, name)) = icon.icon.split_once(':') { @@ -238,6 +246,18 @@ fn validate_children( } } +fn drives_draw_progress(style: &CssStyle) -> bool { + style.animation.iter().any(|effect| match effect { + AnimationEffect::DrawIn(_) | AnimationEffect::StrokeReveal(_) => true, + AnimationEffect::Keyframes(k) => k + .keyframes + .iter() + .any(|anim| anim.property == "draw_progress" || anim.property == "draw_start"), + AnimationEffect::Wiggle(w) => w.property == "draw_progress" || w.property == "draw_start", + _ => false, + }) +} + fn check_style_colors(style: &CssStyle, path: &str, errors: &mut Vec) { if let Some(c) = &style.color { check_color(c, "color", path, errors); @@ -1023,6 +1043,102 @@ mod style_warning_tests { } } +#[cfg(test)] +mod svg_draw_driver_tests { + use super::*; + + const MARK: &str = "\ + "; + + fn errors_for(style: serde_json::Value) -> Vec { + let child: ChildComponent = serde_json::from_value(serde_json::json!({ + "type": "svg", + "data": MARK, + "draw": true, + "style": style + })) + .unwrap(); + let mut errors = Vec::new(); + let mut warnings = Vec::new(); + validate_children(&[child], "test", 4.0, &mut errors, &mut warnings); + errors + } + + #[test] + fn draw_with_no_driver_is_rejected_rather_than_rendering_the_finished_mark() { + let errors = errors_for(serde_json::json!({ "width": 200, "height": 200 })); + assert!( + errors.iter().any(|e| e.contains("draw_progress")), + "an undriven draw is pixel-identical to draw: false, so it must be named: {errors:?}" + ); + } + + #[test] + fn the_draw_in_preset_is_a_driver() { + let errors = errors_for(serde_json::json!({ + "width": 200, "height": 200, + "animation": [{ "name": "draw_in", "delay": 0.1, "duration": 1.0 }] + })); + assert!( + errors.is_empty(), + "draw_in drives draw_progress: {errors:?}" + ); + } + + #[test] + fn the_stroke_reveal_preset_is_a_driver() { + let errors = errors_for(serde_json::json!({ + "width": 200, "height": 200, + "animation": [{ "name": "stroke_reveal", "duration": 1.0 }] + })); + assert!( + errors.is_empty(), + "stroke_reveal drives draw_progress too: {errors:?}" + ); + } + + #[test] + fn keyframes_on_draw_progress_are_a_driver() { + let errors = errors_for(serde_json::json!({ + "width": 200, "height": 200, + "animation": [{ "name": "keyframes", "duration": 1.0, "keyframes": [{ + "property": "draw_progress", + "keyframes": [{ "time": 0.0, "value": 0.0 }, { "time": 1.0, "value": 1.0 }] + }]}] + })); + assert!( + errors.is_empty(), + "hand-written keyframes count as a driver: {errors:?}" + ); + } + + #[test] + fn an_animation_on_some_other_property_is_not_a_driver() { + let errors = errors_for(serde_json::json!({ + "width": 200, "height": 200, + "animation": [{ "name": "fade_in", "duration": 0.5 }] + })); + assert!( + errors.iter().any(|e| e.contains("draw_progress")), + "fade_in animates opacity, which leaves draw_progress at rest: {errors:?}" + ); + } + + #[test] + fn draw_false_never_asks_for_a_driver() { + let child: ChildComponent = serde_json::from_value(serde_json::json!({ + "type": "svg", + "data": MARK, + "style": { "width": 200, "height": 200 } + })) + .unwrap(); + let mut errors = Vec::new(); + let mut warnings = Vec::new(); + validate_children(&[child], "test", 4.0, &mut errors, &mut warnings); + assert!(errors.is_empty(), "a plain svg is not affected: {errors:?}"); + } +} + #[cfg(test)] mod color_validation_tests { use super::*;