From c313a018ec2b4f18295279b5fac736bbb10a52ed Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Wed, 26 Jan 2022 11:55:00 -0500 Subject: [PATCH 1/4] Fix group command not finding unique names Delays checking for name unicity until execution starts. Adds unit test for the issue. --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 33 +++--- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h | 1 + test/lib/ufe/testGroupCmd.py | 112 ++++++++++++++++++ 3 files changed, 132 insertions(+), 14 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index b3fa1d7984..a457b86c8e 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -59,6 +59,7 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand( const UsdSceneItem::Ptr& /* pos */) : Ufe::InsertChildCommand() , _ufeDstItem(nullptr) + , _ufeParentItem(parent) , _ufeSrcPath(child->path()) , _usdSrcPath(child->prim().GetPath()) { @@ -90,20 +91,6 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand( ufe::applyCommandRestriction(childPrim, "reparent"); ufe::applyCommandRestriction(parentPrim, "reparent"); - // First, check if we need to rename the child. - const auto childName = uniqueChildName(parent->prim(), child->path().back().string()); - - // Create a new segment if parent and child are in different run-times. - // parenting a USD node to the proxy shape node implies two different run-times - auto cRtId = child->path().runTimeId(); - if (parent->path().runTimeId() == cRtId) { - _ufeDstPath = parent->path() + childName; - } else { - auto cSep = child->path().getSegments().back().separator(); - _ufeDstPath = parent->path() + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); - } - _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); - _childLayer = childPrim.GetStage()->GetEditTarget().GetLayer(); _parentLayer = getEditRouterLayer(PXR_NS::TfToken("parent"), parentPrim); } @@ -130,6 +117,24 @@ UsdUndoInsertChildCommand::Ptr UsdUndoInsertChildCommand::create( bool UsdUndoInsertChildCommand::insertChildRedo() { + if (_usdDstPath.IsEmpty()) { + const auto& parentPrim = ufePathToPrim(_ufeParentItem->path()); + + // First, check if we need to rename the child. + const auto childName = uniqueChildName(parentPrim, _ufeSrcPath.back().string()); + + // Create a new segment if parent and child are in different run-times. + // parenting a USD node to the proxy shape node implies two different run-times + auto cRtId = _ufeSrcPath.runTimeId(); + if (_ufeParentItem->path().runTimeId() == cRtId) { + _ufeDstPath = _ufeParentItem->path() + childName; + } else { + auto cSep = _ufeSrcPath.getSegments().back().separator(); + _ufeDstPath = _ufeParentItem->path() + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); + } + _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); + } + bool status = SdfCopySpec(_childLayer, _usdSrcPath, _parentLayer, _usdDstPath); if (status) { // remove all scene description for the given path and diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h index 030b72dfab..6b8a26b47b 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h @@ -64,6 +64,7 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::InsertChildCom bool insertChildUndo(); UsdSceneItem::Ptr _ufeDstItem; + UsdSceneItem::Ptr _ufeParentItem; Ufe::Path _ufeSrcPath; Ufe::Path _ufeDstPath; diff --git a/test/lib/ufe/testGroupCmd.py b/test/lib/ufe/testGroupCmd.py index e79643899b..6e6b3112bc 100644 --- a/test/lib/ufe/testGroupCmd.py +++ b/test/lib/ufe/testGroupCmd.py @@ -647,5 +647,117 @@ def checkAfter(a, b, c, e, f, g): children = hierarchyAfter() checkAfter(*children) + @unittest.skipIf(mayaUtils.previewReleaseVersion() < 128, 'Test requires fix in Maya Preview Release 128 or greater.') + def testGroupHierarchyWithRenaming(self): + '''Grouping a node and a descendant when all descendants share the same name''' + # MAYA-113532: when grouping a node and its descendant sharing the same name, we need to + # detect the name conflicts and rename as we reparent into the group. + + cmds.file(new=True, force=True) + import mayaUsd_createStageWithNewLayer + + # Create the following hierarchy: + # + # ps + # |_ A + # |_ A + # |_ A + # |_ A + # |_ A + # + # And group them all at the same level, which implies renaming. + + psPathStr = mayaUsd_createStageWithNewLayer.createStageWithNewLayer() + stage = mayaUsd.lib.GetPrim(psPathStr).GetStage() + stage.DefinePrim('/A', 'Xform') + stage.DefinePrim('/A/B', 'Cube') + stage.DefinePrim('/A/A', 'Xform') + stage.DefinePrim('/A/A/C', 'Cone') + stage.DefinePrim('/A/A/A', 'Xform') + stage.DefinePrim('/A/A/A/D', 'Sphere') + stage.DefinePrim('/A/A/A/A', 'Xform') + stage.DefinePrim('/A/A/A/A/E', 'Capsule') + stage.DefinePrim('/A/A/A/A/A', 'Xform') + stage.DefinePrim('/A/A/A/A/A/F', 'Cylinder') + + psPath = ufe.PathString.path(psPathStr) + psPathSegment = psPath.segments[0] + ps = ufe.Hierarchy.createItem(psPath) + psHier = ufe.Hierarchy.hierarchy(ps) + groupPath = ufe.Path([psPathSegment, usdUtils.createUfePathSegment('/group1')]) + + def hierarchyBefore(): + retVal = [] + aPath = ufe.Path([psPathSegment, usdUtils.createUfePathSegment('/A')]) + retVal.append(ufe.Hierarchy.createItem(aPath)) + for i in range(4): + aPath = aPath + ufe.PathComponent('A') + retVal.append(ufe.Hierarchy.createItem(aPath)) + return retVal + + a, aa, aaa, aaaa, aaaaa = hierarchyBefore() + + def checkBefore(): + # We care about 2 things: + # - All Xforms are named A + # - The non-xform stay in the right order: + order = [("B", "Cube"), ("C", "Cone"), ("D", "Sphere"), ("E", "Capsule"), ("F", "Cylinder")] + psChildren = psHier.children() + self.assertEqual(len(psChildren), 1) + a = psChildren[0] + for gprimName, gprimTypeName in order: + aHier = ufe.Hierarchy.hierarchy(a) + aChildren = aHier.children() + if gprimName == "F": + self.assertEqual(len(aChildren), 1) + else: + self.assertEqual(len(aChildren), 2) + for child in aChildren: + prim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(child.path())) + if child.nodeName() == "A": + self.assertEqual(prim.GetTypeName(), "Xform") + a = child + else: + self.assertEqual(child.nodeName(), gprimName) + self.assertEqual(prim.GetTypeName(), gprimTypeName) + + def checkAfter(): + # We care about 2 things: + # - Group has 5 Xform children. Don't care about names: they will be unique. + # - Each child has a one of the non-xform prims. + members = set([("B", "Cube"), ("C", "Cone"), ("D", "Sphere"), ("E", "Capsule"), ("F", "Cylinder")]) + group = ufe.Hierarchy.createItem(groupPath) + groupHier = ufe.Hierarchy.hierarchy(group) + groupChildren = groupHier.children() + self.assertEqual(len(groupChildren), 5) + foundMembers = set() + for child in groupChildren: + prim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(child.path())) + self.assertEqual(prim.GetTypeName(), "Xform") + childHier = ufe.Hierarchy.hierarchy(child) + childChildren = childHier.children() + self.assertEqual(len(childChildren), 1) + member = childChildren[0] + prim = mayaUsd.ufe.ufePathToPrim(ufe.PathString.string(member.path())) + foundMembers.add((member.nodeName(), prim.GetTypeName())) + self.assertEqual(members, foundMembers) + + sn = ufe.GlobalSelection.get() + sn.clear() + # We randomize the order a bit to make sure previously moved items do + # not affect the next one. + sn.append(a) + sn.append(aaa) + sn.append(aa) + sn.append(aaaaa) + sn.append(aaaa) + + cmds.group() + checkAfter() + cmds.undo() + checkBefore() + cmds.redo() + checkAfter() + if __name__ == '__main__': unittest.main(verbosity=2) From 68c8f1c935d6e1236af77dcb05e3a98bd2a3087d Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Wed, 26 Jan 2022 12:15:01 -0500 Subject: [PATCH 2/4] clang-format --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index a457b86c8e..24369eb974 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -130,7 +130,8 @@ bool UsdUndoInsertChildCommand::insertChildRedo() _ufeDstPath = _ufeParentItem->path() + childName; } else { auto cSep = _ufeSrcPath.getSegments().back().separator(); - _ufeDstPath = _ufeParentItem->path() + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); + _ufeDstPath = _ufeParentItem->path() + + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); } _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); } From 746e9d614b61cbf8add18981026f17c8423a9a4c Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Wed, 26 Jan 2022 13:49:03 -0500 Subject: [PATCH 3/4] Keep only the parent path. --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 11 +++++------ lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h | 2 +- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index 24369eb974..5ae352582f 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -59,8 +59,8 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand( const UsdSceneItem::Ptr& /* pos */) : Ufe::InsertChildCommand() , _ufeDstItem(nullptr) - , _ufeParentItem(parent) , _ufeSrcPath(child->path()) + , _ufeParentPath(parent->path()) , _usdSrcPath(child->prim().GetPath()) { const auto& childPrim = child->prim(); @@ -118,7 +118,7 @@ UsdUndoInsertChildCommand::Ptr UsdUndoInsertChildCommand::create( bool UsdUndoInsertChildCommand::insertChildRedo() { if (_usdDstPath.IsEmpty()) { - const auto& parentPrim = ufePathToPrim(_ufeParentItem->path()); + const auto& parentPrim = ufePathToPrim(_ufeParentPath); // First, check if we need to rename the child. const auto childName = uniqueChildName(parentPrim, _ufeSrcPath.back().string()); @@ -126,12 +126,11 @@ bool UsdUndoInsertChildCommand::insertChildRedo() // Create a new segment if parent and child are in different run-times. // parenting a USD node to the proxy shape node implies two different run-times auto cRtId = _ufeSrcPath.runTimeId(); - if (_ufeParentItem->path().runTimeId() == cRtId) { - _ufeDstPath = _ufeParentItem->path() + childName; + if (_ufeParentPath.runTimeId() == cRtId) { + _ufeDstPath = _ufeParentPath + childName; } else { auto cSep = _ufeSrcPath.getSegments().back().separator(); - _ufeDstPath = _ufeParentItem->path() - + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); + _ufeDstPath = _ufeParentPath + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); } _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); } diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h index 6b8a26b47b..040fd55323 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h @@ -64,9 +64,9 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::InsertChildCom bool insertChildUndo(); UsdSceneItem::Ptr _ufeDstItem; - UsdSceneItem::Ptr _ufeParentItem; Ufe::Path _ufeSrcPath; + Ufe::Path _ufeParentPath; Ufe::Path _ufeDstPath; PXR_NS::SdfPath _usdSrcPath; From b3913d0949ac9899523e212599fdebe5980ad661 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Wed, 26 Jan 2022 13:53:42 -0500 Subject: [PATCH 4/4] clang-format --- lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp index 5ae352582f..5567763a80 100644 --- a/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp +++ b/lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp @@ -130,7 +130,8 @@ bool UsdUndoInsertChildCommand::insertChildRedo() _ufeDstPath = _ufeParentPath + childName; } else { auto cSep = _ufeSrcPath.getSegments().back().separator(); - _ufeDstPath = _ufeParentPath + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); + _ufeDstPath + = _ufeParentPath + Ufe::PathSegment(Ufe::PathComponent(childName), cRtId, cSep); } _usdDstPath = parentPrim.GetPath().AppendChild(TfToken(childName)); }