Tremblp/maya 103815/ufe parenting usd 01

build.py

@@ -141,7 +141,9 @@ def Run(context, cmd):
             while True:
                 l = p.stdout.readline().decode(encoding)
                 if l != "":
-                    logfile.write(l)
+                    # Avoid "UnicodeEncodeError: 'ascii' codec can't encode 
+                    # character" errors by serializing utf8 byte strings.
+                    logfile.write(l.encode("utf8"))
                     PrintCommandOutput(l)
                 elif p.poll() is not None:
                     break

lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp

@@ -35,6 +35,18 @@
 #include <pxr/usd/sdf/copyUtils.h>
 #include <pxr/base/tf/token.h>
 
+namespace {
+// shared_ptr requires public ctor, dtor, so derive a class for it.
+template<class T>
+struct MakeSharedEnabler : public T {
+    MakeSharedEnabler(
+        const MayaUsd::ufe::UsdSceneItem::Ptr& parent,
+        const MayaUsd::ufe::UsdSceneItem::Ptr& child,
+        const MayaUsd::ufe::UsdSceneItem::Ptr& pos
+    ) : T(parent, child, pos) {}
+};
+}
+
 MAYAUSD_NS_DEF {
 namespace ufe {
 
@@ -64,9 +76,21 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(
     }
     fUsdDstPath = parent->prim().GetPath().AppendChild(TfToken(childName));
 
-    fLayer = MayaUsdUtils::defPrimSpecLayer(childPrim);
-    if (!fLayer) {
-        std::string err = TfStringPrintf("No prim found at %s", childPrim.GetPath().GetString().c_str());
+    fChildLayer = MayaUsdUtils::defPrimSpecLayer(childPrim);
+    if (!fChildLayer) {
+        std::string err = TfStringPrintf("No child prim found at %s", childPrim.GetPath().GetString().c_str());
+        throw std::runtime_error(err.c_str());
+    }
+
+    auto parentPrim = parent->prim();
+    // If parent prim is the pseudo-root, no def primSpec will be found, so
+    // just use the edit target layer.
+    fParentLayer = parentPrim.IsPseudoRoot() ?
+        fStage->GetEditTarget().GetLayer() :
+        MayaUsdUtils::defPrimSpecLayer(parentPrim);
+
+    if (!fParentLayer) {
+        std::string err = TfStringPrintf("No parent prim found at %s", parentPrim.GetPath().GetString().c_str());
         throw std::runtime_error(err.c_str());
     }
 }
@@ -82,13 +106,18 @@ UsdUndoInsertChildCommand::Ptr UsdUndoInsertChildCommand::create(
     const UsdSceneItem::Ptr& pos
 )
 {
-    return std::make_shared<UsdUndoInsertChildCommand>(parent, child, pos);
+    // Error if requested parent is currently a child of requested child.
+    if (parent->path().startsWith(child->path())) {
+        return nullptr;
+    }
+    return std::make_shared<MakeSharedEnabler<UsdUndoInsertChildCommand>>(
+        parent, child, pos);
 }
 
 bool UsdUndoInsertChildCommand::insertChildRedo()
 {
     // See comments in UsdUndoRenameCommand.cpp.
-    bool status = SdfCopySpec(fLayer, fUsdSrcPath, fLayer, fUsdDstPath);
+    bool status = SdfCopySpec(fChildLayer, fUsdSrcPath, fParentLayer, fUsdDstPath);
     if (status)
     {
         auto srcPrim = fStage->GetPrimAtPath(fUsdSrcPath);
@@ -120,7 +149,7 @@ bool UsdUndoInsertChildCommand::insertChildUndo()
         // Regardless of where the edit target is currently set, switch to the
         // layer where we copied the source prim into the destination, then
         // restore the edit target.
-        UsdEditContext ctx(fStage, fLayer);
+        UsdEditContext ctx(fStage, fParentLayer);
         status = fStage->RemovePrim(fUsdDstPath);
     }
     if (status) {

lib/mayaUsd/ufe/UsdUndoInsertChildCommand.h

@@ -34,13 +34,6 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::UndoableComman
 public:
     typedef std::shared_ptr<UsdUndoInsertChildCommand> Ptr;
 
-    //! Construct a UsdUndoInsertChildCommand.  Note that as of 4-May-2020 the
-    //! pos argument is ignored, and only append is supported.
-    UsdUndoInsertChildCommand(
-        const UsdSceneItem::Ptr& parent,
-        const UsdSceneItem::Ptr& child,
-        const UsdSceneItem::Ptr& pos
-    );
     ~UsdUndoInsertChildCommand() override;
 
     UsdUndoInsertChildCommand(const UsdUndoInsertChildCommand&) = delete;
@@ -56,6 +49,15 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::UndoableComman
         const UsdSceneItem::Ptr& pos
     );
 
+protected:
+    //! Construct a UsdUndoInsertChildCommand.  Note that as of 4-May-2020 the
+    //! pos argument is ignored, and only append is supported.
+    UsdUndoInsertChildCommand(
+        const UsdSceneItem::Ptr& parent,
+        const UsdSceneItem::Ptr& child,
+        const UsdSceneItem::Ptr& pos
+    );
+
 private:
     void undo() override;
     void redo() override;
@@ -64,7 +66,8 @@ class MAYAUSD_CORE_PUBLIC UsdUndoInsertChildCommand : public Ufe::UndoableComman
     bool insertChildUndo();
 
     UsdStageWeakPtr fStage;
-    SdfLayerHandle fLayer;
+    SdfLayerHandle fChildLayer;
+    SdfLayerHandle fParentLayer;
     UsdSceneItem::Ptr fUfeSrcItem;
     SdfPath fUsdSrcPath;
     UsdSceneItem::Ptr fUfeDstItem;

test/lib/ufe/testParentCmd.py

@@ -35,6 +35,18 @@ def matrixToList(m):
         mList = mList + i
     return mList
 
+class OpenFileCtx(object):
+    def __init__(self, fileName):
+        self._fileName = fileName
+
+    def __enter__(self):
+        filePath = os.path.join(os.path.dirname(os.path.realpath(__file__)), "test-samples", "parentCmd", self._fileName)
+        cmds.file(filePath, force=True, open=True)
+
+    def __exit__(self, type, value, traceback):
+        # Close the file.
+        cmds.file(force=True, new=True)
+
 class ParentCmdTestCase(unittest.TestCase):
     '''Verify the Maya parent command on a USD scene.'''
 
@@ -221,69 +233,130 @@ def testParentAbsolute(self):
         cylChildren = cylHier.children()
         self.assertEqual(len(cylChildren), 0)
 
-    @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2013', 'testParentAbsolute only available in UFE preview version 0.2.13 and greater')
+    @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2013', 'testParentToProxyShape only available in UFE preview version 0.2.13 and greater')
     def testParentToProxyShape(self):
 
         # Load a file with a USD hierarchy at least 2-levels deep.
-        filePath = os.path.join(os.path.dirname(os.path.realpath(__file__)), "test-samples", "parentCmd", "simpleHierarchy.ma" )
-        cmds.file(filePath, force=True, open=True)
-
-        # Create scene items for the proxy shape and the sphere.
-        shapeSegment = mayaUtils.createUfePathSegment(
-            "|world|mayaUsdProxy1|mayaUsdProxyShape1")
-        shapePath = ufe.Path([shapeSegment])
-        shapeItem = ufe.Hierarchy.createItem(shapePath)
-
-        spherePath = ufe.Path(
-            [shapeSegment, usdUtils.createUfePathSegment("/pCylinder1/pCube1/pSphere1")])
-        sphereItem = ufe.Hierarchy.createItem(spherePath)
-
-        # The sphere is not a child of the proxy shape.
-        shapeHier = ufe.Hierarchy.hierarchy(shapeItem)
-        shapeChildren = shapeHier.children()
-        self.assertNotIn("pSphere1", childrenNames(shapeChildren))
-
-        # Get its world space transform.
-        sphereT3d = ufe.Transform3d.transform3d(sphereItem)
-        sphereWorld = sphereT3d.inclusiveMatrix()
-        sphereWorldListPre = matrixToList(sphereWorld)
-
-        # Parent sphere to proxy shape in absolute mode (default), using UFE
-        # path strings.
-        cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1/pCube1/pSphere1", "|mayaUsdProxy1|mayaUsdProxyShape1")
-
-        # Confirm that the sphere is now a child of the proxy shape.
-        shapeChildren = shapeHier.children()
-        self.assertIn("pSphere1", childrenNames(shapeChildren))
-
-        # Undo: the sphere is no longer a child of the proxy shape.
-        cmds.undo()
-
-        shapeChildren = shapeHier.children()
-        self.assertNotIn("pSphere1", childrenNames(shapeChildren))
-
-        # Redo: confirm that the sphere is again a child of the proxy shape.
-        cmds.redo()
+        with OpenFileCtx("simpleHierarchy.ma"):
 
-        shapeChildren = shapeHier.children()
-        self.assertIn("pSphere1", childrenNames(shapeChildren))
+            # Create scene items for the proxy shape and the sphere.
+            shapeSegment = mayaUtils.createUfePathSegment(
+                "|world|mayaUsdProxy1|mayaUsdProxyShape1")
+            shapePath = ufe.Path([shapeSegment])
+            shapeItem = ufe.Hierarchy.createItem(shapePath)
+
+            spherePath = ufe.Path(
+                [shapeSegment, usdUtils.createUfePathSegment("/pCylinder1/pCube1/pSphere1")])
+            sphereItem = ufe.Hierarchy.createItem(spherePath)
+
+            # The sphere is not a child of the proxy shape.
+            shapeHier = ufe.Hierarchy.hierarchy(shapeItem)
+            shapeChildren = shapeHier.children()
+            self.assertNotIn("pSphere1", childrenNames(shapeChildren))
+
+            # Get its world space transform.
+            sphereT3d = ufe.Transform3d.transform3d(sphereItem)
+            sphereWorld = sphereT3d.inclusiveMatrix()
+            sphereWorldListPre = matrixToList(sphereWorld)
+
+            # Parent sphere to proxy shape in absolute mode (default), using UFE
+            # path strings.
+            cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1/pCube1/pSphere1", "|mayaUsdProxy1|mayaUsdProxyShape1")
+
+            # Confirm that the sphere is now a child of the proxy shape.
+            shapeChildren = shapeHier.children()
+            self.assertIn("pSphere1", childrenNames(shapeChildren))
+
+            # Undo: the sphere is no longer a child of the proxy shape.
+            cmds.undo()
+
+            shapeChildren = shapeHier.children()
+            self.assertNotIn("pSphere1", childrenNames(shapeChildren))
+
+            # Redo: confirm that the sphere is again a child of the proxy shape.
+            cmds.redo()
+
+            shapeChildren = shapeHier.children()
+            self.assertIn("pSphere1", childrenNames(shapeChildren))
+
+            # Confirm that the sphere's world transform has not changed.  Must
+            # re-create the item, as its path has changed.
+            sphereChildPath = ufe.Path(
+                [shapeSegment, usdUtils.createUfePathSegment("/pSphere1")])
+            sphereChildItem = ufe.Hierarchy.createItem(sphereChildPath)
+            sphereChildT3d = ufe.Transform3d.transform3d(sphereChildItem)
+
+            sphereWorld = sphereChildT3d.inclusiveMatrix()
+            assertVectorAlmostEqual(
+                self, sphereWorldListPre, matrixToList(sphereWorld), places=6)
+
+            # Undo.
+            cmds.undo()
+
+            shapeChildren = shapeHier.children()
+            self.assertNotIn("pSphere1", childrenNames(shapeChildren))
 
-        # Confirm that the sphere's world transform has not changed.  Must
-        # re-create the item, as its path has changed.
-        sphereChildPath = ufe.Path(
-            [shapeSegment, usdUtils.createUfePathSegment("/pSphere1")])
-        sphereChildItem = ufe.Hierarchy.createItem(sphereChildPath)
-        sphereChildT3d = ufe.Transform3d.transform3d(sphereChildItem)
+    @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2013', 'testIllegalChild only available in UFE preview version 0.2.13 and greater')
+    def testIllegalChild(self):
+        '''Parenting an object to a descendant must report an error.'''
 
-        sphereWorld = sphereChildT3d.inclusiveMatrix()
-        assertVectorAlmostEqual(
-            self, sphereWorldListPre, matrixToList(sphereWorld), places=6)
+        with OpenFileCtx("simpleHierarchy.ma"):
+            with self.assertRaises(RuntimeError):
+                cmds.parent(
+                    "|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1",
+                    "|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1/pCube1")
+
+    @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '2016', 'testAlreadyChild only available in UFE preview version 0.2.16 and greater')
+    def testAlreadyChild(self):
+        '''Parenting an object to its current parent is a no-op.'''
+
+        with OpenFileCtx("simpleHierarchy.ma"):
+            shapeSegment = mayaUtils.createUfePathSegment(
+                "|world|mayaUsdProxy1|mayaUsdProxyShape1")
+            spherePath = ufe.Path(
+                [shapeSegment,
+                 usdUtils.createUfePathSegment("/pCylinder1/pCube1/pSphere1")])
+            sphereItem = ufe.Hierarchy.createItem(spherePath)
+            cylinderShapePath = ufe.Path(
+                [shapeSegment,
+                 usdUtils.createUfePathSegment("/pCylinder1/pCylinderShape1")])
+            cylinderShapeItem = ufe.Hierarchy.createItem(cylinderShapePath)
+            parentPath = ufe.Path(
+                [shapeSegment, usdUtils.createUfePathSegment("/pCylinder1")])
+            parentItem = ufe.Hierarchy.createItem(parentPath)
+
+            parent = ufe.Hierarchy.hierarchy(parentItem)
+            childrenPre = parent.children()
+
+            # The sphere is not a child of the cylinder
+            self.assertNotIn("pSphere1", childrenNames(childrenPre))
+
+            # The cylinder shape is a child of the cylinder
+            self.assertIn("pCylinderShape1", childrenNames(childrenPre))
+
+            # Parent the sphere and the cylinder shape to the cylinder.  This
+            # is a no-op for the cylinder shape, as it's already a child of the
+            # cylinder, and the sphere is parented to the cylinder.
+            cmds.parent(ufe.PathString.string(spherePath),
+                        ufe.PathString.string(cylinderShapePath),
+                        ufe.PathString.string(parentPath))
+
+            children = parent.children()
+            self.assertEqual(len(childrenPre)+1, len(children))
+            self.assertIn("pSphere1", childrenNames(children))
+            self.assertIn("pCylinderShape1", childrenNames(children))
+
+            # Undo / redo
+            cmds.undo()
 
-        # Undo.
-        cmds.undo()
+            children = parent.children()
+            self.assertEqual(len(childrenPre), len(children))
+            self.assertNotIn("pSphere1", childrenNames(children))
+            self.assertIn("pCylinderShape1", childrenNames(children))
 
-        shapeChildren = shapeHier.children()
-        self.assertNotIn("pSphere1", childrenNames(shapeChildren))
+            cmds.redo()
 
-        # Close the file.
-        cmds.file(force=True, new=True)
+            children = parent.children()
+            self.assertEqual(len(childrenPre)+1, len(children))
+            self.assertIn("pSphere1", childrenNames(children))
+            self.assertIn("pCylinderShape1", childrenNames(children))