From 8029b061af2e8be76975fb7828cd0ef19b2c4c85 Mon Sep 17 00:00:00 2001 From: David Mann Date: Tue, 13 Oct 2026 03:17:45 +0200 Subject: [PATCH] EXR: an unnamed part can be opened by the name the part list gives it (#4210) (#4211) file.exrParts lists a part without a name attribute as "part ", but file.openExrParts only matched parts whose name attribute equalled the requested name, so "part/part 0" failed with "no flat part named part 0". One helper now gives the display name for the listing, the lookup and the layer name; a named part still wins over an unnamed one with the same fallback name. Co-authored-by: Claude Opus 5.5 (1M context) --- crates/engine/src/exr_cmds.rs | 40 +++++++++++++++++++++++++++++++++++ crates/io/src/exr_parts.rs | 20 ++++++++++++++---- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/exr_cmds.rs b/crates/engine/src/exr_cmds.rs index c0a3310f9f..908942d4fd 100644 --- a/crates/engine/src/exr_cmds.rs +++ b/crates/engine/src/exr_cmds.rs @@ -210,6 +210,46 @@ mod tests { std::fs::remove_dir_all(&dir).unwrap(); } + /// A single-part EXR with no part name (the usual render output) plus an AOV group. + fn write_unnamed_grouped(path: &std::path::Path) { + use exr::prelude::*; + let (w, h) = (4usize, 4usize); + let mk = |n: &str, v: f32| AnyChannel::new(n, FlatSamples::F32(vec![v; w * h])); + let list = vec![mk("R", 0.7), mk("G", 0.7), mk("B", 0.7), mk("diffuse.R", 0.3), mk("diffuse.G", 0.3), mk("diffuse.B", 0.3)]; + let layer = Layer::new( + (w, h), + LayerAttributes::default(), + Encoding { compression: Compression::Uncompressed, blocks: Blocks::ScanLines, line_order: LineOrder::Increasing }, + AnyChannels::sort(list.into_iter().collect()), + ); + let image = Image::from_layers(ImageAttributes::new(IntegerBounds::new((0, 0), (w, h))), vec![layer]); + image.write().to_buffered(&mut std::io::BufWriter::new(std::fs::File::create(path).expect("create fixture"))).expect("write fixture"); + } + + #[test] + fn an_unnamed_part_opens_by_the_name_the_listing_gives_it() { + // #4210: the listing names an unnamed part "part 0"; feeding that name back must open it. + let dir = std::env::temp_dir().join(format!("pcraft-exr-unnamed-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let path = dir.join("unnamed.exr"); + write_unnamed_grouped(&path); + let info = entries_json(path.to_str().unwrap()).unwrap(); + let part = info.as_array().unwrap().iter().find(|e| e["kind"] == "part").expect("a part entry"); + let name = part["name"].as_str().unwrap(); + assert_eq!(name, "part 0", "{info}"); + let key = format!("part/{name}"); + let mut s = session(); + s.execute("file.openExrParts", json!({"path": path.to_str().unwrap(), key: true})).unwrap(); + let doc = &s.active().unwrap().doc; + assert_eq!(doc.layers.len(), 1); + assert_eq!(doc.layers[0].name, "part 0"); + if let photocraft_doc::LayerContent::Raster(surface) = &doc.layers[0].content { + let px = surface.pixel(0, 0); + assert!((px[0] - 0.7).abs() < 1e-4, "{:?}", px); + } + std::fs::remove_dir_all(&dir).unwrap(); + } + #[test] fn precomp_sums_the_light_path_set_and_refuses_an_incomplete_one() { let dir = std::env::temp_dir().join(format!("pcraft-exr-precomp-{}", std::process::id())); diff --git a/crates/io/src/exr_parts.rs b/crates/io/src/exr_parts.rs index 93b8e85f36..5474d00310 100644 --- a/crates/io/src/exr_parts.rs +++ b/crates/io/src/exr_parts.rs @@ -98,6 +98,12 @@ fn channel_base_split(name: &str) -> Option<(&str, &str)> { (!prefix.is_empty()).then_some((prefix, base)) } +/// The name a part is listed, selected and imported as: its `name` attribute, or +/// `part ` when it has none (a single-part render usually carries no name). +fn part_display_name(part: &codecs::ExrPartInfo) -> String { + part.name.clone().unwrap_or_else(|| format!("part {}", part.index)) +} + /// Lists the entries of a multi-part or channel-grouped EXR for a chooser: flat parts in /// file order, then the channel groups, then the Cryptomatte layers as information. A file /// whose parts are all deep is refused (the deep decoder composites those). @@ -113,7 +119,7 @@ pub fn entries(bytes: &[u8]) -> Result, IoError> { } out.push(Entry { kind: EntryKind::Part, - name: part.name.clone().unwrap_or_else(|| format!("part {}", part.index)), + name: part_display_name(part), view: part.view.clone(), width: part.width, height: part.height, @@ -186,8 +192,14 @@ pub fn import(name: &str, bytes: &[u8], sel: &Selection) -> Result| view.as_ref().is_none_or(|v| sel.view.eq_ignore_ascii_case("both") || v.eq_ignore_ascii_case(&sel.view)); - let part_by_name = - |want: &str| info.iter().find(|p| !p.deep && p.name.as_deref() == Some(want)).ok_or_else(|| IoError::Unsupported(format!("no flat part named {want}"))); + // A named part wins over an unnamed one listed under the same fallback name ("part 0"). + let part_by_name = |want: &str| { + let flat = || info.iter().filter(|p| !p.deep); + flat() + .find(|p| p.name.as_deref() == Some(want)) + .or_else(|| flat().find(|p| p.name.is_none() && part_display_name(p) == want)) + .ok_or_else(|| IoError::Unsupported(format!("no flat part named {want}"))) + }; let mut chosen_parts: Vec<&codecs::ExrPartInfo> = Vec::new(); for want in &sel.parts { let p = part_by_name(want)?; @@ -235,7 +247,7 @@ pub fn import(name: &str, bytes: &[u8], sel: &Selection) -> Result