Merge pull request #2852 from Autodesk/degiroa/MAYA-127483/delete-pro…

…perties-when-deleting-connection

MAYA-127483 - Deleting a connection does not delete source and destination properties.

lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp

@@ -68,20 +68,6 @@ UsdAttribute* usdAttrFromUfeAttr(const Ufe::Attribute::Ptr& attr)
     return dynamic_cast<UsdAttribute*>(attr.get());
 }
 
-bool isConnected(const PXR_NS::UsdAttribute& srcUsdAttr, const PXR_NS::UsdAttribute& dstUsdAttr)
-{
-    PXR_NS::SdfPathVector connectedAttrs;
-    dstUsdAttr.GetConnections(&connectedAttrs);
-
-    for (PXR_NS::SdfPath path : connectedAttrs) {
-        if (path == srcUsdAttr.GetPath()) {
-            return true;
-        }
-    }
-
-    return false;
-}
-
 PXR_NS::SdrShaderNodeConstPtr
 _GetShaderNodeDef(const PXR_NS::UsdPrim& prim, const PXR_NS::TfToken& attrName)
 {
@@ -171,7 +157,7 @@ void UsdUndoCreateConnectionCommand::execute()
         return;
     }
 
-    if (isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) {
+    if (MayaUsd::ufe::isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) {
         return;
     }
 
@@ -303,7 +289,7 @@ void UsdUndoDeleteConnectionCommand::execute()
     UsdAttribute* dstUsdAttr = usdAttrFromUfeAttr(dstAttr);
 
     if (!srcUsdAttr || !dstUsdAttr
-        || !isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) {
+        || !MayaUsd::ufe::isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) {
         return;
     }
 
@@ -321,19 +307,12 @@ void UsdUndoDeleteConnectionCommand::execute()
         // Remove attribute if it does not have a value, default value, or time samples. We do this
         // on Shader nodes and on the Material outputs since they are re-created automatically.
         // Other NodeGraph inputs and outputs require explicit removal.
-        if (!dstUsdAttr->usdAttribute().HasValue()) {
-            UsdShadeShader asShader(dstUsdAttr->usdPrim());
-            if (asShader) {
-                dstUsdAttr->usdPrim().RemoveProperty(dstUsdAttr->usdAttribute().GetName());
-            }
-            UsdShadeMaterial asMaterial(dstUsdAttr->usdPrim());
-            if (asMaterial) {
-                const TfToken baseName = dstUsdAttr->usdAttribute().GetBaseName();
-                if (baseName == UsdShadeTokens->surface || baseName == UsdShadeTokens->volume
-                    || baseName == UsdShadeTokens->displacement) {
-                    dstUsdAttr->usdPrim().RemoveProperty(dstUsdAttr->usdAttribute().GetName());
-                }
-            }
+        if (MayaUsd::ufe::canRemoveDstProperty(dstUsdAttr->usdAttribute())) {
+            dstUsdAttr->usdPrim().RemoveProperty(dstUsdAttr->usdAttribute().GetName());
+        }
+
+        if (MayaUsd::ufe::canRemoveSrcProperty(srcUsdAttr->usdAttribute())) {
+            srcUsdAttr->usdPrim().RemoveProperty(srcUsdAttr->usdAttribute().GetName());
         }
     }
 

lib/mayaUsd/ufe/Utils.cpp

@@ -447,6 +447,120 @@ TfTokenVector getProxyShapePurposes(const Ufe::Path& path)
     return purposes;
 }
 
