Skip to content

Commit cbcde05

Browse files
authored
fix(timing): a composited view keeps the transitions that do not overlap (#389)
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
1 parent 97a5c95 commit cbcde05

2 files changed

Lines changed: 146 additions & 2 deletions

File tree

‎crates/rustmotion/src/cli/commands/validation.rs‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ pub fn run_checks(loaded: &LoadedScenario, strict_anim: bool) -> ValidationRepor
142142
warnings.extend(warn_misplaced_animation(&loaded.raw));
143143
warnings.extend(check_legibility(&loaded.scenario));
144144
warnings.extend(check_off_grid_cuts(&loaded.scenario));
145+
warnings.extend(transitions_with_nothing_to_come_from(&loaded.scenario));
145146
schema_errors.extend(check_node_references(&loaded.scenario));
146147
let (attr_errors, mut attr_warnings) =
147148
super::validate_attrs::check_component_attrs(&loaded.scenario);
@@ -254,6 +255,23 @@ pub fn warn_on_silent_defaults(loaded: &LoadedScenario) {
254255
}
255256
}
256257

258+
pub fn transitions_with_nothing_to_come_from(scenario: &ResolvedScenario) -> Vec<String> {
259+
scenario
260+
.views
261+
.iter()
262+
.enumerate()
263+
.filter(|(_, view)| view.scenes.first().is_some_and(|s| s.transition.is_some()))
264+
.map(|(view_idx, _)| {
265+
format!(
266+
"the `transition` on view {view_idx}'s first scene has no effect — a \
267+
transition belongs to the scene being entered, and the first scene has \
268+
nothing to come from. Move it to the next scene, or use the view's own \
269+
`transition` to come in from the view before it."
270+
)
271+
})
272+
.collect()
273+
}
274+
257275
pub fn check_codec(codec: Option<&str>) -> Result<()> {
258276
if let Some(c) = codec {
259277
let allowed = ["h264", "h265", "vp9", "prores"];

‎crates/rustmotion/src/encode/video/tasks.rs‎

Lines changed: 128 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -747,7 +747,15 @@ fn build_slide_view_tasks_v2(
747747
let starts = v2_scene_starts(scenes, &duration_frames, fps, SnapDuringPlacement::Apply);
748748

749749
if author_overlaps {
750-
v2_build_composited(tasks, view_idx, scenes, &duration_frames, &starts);
750+
v2_build_composited(
751+
tasks,
752+
view_idx,
753+
scenes,
754+
&duration_frames,
755+
&transition_frames,
756+
&starts,
757+
fps,
758+
);
751759
return;
752760
}
753761

@@ -894,17 +902,36 @@ fn v2_build_sequential(
894902
}
895903
}
896904

905+
fn v2_transition_window(
906+
scenes: &[Scene],
907+
duration_frames: &[u32],
908+
transition_frames: &[u32],
909+
starts: &[u32],
910+
incoming: usize,
911+
) -> Option<std::ops::Range<u32>> {
912+
if incoming == 0 || transition_frames[incoming] == 0 || scenes[incoming].transition.is_none() {
913+
return None;
914+
}
915+
let previous_end = starts[incoming - 1] + duration_frames[incoming - 1];
916+
if starts[incoming] + transition_frames[incoming] < previous_end {
917+
return None;
918+
}
919+
Some(starts[incoming]..starts[incoming] + transition_frames[incoming])
920+
}
921+
897922
fn v2_build_composited(
898923
tasks: &mut Vec<FrameTask>,
899924
view_idx: usize,
900925
scenes: &[Scene],
901926
duration_frames: &[u32],
927+
transition_frames: &[u32],
902928
starts: &[u32],
929+
fps: u32,
903930
) {
904931
for (i, scene) in scenes.iter().enumerate() {
905932
if i > 0 && scene.transition.is_some() {
906933
let previous_end = starts[i - 1] + duration_frames[i - 1];
907-
if starts[i] < previous_end {
934+
if starts[i] + transition_frames[i] < previous_end {
908935
eprintln!(
909936
"warning: scene {i} both overlaps scene {} on the absolute timeline and \
910937
declares a `transition`. A transition composites two finished frame \
@@ -937,6 +964,45 @@ fn v2_build_composited(
937964
})
938965
.collect();
939966

967+
let transitioning = (1..scenes.len()).find(|&incoming| {
968+
v2_transition_window(scenes, duration_frames, transition_frames, starts, incoming)
969+
.is_some_and(|window| window.contains(&frame))
970+
});
971+
if let Some(incoming) = transitioning {
972+
if let Some(window) =
973+
v2_transition_window(scenes, duration_frames, transition_frames, starts, incoming)
974+
{
975+
{
976+
let outgoing = incoming - 1;
977+
let transition = scenes[incoming]
978+
.transition
979+
.as_ref()
980+
.expect("v2_transition_window returns None without a transition");
981+
let advance = matches!(scenes[outgoing].tail, SceneTail::Continue);
982+
tasks.push(FrameTask::SlideTransition {
983+
global_frame: tasks.len() as u32,
984+
view_idx,
985+
scene_a_idx: outgoing,
986+
scene_b_idx: incoming,
987+
frame_in_transition: frame - window.start,
988+
scene_a_frame_offset: if advance {
989+
duration_frames[outgoing]
990+
} else {
991+
duration_frames[outgoing].saturating_sub(1)
992+
},
993+
scene_a_frame_advance: advance,
994+
scene_a_total_frames: duration_frames[outgoing],
995+
scene_b_total_frames: duration_frames[incoming],
996+
transition_type: transition.transition_type.clone(),
997+
options: transition.into(),
998+
transition_duration: transition_frames[incoming] as f64 / fps as f64,
999+
easing: transition.easing.clone(),
1000+
});
1001+
continue;
1002+
}
1003+
}
1004+
}
1005+
9401006
match participants.len() {
9411007
0 => {
9421008
let last_live = starts
@@ -1702,6 +1768,66 @@ mod timing_v2_tests {
17021768
.collect()
17031769
}
17041770

1771+
#[test]
1772+
fn one_overlapping_pair_does_not_disable_the_other_scenes_transitions() {
1773+
let scenario = load(
1774+
r##"{
1775+
"video": {"width": 32, "height": 32, "fps": 30},
1776+
"timing": "v2",
1777+
"composition": [{"type": "slide", "scenes": [
1778+
{"at": 0, "duration": 2.0, "children": []},
1779+
{"at": 1.0, "duration": 2.0, "children": []},
1780+
{"duration": 2.0, "transition": {"type": "fade", "duration": 1.0}, "children": []}
1781+
]}]
1782+
}"##,
1783+
);
1784+
let tasks = build_frame_tasks(&scenario);
1785+
1786+
let transition_frames: Vec<u32> = tasks
1787+
.iter()
1788+
.filter_map(|t| match t {
1789+
FrameTask::SlideTransition {
1790+
scene_a_idx,
1791+
scene_b_idx,
1792+
frame_in_transition,
1793+
..
1794+
} => (*scene_a_idx == 1 && *scene_b_idx == 2).then_some(*frame_in_transition),
1795+
_ => None,
1796+
})
1797+
.collect();
1798+
1799+
assert_eq!(
1800+
transition_frames,
1801+
(0..30).collect::<Vec<u32>>(),
1802+
"scene 2 does not overlap anything and declares a 1.0s fade, so it must still get \
1803+
its 30 transition frames — one overlapping pair earlier in the view routed the \
1804+
whole thing through the composited path and dropped every transition silently"
1805+
);
1806+
}
1807+
1808+
#[test]
1809+
fn a_scene_that_overlaps_beyond_its_own_transition_still_loses_it() {
1810+
let scenario = load(
1811+
r##"{
1812+
"video": {"width": 32, "height": 32, "fps": 30},
1813+
"timing": "v2",
1814+
"composition": [{"type": "slide", "scenes": [
1815+
{"at": 0, "duration": 2.0, "children": []},
1816+
{"at": 0.5, "duration": 2.0,
1817+
"transition": {"type": "fade", "duration": 0.2}, "children": []}
1818+
]}]
1819+
}"##,
1820+
);
1821+
let tasks = build_frame_tasks(&scenario);
1822+
assert!(
1823+
!tasks
1824+
.iter()
1825+
.any(|t| matches!(t, FrameTask::SlideTransition { .. })),
1826+
"a 0.2s transition cannot describe a 1.5s overlap, so the overlap wins and the \
1827+
transition is dropped — loudly, which the warning covers"
1828+
);
1829+
}
1830+
17051831
#[test]
17061832
fn an_explicit_at_that_overlaps_composites_instead_of_being_clamped() {
17071833
let scenario = load(&overlapping_json("@1.0s"));

0 commit comments

Comments
 (0)