UsdShade API concept separation.

Remove UsdShadeConnectableAPI implicit ctors from Shader & NodeGraph, to emphasize & enforce independence of connectability from shader graphs. Instead, require callers to convert explicitly. (Internal change: 2138637)
PixarAnimationStudios · Jan 21, 2021 · 7944738 · 7944738
1 parent 3f9d954
commit 7944738
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 49 deletions.
diff --git a/pxr/usd/plugin/usdMtlx/reader.cpp b/pxr/usd/plugin/usdMtlx/reader.cpp
@@ -699,7 +699,7 @@ _NodeGraphBuilder::Build(ShaderNamesByOutputName* outputs)
         // Create the interface.
         if (_mtlxNodeDef) {
             for (auto& i: _GetInheritanceStack(_mtlxNodeDef)) {
-                _CreateInterface(i, usdNodeGraph);
+                _CreateInterface(i, usdNodeGraph.ConnectableAPI());
             }
         }
     }
@@ -714,7 +714,7 @@ _NodeGraphBuilder::Build(ShaderNamesByOutputName* outputs)
     _ConnectNodes();
 
     if (isInsideNodeGraph) {
-        _ConnectTerminals(_mtlxContainer,  UsdShadeNodeGraph(usdPrim));
+        _ConnectTerminals(_mtlxContainer, UsdShadeConnectableAPI(usdPrim));
     }
     else if (outputs) {
         // Collect the outputs on the existing shader nodes.
@@ -1298,7 +1298,8 @@ _Context::BeginMaterial(const mx::ConstMaterialPtr& mtlxMaterial)
             _SetCoreUIAttributes(usdMaterial.GetPrim(), mtlxMaterial);
 
             // Record the material for later variants.
-            _shaders[_Name(mtlxMaterial)][""] = usdMaterial;
+            _shaders[_Name(mtlxMaterial)][""] =
+                UsdShadeConnectableAPI(usdMaterial);
 
             // Cut over.
             _mtlxMaterial = mtlxMaterial;
@@ -1412,7 +1413,8 @@ _Context::AddShaderRef(const mx::ConstShaderRefPtr& mtlxShaderRef)
     usdShader.GetPrim().GetReferences().AddInternalReference(shaderImplPath);
 
     // Record the referencing shader for later variants.
-    _shaders[_Name(_mtlxMaterial)][_Name(mtlxShaderRef)] = usdShader;
+    _shaders[_Name(_mtlxMaterial)][_Name(mtlxShaderRef)] =
+        UsdShadeConnectableAPI(usdShader);
 
     // Connect to material interface.
     for (auto& i: _GetInheritanceStack(mtlxNodeDef)) {
@@ -1433,13 +1435,14 @@ _Context::AddShaderRef(const mx::ConstShaderRefPtr& mtlxShaderRef)
 
     // Translate bindings.
     for (auto mtlxParam: mtlxShaderRef->getBindParams()) {
-        if (auto input = _AddInputWithValue(mtlxParam, _usdMaterial)) {
+        if (auto input = _AddInputWithValue(mtlxParam,
+                                            UsdShadeConnectableAPI(_usdMaterial))) {
             input.SetConnectability(UsdShadeTokens->interfaceOnly);
         }
     }
     for (auto mtlxInput: mtlxShaderRef->getBindInputs()) {
         // Simple binding.
-        _AddInputWithValue(mtlxInput, _usdMaterial);
+        _AddInputWithValue(mtlxInput, UsdShadeConnectableAPI(_usdMaterial));
 
         // Check if this input references an output.
         if (auto outputName = _Attr(mtlxInput, names.output)) {
@@ -1454,7 +1457,8 @@ _Context::AddShaderRef(const mx::ConstShaderRefPtr& mtlxShaderRef)
                         ? AddNodeGraph(mtlxNodeGraph)
                         : AddImplicitNodeGraph(mtlxInput->getDocument())) {
                 _BindNodeGraph(mtlxInput, _usdMaterial.GetPath(),
-                               usdShader, usdNodeGraph);
+                               UsdShadeConnectableAPI(usdShader),
+                               usdNodeGraph);
             }
         }
     }

diff --git a/pxr/usd/usdLux/lightDefParser.cpp b/pxr/usd/usdLux/lightDefParser.cpp
@@ -26,6 +26,7 @@
 #include "pxr/usd/usdShade/connectableAPI.h"
 #include "pxr/usd/usdShade/shaderDefUtils.h"
 #include "pxr/usd/usdShade/tokens.h"
+#include "pxr/usd/sdr/shaderNode.h"
 
 PXR_NAMESPACE_OPEN_SCOPE
 

diff --git a/pxr/usd/usdShade/connectableAPI.h b/pxr/usd/usdShade/connectableAPI.h
@@ -32,8 +32,10 @@
 #include "pxr/usd/usd/prim.h"
 #include "pxr/usd/usd/stage.h"
 
-#include "pxr/usd/usdShade/shader.h"
-#include "pxr/usd/usdShade/nodeGraph.h"
+#include "pxr/usd/usd/typed.h"
+#include "pxr/usd/usdShade/input.h"
+#include "pxr/usd/usdShade/output.h"
+#include "pxr/usd/usdShade/tokens.h"
 #include "pxr/usd/usdShade/types.h"
 
 #include "pxr/base/vt/value.h"
@@ -175,23 +177,6 @@ class UsdShadeConnectableAPI : public UsdAPISchemaBase
     bool _IsCompatible() const override;
 
 public:
-
-    /// Constructor that takes a UsdShadeShader.
-    /// Allow implicit (auto) conversion of UsdShadeShader to 
-    /// UsdShadeConnectableAPI, so that a shader can be passed into any function
-    /// that accepts a ConnectableAPI.
-    UsdShadeConnectableAPI(const UsdShadeShader &shader):
-        UsdShadeConnectableAPI(shader.GetPrim())
-    { }
-
-    /// Constructor that takes a UsdShadeNodeGraph.
-    /// Allow implicit (auto) conversion of UsdShadeNodeGraph to 
-    /// UsdShadeConnectableAPI, so that a nodegraph can be passed into any function
-    /// that accepts a ConnectableAPI.
-    UsdShadeConnectableAPI(const UsdShadeNodeGraph &nodeGraph):
-        UsdShadeConnectableAPI(nodeGraph.GetPrim())
-    { }
-
     /// Returns true if the prim is a container.
     ///
     /// The underlying prim type may provide runtime behavior

diff --git a/pxr/usd/usdShade/schema.usda b/pxr/usd/usdShade/schema.usda
@@ -367,8 +367,10 @@ class "ConnectableAPI"
     customData = {
         token apiSchemaType = "nonApplied"
         string extraIncludes = '''
-#include "pxr/usd/usdShade/shader.h"
-#include "pxr/usd/usdShade/nodeGraph.h"
+#include "pxr/usd/usd/typed.h"
+#include "pxr/usd/usdShade/input.h"
+#include "pxr/usd/usdShade/output.h"
+#include "pxr/usd/usdShade/tokens.h"
 #include "pxr/usd/usdShade/types.h"'''
     }
 )

