Merge pull request #1830 from nanox333/fix/1743-duplicate-outputs

#1743, part 3: two Render Queue outputs never write the same file, and default names avoid the queue's other outputs
2026-11-01 05:58:19 -06:00 · 2026-10-30 16:36:32 +07:00
parent 44f95f766c 2112e553ec
commit 7174c81b2f
8 changed files with 1114 additions and 27 deletions
@@ -1,5 +1,6 @@
 //! Rendering never replaces a file the project reads (#1743). Ordinary-file replacement
-//! behavior stays with the exporter and its confirmation API.
+//! behavior stays with the exporter and its confirmation API. Two items of one job never write
+//! the same file either.

 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Mutex};
@@ -27,6 +28,9 @@ impl Exporter for Writer {
    fn formats(&self) -> Vec<OutputFormat> {
        OutputFormat::ALL.to_vec()
    }
+    fn supports_skip_existing(&self, _: &effectcraft_project::render_queue::RenderSettings) -> bool {
+        true
+    }
    fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result<ExportResult, String> {
        self.jobs.lock().unwrap().push(job.path.to_string());
        let comp = job.project.comp(job.item.comp).ok_or("no comp")?;
@@ -604,3 +608,430 @@ fn a_watched_folder_still_renders_a_project_again_over_its_outputs() {
    assert_eq!(done[0]["state"], "done", "{:?}", done[0]);
    assert_eq!(f.read("Main.mp4"), RENDERED);
 }
+
+// ------------------------------------------------------------ one file, two items of one job
+//
+// Nothing is on disk when a job starts, so the check for files that exist said nothing about two
+// queue items that resolve to one output: the item rendered last replaced the file of the one
+// before it, inside the same job.
+
+impl Fixture {
+    /// The output path of every queue item, in queue order.
+    fn outputs(&self) -> Vec<PathBuf> {
+        (0..self.s.project.render_queue.len()).map(|i| self.output(i)).collect()
+    }
+
+    fn add(&mut self, params: Value) {
+        self.s.execute("renderQueue.add", params).unwrap();
+    }
+
+    /// What the queue would write twice, in the shape the tests read: `{"duplicates": [...]}`.
+    fn check(&mut self) -> Value {
+        let formats = self.s.exporter.as_ref().map(|e| e.formats()).unwrap_or_default();
+        let (items, _) = self.s.queued_outputs(&formats);
+        let refused = self.s.check_outputs(&items);
+        let duplicates: Vec<Value> = self
+            .s
+            .duplicate_outputs(&items, &refused)
+            .unwrap()
+            .iter()
+            .map(|d| {
+                json!({
+                    "item": d.item, "index": d.index, "comp": d.comp, "path": d.path, "files": d.files, "sequence": d.sequence,
+                    "with": {"item": d.with, "index": d.with_index, "comp": d.with_comp}, "reason": d.reason(),
+                })
+            })
+            .collect();
+        json!({"duplicates": duplicates})
+    }
+
+    /// What `check` lists under `duplicates`: for each, the place in the queue of the item that
+    /// would write over a file, the place of the item that wrote it first, and how many files.
+    fn clashes(&mut self) -> Vec<(u64, u64, u64)> {
+        let found = self.check();
+        let list = found["duplicates"].as_array().unwrap_or_else(|| panic!("checkOutputs lists no duplicates: {found}"));
+        list.iter().map(|d| (d["index"].as_u64().unwrap_or(0), d["with"]["index"].as_u64().unwrap_or(0), d["files"].as_u64().unwrap_or(0))).collect()
+    }
+}
+
+/// A PNG sequence of `pattern` for the composition's seconds `start..end`.
+fn span(pattern: &str, start: f64, end: f64) -> Value {
+    json!({"format": "png", "output": pattern, "timeSpan": "custom", "start": start, "end": end})
+}
+
+fn with_overflow(f: &mut Fixture) {
+    struct Room;
+    impl effectcraft_project::render_queue::StorageQuota for Room {
+        fn has_room(&self, _: &str, _: u64) -> bool {
+            true
+        }
+    }
+    f.s.storage_quota = Some(Arc::new(Room));
+    std::fs::create_dir_all(f.path("overflow")).unwrap();
+    Arc::make_mut(&mut f.s.project).render_prefs.overflow_folders = vec![f.text("overflow")];
+}
+
+#[test]
+fn default_names_avoid_possible_storage_overflow_collisions() {
+    let mut f = Fixture::new("overflow-default");
+    f.comp("Main");
+    with_overflow(&mut f);
+    f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true}));
+    f.add(json!({"storageOverflow": true}));
+    assert_eq!(f.output(1), f.path("Main_1.mp4"));
+    assert!(f.clashes().is_empty());
+}
+
+#[test]
+fn possible_storage_overflow_destinations_cannot_replace_footage() {
+    let mut f = Fixture::new("overflow-footage");
+    f.comp("Main");
+    with_overflow(&mut f);
+    f.footage("overflow/Main.mp4");
+    f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true}));
+    let error = f.render(json!({})).unwrap_err().to_string();
+    assert!(error.contains("footage") && error.contains("Main.mp4"), "{error}");
+    assert!(f.exported().is_empty());
+    assert_eq!(f.read("overflow/Main.mp4"), SOURCE);
+}
+
+#[test]
+fn default_names_avoid_protected_storage_overflow_destinations() {
+    let mut f = Fixture::new("overflow-protected-default");
+    f.comp("Main");
+    with_overflow(&mut f);
+    f.footage("overflow/Main.mp4");
+    f.add(json!({"storageOverflow": true}));
+    assert_eq!(f.output(0), f.path("Main_1.mp4"));
+    f.render(json!({})).unwrap();
+    assert_eq!(f.exported().len(), 1);
+    assert_eq!(f.read("overflow/Main.mp4"), SOURCE);
+}
+
+/// After Effects numbers the second item of one composition (`Comp 1_1.mov`). A default output
+/// name that another item of the queue already has is numbered the same way.
+#[test]
+fn a_second_default_item_for_one_comp_gets_the_next_free_name() {
+    let mut f = Fixture::new("second-default");
+    f.comp("Main");
+    for _ in 0..3 {
+        f.add(json!({}));
+    }
+    assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main_1.mp4"), f.path("Main_2.mp4")]);
+    // The name is stored in the item: taking the first one out renames nothing, and the name it
+    // frees is the next default.
+    f.s.execute("renderQueue.remove", json!({"index": 1})).unwrap();
+    assert_eq!(f.outputs(), [f.path("Main_1.mp4"), f.path("Main_2.mp4")]);
+    f.add(json!({}));
+    assert_eq!(f.output(2), f.path("Main.mp4"));
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    for file in ["Main.mp4", "Main_1.mp4", "Main_2.mp4"] {
+        assert_eq!(f.read(file), RENDERED, "{file}");
+    }
+    assert_eq!(f.exported().len(), 3);
+}
+
+#[test]
+fn only_outputs_that_name_one_file_are_numbered() {
+    let mut f = Fixture::new("same-file-only");
+    f.comp("Main");
+    f.add(json!({"format": "h264"}));
+    f.add(json!({"format": "prores"}));
+    f.add(json!({"format": "hevc"})); // H.264 and HEVC both write .mp4
+    f.add(json!({"format": "png"}));
+    f.add(json!({"format": "png"})); // the same five frames of the same sequence
+    assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main.mov"), f.path("Main_1.mp4"), f.path("Main_[#####].png"), f.path("Main_1_[#####].png")]);
+    assert!(f.clashes().is_empty());
+}
+
+#[test]
+fn a_default_name_avoids_every_output_module_of_the_other_items() {
+    let mut f = Fixture::new("modules");
+    f.comp("Main");
+    f.add(json!({}));
+    // The item's second module is named Main_2.mp4, so that it does not clash with its first.
+    let item = f.s.execute("render.addOutputModule", json!({})).unwrap();
+    assert_eq!(Path::new(item["extraOutputs"][0]["outputPath"].as_str().unwrap()), f.path("Main_2.mp4"), "{item}");
+    f.add(json!({}));
+    f.add(json!({}));
+    assert_eq!(f.output(1), f.path("Main_1.mp4"));
+    assert_eq!(f.output(2), f.path("Main_3.mp4"), "Main_2.mp4 is the first item's second module");
+    assert!(f.clashes().is_empty());
+}
+
+#[test]
+fn a_name_the_caller_gave_is_never_numbered() {
+    let mut f = Fixture::new("named");
+    f.comp("Main");
+    let out = f.text("final.mp4");
+    f.add(json!({"output": out}));
+    f.add(json!({"output": out}));
+    f.add(json!({"path": out}));
+    assert_eq!(f.outputs(), vec![f.path("final.mp4"); 3]);
+    assert!(f.s.project.render_queue.iter().all(|i| i.output.output == out), "the names stored in the items are the ones given");
+    // Rendering them is what is refused (below): items 2 and 3 each clash with item 1.
+    assert_eq!(f.clashes(), [(2, 1, 1), (3, 1, 1)]);
+}
+
+#[test]
+fn a_pre_render_numbers_its_default_name_too() {
+    let mut f = Fixture::new("pre-render");
+    f.comp("Main");
+    f.s.execute("render.preRender", json!({})).unwrap();
+    f.s.execute("render.preRender", json!({})).unwrap();
+    f.s.execute("render.preRender", json!({"output": f.text("mine.mov")})).unwrap();
+    f.s.execute("render.preRender", json!({"output": f.text("mine.mov")})).unwrap();
+    assert_eq!(f.outputs(), [f.path("Main_prerender.mov"), f.path("Main_prerender_1.mov"), f.path("mine.mov"), f.path("mine.mov")]);
+}
+
+/// File ▸ Create Proxy queues a render with a default name of its own too.
+#[test]
+fn create_proxy_numbers_its_default_name_too() {
+    let mut f = Fixture::new("proxy-names");
+    f.comp("Main");
+    f.s.execute("file.createProxy", json!({"kind": "movie"})).unwrap();
+    f.s.execute("file.createProxy", json!({"kind": "movie"})).unwrap();
+    f.s.execute("file.createProxy", json!({"kind": "movie", "output": f.text("mine.mov")})).unwrap();
+    f.s.execute("file.createProxy", json!({"kind": "movie", "output": f.text("mine.mov")})).unwrap();
+    assert_eq!(f.outputs(), [f.path("Main_proxy.mov"), f.path("Main_proxy_1.mov"), f.path("mine.mov"), f.path("mine.mov")]);
+}
+
+/// Save Frame As ▸ File… with `queue`: every file of a sequence has its frame number in its name,
+/// so two frames of one composition share no file, and the same frame twice does.
+#[test]
+fn queued_frames_number_their_default_names_only_when_they_are_the_same_frame() {
+    let mut f = Fixture::new("frames");
+    f.comp("Main");
+    for time in [0.0, 0.2, 0.0] {
+        f.s.execute("comp.saveFrameAs", json!({"queue": true, "time": time})).unwrap();
+    }
+    assert_eq!(f.outputs(), [f.path("Main_[#####].png"), f.path("Main_[#####].png"), f.path("Main_1_[#####].png")]);
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    for file in ["Main_00000.png", "Main_00002.png", "Main_1_00000.png"] {
+        assert_eq!(f.read(file), RENDERED, "{file}");
+    }
+}
+
+/// The report: two queue items forced to one path (the same Output To) replaced each other's file
+/// inside one job. The render is refused before anything is written, whatever `overwrite` says:
+/// that flag is for files that exist, and this is the render replacing its own output.
+#[test]
+fn two_items_forced_to_one_file_are_reported_and_the_render_is_refused() {
+    let mut f = Fixture::new("one-file");
+    f.comp("Main");
+    let out = f.text("render.mp4");
+    f.add(json!({"output": out}));
+    f.add(json!({"output": out}));
+    let found = f.check();
+    let list = found["duplicates"].as_array().unwrap_or_else(|| panic!("checkOutputs lists no duplicates: {found}"));
+    assert_eq!(list.len(), 1, "{found}");
+    let d = &list[0];
+    let (first, second) = (f.s.project.render_queue[0].id, f.s.project.render_queue[1].id);
+    assert_eq!((d["item"].as_u64(), d["index"].as_u64(), d["comp"].as_str()), (Some(second), Some(2), Some("Main")), "{found}");
+    assert_eq!((d["with"]["item"].as_u64(), d["with"]["index"].as_u64(), d["with"]["comp"].as_str()), (Some(first), Some(1), Some("Main")), "{found}");
+    assert_eq!((d["files"].as_u64(), d["sequence"].as_bool()), (Some(1), Some(false)), "{found}");
+    assert_eq!(Path::new(d["path"].as_str().unwrap_or_default()), f.path("render.mp4"), "{found}");
+    let why = d["reason"].as_str().unwrap_or_default();
+    assert!(why.contains("#1") && why.contains("#2") && why.contains("render.mp4") && why.contains("Output To"), "{found}");
+
+    let queue = f.s.project.render_queue.clone();
+    let e = f.render(json!({})).unwrap_err().to_string();
+    assert!(e.contains("#1") && e.contains("#2") && e.contains("render.mp4") && e.contains("Nothing was rendered"), "{e}");
+    assert!(!f.path("render.mp4").exists());
+    assert!(f.exported().is_empty(), "the exporter was not started");
+    assert_eq!(f.s.project.render_queue, queue, "a refused render changes nothing in the queue");
+
+    // A file that exists as well does not change that: replacing it would not help.
+    f.write("render.mp4", EARLIER);
+    let found = f.check();
+    assert_eq!(found["duplicates"].as_array().map(Vec::len), Some(1), "{found}");
+    let e = f.render(json!({})).unwrap_err().to_string();
+    assert!(e.contains("#2") && e.contains("Nothing was rendered"), "{e}");
+    assert_eq!(f.read("render.mp4"), EARLIER);
+
+    // Another Output To for one of them, and both render.
+    f.s.execute("renderQueue.setOutput", json!({"index": 2, "path": f.text("second.mp4")})).unwrap();
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    assert_eq!((f.read("render.mp4"), f.read("second.mp4")), (RENDERED.to_vec(), RENDERED.to_vec()));
+}
+
+/// `renderQueue.duplicate` copies the item as it is, output included: it is reported until one of
+/// the two gets another Output To (or is turned off).
+#[test]
+fn a_duplicated_item_is_a_copy_and_clashes_until_one_of_them_changes() {
+    let mut f = Fixture::new("duplicate");
+    f.comp("Main");
+    f.add(json!({}));
+    f.s.execute("renderQueue.duplicate", json!({"index": 1})).unwrap();
+    assert_eq!(f.outputs(), [f.path("Main.mp4"), f.path("Main.mp4")], "a duplicate is a copy, its output included");
+    assert_eq!(f.clashes(), [(2, 1, 1)]);
+    let e = f.render(json!({})).unwrap_err().to_string();
+    assert!(e.contains("#1") && e.contains("#2") && e.contains("Main.mp4"), "{e}");
+    assert!(f.exported().is_empty());
+    // Turned off, the copy is not part of the job.
+    f.s.execute("renderQueue.setRender", json!({"index": 2, "render": false})).unwrap();
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    assert_eq!(f.read("Main.mp4"), RENDERED);
+    assert_eq!(f.exported().len(), 1);
+}
+
+/// Only the items of the job are compared: an earlier job's file is a file that exists, and
+/// replacing it is up to `overwrite`.
+#[test]
+fn an_item_that_is_not_in_the_job_does_not_clash() {
+    let mut f = Fixture::new("not-in-job");
+    f.comp("Main");
+    let out = f.text("render.mp4");
+    f.add(json!({"output": out}));
+    f.add(json!({"output": out}));
+    f.s.execute("renderQueue.setRender", json!({"index": 2, "render": false})).unwrap();
+    f.render(json!({})).unwrap(); // item 1 renders alone
+    f.s.execute("renderQueue.setRender", json!({"index": 2, "render": true})).unwrap();
+    assert!(f.clashes().is_empty(), "item 1 is done and is not in this job");
+    // item 2 now replaces the file item 1 wrote in the earlier job: an ordinary file, left to the exporter
+    f.render(json!({})).unwrap();
+}
+
+#[test]
+fn another_spelling_of_one_file_is_one_file_where_the_volume_ignores_case() {
+    let mut f = Fixture::new("spellings");
+    f.comp("Main");
+    f.write("probe.txt", b"x");
+    if !f.path("PROBE.TXT").exists() {
+        eprintln!("this volume tells the two names apart: skipped");
+        return;
+    }
+    f.add(json!({"output": f.text("Render.mp4")}));
+    f.add(json!({"output": f.text("render.MP4")}));
+    f.add(json!({"output": f.dir.join("sub").join("..").join("RENDER.mp4").to_string_lossy()}));
+    f.add(json!({"output": f.text("other.mp4")}));
+    assert_eq!(f.clashes(), [(2, 1, 1), (3, 1, 1)]);
+}
+
+/// A sequence writes one file per frame: two items of one pattern clash on the frames they share,
+/// and not at all when their frames differ.
+#[test]
+fn sequences_clash_only_on_the_frames_they_both_write() {
+    let mut f = Fixture::new("sequence-halves");
+    f.comp("Main");
+    let pattern = f.text("seq_[#####].png");
+    f.add(span(&pattern, 0.0, 0.2)); // frames 0-1
+    f.add(span(&pattern, 0.2, 0.5)); // frames 2-4
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    for n in 0..5 {
+        assert_eq!(f.read(&format!("seq_{n:05}.png")), RENDERED, "frame {n}");
+    }
+
+    let mut f = Fixture::new("sequence-overlap");
+    f.comp("Main");
+    let pattern = f.text("seq_[#####].png");
+    f.add(span(&pattern, 0.0, 0.2));
+    f.add(span(&pattern, 0.2, 0.5));
+    f.add(span(&pattern, 0.3, 0.5)); // frames 3-4 again
+    assert_eq!(f.clashes(), [(3, 2, 2)]);
+    let found = f.check();
+    assert_eq!(found["duplicates"][0]["sequence"], true, "{found}");
+    let e = f.render(json!({"overwrite": true})).unwrap_err().to_string();
+    assert!(e.contains("#2") && e.contains("#3") && e.contains("2 frames") && e.contains("seq_[#####].png"), "{e}");
+    assert!(!f.path("seq_00000.png").exists(), "the frames before the clash were not rendered either");
+    assert!(f.exported().is_empty());
+}
+
+#[test]
+fn differently_padded_sequences_cannot_replace_the_same_frames() {
+    let mut f = Fixture::new("padding-width-clash");
+    f.s.execute("comp.new", json!({"name": "Main", "width": 16, "height": 16, "frameRate": 10, "duration": 2})).unwrap();
+    f.add(span(&f.text("seq_[#].png"), 0.9, 1.2));
+    f.add(span(&f.text("seq_[##].png"), 0.9, 1.2));
+    assert_eq!(f.clashes(), [(2, 1, 2)]);
+    assert!(f.render(json!({})).is_err());
+    assert!(f.exported().is_empty());
+}
+
+/// Skip Existing Files leaves a frame alone when it is there: the item that skips and comes second
+/// writes only what the first did not. The item that skips and comes first is written over.
+#[test]
+fn a_later_item_that_skips_existing_frames_leaves_the_earlier_ones_alone() {
+    let mut f = Fixture::new("skip-later");
+    f.comp("Main");
+    let pattern = f.text("seq_[#####].png");
+    f.add(json!({"format": "png", "output": pattern}));
+    f.add(json!({"format": "png", "output": pattern, "skipExisting": true}));
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    assert_eq!(f.read("seq_00004.png"), RENDERED);
+
+    let mut f = Fixture::new("skip-earlier");
+    f.comp("Main");
+    let pattern = f.text("seq_[#####].png");
+    f.add(json!({"format": "png", "output": pattern, "skipExisting": true}));
+    f.add(json!({"format": "png", "output": pattern}));
+    assert_eq!(f.clashes(), [(2, 1, 5)]);
+}
+
+#[test]
+fn two_output_modules_of_one_item_cannot_share_a_file() {
+    let mut f = Fixture::new("two-modules");
+    f.comp("Main");
+    let out = f.text("render.mp4");
+    f.add(json!({"output": out}));
+    f.s.execute("render.addOutputModule", json!({"output": out})).unwrap();
+    assert_eq!(f.clashes(), [(1, 1, 1)]);
+    let e = f.render(json!({})).unwrap_err().to_string();
+    assert!(e.contains("#1") && e.contains("twice"), "{e}");
+    assert!(f.exported().is_empty());
+}
+
+/// Settings ▸ Export ▸ Segment Sequences puts each item's frames in numbered folders: default
+/// names keep two items' folders apart, and items given one name clash segment by segment.
+#[test]
+fn segmented_sequences_clash_segment_by_segment() {
+    let mut f = Fixture::new("segments");
+    f.comp("Main");
+    f.s.prefs.export.segment_sequences = true;
+    f.s.prefs.export.segment_sequence_files = 2; // five frames: segments of 2, 2 and 1
+    f.add(json!({"format": "png"}));
+    f.add(json!({"format": "png"}));
+    assert_eq!(f.output(1), f.path("Main_1_[#####].png"));
+    assert!(f.clashes().is_empty());
+    f.render(json!({})).unwrap();
+    assert_eq!(f.read("Main_001/Main_00000.png"), RENDERED);
+    assert_eq!(f.read("Main_1_003/Main_1_00004.png"), RENDERED);
+
+    let mut f = Fixture::new("segments-named");
+    f.comp("Main");
+    f.s.prefs.export.segment_sequences = true;
+    f.s.prefs.export.segment_sequence_files = 2;
+    let pattern = f.text("seg_[#####].png");
+    f.add(json!({"format": "png", "output": pattern}));
+    f.add(json!({"format": "png", "output": pattern}));
+    assert_eq!(f.clashes(), [(2, 1, 2), (2, 1, 2), (2, 1, 1)]);
+    let e = f.render(json!({})).unwrap_err().to_string();
+    assert!(e.contains("2 more"), "the first clash is named, the rest counted: {e}");
+    assert!(f.exported().is_empty());
+}
+
+/// An item that is refused for the file it would replace does not write anything, so it cannot
+/// clash with another item.
+#[test]
+fn items_refused_for_footage_are_not_compared_with_each_other() {
+    let mut f = Fixture::new("refused-pair");
+    f.footage("clip.mov");
+    f.comp("Main");
+    let clip = f.text("clip.mov");
+    f.add(json!({"format": "prores", "output": clip}));
+    f.add(json!({"format": "prores", "output": clip}));
+    f.add(json!({"output": f.text("ok.mp4")}));
+    let found = f.check();
+    assert_eq!(found["duplicates"].as_array().map(Vec::len), Some(0), "{found}");
+    let r = f.render(json!({})).unwrap();
+    let failed = r["items"].as_array().unwrap().iter().filter(|i| i["status"].get("Failed").is_some()).count();
+    assert_eq!(failed, 2, "both items on the footage are refused: {r}");
+    assert_eq!(f.read("clip.mov"), SOURCE);
+    assert_eq!(f.read("ok.mp4"), RENDERED);
+}