MAYA-122218 showing dactivated children #2265

pierrebai-adsk · 2022-04-07T17:17:46Z

Some code paths were ignoring deactivated children when they should not because hasChildren() was ignoring deactivated children.

…iour

seando-adsk · 2022-04-07T18:36:26Z

I'm not sure it is that simple. Did you look at the history of this file (and the pulledObjectHierarchy)? We (Krystian and I) have modified this code a few times to fix different bugs. Did you test that those scenarios (from prior bugs) are still fixed with these changes?

Reverted the children behaviour to not return deactivated items. Only made hasChildren() consider deactivated children.

pierrebai-adsk · 2022-04-07T19:54:41Z

I looked at the most recent changes through git file history, like MAYA 113820 and MAYA-121568

lib/mayaUsd/ufe/UsdHierarchy.cpp

Proxy shape has duplicated code that also need the fix. Also put back in place the old code form in children() to minimize the diff.

Maya 2022 desctivate the prim when deleting, but 2023 and later permanently deletes them.

pierrebai-adsk · 2022-04-08T17:06:28Z

Only failure is the PF on OSX python 2 Maya 2020 failed to launch, but all others passed.

pierrebai-adsk · 2022-04-12T17:13:56Z

Th eonyl PF failure is the OSX python build that constantly fails to start.

seando-adsk · 2022-04-12T17:44:53Z

test/lib/ufe/testContextOps.py

+        # The proxy shape should now have no UFE child items (since we skip inactive)
+        # but hasChildren still reports true for inactive to allow the caller to then
+        # do conditional inactive filtering.
+        if mayaUtils.mayaMajorVersion() >= 2023:


I'm confused why this check is against a Maya version. What change is there in Maya that is affecting this?

I'm confused as to why we changed the test at all.

The behaviour of the delete command vs prim changed from deactibvating to really deleting. When deactivated, hasChildren() returns true.

Okay then instead of using then the Maya test should also surround lines 363 (which does the delete) and the comment you wrote here should be added to the test. Then for Maya 2023 we should not use delete, but instead deactivate the prim.

lib/mayaUsd/ufe/ProxyShapeHierarchy.cpp

ppt-adsk · 2022-04-12T19:46:03Z

test/lib/ufe/testContextOps.py

+        # The proxy shape should now have no UFE child items (since we skip inactive)
+        # but hasChildren still reports true for inactive to allow the caller to then
+        # do conditional inactive filtering.
+        if mayaUtils.mayaMajorVersion() >= 2023:


I'm confused as to why we changed the test at all.

Need to detect patch level 4.

seando-adsk · 2022-04-19T14:09:06Z

lib/mayaUsd/ufe/ProxyShapeHierarchy.cpp

@@ -133,6 +133,8 @@ const UsdPrim& ProxyShapeHierarchy::getUsdRootPrim() const

 Ufe::SceneItem::Ptr ProxyShapeHierarchy::sceneItem() const { return fItem; }

+#if defined(UFE_V4_FEATURES_AVAILABLE) && (UFE_MINOR_VERSION > 4 || UFE_PATCH_LEVEL > 3)


This should just use a single test like this:

Suggested change

#if defined(UFE_V4_FEATURES_AVAILABLE) && (UFE_MINOR_VERSION > 4 || UFE_PATCH_LEVEL > 3)

#if (UFE_PREVIEW_VERSION_NUM >= 4004)

seando-adsk · 2022-04-19T14:12:07Z

lib/mayaUsd/ufe/PulledObjectHierarchy.h

+#if defined(UFE_V4_FEATURES_AVAILABLE) && (UFE_MINOR_VERSION > 4 || UFE_PATCH_LEVEL > 3)
+    bool hasFilteredChildren(const ChildFilter&) const override;
+#endif
+    UFE_V2(Ufe::SceneItemList filteredChildren(const ChildFilter&) const override;)


Why did you add the UFE_V2 here? It is not needed as this file is only included in Ufe v3

Ah, I did not realize it the entire file was already a UFE-3 only. It's just that other headers implementing the same interface had this UFE_V2 conditional, so I thought it was missing in this one. I got confused by the fact some implementations of the UFE Hierarchy interface use the UFE_V2 macro, others don't.

