diff --git a/apps/filmcraft-cli/src/main.rs b/apps/filmcraft-cli/src/main.rs index d0028195d..830ab1db0 100644 --- a/apps/filmcraft-cli/src/main.rs +++ b/apps/filmcraft-cli/src/main.rs @@ -195,7 +195,8 @@ impl Backend { // this process exits after the command, so a background export would never be written: // long commands wait unless the caller says otherwise (#2342) Backend::Local(s) => { - if filmcraft_automation::long_job::LONG_COMMANDS.contains(&id) + // mask tracking is applied when its job finishes, which this process never sees (#3302) + if (filmcraft_automation::long_job::LONG_COMMANDS.contains(&id) || id == "masks.track") && let Some(p) = params.as_object_mut() { // anything but an explicit `false` (missing, null, a typo'd value) waits diff --git a/crates/engine/src/commands.rs b/crates/engine/src/commands.rs index 9088e8509..f29685530 100644 --- a/crates/engine/src/commands.rs +++ b/crates/engine/src/commands.rs @@ -1466,6 +1466,7 @@ fn build() -> Vec { // ================= File ================= cmd!("file.newProject", "Project…", ["File", "New"], Some("Cmd+Alt+N"), r#"{"name":str}"#, always, |s, p| { let name = str_p(p, "name").unwrap_or("Untitled").to_string(); + crate::masks::discard_jobs(s); s.project = std::sync::Arc::new(filmcraft_project::Project::new(&name)); s.project_instance = std::sync::Arc::new(()); s.document_epoch = s.document_epoch.wrapping_add(1); @@ -5182,7 +5183,7 @@ fn schema_backup_path(path: &str, schema: u32) -> String { } /// Make `proj` the session's project (fresh history, media pool and editor state). -fn install_project(s: &mut Session, proj: filmcraft_project::Project, path: Option, clean: bool) { +pub(crate) fn install_project(s: &mut Session, proj: filmcraft_project::Project, path: Option, clean: bool) { let proxies = s.media.use_proxies(); s.media = std::sync::Arc::new(crate::MediaPool::default()); s.media.set_use_proxies(proxies); @@ -5215,6 +5216,7 @@ fn open_project(s: &mut Session, path: &str) -> Result { let bytes = s.services.read_file(path).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let loaded = filmcraft_format::decode(&bytes).map_err(|e| EngineError::Other(format!("{path}: {e}")))?; let (from, migrated) = (loaded.schema_version, loaded.migrated()); + crate::masks::discard_jobs(s); install_project(s, loaded.project, Some(path.to_string()), true); if let Some(view) = loaded.view.filter(|_| s.prefs.timeline.restore_open_sequences) { s.restore_project_view(view); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 153529648..56d05b708 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -771,6 +771,11 @@ impl Session { /// Run a command by id. pub fn execute(&mut self, id: &str, params: Value) -> Result { let spec = commands::find(id).ok_or_else(|| EngineError::UnknownCommand(id.to_string()))?; + // headless sessions (CLI, MCP, control channel) have no UI frame loop calling + // `poll_persistence`: apply background mask tracking before each command (#3302) + if self.exec_depth == 0 && !self.mask_jobs.is_empty() { + masks::poll(self); + } let journal = spec.journal && !commands::reports_only(id, ¶ms); if self.exec_depth == 0 && journal && self.trim_play.active() { self.settle_trim_playback(id); diff --git a/crates/engine/src/masks.rs b/crates/engine/src/masks.rs index e18d7e5a1..bdff28fdc 100644 --- a/crates/engine/src/masks.rs +++ b/crates/engine/src/masks.rs @@ -603,6 +603,8 @@ pub struct PendingTrack { /// (media time, path) in tracking order. pub keys: Arc>>, applied: usize, + /// `Session::document_epoch` when the job started; a different value means the project was replaced. + epoch: u64, } /// Longest side of the frames the tracker works on (speed; the result is in clip pixels). @@ -722,7 +724,7 @@ fn track(s: &mut Session, p: &Value) -> Result { }; let run = crate::export_tools::guard_job(job.progress.clone(), job.result.clone(), run); s.jobs.push(job); - s.mask_jobs.push(PendingTrack { job: id, target: MaskSel { clip, effect: ei, mask: mi }, seq: seq_id, keys, applied: 0 }); + s.mask_jobs.push(PendingTrack { job: id, target: MaskSel { clip, effect: ei, mask: mi }, seq: seq_id, keys, applied: 0, epoch: s.document_epoch }); let wait = crate::commands::bool_p(p, "wait").unwrap_or(false); if wait || cfg!(target_arch = "wasm32") { run(); @@ -738,13 +740,33 @@ fn lock(m: &Mutex) -> std::sync::MutexGuard<'_, T> { m.lock().unwrap_or_else(|e| e.into_inner()) } +/// Cancel and drop every pending tracking job. Called when the project is replaced, so a job started +/// on the old project can never write its result into the new one. +pub fn discard_jobs(s: &mut Session) { + use std::sync::atomic::Ordering; + for pj in std::mem::take(&mut s.mask_jobs) { + if let Some(j) = s.jobs.iter().find(|j| j.id == pj.job) { + j.progress.cancel.store(true, Ordering::Relaxed); + } + } +} + /// Write newly tracked paths as Mask Path keyframes (one merged undo step per tracking job) and -/// drop finished jobs. Called from [`Session::poll_persistence`] (once per UI frame) and after -/// synchronous runs. +/// drop finished jobs. Called from [`Session::poll_persistence`] (once per UI frame), before each +/// top-level [`Session::execute`] (headless sessions) and after synchronous runs. pub fn poll(s: &mut Session) { use std::sync::atomic::Ordering; let mut i = 0; while i < s.mask_jobs.len() { + if s.mask_jobs[i].epoch != s.document_epoch { + // the project was replaced under the job: its result belongs to a document that is gone + s.error_toast("masks.track", "the project was replaced; mask tracking stopped"); + if let Some(j) = s.jobs.iter().find(|j| j.id == s.mask_jobs[i].job) { + j.progress.cancel.store(true, Ordering::Relaxed); + } + s.mask_jobs.remove(i); + continue; + } let finished = s.jobs.iter().find(|j| j.id == s.mask_jobs[i].job).is_none_or(|j| j.progress.finished.load(Ordering::Relaxed)); let new: Vec<(Tick, MaskPath)> = { let pj = &s.mask_jobs[i]; diff --git a/crates/engine/src/masks_tests.rs b/crates/engine/src/masks_tests.rs index 7c538c28d..c65a52916 100644 --- a/crates/engine/src/masks_tests.rs +++ b/crates/engine/src/masks_tests.rs @@ -435,3 +435,85 @@ fn move_vertex_moves_a_list_of_vertices_together() { s.undo(); assert_eq!(pts(&mut s), before); } + +#[test] +fn background_tracking_is_applied_without_a_ui_frame_loop() { + // headless sessions (CLI, MCP) never call `poll_persistence`: commands alone must apply the job (#3302) + let (mut s, _) = tracking_session(); + let c = truth(0.0).apply(filmcraft_geom::Vec2::ZERO); + s.execute("masks.add", json!({"effect": "opacity", "shape": "ellipse", "center": [c.x, c.y], "size": [90, 90]})).unwrap(); + let r = s.execute("masks.track", json!({"direction": "forward", "frames": 6})).unwrap(); + let job = r["job"].clone(); + let t0 = std::time::Instant::now(); + loop { + let jobs = s.execute("jobs.list", json!({})).unwrap(); + let done = jobs.as_array().unwrap().iter().any(|j| j["id"] == job && j["finished"] == true); + if done || t0.elapsed().as_secs() > 60 { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let l = s.execute("masks.list", json!({})).unwrap(); + let keys = l["masks"][0]["pathKeyframes"].as_array().unwrap().len(); + assert_eq!(keys, 7, "{l}"); + assert!(s.mask_jobs.is_empty()); + s.execute("masks.track", json!({"direction": "forward", "frames": 2, "wait": true})).unwrap(); +} + +#[test] +fn replacing_the_project_drops_pending_tracking_jobs() { + // a job started on the old project must not write into the new one + let (mut s, _) = tracking_session(); + let c = truth(0.0).apply(filmcraft_geom::Vec2::ZERO); + s.execute("masks.add", json!({"effect": "opacity", "shape": "ellipse", "center": [c.x, c.y], "size": [90, 90]})).unwrap(); + let job = s.execute("masks.track", json!({"direction": "forward"})).unwrap()["job"].clone(); + assert_eq!(s.mask_jobs.len(), 1); + s.execute("file.newProject", json!({"name": "Fresh"})).unwrap(); + assert!(s.mask_jobs.is_empty(), "stale job survived file.newProject"); + for _ in 0..20 { + s.poll_persistence(); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(s.mask_jobs.is_empty()); + let h = s.execute("history.list", json!({})).unwrap(); + assert!(!h["undo"].as_array().unwrap().iter().any(|v| v == "Track Mask"), "{h}"); + // the new project has no masks and no Mask Path keyframes + if let Ok(l) = s.execute("masks.list", json!({})) { + assert!(l["masks"].as_array().is_none_or(|m| m.is_empty()), "{l}"); + } + let project = serde_json::to_string(&*s.project).unwrap().to_lowercase(); + assert!(!project.contains("mask"), "stale mask data in new project"); + let jobs = s.execute("jobs.list", json!({})).unwrap(); + let j = jobs.as_array().unwrap().iter().find(|j| j["id"] == job).unwrap().clone(); + assert!(j["cancelled"] == true || j["finished"] == true, "{j}"); +} + +#[test] +fn every_project_replacing_path_drops_pending_tracking_jobs() { + // the epoch check covers paths that never call `discard_jobs`: recover (`install_project`), demo, close, template + let tpl = std::env::temp_dir().join(format!("filmcraft-mask-epoch-{}.fcproj", std::process::id())).to_string_lossy().to_string(); + for path in ["recover", "demo", "close", "template"] { + let (mut s, _) = tracking_session(); + s.execute("file.saveAsTemplate", json!({"path": tpl})).unwrap(); + let c = truth(0.0).apply(filmcraft_geom::Vec2::ZERO); + s.execute("masks.add", json!({"effect": "opacity", "shape": "ellipse", "center": [c.x, c.y], "size": [90, 90]})).unwrap(); + s.execute("masks.track", json!({"direction": "forward"})).unwrap(); + assert_eq!(s.mask_jobs.len(), 1); + match path { + "recover" => crate::commands::install_project(&mut s, filmcraft_project::Project::new("Recovered"), None, false), + "demo" => drop(s.execute("file.openDemoProject", json!({})).unwrap()), + "close" => drop(s.execute("file.closeProject", json!({"force": true})).unwrap()), + _ => drop(s.execute("file.newProjectFromTemplate", json!({"template": tpl, "force": true})).unwrap()), + } + for _ in 0..20 { + s.poll_persistence(); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + assert!(s.mask_jobs.is_empty(), "stale job survived {path}"); + let h = s.execute("history.list", json!({})).unwrap(); + assert!(!h["undo"].as_array().unwrap().iter().any(|v| v == "Track Mask"), "{path}: {h}"); + let project = serde_json::to_string(&*s.project).unwrap(); + assert!(!project.contains("pathKeyframes") && !project.to_lowercase().contains("\"masks\":[{"), "{path}: stale mask data"); + } + let _ = std::fs::remove_file(&tpl); +}