diff --git a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp index cd252850d6..5c97da5e39 100644 --- a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp @@ -68,20 +68,6 @@ UsdAttribute* usdAttrFromUfeAttr(const Ufe::Attribute::Ptr& attr) return dynamic_cast(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) { @@ -169,7 +155,7 @@ void UsdUndoCreateConnectionCommand::execute() return; } - if (isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) { + if (MayaUsd::ufe::isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) { return; } @@ -294,7 +280,7 @@ void UsdUndoDeleteConnectionCommand::execute() UsdAttribute* dstUsdAttr = usdAttrFromUfeAttr(dstAttr); if (!srcUsdAttr || !dstUsdAttr - || !isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) { + || !MayaUsd::ufe::isConnected(srcUsdAttr->usdAttribute(), dstUsdAttr->usdAttribute())) { return; } @@ -312,19 +298,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()); } } diff --git a/lib/mayaUsd/ufe/Utils.cpp b/lib/mayaUsd/ufe/Utils.cpp index 43c1b95283..dbbe0fcab0 100644 --- a/lib/mayaUsd/ufe/Utils.cpp +++ b/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(); + 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(); + 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( diff --git a/lib/mayaUsd/ufe/Utils.h b/lib/mayaUsd/ufe/Utils.h index f7a3f50e8d..20c4296ecc 100644 --- a/lib/mayaUsd/ufe/Utils.h +++ b/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); diff --git a/test/lib/ufe/testConnections.py b/test/lib/ufe/testConnections.py index 60d91349aa..84176a5aa5 100644 --- a/test/lib/ufe/testConnections.py +++ b/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) diff --git a/test/testSamples/properties/properties.usda b/test/testSamples/properties/properties.usda new file mode 100644 index 0000000000..de028e814e --- /dev/null +++ b/test/testSamples/properties/properties.usda @@ -0,0 +1,91 @@ +#usda 1.0 + +def Sphere "Sphere1" ( + prepend apiSchemas = ["MaterialBindingAPI"] +) +{ + rel material:binding = +} + +def Scope "mtl" +{ + def Material "UsdPreviewSurface1" ( + prepend apiSchemas = ["NodeGraphNodeAPI"] + ) + { + float inputs:clearcoat = 0 ( + sdrMetadata = { + string uiname = "Clearcoat" + string uisoftmax = "1.0" + string uisoftmin = "0.0" + } + ) + float inputs:clearcoatRoughness = 0.01 ( + sdrMetadata = { + string uiname = "Clearcoat Roughness" + string uisoftmax = "1.0" + string uisoftmin = "0.0" + } + ) + float inputs:port = 0 ( + sdrMetadata = { + string uiname = "Port" + } + ) + token outputs:displacement.connect = + float outputs:port.connect = + token outputs:surface.connect = + uniform float2 ui:nodegraph:node:pos = (0.055555556, 0.055555556) + + def Shader "UsdPreviewSurface1" ( + prepend apiSchemas = ["NodeGraphNodeAPI"] + ) + { + uniform token info:id = "UsdPreviewSurface" + float inputs:clearcoat.connect = + token outputs:surface + uniform float2 ui:nodegraph:node:pos = (4.0055556, 0.46666667) + } + + def Shader "surface1" ( + prepend apiSchemas = ["NodeGraphNodeAPI"] + ) + { + uniform token info:id = "ND_surface" + token inputs:bsdf.connect = + uniform float2 ui:nodegraph:node:pos = (6.6200557, 0.98951113) + } + + def NodeGraph "compound" ( + prepend apiSchemas = ["NodeGraphNodeAPI"] + ) + { + float inputs:clearcoat = 0 ( + sdrMetadata = { + string uiname = "Clearcoat" + string uisoftmax = "1.0" + string uisoftmin = "0.0" + } + ) + float inputs:clearcoatRoughness.connect = + float inputs:port.connect = + token outputs:displacement.connect = + float outputs:port + token outputs:surface.connect = + uniform float2 ui:nodegraph:node:pos = (4.1355443, 3.8365722) + + def Shader "UsdPreviewSurface2" ( + prepend apiSchemas = ["NodeGraphNodeAPI"] + ) + { + uniform token info:id = "UsdPreviewSurface" + float inputs:clearcoat.connect = + float inputs:clearcoatRoughness.connect = + token outputs:displacement + token outputs:surface + uniform float2 ui:nodegraph:node:pos = (-0.41053337, 0.045313444) + } + } + } +} +