diff --git a/pxr/usd/usdShade/shaderDefParser.cpp b/pxr/usd/usdShade/shaderDefParser.cpp
@@ -57,7 +57,7 @@ _GetSdrMetadata(const UsdShadeShader &shaderDef,
 
     metadata[SdrNodeMetadata->Primvars] = 
         UsdShadeShaderDefUtils::GetPrimvarNamesMetadataString(
-            metadata, shaderDef);
+            metadata, shaderDef.ConnectableAPI());
 
     return metadata;
 }
@@ -127,7 +127,8 @@ UsdShadeShaderDefParserPlugin::Parse(
         discoveryResult.sourceType, /* sourceType */
         nodeUriAssetPath.GetResolvedPath(),
         resolvedImplementationUri,
-        UsdShadeShaderDefUtils::GetShaderProperties(shaderDef),
+        UsdShadeShaderDefUtils::GetShaderProperties(
+            shaderDef.ConnectableAPI()),
         _GetSdrMetadata(shaderDef, discoveryResult.metadata),
         discoveryResult.sourceCode
     ));

diff --git a/pxr/usd/usdShade/testenv/testUsdShadeConnectability.py b/pxr/usd/usdShade/testenv/testUsdShadeConnectability.py
@@ -121,7 +121,7 @@ def test_Basic(self):
         self._CanConnect(shaderInputColor, colorInterfaceInput)
         # Make the connection.
         self.assertTrue(shaderInputColor.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(material,
+            UsdShade.ConnectionSourceInfo(material.ConnectableAPI(),
                                           colorInterfaceInput.GetBaseName(),
                                           UsdShade.AttributeType.Input)))
 

