From 7bc4717cc719342a0646d872e284c5cf75ae8bb6 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Sun, 27 Sep 2026 15:57:31 +0200 Subject: [PATCH] fix(timing): a composited view keeps the transitions that do not overlap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways a declared transition disappeared with nothing said about it. **One overlapping pair disabled every transition in the view.** This is a regression from #350: a single overlap routes the whole view through `v2_build_composited`, which walks output frames and never builds transition frames. The warning there only fired for a scene that itself overlaps, so a later scene sitting clear of everything lost its fade in silence. The composited path now emits `SlideTransition` for any pair whose only overlap is its own declared transition. The condition took a second attempt: the first version checked that both scenes were live at that frame, which never fires — the whole point of a v2 transition is that the outgoing scene is rendered *past* its own end, so it is not live there. The window is tested on its own. Measured on the issue's reproduction, three scenes where the first two overlap and the third declares a 1.0 s fade, sampling the centre pixel: before after 3.2s (0,0,255) (0,246,9) 3.5s (0,0,255) (0,115,140) 3.8s (0,0,255) (0,5,250) `(0,115,140)` at 3.5 s is exactly what the issue reports for the same two scenes without the overlapping one in front — so the fade is not merely present, it matches the uncomposited reference. The existing warning is now correctly scoped: it fires only when the overlap is *larger* than the declared transition, which is the case where the two really cannot both describe the same frames. **A transition on a view's first scene did nothing, silently.** A transition belongs to the scene being entered and the first has nothing to come from. It now warns at validate time and says where to move it — the failure mode is an author writing the fade on the wrong scene and counting frames to find out. Two tests. The first fails against the regression with the 30 missing transition frames; the second pins that a scene overlapping *beyond* its own transition still loses it, which is deliberate and stays loud. Closes #371 --- .../rustmotion/src/cli/commands/validation.rs | 18 +++ crates/rustmotion/src/encode/video/tasks.rs | 130 +++++++++++++++++- 2 files changed, 146 insertions(+), 2 deletions(-) diff --git a/crates/rustmotion/src/cli/commands/validation.rs b/crates/rustmotion/src/cli/commands/validation.rs index aba4ab5..a7a31e3 100644 --- a/crates/rustmotion/src/cli/commands/validation.rs +++ b/crates/rustmotion/src/cli/commands/validation.rs @@ -142,6 +142,7 @@ pub fn run_checks(loaded: &LoadedScenario, strict_anim: bool) -> ValidationRepor warnings.extend(warn_misplaced_animation(&loaded.raw)); warnings.extend(check_legibility(&loaded.scenario)); warnings.extend(check_off_grid_cuts(&loaded.scenario)); + warnings.extend(transitions_with_nothing_to_come_from(&loaded.scenario)); schema_errors.extend(check_node_references(&loaded.scenario)); let (attr_errors, mut attr_warnings) = super::validate_attrs::check_component_attrs(&loaded.scenario); @@ -254,6 +255,23 @@ pub fn warn_on_silent_defaults(loaded: &LoadedScenario) { } } +pub fn transitions_with_nothing_to_come_from(scenario: &ResolvedScenario) -> Vec { + scenario + .views + .iter() + .enumerate() + .filter(|(_, view)| view.scenes.first().is_some_and(|s| s.transition.is_some())) + .map(|(view_idx, _)| { + format!( + "the `transition` on view {view_idx}'s first scene has no effect — a \ + transition belongs to the scene being entered, and the first scene has \ + nothing to come from. Move it to the next scene, or use the view's own \ + `transition` to come in from the view before it." + ) + }) + .collect() +} + pub fn check_codec(codec: Option<&str>) -> Result<()> { if let Some(c) = codec { let allowed = ["h264", "h265", "vp9", "prores"]; diff --git a/crates/rustmotion/src/encode/video/tasks.rs b/crates/rustmotion/src/encode/video/tasks.rs index 750e4f7..3cb2ce7 100644 --- a/crates/rustmotion/src/encode/video/tasks.rs +++ b/crates/rustmotion/src/encode/video/tasks.rs @@ -747,7 +747,15 @@ fn build_slide_view_tasks_v2( let starts = v2_scene_starts(scenes, &duration_frames, fps, SnapDuringPlacement::Apply); if author_overlaps { - v2_build_composited(tasks, view_idx, scenes, &duration_frames, &starts); + v2_build_composited( + tasks, + view_idx, + scenes, + &duration_frames, + &transition_frames, + &starts, + fps, + ); return; } @@ -894,17 +902,36 @@ fn v2_build_sequential( } } +fn v2_transition_window( + scenes: &[Scene], + duration_frames: &[u32], + transition_frames: &[u32], + starts: &[u32], + incoming: usize, +) -> Option> { + if incoming == 0 || transition_frames[incoming] == 0 || scenes[incoming].transition.is_none() { + return None; + } + let previous_end = starts[incoming - 1] + duration_frames[incoming - 1]; + if starts[incoming] + transition_frames[incoming] < previous_end { + return None; + } + Some(starts[incoming]..starts[incoming] + transition_frames[incoming]) +} + fn v2_build_composited( tasks: &mut Vec, view_idx: usize, 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() { let previous_end = starts[i - 1] + duration_frames[i - 1]; - if starts[i] < previous_end { + 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 \ @@ -937,6 +964,45 @@ fn v2_build_composited( }) .collect(); + let transitioning = (1..scenes.len()).find(|&incoming| { + v2_transition_window(scenes, duration_frames, transition_frames, starts, incoming) + .is_some_and(|window| window.contains(&frame)) + }); + if let Some(incoming) = transitioning { + if let Some(window) = + v2_transition_window(scenes, duration_frames, transition_frames, starts, incoming) + { + { + let outgoing = incoming - 1; + let transition = scenes[incoming] + .transition + .as_ref() + .expect("v2_transition_window returns None without a transition"); + let advance = matches!(scenes[outgoing].tail, SceneTail::Continue); + tasks.push(FrameTask::SlideTransition { + global_frame: tasks.len() as u32, + view_idx, + scene_a_idx: outgoing, + scene_b_idx: incoming, + frame_in_transition: frame - window.start, + scene_a_frame_offset: if advance { + duration_frames[outgoing] + } else { + duration_frames[outgoing].saturating_sub(1) + }, + scene_a_frame_advance: advance, + scene_a_total_frames: duration_frames[outgoing], + scene_b_total_frames: duration_frames[incoming], + transition_type: transition.transition_type.clone(), + options: transition.into(), + transition_duration: transition_frames[incoming] as f64 / fps as f64, + easing: transition.easing.clone(), + }); + continue; + } + } + } + match participants.len() { 0 => { let last_live = starts @@ -1702,6 +1768,66 @@ mod timing_v2_tests { .collect() } + #[test] + fn one_overlapping_pair_does_not_disable_the_other_scenes_transitions() { + let scenario = load( + r##"{ + "video": {"width": 32, "height": 32, "fps": 30}, + "timing": "v2", + "composition": [{"type": "slide", "scenes": [ + {"at": 0, "duration": 2.0, "children": []}, + {"at": 1.0, "duration": 2.0, "children": []}, + {"duration": 2.0, "transition": {"type": "fade", "duration": 1.0}, "children": []} + ]}] + }"##, + ); + let tasks = build_frame_tasks(&scenario); + + let transition_frames: Vec = tasks + .iter() + .filter_map(|t| match t { + FrameTask::SlideTransition { + scene_a_idx, + scene_b_idx, + frame_in_transition, + .. + } => (*scene_a_idx == 1 && *scene_b_idx == 2).then_some(*frame_in_transition), + _ => None, + }) + .collect(); + + assert_eq!( + transition_frames, + (0..30).collect::>(), + "scene 2 does not overlap anything and declares a 1.0s fade, so it must still get \ + its 30 transition frames — one overlapping pair earlier in the view routed the \ + whole thing through the composited path and dropped every transition silently" + ); + } + + #[test] + fn a_scene_that_overlaps_beyond_its_own_transition_still_loses_it() { + 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 { .. })), + "a 0.2s transition cannot describe a 1.5s overlap, so the overlap wins and the \ + transition is dropped — loudly, which the warning covers" + ); + } + #[test] fn an_explicit_at_that_overlaps_composites_instead_of_being_clamped() { let scenario = load(&overlapping_json("@1.0s"));