From da67ea95533b3b905173a64f9042974b0b995271 Mon Sep 17 00:00:00 2001 From: Vikram Bhaskaran Date: Sat, 10 Oct 2026 21:38:57 +1100 Subject: [PATCH] Keep a composition work area inside the composition and at least one frame long. Setting the start past the current end, or shortening the composition, stored an inverted interval. Trimming that interval then saved a negative duration. --- crates/engine/src/commands/comp.rs | 25 +++++++++++ crates/engine/src/tests_menu_cmds.rs | 64 ++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) diff --git a/crates/engine/src/commands/comp.rs b/crates/engine/src/commands/comp.rs index a3267fa1..877993e4 100644 --- a/crates/engine/src/commands/comp.rs +++ b/crates/engine/src/commands/comp.rs @@ -76,6 +76,8 @@ fn apply_settings(c: &mut Comp, p: &Value) { c.work_area.1 = c.duration; } c.work_area.1 = c.work_area.1.min(c.duration); + // A shorter composition must not leave the start past the end. + keep_work_area_ordered(c); } if let Some(pa) = f_p(p, "pixelAspect") { c.pixel_aspect = pa.max(0.1); @@ -196,17 +198,40 @@ fn work_area(s: &mut Session, p: &Value) -> Result { if let Some(e) = end { c.work_area.1 = e.clamp(c.work_area.0 + fd, c.duration); } + // A start on its own can land after the current end. Pull it back so the interval + // still covers one frame, the same rule as `set: "begin"`. + keep_work_area_ordered(c); Ok(()) })?; let c = s.project.comp(cid).ok_or(EngineError::NoComp)?; Ok(json!({"start": c.work_area.0.seconds(), "end": c.work_area.1.seconds()})) } +/// Keep `work_area` inside the composition and at least one frame long. +fn keep_work_area_ordered(c: &mut Comp) { + let fd = c.frame_duration(); + c.work_area.1 = c.work_area.1.min(c.duration); + if c.work_area.0 < Tick::ZERO { + c.work_area.0 = Tick::ZERO; + } + if c.work_area.0 + fd > c.work_area.1 { + c.work_area.0 = (c.work_area.1 - fd).max(Tick::ZERO); + } + if c.work_area.0 + fd > c.work_area.1 { + c.work_area.1 = (c.work_area.0 + fd).min(c.duration); + } +} + fn trim_to_wa(s: &mut Session, p: &Value) -> Result { let cid = comp_id(s, p)?; s.edit("Trim Comp to Work Area", None, |proj, _| { let c = proj.comp_mut(cid).ok_or(EngineError::NoComp)?; let (a, b) = c.work_area; + let fd = c.frame_duration(); + // An interval that is already inverted must not become a negative duration. + if a < Tick::ZERO || b > c.duration || a + fd > b { + return Err(bad("comp.trimToWorkArea", "the work area must cover at least one frame inside the composition")); + } for l in &mut c.layers { l.start_time -= a; l.in_point = (l.in_point - a).max(Tick::ZERO); diff --git a/crates/engine/src/tests_menu_cmds.rs b/crates/engine/src/tests_menu_cmds.rs index c342f72f..b8c534b8 100644 --- a/crates/engine/src/tests_menu_cmds.rs +++ b/crates/engine/src/tests_menu_cmds.rs @@ -3,6 +3,7 @@ use effectcraft_keyframe::Value as KV; use effectcraft_project::{FrameBlend, ItemKind, MatteKind, Quality, Sampling}; +use effectcraft_time::Tick; use serde_json::{Value, json}; use crate::{EngineError, Event, Session}; @@ -253,6 +254,69 @@ fn lift_and_extract_work_area() { assert!((spans[1].0 - 2.0).abs() < 1e-6 && (spans[1].1 - 8.0).abs() < 1e-6, "{spans:?}"); } +#[test] +fn work_area_stays_ordered_when_the_start_passes_the_end() { + let ordered = |s: &Session| { + let c = s.active_comp().unwrap(); + let fd = c.frame_duration(); + assert!(c.duration >= fd, "duration {}", c.duration.seconds()); + assert!(c.work_area.0 >= Tick::ZERO); + assert!(c.work_area.0 + fd <= c.work_area.1, "start {} end {}", c.work_area.0.seconds(), c.work_area.1.seconds()); + assert!(c.work_area.1 <= c.duration); + (c.duration, c.work_area) + }; + let one_frame_at_two = |s: &Session| { + let end = Tick::from_seconds_f64(2.0); + (end - s.active_comp().unwrap().frame_duration(), end) + }; + + let mut s = comp(); + s.execute("comp.workArea", json!({"end": 2.0})).unwrap(); + s.execute("comp.workArea", json!({"start": 8.0})).unwrap(); + assert_eq!(ordered(&s).1, one_frame_at_two(&s)); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(ordered(&s).1, (Tick::ZERO, Tick::from_seconds_f64(2.0))); + + s.execute("comp.workArea", json!({"start": 0.0, "end": 2.0})).unwrap(); + s.execute("comp.trimToWorkArea", json!({})).unwrap(); + assert_eq!(ordered(&s), (Tick::from_seconds_f64(2.0), (Tick::ZERO, Tick::from_seconds_f64(2.0)))); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(ordered(&s).0, Tick::from_seconds_f64(10.0)); + + let mut s = comp(); + s.execute("comp.workArea", json!({"start": 9.0, "end": 10.0})).unwrap(); + let before = ordered(&s); + s.execute("comp.settings", json!({"duration": 2.0})).unwrap(); + let (dur, wa) = ordered(&s); + assert_eq!(dur, Tick::from_seconds_f64(2.0)); + assert_eq!(wa, one_frame_at_two(&s)); + s.execute("edit.undo", json!({})).unwrap(); + assert_eq!(ordered(&s), before); + + let mut s = comp(); + s.execute("comp.workArea", json!({"end": 2.0})).unwrap(); + s.execute("time.set", json!({"time": 8.0})).unwrap(); + s.execute("comp.workArea", json!({"set": "begin"})).unwrap(); + assert_eq!(ordered(&s).1, one_frame_at_two(&s)); + let path = tmp("WorkArea.ecproj"); + s.execute("file.saveAs", json!({"path": path})).unwrap(); + s.execute("file.open", json!({"path": path})).unwrap(); + assert_eq!(ordered(&s).1, one_frame_at_two(&s)); + + // A project that already has an inverted interval must not trim into a negative duration. + let mut s = comp(); + let cid = s.state.active_comp.unwrap(); + { + let c = std::sync::Arc::make_mut(&mut s.project).comp_mut(cid).unwrap(); + c.work_area = (Tick::from_seconds_f64(8.0), Tick::from_seconds_f64(2.0)); + } + let err = s.execute("comp.trimToWorkArea", json!({})).unwrap_err(); + assert!(err.to_string().contains("work area"), "{err}"); + let c = s.active_comp().unwrap(); + assert_eq!(c.duration, Tick::from_seconds_f64(10.0)); + assert_eq!(c.work_area, (Tick::from_seconds_f64(8.0), Tick::from_seconds_f64(2.0))); +} + #[test] fn label_group_purge_and_edit_original() { crate::tests_roto::hold_roto_cache_test_lock();