From 7fdd3f3d22c12ced0b7699874dca1f19e5e285c5 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 28 Jun 2026 22:35:22 +0200 Subject: [PATCH 1/4] fix(usdz): sniff format of agnostic .usd layers --- src/usdz/reader.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/src/usdz/reader.rs b/src/usdz/reader.rs index 8e2e19f1..e2151ac1 100644 --- a/src/usdz/reader.rs +++ b/src/usdz/reader.rs @@ -91,10 +91,23 @@ impl Archive { // TODO: Implement nested USDZ files support. bail!("Nested USDZ files are not yet supported: '{}'", file_path) } else { - bail!( - "Unsupported file format for '{}'. Expected .usda or .usdc extension", - file_path - ) + // Format-agnostic `.usd` (or any other extension): resolve by + // content, mirroring USD's content-based format detection. A USDC + // crate file begins with the magic `PXR-USDC`; otherwise parse as + // USDA text. Per the USDZ spec the root layer may be `.usd`, and + // Pixar's reference assets (e.g. Kitchen_set.usdz) ship it that way. + if buffer.starts_with(usdc::MAGIC) { + let cursor = Cursor::new(buffer); + let data = usdc::CrateData::open(cursor, true) + .with_context(|| format!("Failed to parse USDC data from '{}'", file_path))?; + Ok(Box::new(data)) + } else { + let content = + String::from_utf8(buffer).with_context(|| format!("File '{}' is not valid UTF-8", file_path))?; + let data = + usda::parse(&content).with_context(|| format!("Failed to parse USDA data from '{}'", file_path))?; + Ok(Box::new(data)) + } } } } From dbe9de015a2d948d3dabdd2dcf1073bbde5d3f17 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 28 Jun 2026 22:35:22 +0200 Subject: [PATCH 2/4] feat(usdz): resolve references inside packages --- src/ar.rs | 70 ++++++++++++++++++++++++++++++++++++--- src/pcp/mod.rs | 16 --------- src/pcp/prim_indexer.rs | 25 +------------- src/sdf/layer_registry.rs | 17 +++------- src/usdz/mod.rs | 50 +++++++++++++++++++++++++++- tests/stage.rs | 33 ++++++++++-------- 6 files changed, 140 insertions(+), 71 deletions(-) diff --git a/src/ar.rs b/src/ar.rs index 8e7e4ca3..86677e67 100644 --- a/src/ar.rs +++ b/src/ar.rs @@ -60,6 +60,19 @@ impl ResolvedPath { /// format case-insensitively — `resolved.extension() == "usdz"` — the way the /// format registry's `find_by_extension` does, without juggling `OsStr`. pub(crate) fn extension(&self) -> String { + // Package-relative path (`pkg.usdz[inner/layer.usd]`): the format is the + // innermost packaged layer's extension, not the literal trailing chars + // (`.usd]`). Match the format on the inner path instead. + let s = self.0.to_string_lossy(); + if is_package_relative_path(&s) { + if let Some((_, inner)) = split_package_relative_path_inner(&s) { + return Path::new(&inner) + .extension() + .and_then(|ext| ext.to_str()) + .map(str::to_ascii_lowercase) + .unwrap_or_default(); + } + } self.0 .extension() .and_then(|ext| ext.to_str()) @@ -283,6 +296,11 @@ impl Resolver for DefaultResolver { return String::new(); } + // An already package-relative asset path is its own identifier. + if is_package_relative_path(asset_path) { + return asset_path.to_string(); + } + let path = Path::new(asset_path); // Absolute paths are their own identifier. @@ -290,8 +308,25 @@ impl Resolver for DefaultResolver { return canonical_identifier(path.to_path_buf()); } - // Anchor relative paths to the anchor's directory. + // Anchor relative paths. if let Some(anchor) = anchor { + let anchor_str = anchor.to_string_lossy(); + // Anchor lives inside a package (`pkg.usdz[inner/layer.usd]`): keep + // the reference inside the package, relative to the inner layer's + // directory → `pkg.usdz[inner/asset.usd]`. + if is_package_relative_path(&anchor_str) { + if let Some((package, inner)) = split_package_relative_path_inner(&anchor_str) { + let inner_dir = Path::new(&inner).parent().unwrap_or_else(|| Path::new("")); + let joined = lexically_normalize(&inner_dir.join(path)); + return join_package_relative_path(&package, &joined.to_string_lossy()); + } + } + // Anchor IS a package file (`foo.usdz`): a relative reference from + // the package's root layer is package-relative → `foo.usdz[asset]`. + if anchor_str.to_ascii_lowercase().ends_with(".usdz") { + let joined = lexically_normalize(path); + return join_package_relative_path(&anchor_str, &joined.to_string_lossy()); + } if let Some(dir) = anchor.parent() { return canonical_identifier(dir.join(path)); } @@ -311,16 +346,41 @@ impl Resolver for DefaultResolver { return None; } - // Handle package-relative paths. + // Handle package-relative paths. The package itself is resolved as a + // plain file (not re-anchored to its default layer — that would nest), + // then the inner packaged path is reattached. if is_package_relative_path(asset_path) { let (package, inner) = split_package_relative_path_outer(asset_path)?; - let resolved_package = self.resolve(&package)?; - // Return the resolved package with the inner path reattached. + let resolved_package = self.resolve_with_search_paths(&package)?; let package_str = resolved_package.to_str().unwrap_or_default(); return Some(ResolvedPath::new(join_package_relative_path(package_str, &inner))); } - self.resolve_with_search_paths(asset_path) + let resolved = self.resolve_with_search_paths(asset_path)?; + + // A bare package (`.usdz`) resolves to its default — first — packaged + // layer (`pkg.usdz[first.usd]`), so the package participates in normal + // composition and references inside it resolve in-package. Mirrors C++ + // Ar package resolution; applies uniformly to a root, sublayer, or + // referenced `.usdz`. + if resolved.extension() == "usdz" { + // Read the package directory through the resolver's own asset seam + // (`open_asset`) rather than direct filesystem access, so a future + // host-provided byte source (issue #105's "get me the bytes for + // this layer") flows through unchanged — no async introduced here. + if let Some(first) = self + .open_asset(&resolved) + .ok() + .and_then(|mut asset| asset.read_all().ok()) + .and_then(|bytes| crate::usdz::Archive::from_reader(std::io::Cursor::new(bytes)).ok()) + .and_then(|archive| archive.first_layer_name()) + { + let package_str = resolved.to_str().unwrap_or_default(); + return Some(ResolvedPath::new(join_package_relative_path(package_str, &first))); + } + } + + Some(resolved) } fn resolve_for_new_asset(&self, asset_path: &str) -> Option { diff --git a/src/pcp/mod.rs b/src/pcp/mod.rs index 3d6dad04..523dec87 100644 --- a/src/pcp/mod.rs +++ b/src/pcp/mod.rs @@ -451,22 +451,6 @@ pub enum Error { site_path: Path, }, - /// A reference or payload authored inside a `.usdz` package names another - /// layer, which would require opening a sibling layer within the archive — - /// not yet supported (the eager collector bailed the same way). The arc is - /// dropped while the rest of the prim composes. - #[error("unsupported {arc:?} @{asset_path}@ inside usdz package @{introduced_by}@ at {site_path}")] - UnsupportedUsdzReference { - /// The reference/payload asset path authored inside the package. - asset_path: String, - /// The composition arc type. - arc: ArcType, - /// Identifier of the usdz package layer that authored the arc. - introduced_by: String, - /// The prim path where the arc was authored. - site_path: Path, - }, - /// A reference/payload resolved its target layer, but the named prim path /// authors no spec there (C++ `PcpErrorUnresolvedPrimPath`). The arc is /// dropped while the rest of the prim still composes. diff --git a/src/pcp/prim_indexer.rs b/src/pcp/prim_indexer.rs index 9ab5ece4..6cba2720 100644 --- a/src/pcp/prim_indexer.rs +++ b/src/pcp/prim_indexer.rs @@ -2566,31 +2566,8 @@ impl<'a, 'f> Indexer<'a, 'f> { } else { let layer_index = match self.inputs.stack.id_of(asset_path) { Some(layer_index) => layer_index, - // The target is not loaded. The authoring layer's location gates - // both the `.usdz` guard and the relative-mute anchor, so resolve - // it once here — only on a lookup miss, leaving an already-loaded - // arc (the steady state) doing no filesystem work. + // The target is not loaded. None => { - // TODO(perf): this resolves the authoring layer's location - // only to read its extension for the usdz guard; the layer's - // in-memory identifier already carries the extension, so the - // guard could test that and skip the filesystem resolve. - let anchor = self.inputs.stack.anchor_location(Some(self.node(parent).layer_id())); - // A reference/payload authored inside a `.usdz` package would - // need to open a sibling layer within the archive, which is not - // yet supported (the eager collector bailed the same way for a - // usdz inner layer's sublayers). Report it explicitly rather - // than letting the package-relative target fail as a generic - // unresolved layer. - if anchor.as_ref().is_some_and(|resolved| resolved.extension() == "usdz") { - self.errors.push(Error::UnsupportedUsdzReference { - asset_path: asset_path.to_string(), - arc, - introduced_by: self.introducing_layer(parent), - site_path: parent_path, - }); - return Ok(()); - } // A muted target contributes nothing and is never opened: drop // the arc (it then composes as if absent) and record the muted // reference as a diagnostic. Checked before the load demand so a diff --git a/src/sdf/layer_registry.rs b/src/sdf/layer_registry.rs index dfbdabef..94ceb3b6 100644 --- a/src/sdf/layer_registry.rs +++ b/src/sdf/layer_registry.rs @@ -19,7 +19,7 @@ use std::collections::{HashMap, HashSet}; use std::path::PathBuf; -use anyhow::{bail, Context, Result}; +use anyhow::{Context, Result}; use crate::ar; use crate::sdf::{self, expr}; @@ -300,7 +300,6 @@ impl LayerRegistry { let mut expr_vars = expr::read_expression_variables(data.as_ref())?.into_owned(); expr::compose_over(&mut expr_vars, ancestor_expr_vars); - let is_usdz = resolved.extension() == "usdz"; let sub_paths = Self::sublayer_paths(data.as_ref()); // Emit this layer ahead of its sublayers so the collected stack is @@ -312,16 +311,10 @@ impl LayerRegistry { layers.push(sdf::Layer::new(identifier.clone(), data)); } - // A layer inside a `.usdz` package cannot reach a sibling layer within the - // archive (not yet supported), so a usdz layer declaring any sublayers - // fails the open. - if is_usdz && !sub_paths.is_empty() { - bail!( - "cross-file references within USDZ archives are not yet supported: {}", - resolved - ); - } - + // Sublayers (and references) reached from inside a `.usdz` resolve + // in-package: a package root is anchored to its first layer, so this + // layer's `resolved` is already package-relative and its sublayer paths + // anchor against it the same way any other layer's do. for sub_path in sub_paths { // Evaluate the (possibly expression-valued) sublayer path. An // unevaluable expression drops only this sublayer — like an unresolved diff --git a/src/usdz/mod.rs b/src/usdz/mod.rs index 1a9bbbed..583da5b5 100644 --- a/src/usdz/mod.rs +++ b/src/usdz/mod.rs @@ -46,7 +46,20 @@ impl sdf::FileFormat for UsdzFileFormat { } fn read(&self, resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Result { - let bytes = resolver.open_asset(resolved)?.read_all()?; + // `resolved` may be the package file (`pkg.usdz`) or a package-relative + // path (`pkg.usdz[inner]`) once a package root has been anchored. This + // format always reads the archive's first layer, so open the package + // itself, not an inner entry. + let s = resolved.to_string(); + let package = if ar::is_package_relative_path(&s) { + match ar::split_package_relative_path_outer(&s) { + Some((pkg, _)) => ar::ResolvedPath::new(pkg), + None => resolved.clone(), + } + } else { + resolved.clone() + }; + let bytes = resolver.open_asset(&package)?.read_all()?; let mut archive = Archive::from_reader(Cursor::new(bytes)).context("failed to open USDZ archive")?; archive .read_first_layer() @@ -62,3 +75,38 @@ impl sdf::FileFormat for UsdzFileFormat { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::usd::{Stage, TimeCode}; + + /// A `.usdz` whose root layer references another layer *inside the same + /// archive*. The reference (`@./inner.usda@`) must resolve in-package — + /// not against the host filesystem — for the inner opinion to compose onto + /// the root prim. Exercises the full package-relative resolution path + /// (bare-package anchoring + `create_identifier` + inner-layer read). + #[test] + fn resolves_packaged_reference() -> Result<()> { + let root = + b"#usda 1.0\n(defaultPrim = \"World\")\ndef \"World\" (prepend references = @./inner.usda@) {}\n"; + let inner = b"#usda 1.0\ndef \"Inner\" { custom int probe = 42 }\n"; + + let dir = tempfile::tempdir()?; + let path = dir.path().join("pkg.usdz"); + let mut writer = ArchiveWriter::create(&path)?; + writer.add_layer("root.usda", root)?; // first entry is the root layer + writer.add_layer("inner.usda", inner)?; + writer.finish()?; + + let stage = Stage::open(path.to_str().unwrap())?; + assert_eq!( + stage + .attribute("/World.probe") + .get_at::(TimeCode::new(0.0))?, + Some(sdf::Value::Int(42)), + "reference to a layer inside the package should compose" + ); + Ok(()) + } +} diff --git a/tests/stage.rs b/tests/stage.rs index 9f40a8fe..d4ec3067 100644 --- a/tests/stage.rs +++ b/tests/stage.rs @@ -367,37 +367,44 @@ fn cross_ref_expr_sublayer() -> Result<()> { } /// A reference authored inside a `.usdz` package targets a sibling layer in the -/// archive, which is unsupported: composition reports the explicit -/// [`UnsupportedUsdzReference`](pcp::Error::UnsupportedUsdzReference) error and -/// drops the arc rather than failing the package layer as a generic unresolved -/// target. +/// same archive: it resolves package-relative (not against the host +/// filesystem), so the sibling's opinion composes onto the prim and no +/// composition error is reported. #[test] -fn lazy_ref_inside_usdz_unsupported() -> Result<()> { +fn lazy_ref_inside_usdz_resolves() -> Result<()> { let dir = tempfile::tempdir()?; let root = dir.path().join("root.usda"); let package = dir.path().join("package.usdz"); std::fs::write(&root, "#usda 1.0\ndef \"P\" (\n references = @package.usdz@\n) {}\n")?; - // The usdz's inner layer authors a reference to another layer. + // The package's first (root) layer references a sibling layer inside the + // same archive, which authors an opinion on the prim. { let mut writer = ArchiveWriter::create(&package)?; writer.add_layer( "scene.usda", b"#usda 1.0\n(\n defaultPrim = \"P\"\n)\ndef \"P\" (\n references = @other.usda@\n) {}\n", )?; + writer.add_layer( + "other.usda", + b"#usda 1.0\n(\n defaultPrim = \"P\"\n)\ndef \"P\" {\n custom int probe = 7\n}\n", + )?; writer.finish()?; } let stage = Stage::open(root.to_str().unwrap())?; - assert!(stage.prim("/P").is_valid()?, "/P still composes from the package layer"); + assert!(stage.prim("/P").is_valid()?, "/P composes from the package"); assert!( - stage.composition_errors().iter().any(|error| matches!( - error, - pcp::Error::UnsupportedUsdzReference { asset_path, introduced_by, .. } - if asset_path.ends_with("other.usda") && introduced_by.ends_with("package.usdz") - )), - "expected UnsupportedUsdzReference, got {:?}", + stage.composition_errors().is_empty(), + "the in-package reference should resolve cleanly, got {:?}", stage.composition_errors() ); + assert_eq!( + stage + .attribute("/P.probe") + .get_at::(usd::TimeCode::new(0.0))?, + Some(sdf::Value::Int(7)), + "the sibling layer's opinion composes through the in-package reference" + ); Ok(()) } From 32944325470067238a47b4b49cafae75f94efdc4 Mon Sep 17 00:00:00 2001 From: Trim Bresilla Date: Sun, 28 Jun 2026 23:33:50 +0200 Subject: [PATCH 3/4] fix: copilot feedback corrections --- src/ar.rs | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/ar.rs b/src/ar.rs index 86677e67..4c4ea6fc 100644 --- a/src/ar.rs +++ b/src/ar.rs @@ -296,8 +296,14 @@ impl Resolver for DefaultResolver { return String::new(); } - // An already package-relative asset path is its own identifier. + // An already package-relative asset path (`pkg.usdz[inner]`): anchor its + // outer package path like any other path (so a relative `pkg.usdz` is + // resolved against the anchor), then reattach the inner packaged path. if is_package_relative_path(asset_path) { + if let Some((package, inner)) = split_package_relative_path_outer(asset_path) { + let anchored = self.create_identifier(&package, anchor); + return join_package_relative_path(&anchored, &inner); + } return asset_path.to_string(); } @@ -352,8 +358,8 @@ impl Resolver for DefaultResolver { if is_package_relative_path(asset_path) { let (package, inner) = split_package_relative_path_outer(asset_path)?; let resolved_package = self.resolve_with_search_paths(&package)?; - let package_str = resolved_package.to_str().unwrap_or_default(); - return Some(ResolvedPath::new(join_package_relative_path(package_str, &inner))); + let package_str = resolved_package.to_string_lossy(); + return Some(ResolvedPath::new(join_package_relative_path(&package_str, &inner))); } let resolved = self.resolve_with_search_paths(asset_path)?; @@ -375,8 +381,8 @@ impl Resolver for DefaultResolver { .and_then(|bytes| crate::usdz::Archive::from_reader(std::io::Cursor::new(bytes)).ok()) .and_then(|archive| archive.first_layer_name()) { - let package_str = resolved.to_str().unwrap_or_default(); - return Some(ResolvedPath::new(join_package_relative_path(package_str, &first))); + let package_str = resolved.to_string_lossy(); + return Some(ResolvedPath::new(join_package_relative_path(&package_str, &first))); } } @@ -778,6 +784,19 @@ mod tests { ); } + #[test] + fn resolver_create_identifier_package_relative_anchored() { + let resolver = DefaultResolver::new(); + let anchor = ResolvedPath::new(PathBuf::from("/scene/root.usda")); + // A package-relative target anchors its outer package path to the + // anchor's directory (not returned verbatim), then reattaches the inner + // packaged path — the same identifier as anchoring the bare package. + assert_eq!( + resolver.create_identifier("model.usdz[geom.usd]", Some(&anchor)), + join_package_relative_path(&resolver.create_identifier("model.usdz", Some(&anchor)), "geom.usd"), + ); + } + #[test] fn resolver_open_asset() { let manifest = std::env::var("CARGO_MANIFEST_DIR").unwrap(); From 05e4cc6232f94bff8c5f9afddd4144238c0d9f76 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Mon, 29 Jun 2026 12:29:49 -0700 Subject: [PATCH 4/4] fix(usdz): anchor packaged layers by real path Give Layer a resolved real_path (C++ SdfLayer::GetRealPath) distinct from its identifier, and anchor every in-package reference, sublayer, and asset path against it. A package whose default layer lives in a sub-directory previously lost that prefix when anchoring relative paths; it now resolves in-package correctly. The bare-package -> default-layer decision moves out of the generic resolver into a FileFormat::resolve_layer hook, so resolve() stays package-agnostic (asset-value resolution and the resolvability probe get the package path itself) while usdz owns its own package rule. Package-internal joins now emit forward slashes and collapse '..' to match ZIP entry names on every platform, and a nested package-relative target nests well-formed instead of forming a double-bracket identifier. A package that cannot be opened, or lists no default layer, falls back to its bare path so read() surfaces the precise zip/parse error consistently rather than reporting a missing asset. Also record the architectural-correctness preference in CLAUDE.md. --- CLAUDE.md | 9 ++ src/ar.rs | 192 ++++++++++++++++++++++++++++---------- src/pcp/compose_site.rs | 2 +- src/pcp/layer_graph.rs | 25 +++-- src/sdf/file_format.rs | 15 +++ src/sdf/layer.rs | 44 ++++++++- src/sdf/layer_registry.rs | 62 ++++++++---- src/usdz/mod.rs | 53 +++++++---- src/usdz/reader.rs | 59 ++++++------ tests/stage.rs | 136 +++++++++++++++++++++++++++ 10 files changed, 475 insertions(+), 122 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 48f035cc..46a68af1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -90,6 +90,15 @@ When implementing a new feature from the spec: - Comprehensive test coverage (50% minimum) with grcov - Security auditing with cargo-deny - Pre-1.0: backward compatibility is not a constraint. Prefer the cleanest design and change or remove public APIs freely; don't keep deprecated shims, compatibility shims, or worse-but-compatible behavior. Update all call sites in the same change. +- Prefer an architecturally correct design over a quick, dirty, or bandaid fix. + Fix the root cause at the layer that owns the concept, not the symptom at the + call site. A special case layered onto shared infrastructure (an `if this one + format` branch, or a value re-derived from a string because the right field + was not carried) signals the fix is at the wrong altitude — generalize the + mechanism or carry the missing state instead. When a module mirrors C++ + OpenUSD, factor the concern the way the C++ model does (e.g. a layer's lexical + identifier vs. its resolved real path). Note genuinely deferred depth as a + `TODO` naming the missing generalization. ## Code Quality diff --git a/src/ar.rs b/src/ar.rs index 4c4ea6fc..273d76c7 100644 --- a/src/ar.rs +++ b/src/ar.rs @@ -60,21 +60,16 @@ impl ResolvedPath { /// format case-insensitively — `resolved.extension() == "usdz"` — the way the /// format registry's `find_by_extension` does, without juggling `OsStr`. pub(crate) fn extension(&self) -> String { - // Package-relative path (`pkg.usdz[inner/layer.usd]`): the format is the - // innermost packaged layer's extension, not the literal trailing chars - // (`.usd]`). Match the format on the inner path instead. + // For a package-relative path (`pkg.usdz[inner/layer.usd]`) the format is + // the innermost packaged layer's extension, not the literal trailing + // chars (`.usd]`), so inspect the inner path rather than `self.0`. let s = self.0.to_string_lossy(); - if is_package_relative_path(&s) { - if let Some((_, inner)) = split_package_relative_path_inner(&s) { - return Path::new(&inner) - .extension() - .and_then(|ext| ext.to_str()) - .map(str::to_ascii_lowercase) - .unwrap_or_default(); - } - } - self.0 - .extension() + let inner = is_package_relative_path(&s) + .then(|| split_package_relative_path_inner(&s)) + .flatten() + .map(|(_, inner)| inner); + let path = inner.as_deref().map(Path::new).unwrap_or(self.0.as_path()); + path.extension() .and_then(|ext| ext.to_str()) .map(str::to_ascii_lowercase) .unwrap_or_default() @@ -298,11 +293,12 @@ impl Resolver for DefaultResolver { // An already package-relative asset path (`pkg.usdz[inner]`): anchor its // outer package path like any other path (so a relative `pkg.usdz` is - // resolved against the anchor), then reattach the inner packaged path. + // resolved against the anchor), then nest the inner packaged path back + // inside the anchored package. if is_package_relative_path(asset_path) { if let Some((package, inner)) = split_package_relative_path_outer(asset_path) { let anchored = self.create_identifier(&package, anchor); - return join_package_relative_path(&anchored, &inner); + return nest_packaged_path(&anchored, &inner); } return asset_path.to_string(); } @@ -322,16 +318,16 @@ impl Resolver for DefaultResolver { // directory → `pkg.usdz[inner/asset.usd]`. if is_package_relative_path(&anchor_str) { if let Some((package, inner)) = split_package_relative_path_inner(&anchor_str) { - let inner_dir = Path::new(&inner).parent().unwrap_or_else(|| Path::new("")); - let joined = lexically_normalize(&inner_dir.join(path)); - return join_package_relative_path(&package, &joined.to_string_lossy()); + let inner_dir = inner.rsplit_once('/').map(|(dir, _)| dir).unwrap_or(""); + let joined = join_packaged_path(inner_dir, asset_path); + return nest_packaged_path(&package, &joined); } } // Anchor IS a package file (`foo.usdz`): a relative reference from // the package's root layer is package-relative → `foo.usdz[asset]`. if anchor_str.to_ascii_lowercase().ends_with(".usdz") { - let joined = lexically_normalize(path); - return join_package_relative_path(&anchor_str, &joined.to_string_lossy()); + let joined = join_packaged_path("", asset_path); + return join_package_relative_path(&anchor_str, &joined); } if let Some(dir) = anchor.parent() { return canonical_identifier(dir.join(path)); @@ -352,41 +348,22 @@ impl Resolver for DefaultResolver { return None; } - // Handle package-relative paths. The package itself is resolved as a - // plain file (not re-anchored to its default layer — that would nest), - // then the inner packaged path is reattached. + // A package-relative path (`pkg.usdz[inner]`) resolves its outer package + // as a plain file, then reattaches the inner packaged path. The inner + // entry must actually exist in the archive: a path to a missing entry is + // unresolved, not merely unreadable, so the caller reports it as such + // rather than as a malformed layer. if is_package_relative_path(asset_path) { let (package, inner) = split_package_relative_path_outer(asset_path)?; let resolved_package = self.resolve_with_search_paths(&package)?; + if !package_contains(&resolved_package, &inner) { + return None; + } let package_str = resolved_package.to_string_lossy(); return Some(ResolvedPath::new(join_package_relative_path(&package_str, &inner))); } - let resolved = self.resolve_with_search_paths(asset_path)?; - - // A bare package (`.usdz`) resolves to its default — first — packaged - // layer (`pkg.usdz[first.usd]`), so the package participates in normal - // composition and references inside it resolve in-package. Mirrors C++ - // Ar package resolution; applies uniformly to a root, sublayer, or - // referenced `.usdz`. - if resolved.extension() == "usdz" { - // Read the package directory through the resolver's own asset seam - // (`open_asset`) rather than direct filesystem access, so a future - // host-provided byte source (issue #105's "get me the bytes for - // this layer") flows through unchanged — no async introduced here. - if let Some(first) = self - .open_asset(&resolved) - .ok() - .and_then(|mut asset| asset.read_all().ok()) - .and_then(|bytes| crate::usdz::Archive::from_reader(std::io::Cursor::new(bytes)).ok()) - .and_then(|archive| archive.first_layer_name()) - { - let package_str = resolved.to_string_lossy(); - return Some(ResolvedPath::new(join_package_relative_path(&package_str, &first))); - } - } - - Some(resolved) + self.resolve_with_search_paths(asset_path) } fn resolve_for_new_asset(&self, asset_path: &str) -> Option { @@ -430,8 +407,7 @@ impl Resolver for DefaultResolver { ) })?; - let package_file = fs::File::open(&package)?; - let mut archive = zip::ZipArchive::new(package_file).map_err(io::Error::other)?; + let mut archive = open_package_archive(Path::new(&package))?; let mut entry = archive.by_name(&inner).map_err(io::Error::other)?; let mut buffer = Vec::new(); @@ -486,6 +462,42 @@ fn normalize_search_path(path: PathBuf) -> PathBuf { lexically_normalize(&path) } +/// Opens `package` as a ZIP archive, reading only its central directory. +/// +/// Shared by [`DefaultResolver::open_asset`], which then extracts an entry, and +/// [`package_contains`], which only checks an entry's presence. +fn open_package_archive(package: &Path) -> io::Result> { + let file = fs::File::open(package)?; + zip::ZipArchive::new(file).map_err(io::Error::other) +} + +/// Whether `inner` should be treated as present in the package at `package`. +/// +/// Reading only the archive's central directory — not its entry data — a +/// readable archive that genuinely lacks a flat entry reports it absent, so +/// [`DefaultResolver::resolve`] reports a missing inner layer as unresolved +/// rather than as a layer that exists but cannot be read. Two cases are +/// deliberately treated as present so the eventual open surfaces the accurate +/// error instead of a misleading "missing asset": a `package` that cannot be +/// opened right now (a transient IO error or a corrupt archive — distinct from +/// a genuinely absent entry), and a nested packaged path +/// (`inner.usdz[deep.usd]`, which is not a flat entry name and whose support is +/// decided when the inner package opens). +/// +/// TODO(perf): opens the package's central directory on every package-relative +/// resolve (per in-package arc, re-run each composition pass). Cache parsed +/// central directories keyed by resolved package path so repeated probes and +/// the eventual load share one parse. +fn package_contains(package: &Path, inner: &str) -> bool { + if is_package_relative_path(inner) { + return true; + } + match open_package_archive(package) { + Ok(mut archive) => archive.by_name(inner).is_ok(), + Err(_) => true, + } +} + // --------------------------------------------------------------------------- // Package-relative path utilities // --------------------------------------------------------------------------- @@ -575,6 +587,44 @@ pub fn join_package_relative_path(package_path: &str, packaged_path: &str) -> St format!("{}[{}]", package_path, packaged_path) } +/// Joins a package-internal directory with a relative reference authored inside +/// it, producing the entry name a packaged layer is stored under. +/// +/// Package entry names are always `/`-separated and carry no filesystem +/// semantics, so unlike [`lexically_normalize`] this collapses `..` as well as +/// `.` and always emits forward slashes — the spelling +/// [`zip::ZipArchive::by_name`] matches against. `dir` is the directory of the +/// anchoring inner layer (empty for a layer at the package root); `rel` is the +/// relative path it references. A `..` that would climb above the package root +/// is dropped, since there is nothing above it inside the archive. +fn join_packaged_path(dir: &str, rel: &str) -> String { + let mut components: Vec<&str> = dir.split('/').filter(|c| !c.is_empty()).collect(); + for part in rel.split(['/', '\\']) { + match part { + "" | "." => {} + ".." => { + components.pop(); + } + other => components.push(other), + } + } + components.join("/") +} + +/// Attaches `leaf` as a packaged layer inside the deepest package of `base`. +/// +/// For a plain `base` this is [`join_package_relative_path`]. When `base` is +/// itself package-relative — its outer package holds a nested package — the +/// `leaf` is nested into the innermost bracket so the result stays well-formed +/// (`pkg[inner[leaf]]`) rather than gaining a stray second bracket pair +/// (`pkg[inner][leaf]`), which no split or resolve step can interpret. +fn nest_packaged_path(base: &str, leaf: &str) -> String { + match split_package_relative_path_outer(base) { + Some((package, inner)) => join_package_relative_path(&package, &nest_packaged_path(&inner, leaf)), + None => join_package_relative_path(base, leaf), + } +} + #[cfg(test)] mod tests { use super::*; @@ -797,6 +847,46 @@ mod tests { ); } + #[test] + fn packaged_path_join_forward_slash() { + // Package entry names are `/`-separated regardless of host platform, and + // `.`/`..` collapse since there is no filesystem inside the archive. + assert_eq!(join_packaged_path("geom", "mesh.usd"), "geom/mesh.usd"); + assert_eq!(join_packaged_path("geom", "./mesh.usd"), "geom/mesh.usd"); + assert_eq!(join_packaged_path("geom", "../tex/foo.usda"), "tex/foo.usda"); + assert_eq!(join_packaged_path("", "geom/mesh.usd"), "geom/mesh.usd"); + // A back-slash spelling is normalized to forward slashes. + assert_eq!(join_packaged_path("geom", r"sub\mesh.usd"), "geom/sub/mesh.usd"); + } + + #[test] + fn in_package_anchor_crosses_dirs() { + let resolver = DefaultResolver::new(); + // A reference authored in a sub-directory layer that climbs out of it + // resolves to a forward-slash, `..`-collapsed entry name — the spelling + // the archive is keyed by — not a host path with separators or `..`. + let anchor = ResolvedPath::new(PathBuf::from("pkg.usdz[geom/scene.usda]")); + assert_eq!( + resolver.create_identifier("../tex/foo.usda", Some(&anchor)), + "pkg.usdz[tex/foo.usda]", + ); + } + + #[test] + fn nest_packaged_well_formed() { + // A leaf nests into the innermost package rather than appending a stray + // second bracket pair. + assert_eq!(nest_packaged_path("pkg.usdz", "geom.usd"), "pkg.usdz[geom.usd]"); + assert_eq!( + nest_packaged_path("pkg.usdz[model.usdz]", "geom.usd"), + "pkg.usdz[model.usdz[geom.usd]]", + ); + assert_eq!( + nest_packaged_path("a.usdz[b.usdz[c.usdz]]", "d.usd"), + "a.usdz[b.usdz[c.usdz[d.usd]]]", + ); + } + #[test] fn resolver_open_asset() { let manifest = std::env::var("CARGO_MANIFEST_DIR").unwrap(); diff --git a/src/pcp/compose_site.rs b/src/pcp/compose_site.rs index 9dd0c7bb..ac39a00e 100644 --- a/src/pcp/compose_site.rs +++ b/src/pcp/compose_site.rs @@ -267,7 +267,7 @@ fn anchor_asset_path(asset_path: &mut String, authoring_layer: &sdf::Layer, regi if asset_path.is_empty() { return; } - *asset_path = registry.create_identifier_anchored(asset_path, &authoring_layer.identifier); + *asset_path = registry.create_identifier_anchored(asset_path, authoring_layer.real_path()); } /// The retiming scale a reference or payload arc folds into its layer offset diff --git a/src/pcp/layer_graph.rs b/src/pcp/layer_graph.rs index 6ce6a2d1..76af21e5 100644 --- a/src/pcp/layer_graph.rs +++ b/src/pcp/layer_graph.rs @@ -591,7 +591,7 @@ impl LayerGraph { } let anchored = self .registry - .create_identifier_anchored(&sub_path, self.identifier(parent)); + .create_identifier_anchored(&sub_path, self.real_path(parent)); let sub_id = self.anchored_or_bare(&anchored, &sub_path); cache.entry(parent).or_default().insert(sub_path.into_owned(), anchored); sub_id @@ -801,6 +801,15 @@ impl LayerGraph { self.nodes[&id].layer.identifier() } + /// The resolved physical location of the layer with the given id — the + /// anchor for the relative asset paths it authors (C++ + /// `SdfLayer::GetRealPath`). Equals the identifier except for a package, + /// whose real path is its package-relative default layer. Panics if + /// unknown. + pub(crate) fn real_path(&self, id: LayerId) -> &str { + self.nodes[&id].layer.real_path() + } + /// Whether a layer with this id is present. pub(crate) fn contains(&self, id: LayerId) -> bool { self.nodes.contains_key(&id) @@ -811,12 +820,14 @@ impl LayerGraph { &self.registry } - /// Resolves the location of the layer `anchor`, used to anchor the relative - /// asset paths authored there. Returns `None` when there is no anchor layer - /// or it cannot itself be resolved. Resolve a layer once and reuse the - /// result to anchor every asset path it authors. + /// The location that anchors the relative asset paths authored in the layer + /// `anchor`: its resolved [`real_path`](Self::real_path), recorded when the + /// loader opened it. Returns `None` when there is no anchor layer or it is + /// anonymous (no resolvable location, C++ `SdfLayer::GetRealPath` is + /// empty). pub(crate) fn anchor_location(&self, anchor: Option) -> Option { - anchor.and_then(|layer| self.registry.resolve(self.identifier(layer))) + let layer = &self.nodes[&anchor?].layer; + (!layer.is_anonymous()).then(|| ResolvedPath::new(layer.real_path())) } /// Records that the layer at `asset_path` resolved but could not be read or @@ -1431,7 +1442,7 @@ impl LayerGraph { let asset_path = without_dot_segments(asset_path); let anchored = self .registry - .create_identifier_anchored(&asset_path, self.identifier(anchor)); + .create_identifier_anchored(&asset_path, self.real_path(anchor)); self.anchored_or_bare(&anchored, &asset_path) } diff --git a/src/sdf/file_format.rs b/src/sdf/file_format.rs index bd559451..aa664707 100644 --- a/src/sdf/file_format.rs +++ b/src/sdf/file_format.rs @@ -87,6 +87,21 @@ pub trait FileFormat: Sync { /// sibling assets) through `resolver`. fn read(&self, resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Result; + /// Resolves the real path of the layer to open at `resolved` — the location + /// it physically loads from and anchors its relative asset paths against + /// (C++ `SdfLayer::GetRealPath`) — or `None` if this format cannot read it + /// there. + /// + /// The default is the identity: an ordinary layer loads from the location + /// it resolved to. A package format overrides this to select the package's + /// default layer (`pkg.usdz` → `pkg.usdz[root.usd]`), so the package + /// composes as an ordinary layer stack and the paths authored inside it + /// anchor in-package; a package it cannot open returns `None`. Opening the + /// asset goes through `resolver` so a host-provided byte source is honored. + fn resolve_layer(&self, _resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Option { + Some(resolved.clone()) + } + /// Whether this format can read an asset whose leading bytes are `prefix` /// (C++ `SdfFileFormat::CanRead`). Used to disambiguate an extension claimed /// by more than one format — binary vs text `.usd` — by content. The default diff --git a/src/sdf/layer.rs b/src/sdf/layer.rs index 842ce513..1f6f3eef 100644 --- a/src/sdf/layer.rs +++ b/src/sdf/layer.rs @@ -46,6 +46,8 @@ use std::sync::atomic::{AtomicU64, Ordering}; use anyhow::{Context, Result}; +use crate::ar; + use super::schema::FieldKey; use super::{ sink, AbstractData, AttributeSpecMut, AttributeSpecRef, ChangeList, CowData, Data, DataError, LayerData, Patch, @@ -69,6 +71,16 @@ static ANONYMOUS_COUNTER: AtomicU64 = AtomicU64::new(0); pub struct Layer { /// Resolved, canonical identifier for this layer. pub identifier: String, + /// The layer's resolved physical location when it differs from + /// [`identifier`](Self::identifier) — the anchor for the relative asset + /// paths it authors (C++ `SdfLayer::GetRealPath`). `None` (the common case) + /// means it equals the identifier; a package (`.usdz`) opens under the bare + /// package identifier while its real path is the package-relative default + /// layer (`pkg.usdz[root.usd]`), so paths authored inside it anchor + /// in-package against the real path rather than the identifier. Read + /// through [`real_path`](Self::real_path), which falls back to the + /// identifier. + real_path: Option, /// The parsed scene description data, under a copy-on-write [`CowData`] /// staging overlay. Every authoring write stages in the overlay; committing an /// edit derives the composition record and drains the overlay into the backend. @@ -93,8 +105,30 @@ impl Layer { /// for blank in-memory layers, or open a stage (which loads layers from /// disk on demand) for loaded layers. pub(crate) fn new(identifier: impl Into, data: LayerData) -> Self { + Self::build(identifier.into(), None, data) + } + + /// Construct a loaded layer recording its resolved physical location. + /// + /// The loader passes the [`real_path`](Self::real_path) it resolved the + /// layer to: it equals `identifier` for an ordinary layer, but a package + /// opens under its bare identifier while its real path is the + /// package-relative default layer, so the paths it authors anchor + /// in-package. Only a real path that differs from the identifier is stored; + /// otherwise this is [`new`](Self::new). + pub(crate) fn new_resolved(identifier: impl Into, real_path: &ar::ResolvedPath, data: LayerData) -> Self { + let identifier = identifier.into(); + let real_path = real_path.to_string(); + let real_path = (real_path != identifier).then_some(real_path); + Self::build(identifier, real_path, data) + } + + /// Shared constructor backing [`new`](Self::new) and + /// [`new_resolved`](Self::new_resolved). + fn build(identifier: String, real_path: Option, data: LayerData) -> Self { Self { - identifier: identifier.into(), + identifier, + real_path, data: CowData::new(data), changes: ChangeList::new(), sinks: sink::Set::default(), @@ -237,6 +271,14 @@ impl Layer { pub fn identifier(&self) -> &str { &self.identifier } + + /// The layer's resolved physical location, the anchor for the relative + /// asset paths it authors (C++ `SdfLayer::GetRealPath`). Equals the + /// identifier except for a package, whose real path is its package-relative + /// default layer. + pub(crate) fn real_path(&self) -> &str { + self.real_path.as_deref().unwrap_or(&self.identifier) + } } impl Layer { diff --git a/src/sdf/layer_registry.rs b/src/sdf/layer_registry.rs index 94ceb3b6..9765f693 100644 --- a/src/sdf/layer_registry.rs +++ b/src/sdf/layer_registry.rs @@ -115,20 +115,20 @@ impl LayerRegistry { self.resolver.create_identifier(asset_path, anchor) } - /// Resolves `asset_path` against `anchor_identifier` — the identifier of the - /// layer it is authored in — yielding the canonical identifier of the - /// targeted layer. The convenience over [`create_identifier`](Self::create_identifier) - /// for the common case of anchoring against another layer's identifier - /// string rather than a pre-resolved [`ar::ResolvedPath`]. + /// Resolves `asset_path` against `anchor_location` — the resolved real path + /// of the layer it is authored in (its + /// [`real_path`](sdf::Layer::real_path), which for a package is the + /// package-relative default layer, not the bare package identifier) — + /// yielding the canonical identifier of the targeted layer. The convenience + /// over [`create_identifier`](Self::create_identifier) for the common case + /// of anchoring against another layer's location string rather than a + /// pre-resolved [`ar::ResolvedPath`]. /// /// TODO(perf): the resolver canonicalizes via the filesystem, so each call /// runs a `canonicalize`. Cache resolved identifiers per - /// `(anchor_identifier, asset_path)`. - pub(crate) fn create_identifier_anchored(&self, asset_path: &str, anchor_identifier: &str) -> String { - self.create_identifier( - asset_path, - Some(&ar::ResolvedPath::new(PathBuf::from(anchor_identifier))), - ) + /// `(anchor_location, asset_path)`. + pub(crate) fn create_identifier_anchored(&self, asset_path: &str, anchor_location: &str) -> String { + self.create_identifier(asset_path, Some(&ar::ResolvedPath::new(PathBuf::from(anchor_location)))) } /// Resolves an asset identifier to a physical location, or `None` if it does @@ -137,6 +137,33 @@ impl LayerRegistry { self.resolver.resolve(identifier) } + /// Resolves the layer the composition graph should open at `identifier`. + /// + /// This is [`resolve`](Self::resolve) deferred to the selected format's + /// [`resolve_layer`](sdf::FileFormat::resolve_layer): a package + /// (`pkg.usdz`) resolves to its default — first — packaged layer + /// (`pkg.usdz[root.usd]`), so it composes as an ordinary layer stack and + /// the sublayers and references authored inside it anchor in-package, while + /// an ordinary format keeps the resolved location. Generic + /// [`resolve`](Self::resolve) stays package-agnostic for asset-value + /// resolution and the resolvability probe, which want the package path + /// itself rather than a layer inside it. A resolved location no registered + /// format claims passes through unchanged, so [`read`](Self::read) reports + /// the missing format. + /// + /// The format is chosen by extension alone — unlike [`read`](Self::read), + /// which content-sniffs the ambiguous `.usd`. That is sound because every + /// `.usd` claimant keeps the identity default, so which one is picked does + /// not change the real path; a future `.usd` format with a non-identity + /// `resolve_layer` would need the content sniff here too. + fn resolve_layer(&self, identifier: &str) -> Option { + let resolved = self.resolver.resolve(identifier)?; + match Self::find_by_extension(&resolved.extension()) { + Some(format) => format.resolve_layer(self.resolver.as_ref(), &resolved), + None => Some(resolved), + } + } + /// The resolver's [`identity`](ar::Resolver::identity) token — the /// configuration the stack's asset paths resolve under. pub(crate) fn identity(&self) -> String { @@ -162,7 +189,7 @@ impl LayerRegistry { /// `None` when it does not resolve. Used for value-clip and manifest layers, /// which compose outside the layer graph (spec 12.3.4). pub(crate) fn open(&self, identifier: &str) -> Result> { - match self.resolver.resolve(identifier) { + match self.resolve_layer(identifier) { Some(resolved) => self.read(&resolved).map(Some), None => Ok(None), } @@ -214,8 +241,7 @@ impl LayerRegistry { // // The root must resolve and read; both failures propagate. let resolved = self - .resolver - .resolve(&identifier) + .resolve_layer(&identifier) .with_context(|| format!("failed to resolve asset path: {asset_path}"))?; let data = self.read(&resolved)?; visited.insert(identifier.clone()); @@ -308,7 +334,7 @@ impl LayerRegistry { // reload pass an already-interned layer is re-walked (to reach a `${VAR}` // sublayer the new context now resolves) but not re-emitted. if !already_present(&identifier) { - layers.push(sdf::Layer::new(identifier.clone(), data)); + layers.push(sdf::Layer::new_resolved(identifier.clone(), &resolved, data)); } // Sublayers (and references) reached from inside a `.usdz` resolve @@ -341,7 +367,7 @@ impl LayerRegistry { continue; } // Resolve the sublayer; a missing one is routed to `on_error`. - let Some(sub_resolved) = self.resolver.resolve(&sub_id) else { + let Some(sub_resolved) = self.resolve_layer(&sub_id) else { on_error(Error::UnresolvedAsset { asset_path: sub_asset, referencing_layer: identifier.clone(), @@ -417,7 +443,7 @@ impl LayerRegistry { if identifier.is_empty() || visited.contains(&identifier) { return Ok(()); } - let Some(resolved) = self.resolver.resolve(&identifier) else { + let Some(resolved) = self.resolve_layer(&identifier) else { return Ok(()); }; visited.insert(identifier.clone()); @@ -428,7 +454,7 @@ impl LayerRegistry { let dep_asset = expr::evaluate_asset_path(&dep, &expr_vars)?; self.collect_with_arcs_in(&dep_asset, Some(&resolved), &expr_vars, layers, visited)?; } - layers.push(sdf::Layer::new(identifier, data)); + layers.push(sdf::Layer::new_resolved(identifier, &resolved, data)); Ok(()) } diff --git a/src/usdz/mod.rs b/src/usdz/mod.rs index 583da5b5..a40cbaf1 100644 --- a/src/usdz/mod.rs +++ b/src/usdz/mod.rs @@ -13,7 +13,7 @@ pub use writer::ArchiveWriter; use std::io::Cursor; -use anyhow::{Context, Result}; +use anyhow::{bail, Context, Result}; use crate::{ar, sdf, tf, usdc}; @@ -45,23 +45,44 @@ impl sdf::FileFormat for UsdzFileFormat { sdf::FileFormatCaps::READ | sdf::FileFormatCaps::WRITE } + fn resolve_layer(&self, resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Option { + // An already package-relative path (`pkg.usdz[inner]`, including a nested + // `pkg.usdz[inner.usdz]`) already names its entry; only a bare package is + // anchored to its default — first — packaged layer. + let package = resolved.to_string_lossy(); + if ar::is_package_relative_path(&package) { + return Some(resolved.clone()); + } + // A package that cannot be opened, or that lists no default layer, falls + // back to the bare package path so `read` surfaces the precise zip/parse + // error, rather than being demoted to an unresolved (missing) asset. + // + // TODO(perf): `from_asset` slurps the whole package into memory only to + // list its central directory (`first_layer_name`), and `read` then reads + // it again to extract the anchored layer. The resolver's asset is `Seek`, + // so a `ZipArchive` could read just the central directory here (as + // `ar::open_package_archive` does off a `File`); carry that opened archive + // through so a bare-package open touches the file once. + match Archive::from_asset(resolver, resolved) + .ok() + .and_then(|a| a.first_layer_name()) + { + Some(first) => Some(ar::ResolvedPath::new(ar::join_package_relative_path(&package, &first))), + None => Some(resolved.clone()), + } + } + fn read(&self, resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Result { - // `resolved` may be the package file (`pkg.usdz`) or a package-relative - // path (`pkg.usdz[inner]`) once a package root has been anchored. This - // format always reads the archive's first layer, so open the package - // itself, not an inner entry. + // A package-relative path reaches this format only when its named entry is + // itself a package (an ordinary inner layer dispatches to its own format), + // so it is a nested package — unsupported. Reported before opening the + // outer archive, since the whole-package read would only be discarded. let s = resolved.to_string(); - let package = if ar::is_package_relative_path(&s) { - match ar::split_package_relative_path_outer(&s) { - Some((pkg, _)) => ar::ResolvedPath::new(pkg), - None => resolved.clone(), - } - } else { - resolved.clone() - }; - let bytes = resolver.open_asset(&package)?.read_all()?; - let mut archive = Archive::from_reader(Cursor::new(bytes)).context("failed to open USDZ archive")?; - archive + if let Some((_, inner)) = ar::split_package_relative_path_outer(&s) { + bail!("Nested USDZ files are not yet supported: '{inner}'"); + } + // A bare package has no named entry, so read its first (default) layer. + Archive::from_asset(resolver, resolved)? .read_first_layer() .context("failed to read first layer from USDZ archive") } diff --git a/src/usdz/reader.rs b/src/usdz/reader.rs index e2151ac1..c3c8447a 100644 --- a/src/usdz/reader.rs +++ b/src/usdz/reader.rs @@ -9,7 +9,7 @@ use std::{ use anyhow::{bail, Context, Result}; use zip::ZipArchive; -use crate::{sdf, usda, usdc}; +use crate::{ar, sdf, usda, usdc}; /// USDZ archive reader. /// @@ -33,6 +33,15 @@ impl Archive { } } +impl Archive>> { + /// Reads the entire package at `resolved` through the resolver's asset seam + /// and opens it as an archive, so a host-provided byte source is honored. + pub fn from_asset(resolver: &dyn ar::Resolver, resolved: &ar::ResolvedPath) -> Result { + let bytes = resolver.open_asset(resolved)?.read_all()?; + Archive::from_reader(Cursor::new(bytes)).context("failed to open USDZ archive") + } +} + impl Archive { /// Creates an archive from any `Read + Seek` source. /// @@ -74,40 +83,34 @@ impl Archive { file.read_to_end(&mut buffer) .with_context(|| format!("Failed to read file '{}' from archive", file_path))?; - if file_path.ends_with(".usdc") { - let cursor = Cursor::new(buffer); - let data = usdc::CrateData::open(cursor, true) + if file_path.ends_with(".usdz") { + // TODO: Implement nested USDZ files support. + bail!("Nested USDZ files are not yet supported: '{}'", file_path); + } + + // The named extension decides crate vs text; a format-agnostic `.usd` + // (or any other name) falls back to the crate magic, mirroring USD's + // content-based format detection. Per the USDZ spec the root layer may be + // `.usd`, and Pixar's reference assets (e.g. Kitchen_set.usdz) ship it + // that way. + let is_crate = if file_path.ends_with(".usdc") { + true + } else if file_path.ends_with(".usda") { + false + } else { + buffer.starts_with(usdc::MAGIC) + }; + + if is_crate { + let data = usdc::CrateData::open(Cursor::new(buffer), true) .with_context(|| format!("Failed to parse USDC data from '{}'", file_path))?; Ok(Box::new(data)) - } else if file_path.ends_with(".usda") { + } else { let content = String::from_utf8(buffer).with_context(|| format!("File '{}' is not valid UTF-8", file_path))?; - let data = usda::parse(&content).with_context(|| format!("Failed to parse USDA data from '{}'", file_path))?; - Ok(Box::new(data)) - } else if file_path.ends_with(".usdz") { - // TODO: Implement nested USDZ files support. - bail!("Nested USDZ files are not yet supported: '{}'", file_path) - } else { - // Format-agnostic `.usd` (or any other extension): resolve by - // content, mirroring USD's content-based format detection. A USDC - // crate file begins with the magic `PXR-USDC`; otherwise parse as - // USDA text. Per the USDZ spec the root layer may be `.usd`, and - // Pixar's reference assets (e.g. Kitchen_set.usdz) ship it that way. - if buffer.starts_with(usdc::MAGIC) { - let cursor = Cursor::new(buffer); - let data = usdc::CrateData::open(cursor, true) - .with_context(|| format!("Failed to parse USDC data from '{}'", file_path))?; - Ok(Box::new(data)) - } else { - let content = - String::from_utf8(buffer).with_context(|| format!("File '{}' is not valid UTF-8", file_path))?; - let data = - usda::parse(&content).with_context(|| format!("Failed to parse USDA data from '{}'", file_path))?; - Ok(Box::new(data)) - } } } } diff --git a/tests/stage.rs b/tests/stage.rs index d4ec3067..d243340c 100644 --- a/tests/stage.rs +++ b/tests/stage.rs @@ -408,6 +408,142 @@ fn lazy_ref_inside_usdz_resolves() -> Result<()> { Ok(()) } +/// A reference authored inside a `.usdz` package targets a sibling layer that +/// is not present in the archive: the missing entry is unresolved (not merely +/// unreadable), so composition reports +/// [`UnresolvedLayer`](pcp::Error::UnresolvedLayer) and the rest of the prim +/// still composes. +#[test] +fn lazy_ref_inside_usdz_missing() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + let package = dir.path().join("package.usdz"); + std::fs::write(&root, "#usda 1.0\ndef \"P\" (\n references = @package.usdz@\n) {}\n")?; + // The package's first layer references a sibling that is never added. + { + let mut writer = ArchiveWriter::create(&package)?; + writer.add_layer( + "scene.usda", + b"#usda 1.0\n(\n defaultPrim = \"P\"\n)\ndef \"P\" (\n references = @other.usda@\n) {}\n", + )?; + writer.finish()?; + } + + let stage = Stage::open(root.to_str().unwrap())?; + assert!(stage.prim("/P").is_valid()?, "/P still composes from the package layer"); + assert!( + stage.composition_errors().iter().any(|error| matches!( + error, + pcp::Error::UnresolvedLayer { asset_path, .. } if asset_path.ends_with("other.usda]") + )), + "expected UnresolvedLayer for the missing in-package target, got {:?}", + stage.composition_errors() + ); + Ok(()) +} + +/// A reference to a present-but-empty `.usdz` (no packaged USD layer) reports a +/// [`MalformedLayer`](pcp::Error::MalformedLayer) carrying the real reason — +/// the package resolved but could not be read — rather than being silently +/// dropped as a missing asset or surfacing a "failed to resolve" diagnostic. +#[test] +fn lazy_ref_empty_usdz_malformed() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + let package = dir.path().join("empty.usdz"); + std::fs::write(&root, "#usda 1.0\ndef \"P\" (\n references = @empty.usdz@\n) {}\n")?; + // A valid ZIP archive with no packaged USD layer. + ArchiveWriter::create(&package)?.finish()?; + + let stage = Stage::open(root.to_str().unwrap())?; + assert!(stage.prim("/P").is_valid()?, "/P still composes from its own opinion"); + assert!( + stage.composition_errors().iter().any(|error| matches!( + error, + pcp::Error::MalformedLayer { asset_path, reason, .. } + if asset_path.ends_with("empty.usdz") && reason.contains("USDZ archive") + )), + "expected MalformedLayer with the package read reason, got {:?}", + stage.composition_errors() + ); + Ok(()) +} + +/// A `.usdz` whose default (first) layer sits in a sub-directory references a +/// sibling by a relative path. The reference must anchor against the package's +/// real path (the package-relative default layer, +/// `pkg.usdz[Scenes/root.usda]`), +/// not the bare package identifier, so the sibling resolves at +/// `pkg.usdz[Scenes/other.usda]` and its opinion composes. +#[test] +fn usdz_subdir_first_layer_anchors() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + let package = dir.path().join("package.usdz"); + std::fs::write(&root, "#usda 1.0\ndef \"P\" (\n references = @package.usdz@\n) {}\n")?; + // Both packaged layers live under `Scenes/`; the first is the default layer. + { + let mut writer = ArchiveWriter::create(&package)?; + writer.add_layer( + "Scenes/root.usda", + b"#usda 1.0\n(\n defaultPrim = \"P\"\n)\ndef \"P\" (\n references = @other.usda@\n) {}\n", + )?; + writer.add_layer( + "Scenes/other.usda", + b"#usda 1.0\n(\n defaultPrim = \"P\"\n)\ndef \"P\" {\n custom int probe = 9\n}\n", + )?; + writer.finish()?; + } + + let stage = Stage::open(root.to_str().unwrap())?; + assert!( + stage.composition_errors().is_empty(), + "the sub-directory in-package reference should resolve cleanly, got {:?}", + stage.composition_errors() + ); + assert_eq!( + stage + .attribute("/P.probe") + .get_at::(usd::TimeCode::new(0.0))?, + Some(sdf::Value::Int(9)), + "the sibling under Scenes/ composes through the in-package reference" + ); + Ok(()) +} + +/// An `asset`-valued attribute pointing at a `.usdz` package resolves to the +/// package path itself, not a path anchored into the package's first layer: +/// the asset is the package, and consumers key on the package path. +#[test] +fn asset_value_usdz_is_package_path() -> Result<()> { + let dir = tempfile::tempdir()?; + let root = dir.path().join("root.usda"); + let package = dir.path().join("model.usdz"); + std::fs::write(&root, "#usda 1.0\ndef \"P\" {\n custom asset a = @model.usdz@\n}\n")?; + { + let mut writer = ArchiveWriter::create(&package)?; + writer.add_layer("root.usda", b"#usda 1.0\ndef \"M\" {}\n")?; + writer.finish()?; + } + + let stage = Stage::open(root.to_str().unwrap())?; + let value = stage + .attribute("/P.a") + .get_at::(usd::TimeCode::new(0.0))? + .expect("asset value resolves"); + let asset = value.try_as_asset_path().expect("attribute is asset-typed"); + let resolved = asset.resolved_path().expect("asset path is resolved"); + assert!( + resolved.ends_with("model.usdz"), + "asset value should resolve to the bare package path, got {resolved:?}", + ); + assert!( + !resolved.contains('['), + "asset value must not be anchored into the package, got {resolved:?}", + ); + Ok(()) +} + /// A reference target's present-but-corrupt sublayer is dropped on its own — /// reported [`MalformedSublayer`](pcp::Error::MalformedSublayer) — while the /// target itself still composes (its own opinion resolves). The bad sublayer