feat(usda): accept instanceable prim metadata#41
Conversation
USDA parser rejected `instanceable = true/false` with "Unsupported prim metadata: instanceable", preventing files that use USD native instancing from opening. Add a match arm in `read_prim_metadata_entry` that parses the `instanceable` boolean flag and stores it via the already-defined `FieldKey::Instanceable`. No changes to the schema or token layers are required. Add a minimal fixture (`fixtures/instanceable_metadata.usda`) and three tests in `stage::tests`: - `instanceable_true_parses_and_is_readable` - `instanceable_false_parses_and_is_readable` - `instanceable_absent_returns_none` Fixes: Kitchen_set_instanced.usd failing on Stage::open()
There was a problem hiding this comment.
Pull request overview
This PR updates the USDA parser to accept the instanceable prim metadata flag (used by native USD instancing), adds a fixture covering instanceable = true/false, and introduces unit tests to verify the authored flag can be read back via Stage::field.
Changes:
- Parse
instanceableinread_prim_metadata_entryand store it underFieldKey::Instanceable. - Add a new USDA fixture authoring
instanceabletrue/false on separate prims. - Add unit tests asserting
instanceableis readable asSome(true),Some(false), orNonewhen absent.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/usda/parser.rs | Adds a metadata match arm to parse and store the instanceable flag. |
| src/stage.rs | Adds unit tests verifying instanceable parsing and retrieval behavior. |
| fixtures/instanceable_metadata.usda | Adds a fixture with prims authored with instanceable = true/false plus an un-authored parent. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/usda/parser.rs
Outdated
| n if n == FieldKey::Instanceable.as_str() => { | ||
| ensure!(list_op.is_none(), "instanceable metadata does not support list ops"); | ||
| let value = self | ||
| .parse_token::<bool>() |
There was a problem hiding this comment.
instanceable is parsed via parse_token::<bool>(), which relies on bool::from_str and only accepts true/false. Elsewhere the parser has parse_bool() (used for Type::Bool) that supports USD’s more permissive boolean literal forms (identifier/number/string). For consistency and to avoid rejecting valid boolean encodings here, parse instanceable with parse_bool() (and consider aligning active as well).
| .parse_token::<bool>() | |
| .parse_bool() |
src/stage.rs
Outdated
| Ok(()) | ||
| } | ||
|
|
||
| // --- PR #1: instanceable prim metadata --- |
There was a problem hiding this comment.
The section header // --- PR #1: instanceable prim metadata --- hard-codes a PR number, which won’t stay meaningful over time and isn’t used elsewhere in this test module. Consider replacing it with a descriptive header (e.g., // --- Instanceable prim metadata ---) to keep the tests self-contained.
| // --- PR #1: instanceable prim metadata --- | |
| // --- Instanceable prim metadata --- |
|
Hi @mxpv, Apologies — this PR (and the other four in this batch: #41 #42 #43 #44 #45) was opened by an AI agent on my behalf without enough human review of upstream etiquette. I should have filed an issue first to discuss before dropping a stack of code on you. Sorry for the noise. For context: I'm building yw-look, a Windows quick-look style asset inspector for CG workflows, and I need a few small accessors on top of If the API and approach look reasonable to you, I'd be grateful if you could merge them. If not — totally fine to close, I'd just maintain them on my fork instead. Happy to revise based on any feedback. Thanks for the great crate! |
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
|
@yohawing no problem! thanks for contributions. Overall the changes make sense, but it'd take some time for me to review each PR. |
Summary
The USDA parser currently rejects
instanceable = true/falsewithUnsupported prim metadata: instanceable, which prevents loading any file that uses USD native instancing (e.g.Kitchen_set_instanced.usdfrom the usd-wg-assets test set).This PR adds a
matcharm inread_prim_metadata_entrythat parses theinstanceableboolean flag and stores it via the already-definedFieldKey::Instanceable. No changes to the schema or token layers are required; only the parser needed to learn about this entry.Changes
src/usda/parser.rs: handleinstanceableinread_prim_metadata_entry(7 lines).fixtures/instanceable_metadata.usda: new fixture with a prim atinstanceable = true, one atinstanceable = false, and a parent with neither.src/stage.rs: three new unit tests instage::tests:instanceable_true_parses_and_is_readableinstanceable_false_parses_and_is_readableinstanceable_absent_returns_noneThe tests use the existing
stage.field::<bool>(path, FieldKey::Instanceable)API to verify that the flag round-trips correctly.Test plan
cargo testpasses (228 tests, 3 new)cargo clippy --all-targets -- -D warningscleancargo fmt --checkcleanKitchen_set_instanced.usdfrom usd-wg-assets now opens