seando-adsk · 2022-04-19T14:15:36Z

test/lib/ufe/testContextOps.py

+        # The proxy shape should now have no UFE child items (since we skip inactive)
+        # but hasChildren still reports true for inactive to allow the caller to then
+        # do conditional inactive filtering.
+        if mayaUtils.mayaMajorVersion() >= 2023:


Okay then instead of using then the Maya test should also surround lines 363 (which does the delete) and the comment you wrote here should be added to the test. Then for Maya 2023 we should not use delete, but instead deactivate the prim.

seando-adsk · 2022-04-19T14:17:11Z

test/lib/ufe/testChildFilter.py

@@ -92,7 +92,7 @@ def testFilteredChildren(self):
        ball3Prim = mayaUsd.ufe.ufePathToPrim(ball3PathStr)
        ball3Prim.SetActive(False)

-        # Props should now have 5 children and ball3 should not be one of them.
+        # Props children should have 5 children and ball3 should not be one of them.


Why did you remove "now". The comment was correct. At line 84 we test that there are 6 children and now we are testing that there are 5.

I took multiple passes to update the test and did not realized I dropped a word there.

- Simplified the #if used to detect the UFE version. - Removed the UFE_V2 that was not needed. - Simplified the test.

ppt-adsk · 2022-04-19T18:14:58Z

lib/mayaUsd/ufe/PulledObjectHierarchy.cpp

 bool PulledObjectHierarchy::hasChildren() const { return _mayaHierarchy->hasChildren(); }

 Ufe::SceneItemList PulledObjectHierarchy::children() const { return _mayaHierarchy->children(); }

+#ifdef UFE_V2_FEATURES_AVAILABLE


The entire file is UFE v3 (in CMakeLists.txt), so this ifdef is redundant.

ppt-adsk · 2022-04-19T18:16:10Z

test/lib/ufe/testContextOps.py

@@ -358,17 +358,20 @@ def testAddNewPrimWithDelete(self):
        self.assertEqual(len(proxyShapehier.children()), 1)

        # Using UFE, delete this new prim (which doesn't actually delete it but
-        # instead makes it inactive).
+        # instead makes it inactive). In Maya 2023 and vreater, delete really


"vreater" --> "greater"

ppt-adsk · 2022-04-19T18:17:24Z

test/lib/ufe/testContextOps.py

+            item = usdUtils.getPrimFromSceneItem(ufeItem)
+            item.SetActive(False)
+        else:
+            cmds.delete()

        # The proxy shape should now have no UFE child items (since we skip inactive)
        # but hasChildren still reports true for inactive to allow the caller to then


Comment and code don't match.

- Removed redundant conditional compilation. - Fix context ops test to account for different behavior in different versions.

The behavior of hasChildren is based on the UFE version, not the Maya version, unlike the behaviour of cmds.delete.

…ust 2023.1 that really deletes.

ppt-adsk · 2022-04-20T18:11:57Z

test/testUtils/ufeUtils.py

@@ -105,3 +105,26 @@ def ufeFeatureSetVersion():
    #   v0.2.20 (unreleased preview version 2).
    major = ufe.VersionInfo.getMajorVersion()
    return ufe.VersionInfo.getMinorVersion() if major == 0 else major
+
+def hasMinimumVersion(releaseVersions, unreleasedVersions):


You shouldn't need this. See
https://github.com/Autodesk/maya-usd/pull/2096/files
on the approach we've chosen: we use the Maya preview release version and the UFE preview version to determine whether code should be compiled and tests run. Then, when the next Maya preview release is released, we clean up ALL the preview release and UFE preview release conditions we're put in, and replace them with UFE feature set version checks.

This cleans up the code one preview release at a time so that we don't leave old unreleased version checks in the files.

In this case I till need to differentiate between 0.4.3 and 0.4.4 though, so I'll test that only intstead.

seando-adsk · 2022-04-21T13:31:45Z

test/lib/ufe/testContextOps.py

