From ebcdb6447ff3d7832ab8ebe0f0d477c2f9603e88 Mon Sep 17 00:00:00 2001 From: alicedegirolamo Date: Mon, 30 Jan 2023 14:37:03 +0100 Subject: [PATCH 1/6] MAYA-127483 Delete properties when deleting connections. --- lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp | 152 +++++++++++++-- test/lib/ufe/testConnections.py | 175 ++++++++++++++++++ test/testSamples/properties/properties.usd | Bin 0 -> 2110 bytes 3 files changed, 314 insertions(+), 13 deletions(-) create mode 100644 test/testSamples/properties/properties.usd diff --git a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp index cd252850d6..393fd155f3 100644 --- a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp @@ -284,6 +284,139 @@ UsdUndoDeleteConnectionCommand::Ptr UsdUndoDeleteConnectionCommand::create( return std::make_shared(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(); + 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; + } + } + + 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(); + 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(); + 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(); + 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(); + 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()); } } diff --git a/test/lib/ufe/testConnections.py b/test/lib/ufe/testConnections.py index 60d91349aa..8796abd910 100644 --- a/test/lib/ufe/testConnections.py +++ b/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) diff --git a/test/testSamples/properties/properties.usd b/test/testSamples/properties/properties.usd new file mode 100644 index 0000000000000000000000000000000000000000..eb0cc27986edb10d4c0421589a54021a02af5beb GIT binary patch literal 2110 zcmbtVPjC}e82@&&NtSJC(lo*I#I6XU>wizDGy{eMf^3qWB_XK@tog*ZTT{K8*c-Y(E}}Pi)B;e(Vs+1SAg8`?kKFKen%b z1Z@KTARVNE^x>_pH*-!TOXByn>^PAm?)L>#j}~$_l}}1LEJl8XZ#fJ2~Um;L(VLzmYRf$QwG!0D&r-stXQfJ zV4G?|D{87W2=@lSIcBPpaz%whvZGp>Tz*E=6-}SrmpKTsspSeKwIbV4*$9c$%qCdm zF`8kjmZPaQ^krmmIcfp2uFBQowf#S&4tm-I`!5QspADhj%X^$hfAcv5{ z$gRlj$PtfN!4sq)s3k}#Au?H>mqZEzG@@Q-XnL7p>UFj*(Dcm#Zn?oNv&$WeHwWs( zgCKqwk8Wf*2r@RdiXj$~9byT~I8?D5hEBKVge!Ns^7~lE;VcVd!*GFxo_@FYl56ph zE8lW;ODx0(sCC3F#X@9bHzc7$?1pjJF2*AeO`z{&Ka3?{FalH<_5mxtBtUW#e2V@F z_>zTF+}vYN;Rhvt9U=XzEQkr<=oBOq(1$~9f-%=rWFdM8IgOk}Iym0Wy%@1WJQ0Dh z7`%s_r;uk@IR3f&LB!ywy8r<1dNtHSR&mH9{M& zvGnj&4vvppWSOCFS3BbmT!J_#4i5k$W_ftR5qJ;ZGs{E1q1ak-fSivxUwaDpCiwRi<)~=2QjAm zbcLHM;rssEl7Hd3^kmMr1o*XPYI`5lYY`rV+Ny{BF`|<~T9qF3!5g`)BPRrdU%O literal 0 HcmV?d00001 From d83a905195e0af04d262017348b2891618a75e77 Mon Sep 17 00:00:00 2001 From: Jerry Gamache Date: Mon, 30 Jan 2023 14:30:41 -0500 Subject: [PATCH 2/6] Convert test scene to ASCII --- test/lib/ufe/testConnections.py | 2 +- test/testSamples/properties/properties.usd | Bin 2110 -> 0 bytes test/testSamples/properties/properties.usda | 91 ++++++++++++++++++++ 3 files changed, 92 insertions(+), 1 deletion(-) delete mode 100644 test/testSamples/properties/properties.usd create mode 100644 test/testSamples/properties/properties.usda diff --git a/test/lib/ufe/testConnections.py b/test/lib/ufe/testConnections.py index 8796abd910..62dd79bd1c 100644 --- a/test/lib/ufe/testConnections.py +++ b/test/lib/ufe/testConnections.py @@ -985,7 +985,7 @@ def testRemovePropertiesWithConnections(self): # Load a scene. - testFile = testUtils.getTestScene('properties', 'properties.usd') + testFile = testUtils.getTestScene('properties', 'properties.usda') shapeNode,shapeStage = mayaUtils.createProxyFromFile(testFile) ufeParentItem = ufeUtils.createUfeSceneItem(shapeNode, diff --git a/test/testSamples/properties/properties.usd b/test/testSamples/properties/properties.usd deleted file mode 100644 index eb0cc27986edb10d4c0421589a54021a02af5beb..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 2110 zcmbtVPjC}e82@&&NtSJC(lo*I#I6XU>wizDGy{eMf^3qWB_XK@tog*ZTT{K8*c-Y(E}}Pi)B;e(Vs+1SAg8`?kKFKen%b z1Z@KTARVNE^x>_pH*-!TOXByn>^PAm?)L>#j}~$_l}}1LEJl8XZ#fJ2~Um;L(VLzmYRf$QwG!0D&r-stXQfJ zV4G?|D{87W2=@lSIcBPpaz%whvZGp>Tz*E=6-}SrmpKTsspSeKwIbV4*$9c$%qCdm zF`8kjmZPaQ^krmmIcfp2uFBQowf#S&4tm-I`!5QspADhj%X^$hfAcv5{ z$gRlj$PtfN!4sq)s3k}#Au?H>mqZEzG@@Q-XnL7p>UFj*(Dcm#Zn?oNv&$WeHwWs( zgCKqwk8Wf*2r@RdiXj$~9byT~I8?D5hEBKVge!Ns^7~lE;VcVd!*GFxo_@FYl56ph zE8lW;ODx0(sCC3F#X@9bHzc7$?1pjJF2*AeO`z{&Ka3?{FalH<_5mxtBtUW#e2V@F z_>zTF+}vYN;Rhvt9U=XzEQkr<=oBOq(1$~9f-%=rWFdM8IgOk}Iym0Wy%@1WJQ0Dh z7`%s_r;uk@IR3f&LB!ywy8r<1dNtHSR&mH9{M& zvGnj&4vvppWSOCFS3BbmT!J_#4i5k$W_ftR5qJ;ZGs{E1q1ak-fSivxUwaDpCiwRi<)~=2QjAm zbcLHM;rssEl7Hd3^kmMr1o*XPYI`5lYY`rV+Ny{BF`|<~T9qF3!5g`)BPRrdU%O 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) + } + } + } +} + From 9f6cbdcee162fc47732fa14f0752781446b51b59 Mon Sep 17 00:00:00 2001 From: alicedegirolamo Date: Tue, 31 Jan 2023 10:15:45 +0100 Subject: [PATCH 3/6] MAYA-127483 - PR Changes --- lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp | 103 ++++++------------ test/lib/ufe/testConnections.py | 29 +++-- 2 files changed, 48 insertions(+), 84 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp index 393fd155f3..684e5a49fe 100644 --- a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp +++ b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp @@ -286,7 +286,6 @@ UsdUndoDeleteConnectionCommand::Ptr UsdUndoDeleteConnectionCommand::create( bool canRemoveSrcProperty(const PXR_NS::UsdAttribute& srcAttr) { - // Do not remove if it has a value. if (srcAttr.HasValue()) { return false; @@ -306,67 +305,58 @@ bool canRemoveSrcProperty(const PXR_NS::UsdAttribute& srcAttr) return false; } - const auto primParent = prim.GetParent(); - - if (!primParent) { - return false; - } + PXR_NS::UsdShadeNodeGraph ngPrim(prim); - // 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; - } - } - } - } + if (!ngPrim) { + const auto primParent = prim.GetParent(); - // 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)) { + if (!primParent) { return false; } - } - PXR_NS::UsdShadeNodeGraph ngPrim(prim); - - if (!ngPrim) { - return true; - } + // 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 a child prim. - for (const auto& childPrim : prim.GetChildren()) { - for (const auto& attribute : childPrim.GetAttributes()) { + // 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; } - return true; + // Do not remove boundary properties even if there are connections. + return false; } -bool canRemoveDstProperty(const PXR_NS::UsdAttribute& srcAttr) +bool canRemoveDstProperty(const PXR_NS::UsdAttribute& dstAttr) { // Do not remove if it has a value. - if (srcAttr.HasValue()) { + if (dstAttr.HasValue()) { return false; } PXR_NS::SdfPathVector connectedAttrs; - srcAttr.GetConnections(&connectedAttrs); + dstAttr.GetConnections(&connectedAttrs); // Do not remove if it has connections. if (!connectedAttrs.empty()) { return false; } - const auto prim = srcAttr.GetPrim(); + const auto prim = dstAttr.GetPrim(); if (!prim) { return false; @@ -378,43 +368,18 @@ bool canRemoveDstProperty(const PXR_NS::UsdAttribute& srcAttr) 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(); - 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(); - 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(); - if (isConnected(srcAttr, dstUsdAttr)) { - return false; - } - } + 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; } } - return true; + // Do not remove boundary properties even if there are connections. + return false; } void UsdUndoDeleteConnectionCommand::execute() diff --git a/test/lib/ufe/testConnections.py b/test/lib/ufe/testConnections.py index 62dd79bd1c..84176a5aa5 100644 --- a/test/lib/ufe/testConnections.py +++ b/test/lib/ufe/testConnections.py @@ -1051,7 +1051,7 @@ def testRemovePropertiesWithConnections(self): # 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. + # 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). @@ -1060,7 +1060,7 @@ def testRemovePropertiesWithConnections(self): # The property has been deleted since there are no more connections. self.assertFalse(previewPrim.HasProperty('inputs:clearcoat')) - # Property kept since it has values. + # Property kept since it is on the boundary. self.assertTrue(parentPrim.HasProperty('inputs:clearcoat')) # 2. Delete compound connections. @@ -1079,16 +1079,15 @@ def testRemovePropertiesWithConnections(self): 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')) + # 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() - # Property kept since there is a connection with child compound. + # Properties kept since they are on the boundary. 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. @@ -1105,17 +1104,15 @@ def testRemovePropertiesWithConnections(self): 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. + # 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() - # Property kept since there is a connection with child compound. + # Properties kept since they are on the boundary. 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. @@ -1134,22 +1131,24 @@ def testRemovePropertiesWithConnections(self): 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(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() + # 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(compoundPrim.HasProperty('inputs:clearcoatRoughness')) self.assertFalse(childCompoundPrim.HasProperty('inputs:clearcoatRoughness')) cmd = connectionHandler.deleteConnectionCmd(compoundClearcoat, childClearcoat) cmd.execute() - # Property kept since it has values. + # 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')) From 373c3db09e672fc861a0e1364b4c3c013946579e Mon Sep 17 00:00:00 2001 From: alicedegirolamo Date: Thu, 2 Feb 2023 10:26:32 +0100 Subject: [PATCH 4/6] MAYA-127483 - PR Changes. --- lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp | 120 +----------------- lib/mayaUsd/ufe/Utils.cpp | 114 +++++++++++++++++ lib/mayaUsd/ufe/Utils.h | 15 +++ 3 files changed, 133 insertions(+), 116 deletions(-) diff --git a/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp b/lib/mayaUsd/ufe/UsdUndoConnectionCommands.cpp index 684e5a49fe..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; } @@ -284,104 +270,6 @@ UsdUndoDeleteConnectionCommand::Ptr UsdUndoDeleteConnectionCommand::create( return std::make_shared(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; - } - - 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; -} - void UsdUndoDeleteConnectionCommand::execute() { UsdUndoBlock undoBlock(&_undoableItem); @@ -392,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; } @@ -410,11 +298,11 @@ 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 (canRemoveDstProperty(dstUsdAttr->usdAttribute())) { + if (MayaUsd::ufe::canRemoveDstProperty(dstUsdAttr->usdAttribute())) { dstUsdAttr->usdPrim().RemoveProperty(dstUsdAttr->usdAttribute().GetName()); } - if (canRemoveSrcProperty(srcUsdAttr->usdAttribute())) { + 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); From a27a9bcc4e8dad81472b8bc30401b06e8371dd0a Mon Sep 17 00:00:00 2001 From: alicedegirolamo Date: Fri, 10 Feb 2023 16:47:23 +0100 Subject: [PATCH 5/6] MAYA-127483 - Correct attribute base name --- lib/mayaUsd/ufe/Utils.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/mayaUsd/ufe/Utils.cpp b/lib/mayaUsd/ufe/Utils.cpp index dbbe0fcab0..a4b34c0dbc 100644 --- a/lib/mayaUsd/ufe/Utils.cpp +++ b/lib/mayaUsd/ufe/Utils.cpp @@ -549,7 +549,8 @@ bool canRemoveDstProperty(const PXR_NS::UsdAttribute& dstAttr) UsdShadeMaterial asMaterial(prim); if (asMaterial) { - const TfToken baseName = dstAttr.GetBaseName(); + const auto baseNameAndType = PXR_NS::UsdShadeUtils::GetBaseNameAndType(dstAttr.GetName()); + const TfToken baseName = baseNameAndType.first; // Remove Material intrinsic outputs since they are re-created automatically. if (baseName == UsdShadeTokens->surface || baseName == UsdShadeTokens->volume || baseName == UsdShadeTokens->displacement) { From d8dd34075837e1a0d8c882e8a27818822f8a514f Mon Sep 17 00:00:00 2001 From: alicedegirolamo Date: Tue, 21 Feb 2023 10:03:02 +0100 Subject: [PATCH 6/6] Revert "MAYA-127483 - Correct attribute base name" This reverts commit a27a9bcc4e8dad81472b8bc30401b06e8371dd0a. --- lib/mayaUsd/ufe/Utils.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/mayaUsd/ufe/Utils.cpp b/lib/mayaUsd/ufe/Utils.cpp index a4b34c0dbc..dbbe0fcab0 100644 --- a/lib/mayaUsd/ufe/Utils.cpp +++ b/lib/mayaUsd/ufe/Utils.cpp @@ -549,8 +549,7 @@ bool canRemoveDstProperty(const PXR_NS::UsdAttribute& dstAttr) UsdShadeMaterial asMaterial(prim); if (asMaterial) { - const auto baseNameAndType = PXR_NS::UsdShadeUtils::GetBaseNameAndType(dstAttr.GetName()); - const TfToken baseName = baseNameAndType.first; + 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) {