+bool isConnected(const PXR_NS::UsdAttribute& srcUsdAttr, const PXR_NS::UsdAttribute& dstUsdAttr)
+{
+    PXR_NS::SdfPathVector connectedAttrs;
+    dstUsdAttr.GetConnections(&connectedAttrs);
+
+    for (PXR_NS::SdfPath path : connectedAttrs) {
+        if (path == srcUsdAttr.GetPath()) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool canRemoveSrcProperty(const PXR_NS::UsdAttribute& srcAttr)
+{
+
+    // Do not remove if it has a value.
+    if (srcAttr.HasValue()) {
+        return false;
+    }
+
+    PXR_NS::SdfPathVector connectedAttrs;
+    srcAttr.GetConnections(&connectedAttrs);
+
+    // Do not remove if it has connections.
+    if (!connectedAttrs.empty()) {
+        return false;
+    }
+
+    const auto prim = srcAttr.GetPrim();
+
+    if (!prim) {
+        return false;
+    }
+
+    PXR_NS::UsdShadeNodeGraph ngPrim(prim);
+
+    if (!ngPrim) {
+        const auto primParent = prim.GetParent();
+
+        if (!primParent) {
+            return false;
+        }
+
+        // Do not remove if there is a connection with a prim.
+        for (const auto& childPrim : primParent.GetChildren()) {
+            if (childPrim != prim) {
+                for (const auto& attribute : childPrim.GetAttributes()) {
+                    const PXR_NS::UsdAttribute dstUsdAttr = attribute.As<PXR_NS::UsdAttribute>();
+                    if (isConnected(srcAttr, dstUsdAttr)) {
+                        return false;
+                    }
+                }
+            }
+        }
+
+        // Do not remove if there is a connection with the parent prim.
+        for (const auto& attribute : primParent.GetAttributes()) {
+            const PXR_NS::UsdAttribute dstUsdAttr = attribute.As<PXR_NS::UsdAttribute>();
+            if (isConnected(srcAttr, dstUsdAttr)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
+    // Do not remove boundary properties even if there are connections.
+    return false;
+}
+
+bool canRemoveDstProperty(const PXR_NS::UsdAttribute& dstAttr)
+{
+
+    // Do not remove if it has a value.
+    if (dstAttr.HasValue()) {
+        return false;
+    }
+
+    PXR_NS::SdfPathVector connectedAttrs;
+    dstAttr.GetConnections(&connectedAttrs);
+
+    // Do not remove if it has connections.
+    if (!connectedAttrs.empty()) {
+        return false;
+    }
+
+    const auto prim = dstAttr.GetPrim();
+
+    if (!prim) {
+        return false;
+    }
+
+    PXR_NS::UsdShadeNodeGraph ngPrim(prim);
+
+    if (!ngPrim) {
+        return true;
+    }
+
+    UsdShadeMaterial asMaterial(prim);
+    if (asMaterial) {
+        const TfToken baseName = dstAttr.GetBaseName();
+        // Remove Material intrinsic outputs since they are re-created automatically.
+        if (baseName == UsdShadeTokens->surface || baseName == UsdShadeTokens->volume
+            || baseName == UsdShadeTokens->displacement) {
+            return true;
+        }
+    }
+
+    // Do not remove boundary properties even if there are connections.
+    return false;
+}
+
 namespace {
 
 SdfLayerHandle getStrongerLayer(

lib/mayaUsd/ufe/Utils.h

@@ -150,6 +150,21 @@ PXR_NS::UsdTimeCode getTime(const Ufe::Path& path);
 MAYAUSD_CORE_PUBLIC
 PXR_NS::TfTokenVector getProxyShapePurposes(const Ufe::Path& path);
 
+//! Check if the src and dst attributes are connected.
+//! \return True, if they are connected.
+MAYAUSD_CORE_PUBLIC
+bool isConnected(const PXR_NS::UsdAttribute& srcUsdAttr, const PXR_NS::UsdAttribute& dstUsdAttr);
+
+//! Check if a source connection property is allowed to be removed.
+//! \return True, if the property can be removed.
+MAYAUSD_CORE_PUBLIC
+bool canRemoveSrcProperty(const PXR_NS::UsdAttribute& srcAttr);
+
+//! Check if a destination connection property is allowed to be removed.
+//! \return True, if the property can be removed.
+MAYAUSD_CORE_PUBLIC
+bool canRemoveDstProperty(const PXR_NS::UsdAttribute& dstAttr);
+
 #ifdef UFE_V2_FEATURES_AVAILABLE
 MAYAUSD_CORE_PUBLIC
 Ufe::Attribute::Type usdTypeToUfe(const PXR_NS::UsdAttribute& usdAttr);

test/lib/ufe/testConnections.py

@@ -979,5 +979,179 @@ def testCompoundDisplacementPassthrough(self):
         cmd = connectionHandler.createConnectionCmd(compoundDisplacementAttr, materialDisplacementAttr)
         cmd.execute()
 
+    @unittest.skipIf(os.getenv('UFE_PREVIEW_VERSION_NUM', '0000') < '4043', 'Test only available in UFE preview version 0.4.43 and greater since it uses the UsdUndoDeleteConnectionCommand')
+    def testRemovePropertiesWithConnections(self):
+        '''Test delete connections and properties.'''
+
+        # Load a scene.
+
+        testFile = testUtils.getTestScene('properties', 'properties.usda')
+        shapeNode,shapeStage = mayaUtils.createProxyFromFile(testFile)
+
+        ufeParentItem = ufeUtils.createUfeSceneItem(shapeNode,
+            '/mtl/UsdPreviewSurface1')
+        ufePreviewItem = ufeUtils.createUfeSceneItem(shapeNode,
+            '/mtl/UsdPreviewSurface1/UsdPreviewSurface1')
+        ufeSurfaceItem = ufeUtils.createUfeSceneItem(shapeNode,
+            '/mtl/UsdPreviewSurface1/surface1')
+        ufeCompoundItem = ufeUtils.createUfeSceneItem(shapeNode,
+            '/mtl/UsdPreviewSurface1/compound')
+        ufeChildCompoundItem = ufeUtils.createUfeSceneItem(shapeNode,
+            '/mtl/UsdPreviewSurface1/compound/UsdPreviewSurface2')
+
+        self.assertIsNotNone(ufeParentItem)
+        self.assertIsNotNone(ufePreviewItem)
+        self.assertIsNotNone(ufeSurfaceItem)
+        self.assertIsNotNone(ufeCompoundItem)
+        self.assertIsNotNone(ufeChildCompoundItem)
+
+        connectionHandler = ufe.RunTimeMgr.instance().connectionHandler(ufeParentItem.runTimeId())
+        self.assertIsNotNone(connectionHandler)
+
+        # Get the prims.
+        parentPrim = usdUtils.getPrimFromSceneItem(ufeParentItem)
+        previewPrim = usdUtils.getPrimFromSceneItem(ufePreviewItem)
+        surfacePrim = usdUtils.getPrimFromSceneItem(ufeSurfaceItem)
+        compoundPrim = usdUtils.getPrimFromSceneItem(ufeCompoundItem)
+        childCompoundPrim = usdUtils.getPrimFromSceneItem(ufeChildCompoundItem)
+
+        # Get the attributes.
+        parentAttrs = ufe.Attributes.attributes(ufeParentItem)
+        previewAttrs = ufe.Attributes.attributes(ufePreviewItem)
+        surfaceAttrs = ufe.Attributes.attributes(ufeSurfaceItem)
+        compoundAttrs = ufe.Attributes.attributes(ufeCompoundItem)
+        childCompoundAttrs = ufe.Attributes.attributes(ufeChildCompoundItem)
+
+        # 1. Delete node (not compound) connections.
+
+        previewClearcoat = previewAttrs.attribute('inputs:clearcoat')
+        previewSurface = previewAttrs.attribute('outputs:surface')
+        parentClearcoat = parentAttrs.attribute('inputs:clearcoat')
+        parentSurface = parentAttrs.attribute('outputs:surface')
+        surfaceBsdf = surfaceAttrs.attribute('inputs:bsdf')
+
+        self.assertIsNotNone(previewClearcoat)
+        self.assertIsNotNone(previewSurface)
+        self.assertIsNotNone(parentClearcoat)
+        self.assertIsNotNone(parentSurface)
+        self.assertIsNotNone(surfaceBsdf)
+
+        # 1.1 Delete the connection between two nodes (not compounds).
+        cmd = connectionHandler.deleteConnectionCmd(previewSurface, surfaceBsdf)
+        cmd.execute()
+
+        # Property kept since there is a connection to the parent.
+        self.assertTrue(previewPrim.HasProperty('outputs:surface'))
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(surfacePrim.HasProperty('outputs:surface'))
+
+        # 1.2 Delete the connection between the node and its parent (outputs).
+        cmd = connectionHandler.deleteConnectionCmd(previewSurface, parentSurface)
+        cmd.execute()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(previewPrim.HasProperty('outputs:surface'))
+        # Property kept since it is on the boundary.
+        self.assertTrue(parentPrim.HasProperty('outputs:surface'))
+
+        # 1.3 Delete the connection between the node and its parent (inputs).
+        cmd = connectionHandler.deleteConnectionCmd(parentClearcoat, previewClearcoat)
+        cmd.execute()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(previewPrim.HasProperty('inputs:clearcoat'))
+        # Property kept since it is on the boundary.
+        self.assertTrue(parentPrim.HasProperty('inputs:clearcoat'))
+
+        # 2. Delete compound connections.
+
+        compoundDisplacement = compoundAttrs.attribute('outputs:displacement')
+        compoundPort = compoundAttrs.attribute('outputs:port')
+        parentDisplacement = parentAttrs.attribute('outputs:displacement')
+        parentPort = parentAttrs.attribute('outputs:port')
+
+        self.assertIsNotNone(compoundDisplacement)
+        self.assertIsNotNone(compoundPort)
+        self.assertIsNotNone(parentDisplacement)
+        self.assertIsNotNone(parentPort)
+
+        # 2.1 Delete compound connections to the parent.
+        cmd = connectionHandler.deleteConnectionCmd(compoundPort, parentPort)
+        cmd.execute()
+
+        # Properties kept since they are on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('outputs:port'))
+        self.assertTrue(parentPrim.HasProperty('outputs:port'))
+
+        cmd = connectionHandler.deleteConnectionCmd(compoundDisplacement, parentDisplacement)
+        cmd.execute()
+
+        # Properties kept since they are on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('outputs:displacement'))
+        self.assertTrue(parentPrim.HasProperty('outputs:displacement'))
+
+        # 2.2 Delete compound connections from the parent.
+        compoundClearcoatRoughness = compoundAttrs.attribute('inputs:clearcoatRoughness')
+        compoundPort = compoundAttrs.attribute('inputs:port')
+        parentClearcoatRoughness = parentAttrs.attribute('inputs:clearcoatRoughness')
+        parentPort = parentAttrs.attribute('inputs:port')
+
+        self.assertIsNotNone(compoundClearcoatRoughness)
+        self.assertIsNotNone(compoundPort)
+        self.assertIsNotNone(parentClearcoatRoughness)
+        self.assertIsNotNone(parentPort)
+
+        cmd = connectionHandler.deleteConnectionCmd(parentPort, compoundPort)
+        cmd.execute()
+
+        # Properties kept since they are on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('inputs:port'))
+        self.assertTrue(parentPrim.HasProperty('inputs:port'))
+
+        cmd = connectionHandler.deleteConnectionCmd(parentClearcoatRoughness, compoundClearcoatRoughness)
+        cmd.execute()
+
+        # Properties kept since they are on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('inputs:clearcoatRoughness'))
+        self.assertTrue(parentPrim.HasProperty('inputs:clearcoatRoughness'))
+
+        # 3. Delete connections inside the compound.
+
+        childDisplacement = childCompoundAttrs.attribute('outputs:displacement')
+        childClearcoatRoughness = childCompoundAttrs.attribute('inputs:clearcoatRoughness')
+        childClearcoat = childCompoundAttrs.attribute('inputs:clearcoat')
+        compoundClearcoat = compoundAttrs.attribute('inputs:clearcoat')
+
+        self.assertIsNotNone(childDisplacement)
+        self.assertIsNotNone(childClearcoatRoughness)
+        self.assertIsNotNone(childClearcoat)
+        self.assertIsNotNone(compoundClearcoat)
+
+        # 3.1 Delete child compound connections to the parent.
+        cmd = connectionHandler.deleteConnectionCmd(childDisplacement, compoundDisplacement)
+        cmd.execute()
+
+        # Property kept since it is on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('outputs:displacement'))
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(childCompoundPrim.HasProperty('outputs:displacement'))
+
+        # 3.2 Delete child compound connections from the parent.
+        cmd = connectionHandler.deleteConnectionCmd(compoundClearcoatRoughness, childClearcoatRoughness)
+        cmd.execute()
+
+        # Property kept since it is on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('inputs:clearcoatRoughness'))
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(childCompoundPrim.HasProperty('inputs:clearcoatRoughness'))
+
+        cmd = connectionHandler.deleteConnectionCmd(compoundClearcoat, childClearcoat)
+        cmd.execute()
+
+        # Property kept since it is on the boundary.
+        self.assertTrue(compoundPrim.HasProperty('inputs:clearcoat'))
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(childCompoundPrim.HasProperty('inputs:clearcoat'))
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)