UsdNamespaceEditor: rename/reparent/delete with fixup#113
Open
bresilla wants to merge 10 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Introduces stage-level namespace editing (rename/reparent/delete) aligned with OpenUSD’s UsdNamespaceEditor, including per-layer spec moves/removals and stage-wide fixup of relationship targets and attribute connections.
Changes:
- Add
NamespaceEditorAPI with validation (can_apply_edits) and application (apply_edits) including target/connection fixups. - Add
Layer::remove_spec_subtreeandLayer::move_spec_subtreeto mechanically edit namespace subtrees within a single layer. - Update exports and roadmap to reflect the new feature.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| src/usd/namespace_editor.rs | New stage-level namespace editor with validation, per-layer edits, and fixup traversal + tests. |
| src/usd/mod.rs | Wires the new module and re-exports NamespaceEditor. |
| src/sdf/layer.rs | Adds single-layer namespace subtree move/remove helpers and unit tests. |
| ROADMAP.md | Documents the new namespace editing support and remaining gaps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+90
| pub fn rename_prim(&mut self, prim: &super::Prim, new_name: &str) -> &mut Self { | ||
| if let Some(new) = prim.path().parent().and_then(|p| p.append_path(new_name).ok()) { | ||
| self.pending = Some(Edit { | ||
| old_path: prim.path().clone(), | ||
| new_path: Some(new), | ||
| }); | ||
| } | ||
| self | ||
| } | ||
|
|
||
| /// Stage a reparent of `prim` under `new_parent`, keeping its name. | ||
| pub fn reparent_prim(&mut self, prim: &super::Prim, new_parent: &super::Prim) -> &mut Self { | ||
| if let Some(new) = prim.path().name().and_then(|n| new_parent.path().append_path(n).ok()) { | ||
| self.pending = Some(Edit { | ||
| old_path: prim.path().clone(), | ||
| new_path: Some(new), | ||
| }); | ||
| } | ||
| self | ||
| } |
Comment on lines
+288
to
+304
| fn rewrite_paths(paths: &[sdf::Path], old: &sdf::Path, new: Option<&sdf::Path>) -> Option<Vec<sdf::Path>> { | ||
| if !paths.iter().any(|p| p.has_prefix(old)) { | ||
| return None; | ||
| } | ||
| let mut out = Vec::new(); | ||
| for p in paths { | ||
| if p.has_prefix(old) { | ||
| if let Some(new) = new { | ||
| out.push(p.replace_prefix(old, new).unwrap_or_else(|| p.clone())); | ||
| } | ||
| // delete: drop the entry | ||
| } else { | ||
| out.push(p.clone()); | ||
| } | ||
| } | ||
| Some(out) | ||
| } |
Comment on lines
+573
to
+581
| // Ensure the destination parent exists so the moved leaf has an owner to | ||
| // register under. A top-level destination's parent is the pseudo-root. | ||
| if new_parent.is_abs_root() { | ||
| if data.spec_type(&new_parent).is_none() { | ||
| data.create_spec(new_parent.clone(), SpecType::PseudoRoot); | ||
| } | ||
| } else { | ||
| ensure_prim_chain(data, &new_parent)?; | ||
| } |
Comment on lines
+882
to
+893
| fn remove_from_token_vec(spec: &mut Spec, key: ChildrenKey, name: &str) { | ||
| let now_empty = match spec.get_mut(key.as_str()) { | ||
| Some(Value::TokenVec(v)) => { | ||
| v.retain(|n| n != name); | ||
| v.is_empty() | ||
| } | ||
| _ => false, | ||
| }; | ||
| if now_empty { | ||
| spec.remove(key.as_str()); | ||
| } | ||
| } |
Comment on lines
+188
to
+192
| for (layer_id, _) in &stack { | ||
| let idx = identifiers.iter().position(|i| i == layer_id).ok_or_else(|| { | ||
| StageAuthoringError::Composition(anyhow::anyhow!("layer {layer_id} not found in stack")) | ||
| })?; | ||
| self.stage.set_edit_target(EditTarget::for_layer_index(idx))?; |
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.
Builds on #112 (the sdf-tier layer ops). This is the actual editor - a
NamespaceEditorthat holds a stage and does the namespace edits you actually reach for:can_apply_editsfor validation (collision, move-under-self, kind mismatch, missing source, etc.)Single edit at a time, like the C++ public API. Under the hood it uses
Layer::move_spec_subtree/remove_spec_subtreefrom #112 per layer, then does one stage traversal to rewrite any targets/connections referencing the old path - prefix-replace on a move, drop on a delete.Scope for now is the local layer stack. Anything composing across a reference/payload would need relocates to move, so those are rejected (matches what C++ does to start). Remaining after this: relocates, batched multi-edit, dependent stages.
I needed this for renaming robot namespaces in my bevy_openusd work, but it's the general UsdNamespaceEditor.
14 tests cover prim rename/reparent/delete, property rename, and the target + connection fixup.