+        # The proxy shape should now have no UFE child items (since we skip inactive
+        # when returning the children list) but hasChildren still reports true in
+        # UFE version before 0.4.4 for inactive to allow the caller to do conditional
+        # inactive filtering, so we test that hasChildren is true for those versions.
+        ufeVersion = ufeUtils.fullVersion()
+        if (0, 4, 4) <= ufeVersion < (1, 0, 0):
+            self.assertFalse(proxyShapehier.hasChildren())
+        else:
+            self.assertTrue(proxyShapehier.hasChildren())


We have been doing this in a different way that allows us to find and remove these checks at a later date when a new Maya PR ships. Once the Maya PR ships with this new Ufe 0.4.x version we don't need a test on the specific Ufe version anymore - just a check against Ufe v4 is okay. So to find them we always use either the existing ufeFeatureSetVersion function or UFE_PREVIEW_VERSION_NUM and fix/remove them. So I think your test should be something like:
if (os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') > '4004'):
This cmake variable is also set as an env var for all these Ufe tests.

seando-adsk · 2022-04-21T13:32:11Z

test/testUtils/ufeUtils.py

+def fullVersion():
+    '''
+    Return the full UFE version as a tuple of (major, minor, patch),
+    so for example:
+        (0, 4, 4)
+    '''
+    major = ufe.VersionInfo.getMajorVersion()
+    minor = ufe.VersionInfo.getMinorVersion()
+    patch = ufe.VersionInfo.getPatchLevel()
+    return (major, minor, patch)


This should not be needed.

pierrebai-adsk · 2022-04-21T13:33:10Z

The pm;y failure was on Windows 2020 python 2 where a schema test timed out for unknown reason. The test is entirely unrelated to my changes.

MAYA-122218 showing dactivated children

881c1b3

Some code paths were ignoring deactivated children when they should not because hasChildren() was ignoring deactivated children.

pierrebai-adsk assigned ecp-maya-devops-adsk Apr 7, 2022

pierrebai-adsk requested a review from seando-adsk April 7, 2022 17:30

MAYA122218 update the hiearchy unit test to reflect the current behav…

1f19cf7

…iour

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 7, 2022

MAYA-122218 made children not return deactivated items

6397867

Reverted the children behaviour to not return deactivated items. Only made hasChildren() consider deactivated children.

pierrebai-adsk unassigned ecp-maya-devops-adsk Apr 7, 2022

pierrebai-adsk assigned ecp-maya-devops-adsk Apr 7, 2022

seando-adsk reviewed Apr 7, 2022

View reviewed changes

lib/mayaUsd/ufe/UsdHierarchy.cpp Show resolved Hide resolved

MAYA-122218 apply fix to proxy shape

f5c56a2

Proxy shape has duplicated code that also need the fix. Also put back in place the old code form in children() to minimize the diff.

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 8, 2022

MAYA-122218 fix conntext op test in 2022 and earlier

7cdfefe

Maya 2022 desctivate the prim when deleting, but 2023 and later permanently deletes them.

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 8, 2022

pierrebai-adsk requested a review from seando-adsk April 8, 2022 17:06

pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Apr 12, 2022

seando-adsk reviewed Apr 12, 2022

View reviewed changes

seando-adsk previously approved these changes Apr 12, 2022

View reviewed changes

ppt-adsk reviewed Apr 12, 2022

View reviewed changes

pierrebai-adsk added do-not-merge-yet Development is not finished, PR not ready for merge and removed ready-for-merge Development process is finished, PR is ready for merge labels Apr 13, 2022

pierrebai-adsk added 2 commits April 14, 2022 13:55

MAYA-122218 implement new hasFilteredChilden from UFE

e4dbc7a

MAYA-122218 fix detection of UFe version.

ece19c4

Need to detect patch level 4.

pierrebai-adsk dismissed seando-adsk’s stale review via ece19c4 April 14, 2022 20:11

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 14, 2022

MAYA-122218 code format

1e034d1

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 15, 2022

seando-adsk requested changes Apr 19, 2022

View reviewed changes

MAYA-122218 simplify code

23de53b

- Simplified the #if used to detect the UFE version. - Removed the UFE_V2 that was not needed. - Simplified the test.

ppt-adsk requested changes Apr 19, 2022

View reviewed changes

pierrebai-adsk added 3 commits April 20, 2022 09:11

MAYA-122218 fix version checks

ca0eed5

- Removed redundant conditional compilation. - Fix context ops test to account for different behavior in different versions.

MAYA-122218 fix test version check

ab3bdb9

The behavior of hasChildren is based on the UFE version, not the Maya version, unlike the behaviour of cmds.delete.

MAYA-122218 test correct Maya version. It is all 2023 versions, not j…

ab18f84

…ust 2023.1 that really deletes.

ppt-adsk requested changes Apr 20, 2022

View reviewed changes

MAYA-122218 only use UFE pre-releae version for teh check.

b9e2aa8

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 20, 2022

seando-adsk reviewed Apr 21, 2022

View reviewed changes

pierrebai-adsk requested review from ppt-adsk and seando-adsk April 21, 2022 13:32

seando-adsk reviewed Apr 21, 2022

View reviewed changes

MAYA-122218 use UFE version env var.

995a1f4

pierrebai-adsk requested a review from seando-adsk April 21, 2022 13:46

pierrebai-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Apr 21, 2022

ppt-adsk approved these changes Apr 21, 2022

View reviewed changes

pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Apr 21, 2022

seando-adsk approved these changes Apr 21, 2022

View reviewed changes

seando-adsk merged commit 37e43b8 into dev Apr 22, 2022

seando-adsk deleted the t_bailp/MAYA-122218/cancel-edit-break-stage branch April 22, 2022 13:29

seando-adsk added the ufe-usd Related to UFE-USD plugin in Maya-Usd label Apr 22, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MAYA-122218 showing dactivated children #2265

MAYA-122218 showing dactivated children #2265

pierrebai-adsk commented Apr 7, 2022

seando-adsk commented Apr 7, 2022

pierrebai-adsk commented Apr 7, 2022 •

edited

Loading

pierrebai-adsk commented Apr 8, 2022

pierrebai-adsk commented Apr 12, 2022

seando-adsk Apr 12, 2022

ppt-adsk Apr 12, 2022

pierrebai-adsk Apr 12, 2022

seando-adsk Apr 19, 2022

ppt-adsk Apr 12, 2022

seando-adsk Apr 19, 2022

seando-adsk Apr 19, 2022

pierrebai-adsk Apr 19, 2022

seando-adsk Apr 19, 2022

seando-adsk Apr 19, 2022

pierrebai-adsk Apr 19, 2022

ppt-adsk Apr 19, 2022

ppt-adsk Apr 19, 2022

ppt-adsk Apr 19, 2022

ppt-adsk Apr 20, 2022

pierrebai-adsk Apr 20, 2022

seando-adsk Apr 21, 2022

seando-adsk Apr 21, 2022

pierrebai-adsk commented Apr 21, 2022

		@@ -133,6 +133,8 @@ const UsdPrim& ProxyShapeHierarchy::getUsdRootPrim() const

		Ufe::SceneItem::Ptr ProxyShapeHierarchy::sceneItem() const { return fItem; }

		#if defined(UFE_V4_FEATURES_AVAILABLE) && (UFE_MINOR_VERSION > 4 \|\| UFE_PATCH_LEVEL > 3)

	#if defined(UFE_V4_FEATURES_AVAILABLE) && (UFE_MINOR_VERSION > 4 \|\| UFE_PATCH_LEVEL > 3)
	#if (UFE_PREVIEW_VERSION_NUM >= 4004)

MAYA-122218 showing dactivated children #2265

MAYA-122218 showing dactivated children #2265

Conversation

pierrebai-adsk commented Apr 7, 2022

seando-adsk commented Apr 7, 2022

pierrebai-adsk commented Apr 7, 2022 • edited Loading

pierrebai-adsk commented Apr 8, 2022

pierrebai-adsk commented Apr 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pierrebai-adsk commented Apr 21, 2022

pierrebai-adsk commented Apr 7, 2022 •

edited

Loading