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.

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

Fix reparenting/renaming of pulled objects USD ancestor #3861

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

jufrantz
Copy link
Contributor

Hello,

This PR addresses an issue where pulled Maya objects transform and visibility were disconnected from the USD stages after renaming or reparenting a USD ancestor of the pulled prim. Our artists very frequently have multiple pulled objects in the same stage while re-structuring the hierarchy.

Before this fix:
beforeFixPullParentAncestor

Ater this fix:
afterFixPullParentAncestor

Changes included in this PR:

  • Added the ability to cleanly reparent a DAG to a USD item using mayaUsd.lib.proxyAccessor, ensuring it does not leave disconnected proxyAccessor outputs, potentially accessing invalid prims: a25dcd6 and unit-test 415dcb6.
  • Small refactoring of PrimUpdaterManager to mutualize the mayaUsd.lib.proxyAccessor.parent python call: dc89563.
  • Added the proxyAccessor update in OrphanedNodeManager, following the pullInformation update: 2969283.
  • Modified CMake targets to define the HAS_ORPHANED_NODES_MANAGER env to add a unit test in testEditAsMaya.py. Disabled a failing test. It was failing without my changes with Maya2023 + USD-23.11 and Maya2024 + USD-23.11. The CMake/test changes: 243eecd. The new test: 1a93630.

--
Julien

Adds `unparentItems` to correctly un-parent a UFE child from its parent.
- Disconnects the parent source attributes.
- Resets `offsetParentMatrix` and `lodVisibility` to their defaults.
- Removes proxyAccessor outputs that are not in use anymore.

Introduces a `force` flag to proxyAccessor.parent.
- When enabled, the child will be correctly unparented before being
  re-parented.
- When disabled, by default, the action will issue an error, as before.
…S_MANAGER.

However, continue skipping `testReparentAncestorOfEditAsMaya` as it still fails even with orphanedNodesManager.
…after reparenting/renaming its USD ancestors.

The proxyAccessor parenting connection was not updated with the new USD parent.
The pulled maya object was still inheriting the old USD parent xform and visibility.
@seando-adsk
Copy link
Collaborator

@jufrantz Thanks for the PR. We will discuss internally and get it reviewed. In the meantime, I also noticed that the testReparentAncestorOfEditAsMaya wasn't being run because of the missing env var. I have some local changes to add that env and fix the failing test.

Can you try re-enabling the test and adding this code to see if it fixes that test.

index 030e780ad..61b07ce62 100644
--- a/test/lib/mayaUsd/fileio/testEditAsMaya.py
+++ b/test/lib/mayaUsd/fileio/testEditAsMaya.py
@@ -152,11 +152,18 @@ class EditAsMayaTestCase(unittest.TestCase):

         # Note: the parent command changes the selection, so preserve and restore it.
         selToPreserve = ufe.GlobalSelection.get().front()
+        self.assertTrue(mayaUsd.lib.PrimUpdaterManager.discardEdits("A"))
         cmds.parent("stage1", locName)
         ufe.GlobalSelection.get().clear()
         ufe.GlobalSelection.get().append(selToPreserve)

+        # Edit "A" Prim as Maya data.
         aUsdUfePathStr = "|" + locName + aUsdUfePathStr
+        with mayaUsd.lib.OpUndoItemList():
+            self.assertTrue(mayaUsd.lib.PrimUpdaterManager.canEditAsMaya(aUsdUfePathStr))
+            self.assertTrue(mayaUsd.lib.PrimUpdaterManager.editAsMaya(aUsdUfePathStr))
+            validateEditAsMayaMetadata()
+
         validateEditAsMayaMetadata()

Thanks, Sean

@jufrantz
Copy link
Contributor Author

Worked like a charm, added 2dfdf46.
Thank you @seando-adsk

@seando-adsk seando-adsk added the workflows Related to in-context workflows label Aug 7, 2024
@@ -244,6 +253,50 @@ def connectParentChildAttr(parentAttr, childDagPath, attrName, connect):
else:
cmds.disconnectAttr(parentAttr, childAttr)

def unparentItems(ufeChildren):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not knowledgeable on proxy accessor, so I'll assume all this is fine :)

Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk left a comment

Choose a reason for hiding this comment

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

Great work!

@seando-adsk
Copy link
Collaborator

@jufrantz I ran the preflight for you and unfortunately it failed during testing testEditAsMaya:

======================================================================
FAIL: testReparentUsdAncestorOfEditAsMaya (testEditAsMaya.EditAsMayaTestCase)
Test that reparenting an usd ancestor correctly updates the internal data.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "S:\jenkins\workspace\ECP\ecg-maya-usd\ecg-mayausd-branch-preflight-2024-python3-windows\ecg-maya-usd\maya-usd\test\lib\mayaUsd\fileio\testEditAsMaya.py", line 210, in testReparentUsdAncestorOfEditAsMaya
    verifyDagXformAndVis(editedDagPath, aUsdXlation, True)
  File "S:\jenkins\workspace\ECP\ecg-maya-usd\ecg-mayausd-branch-preflight-2024-python3-windows\ecg-maya-usd\maya-usd\test\lib\mayaUsd\fileio\testEditAsMaya.py", line 190, in verifyDagXformAndVis
    assertVectorAlmostEqual(self, mXlation, expectedXlation)
  File "S:\jenkins\workspace\ECP\ecg-maya-usd\ecg-mayausd-branch-preflight-2024-python3-windows\ecg-maya-usd\maya-usd\test\testUtils\testUtils.py", line 37, in assertVectorAlmostEqual
    testCase.assertAlmostEqual(va, vb, places)
