From c5671f6569efd5817646fde30c250eea4e81643e Mon Sep 17 00:00:00 2001 From: Kristian Myram Date: Fri, 30 Oct 2026 08:59:53 +0100 Subject: [PATCH] #1830: require exporter support before skipping duplicate frames --- crates/engine/src/output_guard.rs | 3 ++- crates/engine/src/render_queue.rs | 5 +++++ crates/engine/src/tests/render_overwrite.rs | 3 +++ crates/host/src/lib.rs | 3 +++ crates/host/src/tests.rs | 21 +++++++++++++++++++++ docs/agents.md | 3 ++- docs/mcp.md | 5 +++-- 7 files changed, 39 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/output_guard.rs b/crates/engine/src/output_guard.rs index 7634313f2..86c9fa5e6 100644 --- a/crates/engine/src/output_guard.rs +++ b/crates/engine/src/output_guard.rs @@ -494,7 +494,8 @@ impl Session { } else { Target::file(Path::new(path), by_identity) }; - Write { item: m.id, path: path.to_string(), target, skips: m.settings.skip_existing } + let skips = m.settings.skip_existing && self.exporter.as_ref().is_some_and(|exporter| exporter.supports_skip_existing()); + Write { item: m.id, path: path.to_string(), target, skips } } /// The report of `later` writing over `clash`. diff --git a/crates/engine/src/render_queue.rs b/crates/engine/src/render_queue.rs index 0285a64d2..045b9f782 100644 --- a/crates/engine/src/render_queue.rs +++ b/crates/engine/src/render_queue.rs @@ -57,6 +57,11 @@ pub struct ExportResult { pub trait Exporter: Send + Sync { /// Formats this build can write. fn formats(&self) -> Vec; + /// Whether Skip Existing Files preserves sequence frames written by earlier items. Exporters + /// without an existence check (such as memory sinks) must keep the conservative default. + fn supports_skip_existing(&self) -> bool { + false + } /// Render and write one item (blocking). `progress(done, total)` returns `false` to cancel; /// a cancelled export removes the files it opened for that output and returns `Err("cancelled")`. /// Completed earlier queue items and unrelated files must be preserved. diff --git a/crates/engine/src/tests/render_overwrite.rs b/crates/engine/src/tests/render_overwrite.rs index a94063253..8572a56f8 100644 --- a/crates/engine/src/tests/render_overwrite.rs +++ b/crates/engine/src/tests/render_overwrite.rs @@ -28,6 +28,9 @@ impl Exporter for Writer { fn formats(&self) -> Vec { OutputFormat::ALL.to_vec() } + fn supports_skip_existing(&self) -> bool { + true + } fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result { self.jobs.lock().unwrap().push(job.path.to_string()); let comp = job.project.comp(job.item.comp).ok_or("no comp")?; diff --git a/crates/host/src/lib.rs b/crates/host/src/lib.rs index 08e1616a7..1137a8bce 100644 --- a/crates/host/src/lib.rs +++ b/crates/host/src/lib.rs @@ -33,6 +33,9 @@ impl Exporter for FileExporter { fn formats(&self) -> Vec { effectcraft_export::available_formats() } + fn supports_skip_existing(&self) -> bool { + self.sink.is_none() && !cfg!(target_arch = "wasm32") + } fn export(&self, job: &ExportJob, progress: &mut dyn FnMut(u64, u64) -> bool) -> Result { effectcraft_engine::render::passes::block_on(self.export_async(job, progress)) } diff --git a/crates/host/src/tests.rs b/crates/host/src/tests.rs index 3114c2ddb..447411dc7 100644 --- a/crates/host/src/tests.rs +++ b/crates/host/src/tests.rs @@ -1,5 +1,26 @@ use serde_json::json; +#[test] +fn memory_sink_cannot_overwrite_a_previous_output_even_with_skip_existing() { + use std::sync::{ + Arc, + atomic::{AtomicUsize, Ordering}, + }; + let writes = Arc::new(AtomicUsize::new(0)); + let count = writes.clone(); + let sink: Arc = Arc::new(move |_, _| { + count.fetch_add(1, Ordering::Relaxed); + }); + let mut s = effectcraft_engine::Session { exporter: Some(Arc::new(super::FileExporter { sink: Some(sink) })), ..Default::default() }; + s.execute("comp.new", json!({"name": "Main", "width": 16, "height": 16, "frameRate": 10, "duration": 0.2})).unwrap(); + for _ in 0..2 { + s.execute("renderQueue.add", json!({"format": "png", "output": "/virtual/frames_[#####].png", "skipExisting": true})).unwrap(); + } + let result = s.execute("renderQueue.render", json!({})); + assert!(result.is_err(), "a sink cannot skip previous paths: {result:?}"); + assert_eq!(writes.load(Ordering::Relaxed), 0, "refused before the sink receives bytes"); +} + #[test] fn host_runs_tasks_scheduled_by_panel_handlers() { let mut s = super::session(); diff --git a/docs/agents.md b/docs/agents.md index 882fb13c3..3d1f4dd83 100644 --- a/docs/agents.md +++ b/docs/agents.md @@ -322,7 +322,8 @@ build real projects through these interfaces; they are worked examples of everyt **A render never writes one file twice either:** when two outputs of the job (two items, a `renderQueue.duplicate` copy, output modules, segments) name the same file, the whole render is refused before anything is written, naming both items. Sequences clash only on frames both - write, and a later item with Skip Existing Files leaves the earlier one's frames alone. Default + write. A later item with Skip Existing Files leaves the earlier one's frames alone only when + the exporter can check existing files; browser and other memory sinks still report collisions. Default names are numbered like footage collisions when another queue item writes them (`renderQueue.add`, `render.preRender`, `comp.saveFrameAs {queue}`, `file.createProxy`: a second `Main` renders to `Main_1.mp4`); a name you give is kept, and `renderQueue.duplicate` copies the diff --git a/docs/mcp.md b/docs/mcp.md index 59d41552e..116158706 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -58,8 +58,9 @@ receive `resultType: complete` and list/read cache hints; document reads are nev Two outputs of one job that write the same file (two items with one Output To, a duplicated item, output modules or segments that meet) refuse the whole `renderQueue.render` before anything is written: the reply is `isError: true`, naming both items, and the queue is left as it was. -Sequences clash only on the frames both write, and an item with Skip Existing Files leaves the -earlier one's frames alone. A new item's default output name is numbered (`Main.mp4`, +Sequences clash only on the frames both write. Skip Existing Files leaves earlier frames alone +only when the exporter can check existing files; browser and other memory sinks still report +collisions. A new item's default output name is numbered (`Main.mp4`, `Main_1.mp4`) when another item of the queue writes it. ## Progress and cancellation