Data Library API #2054

ashwinbhat · 2024-10-08T22:12:06Z

MaterialX documents are usually required to be complete self contained with all required node defintions. This is done by using the mx::importLibrary API. While processing large number of documents Import and Merge workflows can cause performance and memory challenges.

The MaterialX library aka Data library hosts defintions and thus could be referenced by a document instead of importing them.

A new set of methods have been introduced that allow registeration data library for a document.

registerDataLibrary(ConstDocumentPtr dataLibrary);
getRegisteredDataLibrary() const
hasDataLibrary() const

Other changes include

Add DataLibrary support for getChildOfType and getChildrenOfType
Updates to redirect Document::get* calls to process datalibrary and local document.

MaterialX documents are usually required to be complete self contained with all required node defintions. This is done by using the mx::importLibrary API. While processing large number of documents Import and Merge workflows can cause performance and memory challenges. The MaterialX library aka Data library hosts defintions and thus could be referenced by a document instead of importing them. A new set of methods have been introduced that allow registeration data library for a document. 1. registerDataLibrary(ConstDocumentPtr dataLibrary); 2. getRegisteredDataLibrary() const 3. hasDataLibrary() const Other changes include - Add DataLibrary support for getChildOfType and getChildrenOfType - Updates to redirect Document::get* calls to process datalibrary and local document.

ashwinbhat · 2024-10-08T22:13:36Z

source/PyMaterialX/PyMaterialXCore/PyElement.cpp

-.def("_getChildOfType" #T, &mx::Element::getChildOfType<mx::T>)                                                 \
-.def("_getChildrenOfType" #T, &mx::Element::getChildrenOfType<mx::T>, py::arg("category") = mx::EMPTY_STRING)   \


@bernardkwok @jstone-lucasfilm I need some guidance on these binding api
template <class T> shared_ptr<T> getChildOfType(ConstDocumentPtr datalibrary, const string& name) const
template <class T> shared_ptr<T> getChildOfType(const string& name) const

@ashwinbhat Initially I think it should be fine to omit these new Python bindings, while we're working out the details of the new API, and we can add these once the C++ API is locked down.

jstone-lucasfilm · 2024-10-08T22:41:59Z

source/MaterialXCore/Document.cpp

@@ -357,6 +357,14 @@ vector<OutputPtr> Document::getMaterialOutputs() const

 vector<NodeDefPtr> Document::getMatchingNodeDefs(const string& nodeName) const
 {
+    // Return all nodedefs from datalibrary if available


@ashwinbhat Wouldn't we also need to handle the case of a content document that contains its own custom nodes, as might be packaged and shared between studios? To handle that case, we'd want to first check for a matching nodedef in the data libraries, and then fall back to checking for a matching nodedef in the content document.

Following that pattern, perhaps we should take the same approach with the other updated methods in this pull request, including Document::getMatchingImplementation below.

@jstone-lucasfilm in the code below, we check in the datalibrary if available and then fallback to checking in local.

I see what you mean, but couldn't there be nodedefs for the given node name in both the data library and the content document? For example, the data library might have a node named multiply, but only the content document would have the specific version of multiply that the caller is looking for.

The proposed workflow, if the caller chooses to Register a datalibrary then the data library gets precedence. The caller can still continue to use the import mechanism.

We can update getMatchingNodeDefs so that it checks both data library and content document. Would that be preferred?

@ashwinbhat I'm imagining that we want the behavior to exactly match what a user would see if they called importLibrary instead of registerDataLibrary, and I believe that would require us to check both the data library and content document, merging the matches from both of them.

Let me know your thoughts, though, and that's just one opinion on the matter!

I think this is an interesting point to discuss - should registerDataLibrary match the importLibrary behavior?

I can certainly imagine circumstances where I might even want to not fallback to any local node definition in the local document and want to be able to validate that my document only uses nodes from the dataLibrary provided.

Perhaps registerDataLibrary() has some concept of strictness? or some input that could control its "fallback" behavior?

ld-kerley · 2024-10-09T00:11:52Z

source/MaterialXCore/Document.h

@@ -680,6 +708,8 @@ class MX_CORE_API Document : public GraphElement
  private:
    class Cache;
    std::unique_ptr<Cache> _cache;
+    // Data library for the document
+    ConstDocumentPtr _dataLibrary;


I think other methods like initialize() and copy() probably also need to be updated to handle this extra member variable.

ld-kerley · 2024-10-09T00:13:35Z

source/MaterialXCore/Element.cpp

@@ -564,7 +564,10 @@ bool ValueElement::validate(string* message) const
        const string& unittype = getUnitType();
        if (!unittype.empty())
        {
-            unitTypeDef = getDocument()->getUnitTypeDef(unittype);
+
+            unitTypeDef = getDocument()->hasDataLibrary() ? 


Is there a reason this logic isn' just in Document::getUnitTypeDef() like the other document query functions?

ld-kerley · 2024-10-09T00:20:36Z

source/MaterialXGenShader/ShaderGraph.cpp

@@ -202,10 +202,15 @@ void ShaderGraph::addDefaultGeomNode(ShaderInput* input, const GeomPropDef& geom
        // input here and ignore the type of the geomprop. They are required to have the same type.
        string geomNodeDefName = "ND_" + geomprop.getGeomProp() + "_" + input->getType().getName();
        NodeDefPtr geomNodeDef = _document->getNodeDef(geomNodeDefName);
-        if (!geomNodeDef)
+        if (!geomNodeDef && _document->hasDataLibrary())


This code here looks to me like we're inspecting the local document for a nodeDef first, and then falling back to the dataLibrary.

But then isn't Document::getNodeDef() above already checking the dataLibrary? so perhaps this additional secondary check isn't necessary?

ashwinbhat commented Oct 8, 2024

View reviewed changes

jstone-lucasfilm reviewed Oct 8, 2024

View reviewed changes

ld-kerley reviewed Oct 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Data Library API #2054

Data Library API #2054

ashwinbhat commented Oct 8, 2024

ashwinbhat Oct 8, 2024

jstone-lucasfilm Oct 8, 2024

jstone-lucasfilm Oct 8, 2024

ashwinbhat Oct 8, 2024

jstone-lucasfilm Oct 8, 2024

ashwinbhat Oct 8, 2024

jstone-lucasfilm Oct 8, 2024

ld-kerley Oct 9, 2024

ld-kerley Oct 9, 2024

ld-kerley Oct 9, 2024

ld-kerley Oct 9, 2024

		.def("_getChildOfType" #T, &mx::Element::getChildOfType<mx::T>) \
		.def("_getChildrenOfType" #T, &mx::Element::getChildrenOfType<mx::T>, py::arg("category") = mx::EMPTY_STRING) \

Data Library API #2054

Are you sure you want to change the base?

Data Library API #2054

Conversation

ashwinbhat commented Oct 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment