feat(usd): attribute connections + ConnectionGraph#81
Merged
Conversation
set/add/add_prepended/remove/clear/has_authored/get on the Attribute handle, mirroring C++ UsdAttribute connection editing, over the sdf-tier connectionPaths list op.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds first-class support for authoring and querying USD attribute connections (connectionPaths), plus a stage-wide index for efficiently traversing connection relationships.
Changes:
- Added
AttributeAPIs to set/add/remove/clear and query composed connections. - Introduced a
ConnectionGraphthat indexes connections across aStagefor repeated source/sink/chain queries. - Extended SDF spec authoring helpers for manipulating
connectionPathsas list ops.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/usd/prim.rs | Adds attribute-level authoring/query methods for connectionPaths and corresponding tests. |
| src/usd/mod.rs | Wires in the new connections module and re-exports ConnectionGraph. |
| src/usd/connections.rs | Implements stage-wide connection indexing and queries with unit tests. |
| src/sdf/spec.rs | Adds spec authoring helpers for adding/removing/clearing connectionPaths. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+599
to
+606
| None => { | ||
| let op = if prepend { | ||
| sdf::PathListOp::prepended([path]) | ||
| } else { | ||
| sdf::PathListOp::explicit([path]) | ||
| }; | ||
| self.add(sdf::FieldKey::ConnectionPaths, sdf::Value::PathListOp(op)); | ||
| } |
Comment on lines
+401
to
+423
| pub fn remove_connection(&self, target: &sdf::Path) -> Result<bool, StageAuthoringError> { | ||
| let path = self.path.clone(); | ||
| let target = target.clone(); | ||
| let mut removed = false; | ||
| self.stage.with_target_layer(|layer| { | ||
| let data = layer.writable_data_mut()?; | ||
| match data.spec_mut(&path).and_then(|s| s.as_attr_mut()) { | ||
| Some(mut spec) => { | ||
| removed = spec.remove_connection_path(&target); | ||
| let mut cl = sdf::ChangeList::new(); | ||
| if removed { | ||
| cl.entry_mut(&path) | ||
| .info_changed | ||
| .insert(sdf::FieldKey::ConnectionPaths.as_str()); | ||
| } | ||
| Ok(cl) | ||
| } | ||
| None => Err(sdf::AuthoringError::InvalidPath { | ||
| path: path.clone(), | ||
| reason: "no attribute spec at path on the edit target layer", | ||
| }), | ||
| } | ||
| })?; |
Comment on lines
+433
to
+439
| /// `true` when any connection is authored on the edit target's layer. | ||
| /// Mirrors C++ `UsdAttribute::HasAuthoredConnections`. | ||
| // | ||
| // TODO: drop `anyhow::Result` once `Stage::field` returns a typed error. | ||
| pub fn has_authored_connections(&self) -> anyhow::Result<bool> { | ||
| Ok(!self.get_connections()?.is_empty()) | ||
| } |
Comment on lines
+146
to
+153
| /// Read the composed, flattened `connectionPaths` of a single property. | ||
| fn connections_of(stage: &Stage, attr: &Path) -> Result<Vec<Path>> { | ||
| Ok(match stage.field::<Value>(attr.clone(), "connectionPaths")? { | ||
| Some(Value::PathListOp(op)) => op.flatten(), | ||
| Some(Value::PathVec(v)) => v, | ||
| _ => Vec::new(), | ||
| }) | ||
| } |
Comment on lines
+126
to
+136
| fn walk_terminals(&self, attr: &Path, visited: &mut HashSet<Path>, out: &mut Vec<Path>) { | ||
| if !visited.insert(attr.clone()) { | ||
| return; | ||
| } | ||
| match self.forward.get(attr) { | ||
| // Terminal: nothing further to chase. | ||
| None => { | ||
| if !out.contains(attr) { | ||
| out.push(attr.clone()); | ||
| } | ||
| } |
Contributor
Author
|
Context: I'm porting the UsdShade schema work from my downstream project (bresilla/bevy_openusd) up into this crate, and this is the foundation it sits on. UsdShade is fundamentally a connection graph (shader inputs ← outputs), so rather than bury connection handling inside the shade module, it felt right to land it first as a general-purpose primitive — The shade PR is next and builds directly on this — should be up soon. |
`add_connection_path(_, prepend=false)` on a property with no prior opinion now authors a `PathListOp::appended` list op so weaker-layer connection opinions still compose, matching C++ `UsdAttribute::AddConnection` with its back-of-append default. The old behaviour authored an `explicit` list op which silently blocked all weaker-layer connections. Also honour `prepend=true` over an existing explicit op by inserting at the front of `explicit_items` instead of silently routing to the back. The op stays `explicit`; only the position changes.
Switch from `!get_connections()?.is_empty()` to a direct field-presence check so an explicit-empty `PathListOp` (the canonical way to block weaker-layer connections via `.connect = []`) is reported as authored. The old form flattened first, then asked whether the flattened list was non-empty, and so missed the block-only case. Also tighten connection test names to the project's terser convention.
Connection reads previously went through `Stage::field`, which returns only the strongest layer's list op verbatim. A prepend on the strongest layer would silently drop weaker-layer connection opinions. Add `PrimIndex::resolve_path_list_op` mirroring `resolve_token_list_op` (used for `apiSchemas`): walk every contributing node strongest to weakest, collect `PathListOp` opinions, stop on `ValueBlock`, and fold them with `ListOp::compose_over`. A bare `PathVec` value is treated as an explicit replacement of weaker opinions. Expose this as `pcp::Cache::connection_paths` and `Stage::connection_paths`, and route `Attribute::get_connections` and `ConnectionGraph` through it, removing the now-dead `connections_of` helper.
`Spec::add_connection_path` and `Spec::clear_connection_paths` now return `bool` (true when the spec actually changed). `Attribute::add_connection`, `Attribute::add_connection_prepended`, and `Attribute::clear_connections` route through a new `edit_connection` helper that only records a `ChangeList` entry when the spec was mutated — matching the `remove_connection` pattern. The classifier still ignores property-path entries today, so the observable change is just to avoid wasted bookkeeping. The fix lands the producer-side invariant so the consumer can come online later without re-walking these sites.
Convert `ConnectionGraph::resolve_chain` from a recursive DFS to an explicit work-stack loop so deep chains can't blow the call stack. Push sources in reverse so the visit order matches the recursive form. Cover the new shape with a 2_000-link chain regression test.
Comment on lines
+598
to
+601
| } else if prepend { | ||
| op.prepended_items.push(path); | ||
| } else { | ||
| op.appended_items.push(path); |
Comment on lines
+445
to
+450
| let mut cl = sdf::ChangeList::new(); | ||
| if f(&mut spec) { | ||
| cl.entry_mut(&path) | ||
| .info_changed | ||
| .insert(sdf::FieldKey::ConnectionPaths.as_str()); | ||
| } |
Comment on lines
+408
to
+413
| let mut cl = sdf::ChangeList::new(); | ||
| if removed { | ||
| cl.entry_mut(&path) | ||
| .info_changed | ||
| .insert(sdf::FieldKey::ConnectionPaths.as_str()); | ||
| } |
Comment on lines
+122
to
+141
| let mut terminals = Vec::new(); | ||
| let mut visited = HashSet::new(); | ||
| let mut stack: Vec<&Path> = vec![attr]; | ||
| while let Some(node) = stack.pop() { | ||
| if !visited.insert(node.clone()) { | ||
| continue; | ||
| } | ||
| match self.forward.get(node) { | ||
| // Terminal: nothing further to chase. | ||
| None => { | ||
| if !terminals.contains(node) { | ||
| terminals.push(node.clone()); | ||
| } | ||
| } | ||
| // Push reversed so the first source is explored first | ||
| // (matches the original recursive order). | ||
| Some(sources) => stack.extend(sources.iter().rev()), | ||
| } | ||
| } | ||
| terminals |
`Attribute connections` (Spec 12.4) flips to supported at `0.4.0`: `Stage::connection_paths` composes list-op edits across every contributing layer, `Attribute` carries the authoring surface, and `usd::ConnectionGraph` indexes edges + resolves chains. `List op resolution` (Spec 12.2.6) stays Partial, but `connectionPaths` moves out of the "Left" list. The new `PrimIndex::resolve_path_list_op` helper is the shared mechanism that `targetPaths` and `clipSets` can reuse when they land.
Connection targets authored through references or other composition arcs were composed as raw Sdf paths from the contributing layer. A referenced asset connection could therefore appear as /Source.outputs:out in the composed stage instead of /Mat.outputs:out, and ConnectionGraph indexed the wrong edge. Map every PathListOp bucket through the contributing node's map_to_root before applying list-op composition. Path prefix matching now treats the property separator as a namespace boundary, so prim-level maps also translate property paths.
RemoveConnection only edited local connection list buckets. When a target came from a weaker layer, the edit-target spec had no local item to remove, so the composed connection stayed visible. Check the composed connection list first and author a delete list-op when the target is present. If the edit target has no attribute spec, create a minimal spec using the composed type name so the delete opinion has a place to live. Also update the reference-remap test to satisfy Clippy's slice-from-ref lint.
AddConnection deduped only against the edit-target list op. Adding a target already contributed by a weaker layer could author a stronger duplicate and change ordering, while re-adding a locally deleted target left the stale delete opinion in place. Check the composed connection list before authoring a new add. At the Sdf layer, clear matching deleted_items before appending so a re-added connection is no longer removed during list-op application.
The public add_connection API documented C++ parity but authored append list-op items. OpenUSD's default AddConnection position is back of the prepend list, so parity callers got weaker ordering than expected. Route add_connection through the prepended bucket and keep an explicit add_connection_appended method for callers that need append semantics. Update tests to cover both authored buckets and the resulting composed order.
Stage authoring recorded connectionPaths in info_changed for set, add, and clear operations, but omitted the CHANGE_ATTRIBUTE_CONNECTION flag. Relationship target edits already set their matching flag. Leaving connection edits unflagged would make future property-level invalidation miss producer intent. Set CHANGE_ATTRIBUTE_CONNECTION whenever the shared connection edit helper reports a real mutation. remove_connection already sets the same flag on its custom edit path.
resolve_chain used terminals.contains for every terminal even though the visited set already ensures each graph node is processed once. In broad branching graphs that made terminal collection perform a redundant linear scan for every terminal. Emit each terminal directly after the visited check. The traversal order and cycle handling stay the same, but terminal collection no longer adds an O(T^2) scan.
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.
Adds first-class attribute connection support (the
.connect/connectionPathswiring) at the usd tier.Attributegets the full C++UsdAttributeconnection-editing surface — set, add (append/prepend), remove, clear, has-authored, get — over new sdf-tier list-op helpers that mirror the existing relationship-target ones.usd::ConnectionGraphindexes every connection edge on a stage in one pass and answers repeated queries: sources, sinks, connectedness, all edges, and chain resolution to terminal sources (cycle-safe). Schema-agnostic — UsdShade is the heaviest user but any connectable network reuses it.fmt + clippy clean.