diff --git a/Cargo.lock b/Cargo.lock index 52812471c..a6611ffba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1653,6 +1653,7 @@ version = "0.18.0" dependencies = [ "effectcraft-geom", "image", + "log", "miniz_oxide 0.9.1", "serde", "serde_json", diff --git a/crates/media/src/pool.rs b/crates/media/src/pool.rs index 4820efa35..748d33c99 100644 --- a/crates/media/src/pool.rs +++ b/crates/media/src/pool.rs @@ -1218,8 +1218,8 @@ mod tests { assert_eq!(px(&f, FloatCurve::Gamma24), g24); } - /// A model's material library is read beside it; one named by a path to another computer - /// is not read. + /// A model's material library is read beside it; one named by a path to another computer is + /// looked up under its own file name beside the model, never fetched from that machine (#1274). #[test] fn model_files_on_other_computers_are_not_read() { let dir = std::env::temp_dir().join(format!("ec-model-files-{}", std::process::id())); @@ -1233,11 +1233,13 @@ mod tests { model.materials.iter().any(|m| m.base_color[..3] == [1.0, 0.0, 0.0]) }; assert!(red("beside.obj", "red.mtl")); - assert!(!red("unc.obj", r"\\192.0.2.1\share\red.mtl")); - assert!(!red("url.obj", "file://192.0.2.1/s/red.mtl")); + // 192.0.2.1 is a test-net address nothing here can reach, so these passes prove the + // material library came from beside the model, not from another computer. + assert!(red("unc.obj", r"\\192.0.2.1\share\red.mtl")); + assert!(red("url.obj", "file://192.0.2.1/s/red.mtl")); // Linux reads `//dir/red.mtl` as the local file. #[cfg(unix)] - assert!(!red("slashes.obj", &format!("/{}", dir.join("red.mtl").display()))); + assert!(red("slashes.obj", &format!("/{}", dir.join("red.mtl").display()))); let _ = std::fs::remove_dir_all(&dir); } } diff --git a/crates/model/Cargo.toml b/crates/model/Cargo.toml index 33ee26726..ebe7da7e8 100644 --- a/crates/model/Cargo.toml +++ b/crates/model/Cargo.toml @@ -14,5 +14,6 @@ effectcraft-geom = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } thiserror = { workspace = true } +log = { workspace = true } image = { workspace = true } miniz_oxide = "0.9" diff --git a/crates/model/src/obj.rs b/crates/model/src/obj.rs index 059278ecd..22d9877a2 100644 --- a/crates/model/src/obj.rs +++ b/crates/model/src/obj.rs @@ -14,18 +14,52 @@ use std::sync::Arc; use crate::{AlphaMode, Material, Mesh, Model, Primitive, Result, TexRef, Texture, perr}; +/// The last component of a referenced path, treating `\` as a separator too: a path from a +/// machine of another kind (`C:\textures\checker.png`) still finds `checker.png` next to the +/// model (#1274). `None` when the name carries no path at all. +fn sibling_name(name: &str) -> Option<&str> { + let file = name.rsplit(['/', '\\']).next()?; + (!file.is_empty() && file.len() < name.len()).then_some(file) +} + +/// A material reference resolved against the model's folder: the name as written (a name +/// containing spaces is one name), then its file name (an absolute path from another machine, +/// #1274), then the final token (the options convention, `-bm 1 file.png`). +fn resolve_reference(resolve: &dyn Fn(&str) -> Option>, name: &str) -> Option> { + let mut tried: Vec<&str> = vec![]; + for candidate in [name, sibling_name(name).unwrap_or(name), name.split_whitespace().last().unwrap_or(name)] { + if tried.contains(&candidate) { + continue; + } + tried.push(candidate); + if let Some(b) = resolve(candidate) { + return Some(b); + } + } + None +} + /// Parse MTL text into named materials (textures appended to `textures`). pub fn parse_mtl(text: &str, resolve: &dyn Fn(&str) -> Option>, textures: &mut Vec>) -> Vec { let mut out: Vec = vec![]; let mut loaded: HashMap> = HashMap::new(); let mut tex = |name: &str, textures: &mut Vec>| -> Option { - // Options (`-bm 1 file.png`): the file name is the last token. - let file = name.split_whitespace().last()?.to_string(); - let t = *loaded.entry(file.clone()).or_insert_with(|| { - let b = resolve(&file)?; - let t = Texture::decode(&b).ok()?; - textures.push(Arc::new(t)); - Some(textures.len() - 1) + let key = name.trim().to_string(); + let t = *loaded.entry(key.clone()).or_insert_with(|| { + let Some(b) = resolve_reference(resolve, &key) else { + log::warn!("model: texture `{key}` was not found next to the model"); + return None; + }; + match Texture::decode(&b) { + Ok(t) => { + textures.push(Arc::new(t)); + Some(textures.len() - 1) + } + Err(e) => { + log::warn!("model: cannot decode texture `{key}`: {e}"); + None + } + } }); t.map(|texture| TexRef { texture, uv_set: 0 }) }; @@ -147,13 +181,18 @@ pub fn parse(bytes: &[u8], resolve: &dyn Fn(&str) -> Option>) -> Result< } "mtllib" => { let name = line["mtllib".len()..].trim(); - for file in std::iter::once(name).chain(name.split_whitespace()) { + let mut found = false; + for file in std::iter::once(name).chain(name.split_whitespace()).chain(sibling_name(name)) { if let Some(b) = resolve(file) { let mut ms = parse_mtl(&String::from_utf8_lossy(&b), resolve, &mut textures); materials.append(&mut ms); + found = true; break; } } + if !found && !name.is_empty() { + log::warn!("model: material library `{name}` was not found next to the model"); + } } "usemtl" => { let name = line["usemtl".len()..].trim(); diff --git a/crates/model/src/tests.rs b/crates/model/src/tests.rs index 3fe0d6525..5ee6a6950 100644 --- a/crates/model/src/tests.rs +++ b/crates/model/src/tests.rs @@ -119,6 +119,92 @@ fn obj_without_normals_gets_them() { assert!(obj::parse(b"v 0 0 0\n", &|_| None).is_err()); } +#[test] +fn obj_textures_from_another_machine_find_the_file_next_to_the_model() { + // No committed media: the material file and its textures are written by the test itself. + let d = std::env::temp_dir().join(format!("ec-obj-sibling-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + let png = { + let img = image::RgbaImage::from_fn(2, 2, |x, y| if (x + y) % 2 == 0 { image::Rgba([255, 0, 0, 255]) } else { image::Rgba([0, 0, 255, 255]) }); + let mut out = std::io::Cursor::new(Vec::new()); + img.write_to(&mut out, image::ImageFormat::Png).unwrap(); + out.into_inner() + }; + std::fs::write(d.join("checker.png"), &png).unwrap(); + std::fs::write(d.join("Diffuse Map.png"), &png).unwrap(); + let mtl = "newmtl Mat\nKd 0.640000 0.640000 0.640000\nmap_Kd C:\\Users\\somebody\\Desktop\\textures\\checker.png\n\ + newmtl Spaced\nKd 1.000000 1.000000 1.000000\nmap_Kd Diffuse Map.png\n\ + newmtl Missing\nKd 0.500000 0.500000 0.500000\nmap_Kd nowhere.png\n"; + let obj = "mtllib quad.mtl\nv -1 -1 0\nv 1 -1 0\nv 1 1 0\nv -1 1 0\nvt 0 0\nvt 1 0\nvt 1 1\nvt 0 1\n\ + usemtl Mat\nf 1/1 2/2 3/3\nf 1/1 3/3 4/4\n"; + // Like the media layer's resolver: a relative name joins the model's folder; an absolute + // path is only looked up where it points (here: nowhere else). + let resolve = |uri: &str| -> Option> { + let p = std::path::Path::new(uri); + if p.is_absolute() { + return None; + } + std::fs::read(d.join(p)).ok() + }; + let resolver = |uri: &str| -> Option> { if uri == "quad.mtl" { Some(mtl.as_bytes().to_vec()) } else { resolve(uri) } }; + let m = obj::parse(obj.as_bytes(), &resolver).unwrap(); + assert_eq!(m.materials.len(), 3); + let mat = &m.materials[0]; + let t = mat.base_color_tex.expect("the absolute map_Kd path finds checker.png next to the .obj"); + let tex = &m.textures[t.texture]; + assert_eq!((tex.width, tex.height), (2, 2)); + assert_eq!(tex.data[0], [1.0, 0.0, 0.0, 1.0]); + assert!((mat.base_color[0] - obj::srgb_to_linear(0.64)).abs() < 1e-6, "Kd still tints the texture"); + assert!(m.materials[1].base_color_tex.is_some(), "a texture name containing spaces is one name"); + assert!(m.materials[2].base_color_tex.is_none(), "a genuinely missing texture keeps the Kd colour"); + assert!((m.materials[2].base_color[0] - obj::srgb_to_linear(0.5)).abs() < 1e-6); + // The same fallback applies to the material library itself. + let here = obj.replace("mtllib quad.mtl", "mtllib C:\\build\\quad.mtl"); + assert_eq!(obj::parse(here.as_bytes(), &resolver).unwrap().materials.len(), 3); + let _ = std::fs::remove_dir_all(&d); +} + +#[test] +fn obj_texture_prefers_the_complete_name_over_its_final_token() { + // Two textures sit beside the model: `Map.png` (the final whitespace token of the reference) + // and `Diffuse Map.png` (the complete name). The complete name must win, its basename next, + // the final token last (the `-bm 1 file.png` options convention). + let d = std::env::temp_dir().join(format!("ec-obj-full-name-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + let png = |w: u32| { + let img = image::RgbaImage::from_fn(w, 2, |_, _| image::Rgba([255, 0, 0, 255])); + let mut out = std::io::Cursor::new(Vec::new()); + img.write_to(&mut out, image::ImageFormat::Png).unwrap(); + out.into_inner() + }; + std::fs::write(d.join("Map.png"), png(2)).unwrap(); + std::fs::write(d.join("Diffuse Map.png"), png(4)).unwrap(); + let mtl = "newmtl Spaced\nmap_Kd Diffuse Map.png\n\ + newmtl Absolute\nmap_Kd C:\\tex\\Diffuse Map.png\n\ + newmtl Options\nmap_Kd -bm 1 Map.png\n"; + let obj = "mtllib quad.mtl\nv -1 -1 0\nv 1 -1 0\nv 1 1 0\nv -1 1 0\nvt 0 0\nvt 1 0\nvt 1 1\nvt 0 1\n\ + usemtl Spaced\nf 1/1 2/2 3/3\nf 1/1 3/3 4/4\n"; + // Like the media layer's resolver: a relative name joins the model's folder; an absolute + // path is only looked up where it points (here: nowhere else). + let resolve = |uri: &str| -> Option> { + let p = std::path::Path::new(uri); + if p.is_absolute() { + return None; + } + std::fs::read(d.join(p)).ok() + }; + let resolver = |uri: &str| -> Option> { if uri == "quad.mtl" { Some(mtl.as_bytes().to_vec()) } else { resolve(uri) } }; + let m = obj::parse(obj.as_bytes(), &resolver).unwrap(); + assert_eq!(m.materials.len(), 3); + let size = |mat: &Material| mat.base_color_tex.map(|t| (m.textures[t.texture].width, m.textures[t.texture].height)); + assert_eq!(size(&m.materials[0]), Some((4, 2)), "the complete `Diffuse Map.png` wins over its final token `Map.png`"); + assert_eq!(size(&m.materials[1]), Some((4, 2)), "an absolute path's spaced basename beats its final token"); + assert_eq!(size(&m.materials[2]), Some((2, 2)), "the `-bm 1 file.png` options convention still finds the final token"); + let _ = std::fs::remove_dir_all(&d); +} + #[test] fn channel_interpolation_modes() { let mut c = Channel { node: 0, path: ChannelPath::Translation, interp: Interp::Step, times: vec![0.0, 1.0], values: vec![0.0, 0.0, 0.0, 10.0, 0.0, 0.0] };