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

lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp

@@ -284,6 +284,139 @@ UsdUndoDeleteConnectionCommand::Ptr UsdUndoDeleteConnectionCommand::create(
     return std::make_shared<UsdUndoDeleteConnectionCommand>(srcAttr, dstAttr);
 }
 
+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;
+    }
+
+    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;
+        }
+    }
+
+    PXR_NS::UsdShadeNodeGraph ngPrim(prim);
+
+    if (!ngPrim) {
+        return true;
+    }
+
+    // Do not remove if there is a connection with a child prim.
+    for (const auto& childPrim : prim.GetChildren()) {
+        for (const auto& attribute : childPrim.GetAttributes()) {
+            const PXR_NS::UsdAttribute dstUsdAttr = attribute.As<PXR_NS::UsdAttribute>();
+            if (isConnected(srcAttr, dstUsdAttr)) {
+                return false;
+            }
+        }
+    }
+
+    return true;
+}
+
+bool canRemoveDstProperty(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) {
+        return true;
+    }
+
+    // Do not remove if there is a connection with a child prim.
+    for (const auto& childPrim : prim.GetChildren()) {
+        for (const auto& attribute : childPrim.GetAttributes()) {
+            const PXR_NS::UsdAttribute dstUsdAttr = attribute.As<PXR_NS::UsdAttribute>();
+            if (isConnected(srcAttr, dstUsdAttr)) {
+                return false;
+            }
+        }
+    }
+
+    const auto primParent = prim.GetParent();
+
+    if (!primParent) {
+        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;
+        }
+    }
+
+    // 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;
+                }
+            }
+        }
+    }
+
+    return true;
+}
+
 void UsdUndoDeleteConnectionCommand::execute()
 {
     UsdUndoBlock undoBlock(&_undoableItem);
@@ -312,19 +445,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 (canRemoveDstProperty(dstUsdAttr->usdAttribute())) {
+            dstUsdAttr->usdPrim().RemoveProperty(dstUsdAttr->usdAttribute().GetName());
+        }
+
+        if (canRemoveSrcProperty(srcUsdAttr->usdAttribute())) {
+            srcUsdAttr->usdPrim().RemoveProperty(srcUsdAttr->usdAttribute().GetName());
         }
     }
 

test/lib/ufe/testConnections.py

@@ -979,5 +979,180 @@ 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.usd')
+        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'))
+        # Material outputs displacement-surface-volume properties are re-created automatically.
+        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 has values.
+        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()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(compoundPrim.HasProperty('outputs:port'))
+        self.assertFalse(parentPrim.HasProperty('outputs:port'))
+
+        cmd = connectionHandler.deleteConnectionCmd(compoundDisplacement, parentDisplacement)
+        cmd.execute()
+
+        # Property kept since there is a connection with child compound.
+        self.assertTrue(compoundPrim.HasProperty('outputs:displacement'))
+        # Material outputs displacement-surface-volume properties are re-created automatically.
+        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()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(compoundPrim.HasProperty('inputs:port'))
+        # Property kept since it has metadata values.
+        self.assertTrue(parentPrim.HasProperty('inputs:port'))
+
+        cmd = connectionHandler.deleteConnectionCmd(parentClearcoatRoughness, compoundClearcoatRoughness)
+        cmd.execute()
+
+        # Property kept since there is a connection with child compound.
+        self.assertTrue(compoundPrim.HasProperty('inputs:clearcoatRoughness'))
+        # Property kept since it has metadata values.
+        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()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(compoundPrim.HasProperty('outputs:displacement'))
+        self.assertFalse(childCompoundPrim.HasProperty('outputs:displacement'))
+
+        # 3.2 Delete child compound connections from the parent.
+        cmd = connectionHandler.deleteConnectionCmd(compoundClearcoatRoughness, childClearcoatRoughness)
+        cmd.execute()
+
+        # The property has been deleted since there are no more connections.
+        self.assertFalse(compoundPrim.HasProperty('inputs:clearcoatRoughness'))
+        self.assertFalse(childCompoundPrim.HasProperty('inputs:clearcoatRoughness'))
+
+        cmd = connectionHandler.deleteConnectionCmd(compoundClearcoat, childClearcoat)
+        cmd.execute()
+
+        # Property kept since it has values.
+        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)