diff --git a/pxr/usd/usdShade/testenv/testUsdShadeShaders.py b/pxr/usd/usdShade/testenv/testUsdShadeShaders.py
@@ -106,14 +106,16 @@ def test_InputOutputConnections(self):
         self.assertTrue(not usdShadeInput.HasConnectedSource())
 
         usdShadeInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, 'Fout', Output))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(),
+            'Fout', Output))
         self.assertTrue(usdShadeInput.HasConnectedSource())
 
         self.assertEqual(usdShadeInput.GetAttr().GetConnections(),
                 [whiterPale.GetPath().AppendProperty("outputs:Fout")])
 
         self.assertEqual(usdShadeInput.GetConnectedSources()[0],
-                [UsdShade.ConnectionSourceInfo(whiterPale, 'Fout', Output)])
+                [UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(),
+                'Fout', Output)])
         usdShadeInput.ClearSources()
         self.assertTrue(not usdShadeInput.HasConnectedSource())
         self.assertEqual(usdShadeInput.GetConnectedSources()[0], [])
@@ -122,16 +124,16 @@ def test_InputOutputConnections(self):
         inheritedInput = shaderClass.CreateInput('myFloatInput', 
                                                  Sdf.ValueTypeNames.Float)
         inheritedInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, 'Fout', Output))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), 'Fout', Output))
         # note we're now testing the inheritING prim's parameter
         self.assertTrue(usdShadeInput.HasConnectedSource())
         self.assertTrue(usdShadeInput.GetConnectedSources()[0],
-            [UsdShade.ConnectionSourceInfo(whiterPale, 'Fout', Output)])
+            [UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), 'Fout', Output)])
         # clearing no longer changes anything
         usdShadeInput.ClearSources()
         self.assertTrue(usdShadeInput.HasConnectedSource())
         self.assertTrue(usdShadeInput.GetConnectedSources()[0],
-            [UsdShade.ConnectionSourceInfo(whiterPale, 'Fout', Output)])
+            [UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), 'Fout', Output)])
         # but disconnecting should
         usdShadeInput.DisconnectSource()
         self.assertTrue(not usdShadeInput.HasConnectedSource())
@@ -160,20 +162,20 @@ def test_InputOutputConnections(self):
         colInput = pale.CreateInput("col1", Sdf.ValueTypeNames.Color3f)
         self.assertTrue(colInput)
         self.assertTrue(colInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, 'colorOut', Output)))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), 'colorOut', Output)))
         outputAttr = whiterPale.GetPrim().GetAttribute("outputs:colorOut")
         self.assertTrue(outputAttr)
         self.assertEqual(outputAttr.GetTypeName(), Sdf.ValueTypeNames.Color3f)
 
         v3fInput = pale.CreateInput("v3f1", Sdf.ValueTypeNames.Float3)
         self.assertTrue(v3fInput)
         self.assertTrue(v3fInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, "colorOut", Output)))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), "colorOut", Output)))
 
         pointInput = pale.CreateInput("point1", Sdf.ValueTypeNames.Point3f)
         self.assertTrue(pointInput)
         self.assertTrue(pointInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, "colorOut", Output)))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), "colorOut", Output)))
 
         floatInput = pale.CreateInput("float1", Sdf.ValueTypeNames.Float)
         self.assertTrue(floatInput)
