Add create options and unit test.

Author: kwokcb

source/MaterialXCore/Document.cpp

@@ -166,7 +166,8 @@ void Document::initialize()
 }
 
 NodeDefPtr Document::addNodeDefFromGraph(NodeGraphPtr nodeGraph, const string& nodeDefName,
-                                         const string& category, const string& newGraphName)
+                                         const string& category, const string& newGraphName,
+                                         DefinitionOptions *options)
 {
     if (category.empty())
     {
@@ -178,26 +179,34 @@ NodeDefPtr Document::addNodeDefFromGraph(NodeGraphPtr nodeGraph, const string& n
         throw Exception("Cannot create duplicate nodedef: " + nodeDefName);
     }
 
-    if (getNodeGraph(newGraphName))
+    bool addAsChild = options ? options->addImplementationAsChild : false;
+
+    if (!addAsChild && getNodeGraph(newGraphName))
     {
         throw Exception("Cannot create duplicate nodegraph: " + newGraphName);
     }
 
+    // Create a new nodedef and set its category
+    NodeDefPtr nodeDef = addNodeDef(nodeDefName, EMPTY_STRING);
+    nodeDef->setNodeString(category);
+
     // Create a new functional nodegraph, and copy over the
     // contents from the compound nodegraph
-    NodeGraphPtr graph = addNodeGraph(newGraphName);
+    NodeGraphPtr graph = !addAsChild ? addNodeGraph(newGraphName) : nodeDef->addChild<NodeGraph>(newGraphName);
     graph->copyContentFrom(nodeGraph);
 
     for (auto graphChild : graph->getChildren())
     {
         graphChild->removeAttribute(Element::XPOS_ATTRIBUTE);
         graphChild->removeAttribute(Element::YPOS_ATTRIBUTE);
     }
-    graph->setNodeDefString(nodeDefName);
 
-    // Create a new nodedef and set its category
-    NodeDefPtr nodeDef = addNodeDef(nodeDefName, EMPTY_STRING);
-    nodeDef->setNodeString(category);
+    // Reference from nodegraph to nodedef is not required
+    // as the graph is a child of the nodedef.
+    if (!addAsChild)
+    {
+        graph->setNodeDefString(nodeDefName);
+    }
 
     // Expose any existing interfaces from the graph.
     // Any connection attributes ("nodegraph", "nodename", "interfacename") on the

source/MaterialXCore/Document.h

@@ -23,6 +23,8 @@ using DocumentPtr = shared_ptr<Document>;
 /// A shared pointer to a const Document
 using ConstDocumentPtr = shared_ptr<const Document>;
 
+class DefinitionOptions;
+
 /// @class Document
 /// A MaterialX document, which represents the top-level element in the
 /// MaterialX ownership hierarchy.
@@ -363,7 +365,8 @@ class MX_CORE_API Document : public GraphElement
     /// @param category Category of the new NodeDef
     /// @return New declaration if successful.
     NodeDefPtr addNodeDefFromGraph(NodeGraphPtr nodeGraph, const string& nodeDefName,
-                                   const string& category, const string& newGraphName);
+                                   const string& category, const string& newGraphName,
+                                   DefinitionOptions * options = nullptr);
 
     /// Return the NodeDef, if any, with the given name.
     NodeDefPtr getNodeDef(const string& name) const
@@ -708,6 +711,15 @@ class MX_CORE_API Document : public GraphElement
     std::unique_ptr<Cache> _cache;
 };
 
+/// @class DefinitionOptions
+/// Options for defining a NodeDef from an immplementation
+class MX_CORE_API DefinitionOptions
+{
+public:
+    /// Add implementation as child of NodeDef as opposed a sibliing.
+    bool addImplementationAsChild = false;
+};
+
 /// Create a new Document.
 /// @relates Document
 MX_CORE_API DocumentPtr createDocument();

source/MaterialXTest/MaterialXCore/Node.cpp

@@ -675,7 +675,7 @@ TEST_CASE("Organization", "[nodegraph]")
     CHECK(nodeGraph->getBackdrops().empty());
 }
 