AssertionError: 0.0 != 1.0 within 7 places (1.0 difference)

@jufrantz
Copy link
Contributor Author

Thank you @seando-adsk and @pierrebai-adsk for the review.

I retested on my end, did a clean checkout of the branch, and ran build.py with --stages configure build install test. The testEditAsMaya tests passed successfully in the following configurations:

  • CentOS 7.8, Maya 2023.update3.2-pfix28, USD 23.11
  • Rocky 8.8, Maya 2024-update2-pfix10.0, USD 23.11
        Start  95: testEditAsMaya
 95/268 Test  #95: testEditAsMaya ..................................   Passed    6.49 sec

There may be something in my environment or workflow causing this discrepancy. This is quite suspicious! I will investigate further to try to reproduce the preflight failures. If you have any insights, I will greatly appreciate them.

@seando-adsk
Copy link
Collaborator

One thing I noticed is that for your testing you are using USD v23.11 for both your tests with Maya 2023 and 2024. In our preflight we use USD v21.11 for Maya 2023 and USD v22.11 with Maya 2024. Not sure if that makes a difference. We include the full USD build in our MayaUsd installers, so you can build with that (first extracting devkit.zip). That way you'll exactly match our internal builds.

Sean

@jufrantz
Copy link
Contributor Author

OK thanks. I built using USD extracted from mayausd-0.29 release packages. Unfortunately, testEditAsMaya is still passing on my end with both Maya 2023.3 + USD 21.11 and Maya 2024.2 + USD 22.11.
I will continue investigating.

@jufrantz
Copy link
Contributor Author

I have a strong lead. After syncing my branch with Autodesk/dev, I’m now encountering the same test failure, which suggests that the combined changes might not work together.
Does the build preflight automatically perform this merge?

@seando-adsk
Copy link
Collaborator

Yes, preflight merges your branch against latest dev.

@jufrantz
Copy link
Contributor Author

jufrantz commented Aug 20, 2024

Hello @seando-adsk,

It appears that the reason for the failure is very likeky an unrelated issue in Autodesk/dev, which has been occurring since PR #3881, commit b9d928d.

The new zealous testEditAsMaya case fails when verifying that the edited Maya worldspace xform matches USD, right after the editAsMaya operation. The pulled transform has inheritsTransform attrib set to False, which seems incorrect since the USD prim does not have a resetXformStack op.

Here is the current behaviour in dev:

maya-usd_inheritXform_issue

And a scripted repro:

import maya.cmds
import mayaUsd.ufe
import mayaUsd_createStageWithNewLayer

maya.cmds.loadPlugin("mayaUsdPlugin", quiet=True)

proxyShape = mayaUsd_createStageWithNewLayer.createStageWithNewLayer()
shapeStage = mayaUsd.ufe.getStage(proxyShape)

shapeStage.GetRootLayer().ImportFromString('''#sdf 1
def Xform "transform1"
{
    double3 xformOp:translate = (0, 10, 0)
    uniform token[] xformOpOrder = ["xformOp:translate"]

    def Mesh "pCube1"
    {
        int[] faceVertexCounts = [4, 4, 4, 4, 4, 4]
        int[] faceVertexIndices = [0, 1, 3, 2, 2, 3, 5, 4, 4, 5, 7, 6, 6, 7, 1, 0, 1, 7, 5, 3, 6, 0, 2, 4]
        point3f[] points = [(-1, -1, 1), (1, -1, 1), (-1, 1, 1), (1, 1, 1), (-1, 1, -1), (1, 1, -1), (-1, -1, -1), (1, -1, -1)]
    }
}
''')

maya.cmds.mayaUsdEditAsMaya("|stage1|stageShape1,/transform1")

If this issue is confirmed, we'll need to wait for it to be fixed before this PR can pass all checks. Please let me know if there’s anything else I can do.

Cheers,
Julien

@samuelliu-adsk
Copy link
Collaborator

Hi @jufrantz , thank you for your investigation. I'm taking a look at that change

@jufrantz
Copy link
Contributor Author

@samuelliu-adsk @seando-adsk,
Are there any updates on the issue blocking the new tests? Beyond this PR, editAsMaya has a major defect in dev and release/v0.30.0 branches. It now incorrectly imports all xformable prims w/o local transformations as inheritsTransform=False in Maya, as seen in the scene from my previous example. This is a very common case, the issue will appear in many production scenes.
Julien

@jufrantz
Copy link
Contributor Author

@seando-adsk,
I tested with latest dev changes and the testEditAsMaya now passes. PR is ready for merge and should pass preflight.
I hope it arrives in time for v0.30.

@pierrebai-adsk pierrebai-adsk assigned jufrantz and unassigned jufrantz Sep 17, 2024
@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Sep 18, 2024
@seando-adsk seando-adsk merged commit 6925a50 into Autodesk:dev Sep 18, 2024
13 of 15 checks passed
@jufrantz jufrantz deleted the fixReparentPulledUsdAncestor branch September 18, 2024 15:16
@seando-adsk
Copy link
Collaborator

@jufrantz Unfortunately it was too late for the v0.30 release, as we had already locked that one down. But as you can see I have merged this PR now so it should be in our next release after v0.30.

@jufrantz
Copy link
Contributor Author

No problem for us, we'll patch v0.30 on our side. Thank you for the information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge workflows Related to in-context workflows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants