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

Jump to bottom

MAYA-110706: Block attribute edits for the remaining transformation stacks (UsdTransform3dCommonAPI, UsdTransform3dMatrixOp) #1308

Merged

kxl-adsk merged 6 commits into dev from sabrih/MAYA-110706/block_attribute_edit_commonapi_3dmatrixop

Apr 8, 2021

Contributor

HamedSabri-adsk commented Apr 6, 2021

This PR completes the blocking of attribute edits for the remaining transformation stacks ( UsdTransform3dCommonAPI, UsdTransform3dMatrixOp )


          MAYA-110706: This PR completes the blocking of attribute edits for th…

…e remaining transformation stacks ( UsdTransform3dCommonAPI, UsdTransform3dMatrixOp )

HamedSabri-adsk added core workflows labels


          Remove the scope resolution operator.

b77942f

HamedSabri-adsk commented

View reviewed changes

lib/mayaUsd/ufe/UsdTransform3d.cpp

-                  return true;
-              }
               } // namespace

Contributor Author

HamedSabri-adsk Apr 6, 2021

Re-factored isAttributeEditAllowed(const PXR_NS::UsdPrim& prim, const std::string& tokenName) and moved it to lib/mayaUsd/ufe/Utils.cpp to avoid code duplication.

HamedSabri-adsk commented

View reviewed changes

lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp

+                  if (!isAttributeEditAllowed(prim(), "xformOp:translate:pivot")) {
+                      return nullptr;
+                  }
                   return std::make_shared<UsdTranslatePivotUndoableCmd>(usdSceneItem(), UsdTimeCode::Default());
               }

Contributor Author

HamedSabri-adsk Apr 6, 2021

Blocking support for UsdTransform3dCommonAPI.

HamedSabri-adsk commented

View reviewed changes

lib/mayaUsd/ufe/UsdTransform3dMatrixOp.cpp

+                  if (!isAttributeEditAllowed(prim(), "xformOp:scale")) {
+                      return nullptr;
+                  }

Contributor Author

HamedSabri-adsk Apr 6, 2021

Blocking support for UsdTransform3dMatrixOp.cpp.

HamedSabri-adsk commented

View reviewed changes

test/lib/ufe/testAttributeBlock.py Show resolved Hide resolved

HamedSabri-adsk requested a review from ppt-adsk

April 6, 2021 14:29

HamedSabri-adsk assigned ecp-maya-devops-adsk


          MAYA-110706: add test coverage for attribute blocking with UsdTransfo…

bba1493

…rm3dMatrixOp.

HamedSabri-adsk unassigned ecp-maya-devops-adsk

HamedSabri-adsk commented

View reviewed changes

test/lib/ufe/testAttributeBlock.py


		# authoring new transformation edit is allowed.
		self.assertTrue(mayaUsdUfe.isAttributeEditAllowed(transformAttr))

Contributor Author

HamedSabri-adsk Apr 6, 2021

@ppt-adsk Unit test to cover attribute blocking using UsdTransform3dMatrixOp.

HamedSabri-adsk assigned ecp-maya-devops-adsk

HamedSabri-adsk commented

View reviewed changes

test/lib/ufe/testAttributeBlock.py

Comment on lines +298 to +304

+                      # create a xformable and give it a matrix4d xformOp:transform
+                      # This will match the UsdTransform3dMatrixOp, but not the Maya API.
+                      cylinderXformable = UsdGeom.Xformable(cylinderPrim)
+                      transformOp = cylinderXformable.AddTransformOp()
+                      xform = Gf.Matrix4d(1.0).SetTranslate(Gf.Vec3d(10, 20, 30))
+                      transformOp.Set(xform)
+                      self.assertTrue(cylinderPrim.GetPrim().HasAttribute("xformOp:transform"))

Contributor Author

HamedSabri-adsk Apr 6, 2021

Line 298 to 304: I create a matrix4d xformOp:transform with the initial translate values set to [10, 20, 30] to match the 3dMatrixOp.

HamedSabri-adsk commented

View reviewed changes

test/lib/ufe/testAttributeBlock.py

Comment on lines +321 to +332

+                      cmds.move(65, 20, 10)
+                      self.assertEqual(cylinderXformable.GetXformOpOrderAttr().Get(),
+                                  Vt.TokenArray(('xformOp:transform', )))
+                      # validate the Matrix4d entires
+                      actual = cylinderXformable.GetLocalTransformation()
+                      expected  = Gf.Matrix4d( 1, 0, 0, 0,
+, 1, 0, 0,
+, 0, 1, 0,
+, 20, 10, 1)
+                      self.assertTrue(Gf.IsClose(actual, expected, 1e-5))

Contributor Author