-TEST_CASE("Node Definition Creation", "[nodedef]")
+void testNodeDefCreationFromGraph(mx::DefinitionOptions options)
 {
     mx::FileSearchPath searchPath = mx::getDefaultDataSearchPath();
     mx::DocumentPtr stdlib = mx::createDocument();
@@ -714,10 +714,12 @@ TEST_CASE("Node Definition Creation", "[nodedef]")
         bool isDefaultVersion = false;
         const std::string NODENAME = graph->getName();
 
+
         // Create a new functional graph and definition from a compound graph
+        bool addAsChild = options.addImplementationAsChild;
         std::string newNodeDefName = doc->createValidChildName("ND_" + graph->getName());
         std::string newGraphName = doc->createValidChildName("NG_" + graph->getName());
-        mx::NodeDefPtr nodeDef = doc->addNodeDefFromGraph(graph, newNodeDefName, NODENAME, newGraphName);
+        mx::NodeDefPtr nodeDef = doc->addNodeDefFromGraph(graph, newNodeDefName, NODENAME, newGraphName, &options);
         REQUIRE(nodeDef != nullptr);
         nodeDef->setVersionString(VERSION1);
         nodeDef->setDefaultVersion(isDefaultVersion);
@@ -759,9 +761,16 @@ TEST_CASE("Node Definition Creation", "[nodedef]")
         }
 
         // Check validity of new functional nodegraph
-        mx::NodeGraphPtr newGraph = doc->getNodeGraph(newGraphName);
+        mx::NodeGraphPtr newGraph = !addAsChild ? doc->getNodeGraph(newGraphName) : nodeDef->getChildOfType<mx::NodeGraph>(newGraphName);
         REQUIRE(newGraph != nullptr);
-        REQUIRE(newGraph->getNodeDefString() == newNodeDefName);
+        if (!addAsChild)
+        {
+            REQUIRE(newGraph->getNodeDefString() == newNodeDefName);
+        }
+        else
+        {
+            REQUIRE(newGraph->getNodeDefString().empty());
+        }
         mx::ConstInterfaceElementPtr decl = newGraph->getDeclaration();
         REQUIRE(decl->getName() == nodeDef->getName());
         REQUIRE(doc->validate());
@@ -826,6 +835,23 @@ TEST_CASE("Node Definition Creation", "[nodedef]")
     REQUIRE(doc->validate());
 }
 
+TEST_CASE("Node Definition Creation", "[nodedef_create]")
+{
+    mx::DefinitionOptions defOptions;
+
+    SECTION("Without implementation as child")
+    {
+        defOptions.addImplementationAsChild = false;
+        testNodeDefCreationFromGraph(defOptions);
+    }
+
+    SECTION("With implementation as child")
+    {
+        defOptions.addImplementationAsChild = true;
+        testNodeDefCreationFromGraph(defOptions);
+    }
+}
+
 TEST_CASE("Set Name Global", "[node, nodegraph]")
 {
     mx::DocumentPtr doc = mx::createDocument();

source/PyMaterialX/PyMaterialXCore/PyDocument.cpp

@@ -26,6 +26,9 @@ void bindPyDocument(py::module& mod)
 {
     mod.def("createDocument", &mx::createDocument);
 
+    py::class_<mx::DefinitionOptions>(mod, "DefinitionOptions")
+        .def_readwrite("addImplementationAsChild", &mx::DefinitionOptions::addImplementationAsChild);
+
     py::class_<mx::Document, mx::DocumentPtr, mx::GraphElement>(mod, "Document")
         .def("initialize", &mx::Document::initialize)
         .def("copy", &mx::Document::copy)
@@ -74,7 +77,7 @@ void bindPyDocument(py::module& mod)
         .def("removeTypeDef", &mx::Document::removeTypeDef)
         .def("addNodeDef", &mx::Document::addNodeDef,
             py::arg("name") = mx::EMPTY_STRING, py::arg("type") = mx::DEFAULT_TYPE_STRING, py::arg("node") = mx::EMPTY_STRING)
-        .def("addNodeDefFromGraph", (mx::NodeDefPtr (mx::Document::*)(mx::NodeGraphPtr, const std::string&, const std::string&, const std::string&)) & mx::Document::addNodeDefFromGraph)
+        .def("addNodeDefFromGraph", (mx::NodeDefPtr (mx::Document::*)(mx::NodeGraphPtr, const std::string&, const std::string&, const std::string&, mx::DefinitionOptions*)) & mx::Document::addNodeDefFromGraph)
         .def("addNodeDefFromGraph", (mx::NodeDefPtr(mx::Document::*)(mx::NodeGraphPtr, const std::string&, const std::string&, const std::string&,
             bool, const std::string&, const std::string& )) & PyBindDocument::old_addNodeDefFromGraph)
         .def("getNodeDef", &mx::Document::getNodeDef)