Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MAYA-107087: Don't allow parenting to a Gprim. #852

Merged
merged 3 commits into from
Oct 21, 2020
+155 −104
Merged

MAYA-107087: Don't allow parenting to a Gprim. #852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
31d9096
MAYA-107087: Don't allow parenting to a Gprim.
Oct 8, 2020
8292d98
Maya-107087: Adopt to the recent changes in simpleSceneTRS.usda
Oct 21, 2020
6d391ed
Merge branch 'dev' into sabrih/MAYA-107087/block_parenting_gprim_to_g…
Oct 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/mayaUsd/ufe/UsdUndoInsertChildCommand.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include <pxr/usd/sdf/copyUtils.h>
#include <pxr/usd/usd/editContext.h>
#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usdGeom/gprim.h>

#include <mayaUsdUtils/util.h>

Expand Down Expand Up @@ -59,6 +60,17 @@ UsdUndoInsertChildCommand::UsdUndoInsertChildCommand(const UsdSceneItem::Ptr& pa
const auto& childPrim = child->prim();
const auto& parentPrim = parent->prim();

// Don't allow parenting to a Gprim.
// USD strongly discourages parenting of one gprim to another.
// https://graphics.pixar.com/usd/docs/USD-Glossary.html#USDGlossary-Gprim
if (parentPrim.IsA<UsdGeomGprim>()) {
std::string err = TfStringPrintf("Parenting geometric prim [%s] under geometric prim [%s] is not allowed "
"Please parent geometric prims under separate XForms and reparent between XForms.",
childPrim.GetName().GetString().c_str(),
parentPrim.GetName().GetString().c_str());
throw std::runtime_error(err.c_str());
}
Copy link
Contributor Author

@HamedSabri-adsk HamedSabri-adsk Oct 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We throw an error if one tries to parent a Gprim under another.


// Apply restriction rules
ufe::applyCommandRestriction(childPrim, "reparent");
ufe::applyCommandRestriction(parentPrim, "reparent");
Expand Down
64 changes: 39 additions & 25 deletions test/lib/ufe/testParentCmd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import mayaUtils, usdUtils
from testUtils import assertVectorAlmostEqual

from pxr import UsdGeom

import ufe
import mayaUsd.ufe
import maya.cmds as cmds
Expand Down Expand Up @@ -56,7 +58,7 @@ class ParentCmdTestCase(unittest.TestCase):
def setUpClass(cls):
if not cls.pluginsLoaded:
cls.pluginsLoaded = mayaUtils.isMayaUsdPluginLoaded()

@classmethod
def tearDownClass(cls):
cmds.file(new=True, force=True)
Expand All @@ -78,10 +80,10 @@ def testParentRelative(self):
shapeSegment = mayaUtils.createUfePathSegment(
"|world|mayaUsdProxy1|mayaUsdProxyShape1")
cubePath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCube1")])
[shapeSegment, usdUtils.createUfePathSegment("/cubeXform")])
cubeItem = ufe.Hierarchy.createItem(cubePath)
cylinderPath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCylinder1")])
[shapeSegment, usdUtils.createUfePathSegment("/cylinderXform")])
cylinderItem = ufe.Hierarchy.createItem(cylinderPath)

# get the USD stage
Expand All @@ -93,7 +95,7 @@ def testParentRelative(self):
# The cube is not a child of the cylinder.
cylHier = ufe.Hierarchy.hierarchy(cylinderItem)
cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)

# Get the components of the cube's local transform.
cubeT3d = ufe.Transform3d.transform3d(cubeItem)
Expand All @@ -102,32 +104,32 @@ def testParentRelative(self):
cubeS = cubeT3d.scale()

# Parent cube to cylinder in relative mode, using UFE path strings.
cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/pCube1",
"|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1",
cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/cubeXform",
"|mayaUsdProxy1|mayaUsdProxyShape1,/cylinderXform",
relative=True)

# Confirm that the cube is now a child of the cylinder.
cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 1)
self.assertIn("pCube1", childrenNames(cylChildren))
self.assertEqual(len(cylChildren), 2)
self.assertIn("cubeXform", childrenNames(cylChildren))

# Undo: the cube is no longer a child of the cylinder.
cmds.undo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)

# Redo: confirm that the cube is again a child of the cylinder.
cmds.redo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 1)
self.assertIn("pCube1", childrenNames(cylChildren))
self.assertEqual(len(cylChildren), 2)
self.assertIn("cubeXform", childrenNames(cylChildren))

# Confirm that the cube's local transform has not changed. Must
# re-create the item, as its path has changed.
cubeChildPath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCylinder1/pCube1")])
[shapeSegment, usdUtils.createUfePathSegment("/cylinderXform/cubeXform")])
cubeChildItem = ufe.Hierarchy.createItem(cubeChildPath)
cubeChildT3d = ufe.Transform3d.transform3d(cubeChildItem)