HamedSabri-adsk Apr 6, 2021

Line 321 to 332: Set the translate to a new value [65, 20, 10] and check the 4x4 affine matrix with the newly translate values.

HamedSabri-adsk commented

View reviewed changes

test/lib/ufe/testAttributeBlock.py

Comment on lines +335 to +348

+                      cmds.mayaUsdEditTarget(proxyShapePath, edit=True, editTarget=subLayerC)
+                      # check if transform attribute is editable
+                      transformAttr = cylinderPrim.GetAttribute('xformOp:transform')
+                      self.assertIsNotNone(transformAttr)
+                      # authoring new transformation edit is not allowed.
+                      self.assertFalse(mayaUsdUfe.isAttributeEditAllowed(transformAttr))
+                      # set the edit target to a stronger layer (LayerA)
+                      cmds.mayaUsdEditTarget(proxyShapePath, edit=True, editTarget=subLayerA)
+                      # authoring new transformation edit is allowed.
+                      self.assertTrue(mayaUsdUfe.isAttributeEditAllowed(transformAttr))

Contributor Author

HamedSabri-adsk Apr 6, 2021

Line 335 to 348: tests blocking attribute authoring in weaker or stronger layers for the xformOp:transform.

ppt-adsk requested changes

View reviewed changes

Collaborator

ppt-adsk left a comment

Great test! Just a couple of minor details and we're done.

lib/mayaUsd/ufe/Utils.cpp Outdated

                   return true;
               }
+              bool isAttributeEditAllowed(const PXR_NS::UsdPrim& prim, const std::string& tokenName)

Collaborator

ppt-adsk Apr 6, 2021

Instead of taking a string argument and converting to a TfToken for the call to GetAttribute(), we should probably take a const TfToken& directly.

lib/mayaUsd/ufe/Utils.cpp Outdated

