Conversation
Resolve relationship `targetPaths` as composed raw targets, folding list-op edits across every contributing layer and remapping through arcs (spec 12.4). Share `Cache::property_targets` with connections, and expose `Stage::relationship_targets`, `Relationship::get_targets`, and `has_authored_targets`. Target forwarding remains unimplemented. Fold the `clipSets` strength order across all layers in `PrimIndex::clip_sets_order` per generic list-op resolution (spec 12.2.6), rather than honoring only the strongest opinion, while keeping the unauthored case as a name-order fallback.
Add forwarded target resolution (spec 12.4): raw targets that resolve to another relationship are replaced, recursively, by that relationship's forwarded targets, leaving only prim and attribute paths. Relationship targets are classified by composed spec type; cycles are broken by tracking followed relationships and duplicates collapse to first occurrence. Expose `Stage::forwarded_relationship_targets` and `Relationship::get_forwarded_targets`, alongside the existing raw `get_targets`. The population mask guards the queried relationship; forwarding then follows the composed chain regardless of the mask.
There was a problem hiding this comment.
Pull request overview
Closes remaining list-op composition gaps for property-level fields by adding full composition of relationship targetPaths (including arc remapping), implementing forwarded-relationship target chasing, and fixing clipSets strength-order resolution to fold list-op edits across all contributing layers.
Changes:
- Add
Stage::relationship_targets/Relationship::get_targets(+has_authored_targets) to compose raw relationshiptargetPathsas a path list-op across the full layer stack and through composition arcs. - Add
Stage::forwarded_relationship_targets/Relationship::get_forwarded_targetsandCache::forwarded_relationship_targetsto recursively forward relationship-to-relationship targets with cycle-breaking and deduplication. - Update
PrimIndex::clip_sets_orderto foldclipSetslist-op edits across layers (vs. strongest-opinion-only), with tests and roadmap updates.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/usd/stage.rs | Adds stage APIs for composed raw and forwarded relationship targets; adds composition/remap tests. |
| src/usd/prim.rs | Adds relationship-level target APIs (has_authored_targets, get_targets, get_forwarded_targets) and forwarding tests. |
| src/pcp/index.rs | Fixes clipSets order resolution to fold list-op edits across contributing layers; adds tests. |
| src/pcp/cache.rs | Reuses generic path-list-op composition for relationship targets and implements forwarded target chasing logic. |
| ROADMAP.md | Updates spec-coverage tracking to reflect implemented relationship target and clipSets composition behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+856
to
+860
| if is_relationship { | ||
| self.forward_targets_into(&target, out, visited)?; | ||
| } else if !out.contains(&target) { | ||
| out.push(target); | ||
| } |
Replace the recursive relationship-target forwarding walk with an explicit-stack DFS (mirroring `ConnectionGraph::resolve_chain`) so a deep relationship chain cannot overflow the call stack. Add a 4000-hop chain test that the recursive form would abort.
Forwarding now skips target relationships on prims outside the working set, so forwarded results no longer leak scene the population mask excludes. A masked-out relationship contributes no composed targets, matching raw `relationship_targets` on that path; direct prim/attribute target values are still returned.
Verified against C++ Usd_ClipSetDefinition: an authored clipSets is the authoritative ordered list (composed over an empty base), so an authored explicit-empty list yields no clip sets, distinct from an unauthored field which falls back to name order. Clarify the three-way return contract and add a regression test; no behavior change.
Cache::spec_type for a property path built and cached an index keyed by the property path, which change classification never invalidates — so a relationship authored after the path was first queried was never picked up, and forwarding kept treating the target as terminal (reported by Codex). Resolve the type from the owning prim's composition nodes, reading the layers live (like has_spec_at); the prim index stays valid across property authoring.
Forwarding through a relationship defined in an instance prototype classifies it correctly (spec_type redirects the instance-proxy path via effective_path) and remaps its targets into the queried instance namespace. Add coverage for two instances sharing a prototype.
Forwarding follows only live relationships; prim paths, attribute paths, and dangling/unloaded targets are kept as-is, matching C++ GetForwardedTargets. Correct the doc overclaim that only prim/attribute paths remain and add a dangling-target test.
Replace the O(n^2) out.contains scan with a HashSet seen-set, making forwarded-target dedup O(1) average-case while preserving first-occurrence order. Add a dedup test. (Reported by Claude review and Copilot.)
Extract compose_list_ops, the strongest-to-weakest reverse fold that was copied verbatim in resolve_token_list_op, resolve_path_list_op, and clip_sets_order. One implementation of the composition semantics keeps the three resolvers from drifting.
Extract masked_property_targets, collapsing the duplicated mask guard + try_or_handle preamble in connection_paths, relationship_targets, and forwarded_relationship_targets into one place.
The Prim-level get_targets / get_forwarded_targets wrappers re-narrated the full spec-12.4 contract already documented on the owning Stage methods. Replace the duplicated prose with intra-doc links so the contract has a single source of truth.
Extract Cache::find_property_node, the strongest-first walk over a property's owning-prim composition nodes that has_spec_at and spec_type each open-coded. It re-anchors the property onto each node's prim with Path::replace_prefix instead of raw string slicing and format!, removing the duplicated ensure_index + node-scan and the manual path arithmetic.
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.
Closes the remaining list-op resolution gaps for property-level fields
(spec §12.2.6, §12.4, §12.3.4).
Relationship raw targets now compose as a path list op, folding edits
across every contributing layer and remapping through arcs — sharing
Cache::property_targetswith attribute connections.New
Stage::relationship_targets,Relationship::get_targets, andhas_authored_targets.clipSetsstrength order folds across all layers inPrimIndex::clip_sets_order(was strongest-opinion-only), keeping theunauthored case as a name-order fallback.
Cache::forwarded_relationship_targetsrecursively chasesrelationship-to-relationship chains to prim/attribute terminals,
classifying targets by composed spec type, breaking cycles, and
deduping (spec §12.4).
New
Stage::forwarded_relationship_targetsandRelationship::get_forwarded_targets.