diff --git a/crates/rustmotion-components/src/gradient_text.rs b/crates/rustmotion-components/src/gradient_text.rs index e64db6a..b37d885 100644 --- a/crates/rustmotion-components/src/gradient_text.rs +++ b/crates/rustmotion-components/src/gradient_text.rs @@ -35,6 +35,9 @@ pub struct GradientTextStop { /// Hex color at this stop, e.g. `"#7C3AED"`. pub color: String, /// Position along the gradient line, from `0.0` (first stop) to `1.0` (last). + /// `offset`, the name a background's gradient stops use for the same + /// value, is also accepted. + #[serde(alias = "offset")] pub position: f32, } @@ -981,6 +984,18 @@ mod tests { ); } + #[test] + fn a_stop_written_with_offset_deserializes_the_same_as_one_written_with_position() { + let json = r##"{"color": "#7C3AED", "offset": 0.7}"##; + let stop: GradientTextStop = + serde_json::from_str(json).expect("offset must be accepted as an alias for position"); + assert_eq!( + stop.position, 0.7, + "a stop using the background gradient's `offset` vocabulary must resolve to the \ + same `position` field gradient_text reads at paint time" + ); + } + fn props_for(gt: &GradientText) -> AnimatedProperties { AnimatedProperties { char_animation: rustmotion_core::engine::animator::extract_effects(>.style.animation) diff --git a/crates/rustmotion/skills/SKILL.md b/crates/rustmotion/skills/SKILL.md index a3a93fc..ce30db1 100644 --- a/crates/rustmotion/skills/SKILL.md +++ b/crates/rustmotion/skills/SKILL.md @@ -1529,7 +1529,7 @@ Style: `font-size`, `font-weight`, `font-family` `angle` follows the CSS convention: `0` points up, `90` points right, growing clockwise. -`stops` places each colour explicitly instead of spreading `colors` evenly along the gradient line. The position key is **`position`**, not the `offset` a background's stops use — the two vocabularies differ, and the validator refuses the wrong one: +`stops` places each colour explicitly instead of spreading `colors` evenly along the gradient line. The position key is **`position`** — the canonical one. `offset`, the key a background's stops use for the same value, is accepted as an alias: ```json { "type": "gradient_text", "content": "Rustmotion", "angle": 90, diff --git a/crates/rustmotion/skills/rules/text-component-parity.md b/crates/rustmotion/skills/rules/text-component-parity.md index 51e9f0f..7313f0d 100644 --- a/crates/rustmotion/skills/rules/text-component-parity.md +++ b/crates/rustmotion/skills/rules/text-component-parity.md @@ -33,7 +33,7 @@ La ligne du dégradé est aussi recalculée : c'est la projection CSS de la boî ## gradient_text : stops explicites -Champ optionnel `stops`. Il ne partage pas la clé des stops d'un fond : `gradient_text` nomme la position `position`, un `background` la nomme `offset`. Les deux sont une fraction `0..1` de la ligne du dégradé — seul le nom diffère, et le validateur refuse l'autre. +Champ optionnel `stops`. La position se nomme `position` — c'est la forme canonique, celle que produit la sérialisation. `offset`, le nom qu'utilisent les stops d'un fond, est accepté en entrée comme alias : les deux désignent le même champ, une fraction `0..1` de la ligne du dégradé. ```json { "type": "gradient_text", "content": "Rustmotion", "angle": 90, diff --git a/crates/rustmotion/src/cli/commands/geometry.rs b/crates/rustmotion/src/cli/commands/geometry.rs index fca618c..3e4a390 100644 --- a/crates/rustmotion/src/cli/commands/geometry.rs +++ b/crates/rustmotion/src/cli/commands/geometry.rs @@ -3,6 +3,7 @@ use std::collections::HashSet; use rustmotion::components::box_builder::{ build_scene_from_refs, component_kind, effective_effects, BuildAnimationCtx, }; +use rustmotion::components::connector::RoutingMode; use rustmotion::components::intrinsic::{ CaptionIntrinsic, GradientTextIntrinsic, RichTextIntrinsic, TableIntrinsic, TextIntrinsic, }; @@ -231,6 +232,29 @@ fn bbox_of(layout: &BoxLayout) -> BBox { const ARROW_HEAD_BBOX_PADDING: f32 = 16.0; +fn quadratic_bulge_point(x1: f32, y1: f32, x2: f32, y2: f32, curve: f32) -> (f32, f32) { + let mid_x = (x1 + x2) / 2.0; + let mid_y = (y1 + y2) / 2.0; + let dx = x2 - x1; + let dy = y2 - y1; + let len = (dx * dx + dy * dy).sqrt(); + if len < f32::EPSILON { + return (mid_x, mid_y); + } + let perp_x = -dy / len * curve * len * 0.3; + let perp_y = dx / len * curve * len * 0.3; + (mid_x + perp_x, mid_y + perp_y) +} + +fn arrowhead_pad(width: f32, arrow_size: f32, arrow_start: bool, arrow_end: bool) -> f32 { + let head_pad = if arrow_start || arrow_end { + ARROW_HEAD_BBOX_PADDING + arrow_size.max(0.0) + } else { + 0.0 + }; + width.max(0.0) / 2.0 + head_pad +} + fn endpoint_extent(component: &Component) -> Option<(f32, f32, f32, f32, f32)> { match component { Component::Line(Line { @@ -255,8 +279,11 @@ fn endpoint_extent(component: &Component) -> Option<(f32, f32, f32, f32, f32)> { cp, cp1, cp2, + curve, width, arrow_size, + arrow_start, + arrow_end, .. }) => { let mut min_x = x1.min(*x2); @@ -272,24 +299,43 @@ fn endpoint_extent(component: &Component) -> Option<(f32, f32, f32, f32, f32)> { min_y = min_y.min(p.y); max_y = max_y.max(p.y); } - let pad = width.max(0.0) / 2.0 + ARROW_HEAD_BBOX_PADDING + arrow_size.max(0.0); + if cp.is_none() && cp1.is_none() && cp2.is_none() { + if let Some(curve) = curve { + let (bulge_x, bulge_y) = quadratic_bulge_point(*x1, *y1, *x2, *y2, *curve); + min_x = min_x.min(bulge_x); + max_x = max_x.max(bulge_x); + min_y = min_y.min(bulge_y); + max_y = max_y.max(bulge_y); + } + } + let pad = arrowhead_pad(*width, *arrow_size, *arrow_start, *arrow_end); Some((min_x, min_y, max_x, max_y, pad)) } Component::Connector(Connector { from, to, + routing, + curvature, width, arrow_size, + arrow_start, + arrow_end, .. }) => { - let pad = width.max(0.0) / 2.0 + ARROW_HEAD_BBOX_PADDING + arrow_size.max(0.0); - Some(( - from.x.min(to.x), - from.y.min(to.y), - from.x.max(to.x), - from.y.max(to.y), - pad, - )) + let mut min_x = from.x.min(to.x); + let mut max_x = from.x.max(to.x); + let mut min_y = from.y.min(to.y); + let mut max_y = from.y.max(to.y); + if matches!(routing, RoutingMode::Curved) { + let (bulge_x, bulge_y) = + quadratic_bulge_point(from.x, from.y, to.x, to.y, *curvature); + min_x = min_x.min(bulge_x); + max_x = max_x.max(bulge_x); + min_y = min_y.min(bulge_y); + max_y = max_y.max(bulge_y); + } + let pad = arrowhead_pad(*width, *arrow_size, *arrow_start, *arrow_end); + Some((min_x, min_y, max_x, max_y, pad)) } _ => None, } @@ -529,19 +575,34 @@ fn hint_for_viewport(component: &Component, axis: Axis, bbox: &BBox, vp: (u32, u "card width must be ≥ {:.0}px (counter natural width)", bbox.w ), - _ => match axis { - Axis::X => format!( - "shift x to fit [0..{:.0}], current right edge is {:.0}", - vw, - bbox.x + bbox.w - ), - Axis::Y => format!( - "shift y to fit [0..{:.0}], current bottom edge is {:.0}", - vh, - bbox.y + bbox.h - ), - Axis::Both => "reposition the component to stay inside the viewport".to_string(), - }, + _ => { + let eps = 0.5; + match axis { + Axis::X if bbox.x < -eps => { + format!( + "shift x to fit [0..{:.0}], current left edge is {:.0}", + vw, bbox.x + ) + } + Axis::X => format!( + "shift x to fit [0..{:.0}], current right edge is {:.0}", + vw, + bbox.x + bbox.w + ), + Axis::Y if bbox.y < -eps => { + format!( + "shift y to fit [0..{:.0}], current top edge is {:.0}", + vh, bbox.y + ) + } + Axis::Y => format!( + "shift y to fit [0..{:.0}], current bottom edge is {:.0}", + vh, + bbox.y + bbox.h + ), + Axis::Both => "reposition the component to stay inside the viewport".to_string(), + } + } } } @@ -1369,6 +1430,9 @@ pub fn check_off_grid_cuts(scenario: &ResolvedScenario) -> Vec { } let tasks = rustmotion::encode::build_frame_tasks(scenario); + warnings.extend(rustmotion::encode::video::v2_dropped_transition_warnings( + scenario, + )); let mut cut_frame: HashMap<(usize, usize), u32> = HashMap::new(); for task in &tasks { @@ -3232,6 +3296,104 @@ mod tests { violations ); } + + #[test] + fn an_arrow_with_no_arrowhead_near_the_edge_does_not_false_positive_on_head_padding() { + let json = r##"{"video":{"width":1920,"height":1080,"fps":30,"background":"#000000"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"arrow","position":"absolute","x":0,"y":0,"x1":2,"y1":500,"x2":100,"y2":500, + "arrow_start":false,"arrow_end":false,"color":"#FFFFFF","width":4}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .all(|v| !(v.component == "arrow" && v.kind == ViolationKind::ViewportOverflow)), + "an arrow with neither arrow_start nor arrow_end must not pay the arrowhead padding \ + it never draws: {:?}", + violations + ); + } + + #[test] + fn a_curved_arrow_that_bulges_off_the_top_edge_is_caught() { + let json = r##"{"video":{"width":1920,"height":1080,"fps":30,"background":"#000000"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"arrow","position":"absolute","x":0,"y":0,"x1":1700,"y1":100,"x2":200,"y2":100, + "curve":0.5,"color":"#FFFFFF","width":4}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + assert!( + violations + .iter() + .any(|v| v.component == "arrow" && v.kind == ViolationKind::ViewportOverflow), + "a `curve` that bulges the arrow's implicit control point above y=0 must be caught, \ + not silently clipped at render: {:?}", + violations + ); + } + + #[test] + fn overflow_hint_names_the_left_edge_not_the_right_for_a_negative_x_overflow() { + let json = r##"{"video":{"width":1920,"height":1080,"fps":30,"background":"#000000"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"shape","shape":"rect","position":"absolute","x":-102,"y":10, + "size":{"width":50,"height":50},"fill":"#ff0000"}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::ViewportOverflow && v.axis == Axis::X) + .unwrap_or_else(|| panic!("expected a ViewportOverflow: {:?}", violations)); + assert!( + v.hint.contains("left edge") && v.hint.contains("-102"), + "a shape poking off the left edge at x=-102 must name the left edge in its hint, \ + not fabricate a right edge: {:?}", + v.hint + ); + } + + #[test] + fn overflow_hint_names_the_top_edge_not_the_bottom_for_a_negative_y_overflow() { + let json = r##"{"video":{"width":1920,"height":1080,"fps":30,"background":"#000000"}, + "scenes":[{"duration":1.0,"children":[ + {"type":"shape","shape":"rect","position":"absolute","x":10,"y":-75, + "size":{"width":50,"height":50},"fill":"#ff0000"}]}]}"##; + let scenario = parse(json); + let violations = validate_geometry(&scenario); + let v = violations + .iter() + .find(|v| v.kind == ViolationKind::ViewportOverflow && v.axis == Axis::Y) + .unwrap_or_else(|| panic!("expected a ViewportOverflow: {:?}", violations)); + assert!( + v.hint.contains("top edge") && v.hint.contains("-75"), + "a shape poking off the top edge at y=-75 must name the top edge in its hint, \ + not fabricate a bottom edge: {:?}", + v.hint + ); + } + + const V2_COMPOSITED_DROPPED_TRANSITION_JSON: &str = r##"{"timing":"v2", + "video":{"width":320,"height":180,"fps":30,"background":"#000000"}, + "composition":[{"type":"slide","scenes":[ + {"at":0,"duration":2.0,"background":"#FF0000","children":[]}, + {"at":0.5,"duration":2.0,"background":"#00FF00", + "transition":{"type":"fade","duration":0.2},"children":[]}]}]}"##; + + #[test] + fn dropped_v2_transition_warning_is_reported_exactly_once_by_off_grid_cuts() { + let scenario = parse(V2_COMPOSITED_DROPPED_TRANSITION_JSON); + let warnings = check_off_grid_cuts(&scenario); + let dropped_transition_warnings = warnings + .iter() + .filter(|w| w.contains("both overlaps scene") && w.contains("declares a `transition`")) + .count(); + assert_eq!( + dropped_transition_warnings, 1, + "expected exactly one dropped-transition warning from the single authoritative \ + call site, got {dropped_transition_warnings}: {warnings:?}" + ); + } } #[cfg(test)] diff --git a/crates/rustmotion/src/cli/commands/schema.rs b/crates/rustmotion/src/cli/commands/schema.rs index 5ef8fef..89e814f 100644 --- a/crates/rustmotion/src/cli/commands/schema.rs +++ b/crates/rustmotion/src/cli/commands/schema.rs @@ -26,9 +26,7 @@ const SERDE_ALIASES: &[(&str, &str, &[&str])] = &[ ("CardJustify", "space_evenly", &["space-evenly"]), ("AnimationPreset", "float3d", &["float_3d"]), ("AnimationEffect", "float3d", &["float_3d"]), - ("Component", "progress", &["progress_bar"]), ("ComponentBase", "progress", &["progress_bar"]), - ("ChildComponent", "progress", &["progress_bar"]), ("ChildComponentBase", "progress", &["progress_bar"]), ]; @@ -63,6 +61,7 @@ const SERDE_FIELD_ALIASES: &[(&str, &str, &[&str])] = &[ ("BorderRadius", "top-right", &["top_right"]), ("BorderRadius", "bottom-right", &["bottom_right"]), ("BorderRadius", "bottom-left", &["bottom_left"]), + ("GradientTextStop", "position", &["offset"]), ]; fn widen_properties_with(value: &mut serde_json::Value, canonical: &str, aliases: &[&str]) { @@ -321,6 +320,67 @@ mod serde_alias_exposure_tests { ); } + #[test] + fn every_entry_in_the_two_tables_actually_lands_in_the_exported_schema() { + let schema = build_schema(); + let defs = schema + .get("definitions") + .and_then(|d| d.as_object()) + .expect("definitions"); + + let mut missing: Vec = Vec::new(); + + for (definition, canonical, aliases) in SERDE_ALIASES { + let Some(entry) = defs.get(*definition) else { + continue; + }; + let text = entry.to_string(); + for alias in *aliases { + if !text.contains(&format!("\"{alias}\"")) { + missing.push(format!("{definition}: enum {canonical} lacks {alias}")); + } + } + } + + for (definition, canonical, aliases) in SERDE_FIELD_ALIASES { + let Some(entry) = defs.get(*definition) else { + missing.push(format!("{definition}: no such definition")); + continue; + }; + let text = entry.to_string(); + for alias in *aliases { + if !text.contains(&format!("\"{alias}\"")) { + missing.push(format!( + "{definition}: property {canonical} has no {alias} alongside it" + )); + } + } + } + + assert!( + missing.is_empty(), + "a table entry that names a definition or a canonical the schema does not carry is \ + inert, and inert is exactly how this drifts — the wider scan over the sources \ + cannot see it, because an alias string often already appears elsewhere in the \ + schema as some other type's field name:\n {}", + missing.join("\n ") + ); + } + + #[test] + fn a_gradient_text_stop_accepts_both_spellings_of_its_position() { + let schema = build_schema(); + let stop = schema + .pointer("/definitions/GradientTextStop/properties") + .and_then(|v| v.as_object()) + .expect("GradientTextStop carries properties"); + assert!( + stop.contains_key("position") && stop.contains_key("offset"), + "the parser accepts offset as an alias, so --strict-attrs must not reject it: {:?}", + stop.keys().collect::>() + ); + } + #[test] fn the_canonical_spelling_is_never_replaced_by_its_alias() { let schema = build_schema(); diff --git a/crates/rustmotion/src/cli/commands/validate.rs b/crates/rustmotion/src/cli/commands/validate.rs index 812ffd6..2f36d1d 100644 --- a/crates/rustmotion/src/cli/commands/validate.rs +++ b/crates/rustmotion/src/cli/commands/validate.rs @@ -14,6 +14,10 @@ fn announced_duration(scenario: &ResolvedScenario) -> f64 { rustmotion::encode::build_frame_tasks(scenario).len() as f64 / fps } +fn format_duration_seconds(seconds: f64) -> String { + format!("{:.3}s", seconds) +} + #[derive(Debug, PartialEq, Eq)] pub(crate) enum FixRefusal { HtmlSource, @@ -207,7 +211,7 @@ pub fn cmd_validate( " Resolution: {}x{} @ {}fps", loaded.scenario.video.width, loaded.scenario.video.height, loaded.scenario.video.fps ); - eprintln!(" Duration: {:.1}s", total_duration); + eprintln!(" Duration: {}", format_duration_seconds(total_duration)); if !report_out.geom_violations.is_empty() { eprintln!(" Geometry warnings: {}", report_out.geom_violations.len()); } @@ -547,6 +551,16 @@ mod tests { ); } + #[test] + fn printed_duration_keeps_frame_level_precision_instead_of_rounding_to_one_decimal() { + let formatted = format_duration_seconds(2.95); + assert_eq!( + formatted, "2.950s", + "rounding 2.95s to one decimal reads as \"3.0s\", which hides real sub-second \ + drift from the author; got {formatted}" + ); + } + mod fix_refusals { use super::super::{refuse_fix, FixRefusal}; use std::path::Path; diff --git a/crates/rustmotion/src/encode/video/tasks.rs b/crates/rustmotion/src/encode/video/tasks.rs index 02d4949..906777c 100644 --- a/crates/rustmotion/src/encode/video/tasks.rs +++ b/crates/rustmotion/src/encode/video/tasks.rs @@ -1009,31 +1009,77 @@ fn v2_transition_window( Some(starts[incoming]..starts[incoming] + transition_frames[incoming]) } -fn v2_build_composited( - tasks: &mut Vec, - view_idx: usize, +fn v2_dropped_transition_warning(i: usize, previous: usize) -> String { + format!( + "warning: scene {i} both overlaps scene {previous} on the absolute timeline and \ + declares a `transition`. A transition composites two finished frame \ + buffers and an overlap composites live scenes; the two cannot both \ + describe the same frames. The transition is ignored here — remove it, or \ + move `at` so the scenes no longer overlap." + ) +} + +fn v2_composited_dropped_transition_warnings( scenes: &[Scene], duration_frames: &[u32], transition_frames: &[u32], starts: &[u32], - fps: u32, -) { - for (i, scene) in scenes.iter().enumerate() { - if i > 0 && scene.transition.is_some() { +) -> Vec { + scenes + .iter() + .enumerate() + .filter_map(|(i, scene)| { + if i == 0 || scene.transition.is_none() { + return None; + } let previous_end = starts[i - 1] + duration_frames[i - 1]; if starts[i] + transition_frames[i] < previous_end { - eprintln!( - "warning: scene {i} both overlaps scene {} on the absolute timeline and \ - declares a `transition`. A transition composites two finished frame \ - buffers and an overlap composites live scenes; the two cannot both \ - describe the same frames. The transition is ignored here — remove it, or \ - move `at` so the scenes no longer overlap.", - i - 1 - ); + Some(v2_dropped_transition_warning(i, i - 1)) + } else { + None } - } + }) + .collect() +} + +pub fn v2_dropped_transition_warnings(scenario: &Scenario) -> Vec { + let fps = scenario.video.fps; + if fps == 0 { + return Vec::new(); } + let mut warnings = Vec::new(); + for view in &scenario.views { + if !matches!(view.view_type, ViewType::Slide) { + continue; + } + if view_timing(view) != TimingMode::V2 { + continue; + } + let placement = v2_placement(&view.scenes, fps); + if !placement.author_overlaps { + continue; + } + let duration_frames: Vec = placement.spans.iter().map(SceneSpan::frames).collect(); + let starts: Vec = placement.spans.iter().map(|s| s.start).collect(); + warnings.extend(v2_composited_dropped_transition_warnings( + &view.scenes, + &duration_frames, + &placement.transition_frames, + &starts, + )); + } + warnings +} +fn v2_build_composited( + tasks: &mut Vec, + view_idx: usize, + scenes: &[Scene], + duration_frames: &[u32], + transition_frames: &[u32], + starts: &[u32], + fps: u32, +) { let total_frames = starts .iter() .zip(duration_frames) @@ -2058,6 +2104,56 @@ mod timing_v2_tests { ); } + #[test] + fn v2_dropped_transition_warnings_reports_the_drop_exactly_once() { + let scenario = load( + r##"{ + "video": {"width": 32, "height": 32, "fps": 30}, + "timing": "v2", + "composition": [{"type": "slide", "scenes": [ + {"at": 0, "duration": 2.0, "children": []}, + {"at": 0.5, "duration": 2.0, + "transition": {"type": "fade", "duration": 0.2}, "children": []} + ]}] + }"##, + ); + let warnings = v2_dropped_transition_warnings(&scenario); + assert_eq!( + warnings.len(), + 1, + "one scene both overlaps its predecessor and declares a transition, so exactly one \ + warning must come out of this pure, single-call-site function: {warnings:?}" + ); + assert!( + warnings[0].contains("scene 1") && warnings[0].contains("scene 0"), + "the warning must name the overlapping pair: {:?}", + warnings[0] + ); + } + + #[test] + fn v2_build_composited_no_longer_prints_the_dropped_transition_warning_itself() { + let scenario = load( + r##"{ + "video": {"width": 32, "height": 32, "fps": 30}, + "timing": "v2", + "composition": [{"type": "slide", "scenes": [ + {"at": 0, "duration": 2.0, "children": []}, + {"at": 0.5, "duration": 2.0, + "transition": {"type": "fade", "duration": 0.2}, "children": []} + ]}] + }"##, + ); + let tasks = build_frame_tasks(&scenario); + assert!( + !tasks + .iter() + .any(|t| matches!(t, FrameTask::SlideTransition { .. })), + "building the tasks must still drop the transition frames (behaviour unchanged); \ + only the diagnostic side effect moved to v2_dropped_transition_warnings" + ); + } + #[test] fn an_explicit_at_that_overlaps_composites_instead_of_being_clamped() { let scenario = load(&overlapping_json("@1.0s"));