From af6697d207b2224fd7a305174abaefb53a68ae9f Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 15:28:08 +0000 Subject: [PATCH] fix(engine): name allocators resume each base's suffix instead of retrying from the start --- crates/engine/src/cmd/file_more.rs | 19 ++++++--- crates/engine/src/cmd/frame_export.rs | 47 ++++++++++++++++----- crates/engine/src/home/templates.rs | 9 ++-- crates/engine/src/tests/export_overwrite.rs | 14 ++++++ 4 files changed, 71 insertions(+), 18 deletions(-) diff --git a/crates/engine/src/cmd/file_more.rs b/crates/engine/src/cmd/file_more.rs index 168529c47..3088e8828 100644 --- a/crates/engine/src/cmd/file_more.rs +++ b/crates/engine/src/cmd/file_more.rs @@ -395,6 +395,9 @@ fn collect_files(s: &mut Session, p: &Value) -> Result { // reads (other footage, a proxy, the project file itself), and then nothing is written. let protected = s.protected_files(); let mut used_names = BTreeSet::new(); + // Where each file name's numbering resumes: a name repeated many times is numbered from its + // own file name, not by prefixing the previous candidate again. + let mut next_k: BTreeMap = BTreeMap::new(); let mut plan: Vec<(ItemId, Vec<(String, String)>)> = vec![]; for it in s.project.items.values() { let ItemKind::Footage(f) = &it.kind else { continue }; @@ -404,11 +407,17 @@ fn collect_files(s: &mut Session, p: &Value) -> Result { let files: Vec<&String> = if f.sequence.is_empty() { vec![&f.path] } else { f.sequence.iter().collect() }; let mut pairs = vec![]; for src in files { - let mut name = std::path::Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "footage".into()); - let mut k = 2; - while !used_names.insert(name.clone()) { - name = format!("{k}_{name}"); - k += 1; + let file = std::path::Path::new(src).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| "footage".into()); + let mut name = file.clone(); + if !used_names.insert(name.clone()) { + let k = next_k.entry(file.clone()).or_insert(2); + loop { + name = format!("{k}_{file}"); + *k += 1; + if used_names.insert(name.clone()) { + break; + } + } } let dst = format!("{folder}/(Footage)/{name}"); // A file that is already where its copy goes stays (it is not copied onto itself). diff --git a/crates/engine/src/cmd/frame_export.rs b/crates/engine/src/cmd/frame_export.rs index d8b49159c..7fb8c61f1 100644 --- a/crates/engine/src/cmd/frame_export.rs +++ b/crates/engine/src/cmd/frame_export.rs @@ -9,6 +9,8 @@ //! premultiplied): the composite as `R`, `G`, `B`, `A` plus one `.R/G/B/A` channel //! set per layer (the layer-prefixed channel convention multi-layer EXR readers use). +use std::collections::{HashMap, HashSet}; + use effectcraft_color::BlendMode; use effectcraft_project::{ItemId, LayerId, Project}; use effectcraft_raster::Image; @@ -142,17 +144,22 @@ fn srgb_to_linear(c: f32) -> f32 { /// A unique EXR layer name (no dots: they separate the layer from the channel). Printable ASCII only: /// the `exr` crate panics on characters it cannot store in a channel name (#1702). -fn exr_name(name: &str, used: &mut Vec) -> String { +/// `used` holds the names given so far, `next` the suffix each base's search resumes at: many +/// layers of one name take a name each without retrying every suffix before theirs. +fn exr_name(name: &str, used: &mut HashSet, next: &mut HashMap) -> String { let base: String = name.chars().map(|c| if c == '.' || !(c == ' ' || c.is_ascii_graphic()) { '_' } else { c }).collect(); let base = if base.trim().is_empty() { "Layer".to_string() } else { base }; - let mut n = base.clone(); - let mut k = 2; - while used.contains(&n) { - n = format!("{base} {k}"); - k += 1; + if used.insert(base.clone()) { + return base; + } + let k = next.entry(base.clone()).or_insert(2); + loop { + let n = format!("{base} {k}"); + *k += 1; + if used.insert(n.clone()) { + return n; + } } - used.push(n.clone()); - n } /// The multi-layer EXR bytes of a frame and the channel names. @@ -182,9 +189,9 @@ pub fn layered_exr(s: &Session, cid: ItemId, t: Tick, scale: f64) -> Result<(Vec } }; push("", &full); - let mut used = vec![]; + let (mut used, mut next) = (HashSet::new(), HashMap::new()); for l in &layers { - let n = exr_name(&l.name, &mut used); + let n = exr_name(&l.name, &mut used, &mut next); push(&n, &l.image); } let layer = Layer::new((w, h), LayerAttributes::default(), Encoding::SMALL_LOSSLESS, AnyChannels::sort(channels.into())); @@ -240,3 +247,23 @@ pub fn specs() -> Vec { ), ] } + +#[cfg(test)] +mod tests { + use std::collections::{HashMap, HashSet}; + + use super::exr_name; + + #[test] + fn many_layers_of_one_name_take_a_name_each_in_order() { + let (mut used, mut next) = (HashSet::new(), HashMap::new()); + // `A 3` is taken by a layer of its own before the repeats reach it. + let names: Vec = ["A", "A", "A 3", "A", "A.b"].iter().map(|n| exr_name(n, &mut used, &mut next)).collect(); + assert_eq!(names, ["A", "A 2", "A 3", "A 4", "A_b"]); + // 20 000 layers of one name: each found without retrying the suffixes before it. + for _ in 0..20_000 { + exr_name("Same", &mut used, &mut next); + } + assert_eq!(next["Same"], 20_001); + } +} diff --git a/crates/engine/src/home/templates.rs b/crates/engine/src/home/templates.rs index 0b15baaf5..f411fa5bd 100644 --- a/crates/engine/src/home/templates.rs +++ b/crates/engine/src/home/templates.rs @@ -751,6 +751,9 @@ fn footage_dir(s: &Session, name: &str, footage_dir: Option<&str>, project_path: fn extract_footage(s: &Session, proj: &mut Project, entries: &[(String, Vec)], dir: &str) -> Result> { let mut written = std::collections::HashMap::new(); let mut used = std::collections::HashSet::new(); + // Where each (item, file) pair's numbering resumes, so repeated names don't retry every + // number before theirs. + let mut next = std::collections::HashMap::new(); for (k, v) in entries.iter().filter(|(k, _)| k.starts_with("media/")) { // `media//` → one folder; a name another item took gets the item's id. let file = safe_file_name(&base_name(k)); @@ -763,10 +766,10 @@ fn extract_footage(s: &Session, proj: &mut Project, entries: &[(String, Vec) // case-insensitive volume (Windows and macOS as they come), and Win32 drops a trailing // dot or space, so `a.png.` is `a.png` there. The name written stays as it is. let mut name = file.clone(); - let mut n = 0usize; + let n = next.entry((item, same_file_key(&file))).or_insert(0usize); while !used.insert(same_file_key(&name)) { - n += 1; - name = if n == 1 { format!("{item}-{file}") } else { format!("{item}-{n}-{file}") }; + *n += 1; + name = if *n == 1 { format!("{item}-{file}") } else { format!("{item}-{n}-{file}") }; } let out = format!("{dir}/{name}"); s.services.store_file(&out, v).map_err(|e| EngineError::Other(format!("cannot write {out}: {e}")))?; diff --git a/crates/engine/src/tests/export_overwrite.rs b/crates/engine/src/tests/export_overwrite.rs index 5cc65b1d5..507a9be4b 100644 --- a/crates/engine/src/tests/export_overwrite.rs +++ b/crates/engine/src/tests/export_overwrite.rs @@ -331,3 +331,17 @@ fn background_pre_render_movies_are_checked_again_before_publication() { assert!(listing(&f.dir).iter().all(|name| !name.starts_with(".effectcraft-prerender-"))); } } + +/// Footage files sharing a name are numbered from that name (`3_a.mov`, not `3_2_a.mov`). +#[test] +fn collect_files_numbers_repeated_names_from_the_name() { + let mut f = collecting("collect-repeated", "a.mov"); + for d in ["in2", "in3"] { + std::fs::create_dir_all(f.path(d)).unwrap(); + f.footage(&format!("{d}/a.mov")); + } + collect(&mut f).unwrap(); + let mut names = listing(&f.path("out/(Footage)")); + names.sort(); + assert_eq!(names, ["2_a.mov", "3_a.mov", "a.mov"]); +}