Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Dependency filtering has correctness gaps, and invalid first-layer names can produce unreadable packages.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds nested USDZ support and a C++-aligned usd_utils module for dependency discovery, asset-path rewriting, and package creation.
Changes:
- Resolves and reads nested USDZ packages efficiently.
- Adds dependency discovery and asset-path modification utilities.
- Packages stages and dependencies into new USDZ archives.
| File | Description |
|---|---|
crates/openusd/src/ar.rs |
Reads recursively nested package entries. |
crates/openusd/src/error.rs |
Adds dependency errors. |
crates/openusd/src/lib.rs |
Exposes usd_utils. |
crates/openusd/src/sdf/layer_registry.rs |
Exposes resolver-backed asset opening internally. |
crates/openusd/src/usd_utils/dependencies.rs |
Implements dependency discovery and path rewriting. |
crates/openusd/src/usd_utils/mod.rs |
Defines the utility module API. |
crates/openusd/src/usd_utils/package.rs |
Implements USDZ dependency packaging. |
crates/openusd/src/usdz/mod.rs |
Anchors nested packages and optimizes default-layer lookup. |
crates/openusd/src/usdz/reader.rs |
Reads nested package default layers. |
crates/openusd/src/usdz/writer.rs |
Documents the new packaging utility. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| continue; | ||
| }; | ||
| let anchor = layer.anchor_location(); | ||
| visit_asset_paths(layer.data(), layer.identifier(), Visit::STRICT, |asset| { |
Comment on lines
+43
to
+46
| let name = match first_layer_name { | ||
| Some(name) => name.to_owned(), | ||
| None => file_name(layer.resolved_path().ok_or_else(unresolved)?), | ||
| }; |
bresilla
force-pushed
the
feat/usdz-packaging
branch
from
October 4, 2026 17:39
b1432b6 to
4e6f5c8
Compare
Contributor
Author
|
PS: maybe some of this should go in another crate??? since you divided into "core", "schema" and "build"... maybe one called "utils"??? But at the moment its just a module |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Third one from the batch (after #139 and #140), the USDZ side. I needed to save an edited stage as a
.usdzwith everything it uses, so I ported the parts of C++UsdUtilsthat do that, plus reading packages nested in packages, which C++ already handles. One thing per commit again: 1 and 2 are standalone, 4 builds on 3, and 5 builds on 3 and 4.feat(usdz): read packages nested in packages: C++ opensouter.usdz[inner.usdz]and composes a reference to a package inside a package, here it was refused. The resolver now reads one bracket level at a time, and a nested package anchors to its default layer (outer.usdz[inner.usdz[first.usda]]).Archive::readon a.usdzentry reads that package's default layer, so theNestedPackageerror is gone.perf(usdz): find default layer without full read: theTODO(perf)inUsdzFileFormat::resolve_layer, it read the whole package just to list the central directory. Now it opens the asset and letsZipArchiveread only the directory.feat(usd_utils): compute all dependencies: a newusd_utilsmodule (C++UsdUtils) withcompute_all_dependencies, the port ofUsdUtilsComputeAllDependencies. It takes the stage because its resolver and already open layers play the part of the C++ global resolver and layer registry, so unsaved edits are seen like in C++. It follows every variant and skips deleted list-op items, and on the same scene it gives the same layers, assets and unresolved paths as C++ 25.05. Variable expressions, UDIM patterns and clip templates, which C++ expands, return an error for now.feat(usd_utils): create new usdz package: the port ofUsdUtilsCreateNewUsdzPackage, with the C++ layout. The root goes first (or underfirst_layer_name), layers keep their format,./and../paths that stay inside the package keep their place, and everything else moves into0/,1/, ... per source directory with its path rewritten. A referenced.usdzis stored whole, deleted references are packaged too so the deletes still match, a missing layer is skipped and returned, and a missing texture fails the package. The one difference: C++ 25.05 writes moved paths asN/file.usdaeven from a layer in a subdirectory (and in one case points at the wrong number), so its own package ends up with broken references. Here they are written relative to the layer, and C++ opens the result with everything composing.feat(usd_utils): modify asset paths: the port ofUsdUtilsModifyAssetPaths. It visits the same paths C++ does (once per distinct path, clip templates and expressions as authored), and an empty result removes the path like in C++. The only difference is that C++ drops every sublayer offset as soon as one sublayer path changes, while here they stay with their sublayers. This is what I use to export a layer to another directory: anchor its paths, then export.One question: are you ok with a
usd_utilsmodule for this? I went with it because it mirrors C++UsdUtils, and I assume more of it will come over the same way (flattening helpers, stage cache, asset localization and so on), so it felt like the right home rather than putting these onStageor inusdz. If you'd rather have them somewhere else, tell me and I'll move them.