+                  // check for xformOp:tokenName in XformOpOrderAttr first
+                  UsdGeomXformable xformable(prim);
+                  if (!isAttributeEditAllowed(xformable.GetXformOpOrderAttr(), &errMsg)) {

Collaborator

ppt-adsk Apr 6, 2021

This feels funky --- we're testing if attribute editing is allowed on the xformOpOrder attribute for any attribute. However, for non xform op attributes, this would be incorrect. We should only be testing xformOpOrder if the attribute is an xformOp --- can use UsdGeomXformOp::IsXformOp(nameToken) for that.

Contributor Author

HamedSabri-adsk Apr 7, 2021

@ppt-adsk Great point.

Addressed in 1855f97

lib/mayaUsd/ufe/Utils.cpp

+                  std::string errMsg;
+                  // check for xformOp:tokenName in XformOpOrderAttr first
+                  UsdGeomXformable xformable(prim);

Collaborator

ppt-adsk Apr 6, 2021

What if the prim doesn't support the UsdGeomXformable schema? We should check for that.

lib/mayaUsd/ufe/Utils.h Outdated

               MAYAUSD_CORE_PUBLIC
               bool isAttributeEditAllowed(const PXR_NS::UsdAttribute& attr, std::string* errMsg = nullptr);
+              MAYAUSD_CORE_PUBLIC
+              bool isAttributeEditAllowed(const PXR_NS::UsdPrim& prim, const std::string& tokenName);

Collaborator

ppt-adsk Apr 6, 2021

As per comment in .cpp file, should take a const TfToken& directly, instead of converting to string.

lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp Outdated

		@@ -19,6 +19,8 @@

		#include <pxr/usd/usdGeom/xformCache.h>

		#include <maya/MGlobal.h>

Collaborator

ppt-adsk Apr 6, 2021

Don't think this one is needed.

Contributor Author

HamedSabri-adsk Apr 7, 2021

Addressed in 0164510

lib/mayaUsd/ufe/UsdTransform3dCommonAPI.cpp Outdated

               Ufe::TranslateUndoableCommand::Ptr
               UsdTransform3dCommonAPI::translateCmd(double x, double y, double z)
               {
+                  if (!isAttributeEditAllowed(prim(), "xformOp:translate")) {

Collaborator

ppt-adsk Apr 6, 2021

Should pass a TfToken.

Hamed Sabri added 2 commits

April 7, 2021 07:19


          Address feedback.

1855f97


          Get rid off the unused headers.

HamedSabri-adsk requested a review from ppt-adsk

April 7, 2021 11:28

Contributor Author

HamedSabri-adsk commented Apr 7, 2021

I noticed last night that testAttributeBlocking3dMatrixOps with UFE1.0 failed which came as a surprise. I haven't looked into detail just yet.

13: Plug-in, "ufeSupport", was not found on MAYA_PLUG_IN_PATH.
13:
13: Unable to load ufeSupport
13: Warning: This file is from an older version of Maya.  If saved, it will not be readable by previous versions.
13: File read in  0.63 seconds.
13: .Clearing USD Stage Cache
13: Clearing USD Stage Cache
13: Warning: This file is from an older version of Maya.  If saved, it will not be readable by previous versions.
13: File read in  0.019 seconds.
13: Warning: Could not determine xform ops for incompatible xformable </pCylinder1>
13: Failed to translate prim |world|mayaUsdTransform|shape/pCylinder1 - Incompatible xform op xformOp:transform:
13:     File: C:\Users\sabrih\Desktop\maya-usd\lib\mayaUsd\ufe\private\Utils.cpp
13:     Line: 212
13:     Function: MayaUsd_v0::ufe::translateOp
13:Clearing USD Stage Cache

13: ======================================================================
13: ERROR: testAttributeBlocking3dMatrixOps (testAttributeBlock.AttributeBlockTestCase)
13: ----------------------------------------------------------------------
13: Traceback (most recent call last):
13:   File "testAttributeBlock.py", line 321, in testAttributeBlocking3dMatrixOps
13:     cmds.move(65, 20, 10)
13: RuntimeError: Maya command error

Collaborator

ppt-adsk commented Apr 7, 2021

I noticed last night that testAttributeBlocking3dMatrixOps with UFE1.0 failed which came as a surprise. I haven't looked into detail just yet.

13: Plug-in, "ufeSupport", was not found on MAYA_PLUG_IN_PATH.
13:
13: Unable to load ufeSupport
13: Warning: This file is from an older version of Maya.  If saved, it will not be readable by previous versions.
13: File read in  0.63 seconds.
13: .Clearing USD Stage Cache
13: Clearing USD Stage Cache
13: Warning: This file is from an older version of Maya.  If saved, it will not be readable by previous versions.
13: File read in  0.019 seconds.
13: Warning: Could not determine xform ops for incompatible xformable </pCylinder1>
13: Failed to translate prim |world|mayaUsdTransform|shape/pCylinder1 - Incompatible xform op xformOp:transform:
13:     File: C:\Users\sabrih\Desktop\maya-usd\lib\mayaUsd\ufe\private\Utils.cpp
13:     Line: 212
13:     Function: MayaUsd_v0::ufe::translateOp
13:Clearing USD Stage Cache

13: ======================================================================
13: ERROR: testAttributeBlocking3dMatrixOps (testAttributeBlock.AttributeBlockTestCase)
13: ----------------------------------------------------------------------
13: Traceback (most recent call last):
13:   File "testAttributeBlock.py", line 321, in testAttributeBlocking3dMatrixOps
13:     cmds.move(65, 20, 10)
13: RuntimeError: Maya command error

Ack, I should have realized this and told you about it. UFE v1 supports the common transform API only, so you can't run the matrix transform op test for UFE v1 --- you'll have to skip it under those circumstances.

ppt-adsk requested changes

View reviewed changes

lib/mayaUsd/ufe/Utils.cpp Outdated

               {
-                  std::string errMsg;
+                  if (prim.IsA<UsdGeomXformable>()) {

Collaborator

ppt-adsk Apr 7, 2021

As you have it now, the code is UsdGeomXformable centric, and it doesn't need to be --- we currently return true if it's not a UsdGeomXformable. Seems to me we could have something like:

UsdGeomXformable xformable(prim);
if (xformable) { // bool operator on schema returns true if valid
if (UsdGeomXformOp::IsXformOp(attrName) {
// Check xformOpOrder
}
}
// Then check the attribute itself.

Contributor Author

HamedSabri-adsk Apr 7, 2021

@ppt-adsk 0b42a42

lib/mayaUsd/ufe/Utils.cpp Outdated

                           MGlobal::displayError(errMsg.c_str());
                           return false;
+                      } else {
+                          // check for xformOp:tokenName
+                          if (UsdGeomXformOp::IsXformOp(attrName)) {

Collaborator

ppt-adsk Apr 7, 2021

I think this test should be done to gate whether we do the xformOpOrder check: if the attrName is an xform op, do the extra check, otherwise just do a single test on the attribute itself.


          address feedback.

0b42a42

HamedSabri-adsk requested a review from ppt-adsk

April 7, 2021 17:52

HamedSabri-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk

ppt-adsk approved these changes

View reviewed changes

HamedSabri-adsk added the ready-for-merge label

kxl-adsk merged commit ad3d0d8 into dev

kxl-adsk deleted the sabrih/MAYA-110706/block_attribute_edit_commonapi_3dmatrixop branch

April 8, 2021 10:46

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core ready-for-merge workflows