Conversation
There was a problem hiding this comment.
Pull request overview
Implements explicit value clips and layer-offset retiming for AOUSD §12.3 value resolution, integrating clip evaluation into the PCP walk so time-sampled value resolution follows composition strength rather than a flattened query.
Changes:
- Add PCP-aware
timeSamplesresolution that retimes authored sample keys through each node’s composedmap_to_rootlayer offset (AOUSD §12.3.2.1). - Introduce value-clip parsing + runtime selection (active clip, stage→clip timing curve incl. jump discontinuities) and wire clip sampling into
Stage::value_atviapcp::Cache::value_at(AOUSD §12.3.4). - Extend USDA/USDC decoding to support
clips/clipSetsmetadata and correct USDC decoding forasset[]arrays (e.g.assetPaths).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/usdc/reader.rs | Fix USDC decoding for asset[] arrays used by value clips (assetPaths). |
| src/usda/parser.rs | Add parsing for clips (dictionary) and clipSets (list-op) prim metadata. |
| src/usd/stage.rs | Route Stage::value_at through composition cache value resolution with clip support; add clip conformance tests. |
| src/usd/prim.rs | Add clip introspection APIs (has_clips, clip_sets) + test coverage. |
| src/sdf/schema.rs | Register clips and clipSets as known Sdf field keys. |
| src/pcp/mod.rs | Export new pcp::clip module. |
| src/pcp/index.rs | Add PCP-node time-sample retiming + clip anchoring helpers + tests. |
| src/pcp/clip.rs | New clip-set model and stage→clip time mapping utilities + unit tests. |
| src/pcp/cache.rs | Add lazy clip-layer loading/caching and integrate clips into composed value resolution. |
| src/layer.rs | Adjust open_layer signature to accept &dyn Resolver for use from cache. |
| ROADMAP.md | Update roadmap entries to reflect retiming + explicit value clip support status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Value resolution maps each contributing node's authored sample times to stage time through the node's composed map_to_root offset (stage = scale * layer + offset), covering sublayer, reference, and payload offsets per spec 12.3.2.1. The strongest timeSamples opinion wins as a whole; a ValueBlock blocks weaker layers.
Add clips/clipSets field keys, USDA parsing for the clips dictionary and clipSets list-op, and pcp::clip::ClipSet — the explicit clip-set model with active-clip selection and stage-to-clip time mapping (linear segments, jump discontinuities, clamping) per spec 12.3.4. Not yet wired into value resolution; carries a temporary dead_code allow removed in the resolution step.
Add Cache::clip_layer, which resolves and loads clip and manifest layers through the stack resolver, anchored to the authoring layer, and caches them by resolved identifier. Clip layers stay out of the composition LayerStack since clip data does not compose (spec 12.3.4). open_layer now takes a &dyn Resolver so the cache can reuse it.
Wire value clips into Stage::value_at via Cache::value_at: local time samples win, then clips anchored on the prim or an ancestor, then reference/payload time samples, then default (spec 12.3.4.5). Clip sets are selected by clipSets order, gated on the manifest, and sampled at the mapped clip time. Local opinions are identified by root-layer-stack membership, since referenced layers also yield Root-arc nodes. Also read usdc asset[] arrays (the assetPaths type), which previously failed.
Add Prim::clip_sets and Prim::has_clips, reading the composed clips dictionary (spec 12.3.4). Attribute::get_at already resolves clip values through Stage::value_at. Authoring helpers remain future work.
Mark Spec 12.3.2.1 layer-offset retiming as supported and explicit value clips (Spec 12.3.4) as partially landed, and refresh the offset-composition and attribute-resolution notes that previously deferred runtime retiming.
Mark the two seams where explicit-clip resolution is intentionally incomplete: template-clip derivation in ClipSet parsing and missing-value handling in clip resolution (spec 12.3.4.1.3, 12.3.4.6-7).
USDC backends may decode the clipSets list op as either a String or Token list op; clip_sets_order now honors both so an authored clip-set strength order is not silently dropped in favor of name ordering.
Fix value clip resolution incorrectly outranking local defaults and returning values for attributes that do not exist on the composed stage. Stage value lookup now requires a composed attribute spec before consulting clips, and checks local defaults before clip data. Preserve clip asset provenance while composing clips dictionaries so relative assetPaths and manifestAssetPath resolve against the layer that authored those fields. This covers sparse stronger overrides and clips introduced through references. Add regression coverage for local defaults, missing attributes, sublayer-authored clip assets, and referenced-layer clip assets.
Apply layer offsets to explicit value clip active and times metadata by retiming the authored stage-time component through the contributing node's map-to-root offset. Build the root composition scan from the actual stage root layer stack before falling back to the flat loaded-layer scan. This keeps local sublayer offsets available to value resolution. Fix duplicate timing curves at the first stage time so exact queries use the right-hand jump value, while earlier queries still clamp to the first value. Add regression coverage for sublayer-retimed clip metadata and initial timing-curve jumps.
Fix cross-platform reference lookup for relative asset paths such as `./asset/model.usda`. The old string suffix check compared USD-authored forward slashes with platform-native collected identifiers, which failed on Windows. Use `std::path::Path` equality, suffix, and `.`/`..` handling instead of manual separator checks. Add tempdir coverage so the test follows each platform's native path behavior.
Add PrimIndex::opinions, an iterator yielding Opinion { node, query_path, value } for each authored opinion, and route
the seven resolve_* field walkers through it, removing the duplicated query-path/layer/try_get preamble. Behavior
preserving.
Extract push_implied_nodes (the layer-scan-and-push block duplicated across all three phases) and collect_ancestor_arcs_from (the shared ancestor walk), and split the three fallback phases into named methods. propagate_parent_specs is now a short dispatcher. Behavior preserving.
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.
Implements explicit value clips (§12.3.4) and the layer-offset retiming
prerequisite (§12.3.2.1), wiring clips into the composition core so
timeSamplesresolution walks the PCP nodes rather than a flattened query.Addresses #82 (explicit clips; template clips,
interpolateMissingClipValues,and
UsdClipsAPIauthoring remain follow-ups).PrimIndex::resolve_time_samplesmapseach contributing node's authored sample times to stage time through its
composed
map_to_rootoffset, covering sublayer/reference/payload offsets.clips/clipSetsfield keys + USDA parsing;pcp::clip::ClipSetwith active-clip selection (§12.3.4.3) and stage→cliptime mapping (§12.3.4.4) including jump discontinuities (§12.3.4.8).
Cache::clip_layerresolves and caches clip andmanifest layers through the stack resolver; clip layers stay out of the
composition
LayerStacksince clip data does not compose.Cache::value_atapplies the strength order(local time samples → clips → reference/payload time samples → default),
gating on the manifest, substituting
primPath, and sampling at the mappedclip time. Local opinions are identified by root-layer-stack membership, since
referenced layers also yield
Root-arc nodes. Also fixes USDCasset[]arrayreading (the
assetPathstype), which previously failed.Prim::has_clips/Prim::clip_sets;Attribute::get_atresolves clip values.
Things deferred:
templateAssetPath/templateStartTime/ …).interpolateMissingClipValuesand missing-value/sentinel semantics(§12.3.4.6–12.3.4.7).
UsdClipsAPIauthoring.