@@ -185,7 +187,7 @@ def test_InputOutputConnections(self):
         #        UsdShade.ConnectionSourceInfo(whiterPale, "colorOut", Output))
 
         self.assertTrue(floatInput.ConnectToSource(
-            UsdShade.ConnectionSourceInfo(whiterPale, "floatInput", Input)))
+            UsdShade.ConnectionSourceInfo(whiterPale.ConnectableAPI(), "floatInput", Input)))
         outputAttr = whiterPale.GetPrim().GetAttribute("outputs:floatInput")
         self.assertFalse(outputAttr)
         outputAttr = whiterPale.GetPrim().GetAttribute("inputs:floatInput")

diff --git a/pxr/usd/usdShade/wrapConnectableAPI.cpp b/pxr/usd/usdShade/wrapConnectableAPI.cpp
@@ -237,9 +237,6 @@ WRAP_CUSTOM {
         &UsdShadeConnectableAPI::HasConnectableAPI;
 
     _class
-        .def(init<UsdShadeShader const &>(arg("shader")))
-        .def(init<UsdShadeNodeGraph const&>(arg("nodeGraph")))
-
         .def("IsContainer", &UsdShadeConnectableAPI::IsContainer)
 
         .def("CanConnect", CanConnect_Input,
@@ -321,9 +318,6 @@ WRAP_CUSTOM {
 
     ;
 
-    implicitly_convertible<UsdShadeNodeGraph, UsdShadeConnectableAPI>();
-    implicitly_convertible<UsdShadeShader, UsdShadeConnectableAPI>();
-
     class_<ConnectionSourceInfo>("ConnectionSourceInfo")
         .def(init<UsdShadeConnectableAPI const &,
                   TfToken const &,

diff --git a/pxr/usdImaging/bin/testusdview/testenv/testUsdviewShaderEdits/testUsdviewShaderEdits.py b/pxr/usdImaging/bin/testusdview/testenv/testUsdviewShaderEdits/testUsdviewShaderEdits.py
@@ -42,7 +42,8 @@ def _bugStep1(s):
     pbrShader.CreateInput("roughness", Sdf.ValueTypeNames.Float).Set(0.0)
     pbrShader.CreateInput("metallic", Sdf.ValueTypeNames.Float).Set(0.0)
     pbrShader.CreateInput("diffuseColor", Sdf.ValueTypeNames.Color3f).Set((0.0, 0.0, 1.0))
-    material.CreateSurfaceOutput().ConnectToSource(pbrShader, "surface")
+    material.CreateSurfaceOutput().ConnectToSource(pbrShader.ConnectableAPI(),
+            "surface")
 
     # Now bind the Material to the card
     mesh = s.GetPrimAtPath('/Scene/Geom/Plane')
@@ -58,11 +59,11 @@ def _bugStep2(s):
     diffuseTextureSampler = UsdShade.Shader.Define(s,'/Scene/Looks/NewMaterial/Texture')
     diffuseTextureSampler.CreateIdAttr('UsdUVTexture')
     diffuseTextureSampler.CreateInput('file', Sdf.ValueTypeNames.Asset).Set("test.png")
-    diffuseTextureSampler.CreateInput("st", Sdf.ValueTypeNames.Float2).ConnectToSource(stReader, 'result')
+    diffuseTextureSampler.CreateInput("st", Sdf.ValueTypeNames.Float2).ConnectToSource(stReader.ConnectableAPI(), 'result')
     diffuseTextureSampler.CreateOutput('rgb', Sdf.ValueTypeNames.Float3)
 
     pbrShader = UsdShade.ConnectableAPI(s.GetPrimAtPath('/Scene/Looks/NewMaterial/PbrPreview'))
-    pbrShader.CreateInput("diffuseColor", Sdf.ValueTypeNames.Color3f).ConnectToSource(diffuseTextureSampler, 'rgb')
+    pbrShader.CreateInput("diffuseColor", Sdf.ValueTypeNames.Color3f).ConnectToSource(diffuseTextureSampler.ConnectableAPI(), 'rgb')
 
 def _bugStep3(s):
     # change diffuse texture