Expand Down Expand Up @@ -160,17 +162,18 @@ def testParentRelative(self):
cmds.undo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)


def testParentAbsolute(self):
# Create scene items for the cube and the cylinder.
shapeSegment = mayaUtils.createUfePathSegment(
"|world|mayaUsdProxy1|mayaUsdProxyShape1")
cubePath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCube1")])
[shapeSegment, usdUtils.createUfePathSegment("/cubeXform")])
cubeItem = ufe.Hierarchy.createItem(cubePath)
cylinderPath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCylinder1")])
[shapeSegment, usdUtils.createUfePathSegment("/cylinderXform")])
cylinderItem = ufe.Hierarchy.createItem(cylinderPath)

# get the USD stage
Expand All @@ -182,7 +185,7 @@ def testParentAbsolute(self):
# The cube is not a child of the cylinder.
cylHier = ufe.Hierarchy.hierarchy(cylinderItem)
cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)

# Get its world space transform.
cubeT3d = ufe.Transform3d.transform3d(cubeItem)
Expand All @@ -191,31 +194,31 @@ def testParentAbsolute(self):

# Parent cube to cylinder in absolute mode (default), using UFE
# path strings.
cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/pCube1",
"|mayaUsdProxy1|mayaUsdProxyShape1,/pCylinder1")
cmds.parent("|mayaUsdProxy1|mayaUsdProxyShape1,/cubeXform",
"|mayaUsdProxy1|mayaUsdProxyShape1,/cylinderXform")

# Confirm that the cube is now a child of the cylinder.
cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 1)
self.assertIn("pCube1", childrenNames(cylChildren))
self.assertEqual(len(cylChildren), 2)
self.assertIn("cubeXform", childrenNames(cylChildren))

# Undo: the cube is no longer a child of the cylinder.
cmds.undo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)

# Redo: confirm that the cube is again a child of the cylinder.
cmds.redo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 1)
self.assertIn("pCube1", childrenNames(cylChildren))
self.assertEqual(len(cylChildren), 2)
self.assertIn("cubeXform", childrenNames(cylChildren))

# Confirm that the cube's world transform has not changed. Must
# re-create the item, as its path has changed.
cubeChildPath = ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment("/pCylinder1/pCube1")])
[shapeSegment, usdUtils.createUfePathSegment("/cylinderXform/cubeXform")])
cubeChildItem = ufe.Hierarchy.createItem(cubeChildPath)
cubeChildT3d = ufe.Transform3d.transform3d(cubeChildItem)

Expand All @@ -241,7 +244,7 @@ def testParentAbsolute(self):
cmds.undo()

cylChildren = cylHier.children()
self.assertEqual(len(cylChildren), 0)
self.assertEqual(len(cylChildren), 1)

def testParentToProxyShape(self):

Expand Down Expand Up @@ -470,3 +473,14 @@ def checkUnparent(done):

cmds.redo()
checkUnparent(done=True)

def testParentingToGPrim(self):
'''Parenting an object to UsdGeomGprim object is not allowed'''

# open tree scene
mayaUtils.openTreeScene()

with self.assertRaises(RuntimeError):
cmds.parent("|Tree_usd|Tree_usdShape,/TreeBase/trunk",
"|Tree_usd|Tree_usdShape,/TreeBase/leavesXform/leaves")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a unit test to back the Gprim parenting restriction.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk To make testParentRelative, testParentAbsolute working under the new rule, I created a top level Xforms for cube, cylinder, sphere in "simpleSceneTRS.usda" and made sure the Translate/Rotate/Scale and Order Ops matches the underlying meshes. Github won't show the diff results. You need to use your diff-editor locally to see the changes.

7 changes: 4 additions & 3 deletions test/lib/ufe/testSelection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,14 @@ def runTestSelection(self, selectCmd):
# time, so we select 6 different items, one at a time.
shapeSegment = mayaUtils.createUfePathSegment(
"|world|mayaUsdProxy1|mayaUsdProxyShape1")
names = ["pCube1", "pCylinder1", "pSphere1"]
ufeNames = ["cubeXform", "cylinderXform", "sphereXform"]
mayaNames = ["pCube1", "pCylinder1", "pSphere1"]
usdPaths = []
for n in ["/" + o for o in names]:
for n in ["/" + o for o in ufeNames]:
usdPaths.append(ufe.Path(
[shapeSegment, usdUtils.createUfePathSegment(n)]))
mayaPaths = []
for n in ["|world|" + o for o in names]:
for n in ["|world|" + o for o in mayaNames]:
mayaPaths.append(ufe.Path(mayaUtils.createUfePathSegment(n)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ppt-adsk Adopt to the recent changes in simpleSceneTRS: Separated "maya" and "ufe" names list.


# Create a list of paths by alternating USD objects and Maya objects
Expand Down
Loading