feat(skel): UsdSkel reader + skinning toolkit#69
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a new skel feature/module implementing a UsdSkel schema reader plus time-independent resolvers and math helpers, with fixtures and integration/unit tests.
Changes:
- Introduces
openusd::skelbehind a new Cargo feature (skel) including tokens, read/decode structs, topology, anim mapping, skinning math, and resolvers. - Adds a comprehensive UsdSkel USDA fixture and integration tests covering schema discovery, decoding, inheritance, and resolver behavior.
- Updates crate exports and roadmap to reflect UsdSkel support.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/skel_reader.rs | Adds integration tests validating end-to-end UsdSkel decoding and resolver behavior using a fixture stage. |
| src/skel/mod.rs | Wires the new skel module and re-exports public APIs/types. |
| src/skel/tokens.rs | Adds centralized UsdSkel token constants used by reader/resolvers. |
| src/skel/types.rs | Introduces decoded data types/enums returned by reader functions. |
| src/skel/read.rs | Implements schema readers, time-sample decoding, and stage-wide prim discovery. |
| src/skel/topology.rs | Adds joint topology representation + validation with unit tests. |
| src/skel/anim_mapper.rs | Adds per-joint remapping utility (UsdSkelAnimMapper-like) with unit tests. |
| src/skel/skinning.rs | Adds pure math helpers for skinning/blend-shapes with unit tests. |
| src/skel/skeleton_query.rs | Adds a static SkeletonResolver that precomputes topology/inverse binds. |
| src/skel/skinning_query.rs | Adds a per-mesh SkinningResolver for remapping and performing skinning. |
| src/skel/binding.rs | Adds subtree binding discovery and SkelRoot enumeration helpers. |
| src/lib.rs | Exposes skel module behind the skel feature flag. |
| fixtures/usdSkel_scene.usda | Adds a fixture USDA scene covering SkelRoot/Skeleton/SkelAnimation/BlendShape/SkelBindingAPI. |
| ROADMAP.md | Marks UsdSkel feature as complete and documents scope/limitations. |
| Cargo.toml | Adds the skel feature and a feature-gated integration test target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
+31
| /// `true` iff `indices == [0, 1, 2, …, n-1]` AND `source.len() == | ||
| /// target.len()`. When set, [`remap`] is a straight clone. | ||
| identity: bool, |
Comment on lines
+85
to
+103
| /// Remap a per-joint array `source` (one element per source | ||
| /// joint) into target order. Missing entries get `default`. | ||
| /// Panics if `source.len()` doesn't match the source length the | ||
| /// mapper was built with. | ||
| pub fn remap<T: Clone>(&self, source: &[T], default: T) -> Vec<T> { | ||
| if self.identity { | ||
| return source.to_vec(); | ||
| } | ||
| self.indices | ||
| .iter() | ||
| .map(|&i| { | ||
| if i == MISSING { | ||
| default.clone() | ||
| } else { | ||
| source[i as usize].clone() | ||
| } | ||
| }) | ||
| .collect() | ||
| } |
Comment on lines
+109
to
+122
| pub fn remap_with_stride<T: Copy>(&self, source: &[T], stride: usize, default: T) -> Vec<T> { | ||
| let mut out = Vec::with_capacity(self.indices.len() * stride); | ||
| for &i in &self.indices { | ||
| if i == MISSING { | ||
| for _ in 0..stride { | ||
| out.push(default); | ||
| } | ||
| } else { | ||
| let start = (i as usize) * stride; | ||
| out.extend_from_slice(&source[start..start + stride]); | ||
| } | ||
| } | ||
| out | ||
| } |
| } | ||
| let attr_path = prim.append_property(name)?; | ||
| let offsets = match stage.field::<Value>(attr_path.clone(), "default")? { | ||
| Some(Value::Vec3fVec(v)) => v, |
Comment on lines
+122
to
+143
| pub fn remap_skinning_xforms(&self, skel_skinning_xforms: &[[f64; 16]]) -> Vec<[f64; 16]> { | ||
| if self.mapper.is_identity() { | ||
| return skel_skinning_xforms.to_vec(); | ||
| } | ||
| // Flatten through the strided helper. Missing joints land as | ||
| // identity (fill value = 1 on the diagonal). | ||
| let flat: Vec<f64> = skel_skinning_xforms.iter().flat_map(|m| m.iter().copied()).collect(); | ||
| let strided = self.mapper.remap_with_stride(&flat, 16, 0.0); | ||
| let mut out = Vec::with_capacity(self.num_mesh_joints); | ||
| for chunk in strided.chunks_exact(16) { | ||
| let mut m = [0.0f64; 16]; | ||
| m.copy_from_slice(chunk); | ||
| // Fill-with-zeros leaves missing joints as the all-zeros | ||
| // matrix; promote to identity so the downstream skin | ||
| // routine doesn't collapse points to the origin. | ||
| if m == [0.0f64; 16] { | ||
| m = IDENTITY_MAT4; | ||
| } | ||
| out.push(m); | ||
| } | ||
| out | ||
| } |
| - m[8] * m[2] * m[5]; | ||
|
|
||
| let det = m[0] * inv[0] + m[1] * inv[4] + m[2] * inv[8] + m[3] * inv[12]; | ||
| if det.abs() < f64::EPSILON { |
Owner
|
./src/schemas/ would be nice. |
Contributor
Author
|
Thanks @mxpv -- |
Owner
|
Thanks. Changes are merge. I followed up with a few changes in the |
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.
Proposal for follow-up reorganisation
Before this lands, a question for @mxpv: if you're open to it, I'd like to move
src/physics/andsrc/skel/(and the matchingtests/+fixtures/) under asrc/noncore/subfolder in a follow-up PR.The plan is to keep contributing domain schemas —
UsdAnimis next, thenUsdLuxandUsdGeom.Camera. Adding one top-level folder per schema family will get crowded fast, and these schemas aren't part of the AOUSD core spec anyway, sononcore(orschemas, or whatever you prefer) feels like a natural grouping. Happy to defer to your naming preference.If you'd rather keep the flat layout, no problem — this PR doesn't depend on the reorg either way.
Summary
Adds the
skelcargo feature with a complete UsdSkel schema reader plus the time-independent half of Pixar's UsdSkel object model. Covers sections 1, 3, 4, and 6 of the canonical UsdSkel landing page (https://openusd.org/release/api/usd_skel_page_front.html), minus animation evaluation (left for a follow-upanimfeature).What's deliberately out
Time-dependent evaluation (
UsdSkelAnimQuery,ComputeSkinningTransforms(time), stage-time interpolation ofSkelAnimation samples). Resolvers take pre-evaluated joint poses so the follow-up anim layer can plug in directly.
Instancing-of-bind-state and a stateful
UsdSkelCachewere intentionally skipped — discovery is exposed as plain stateless functions to fit the existing single-shot Stage API surface.Tests
All green; no regressions in other test binaries.
ROADMAP
Flips the UsdSkel row to
:white_check_mark:/main.EXAMPLE POSSIBLE BECAUSE OF THIS