Skip to content

Commit 9aacd43

Browse files
jstone-lucasfilmjstone-lucasfilm
authored
Simplify NodeDef::getImplementation (#2726)
This changelist builds upon recent work on `NodeDef::getImplementation`, simplifying the method and its underlying caching strategy for clarity, while maintaining all aspects of its behavior. Two important cases that were considered here are shader generation and LibsToOso.cpp, each of which requires different handling of graph indirection.
1 parent c342c0b commit 9aacd43

File tree

6 files changed

+42
-98
lines changed

6 files changed

+42
-98
lines changed

‎source/MaterialXCore/Definition.cpp‎

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -57,12 +57,38 @@ const string& NodeDef::getType() const
5757
}
5858
}
5959

60-
namespace
61-
{
62-
InterfaceElementPtr selectImplementation(const vector<InterfaceElementPtr>& interfaces, const TargetDefPtr targetDef)
60+
InterfaceElementPtr NodeDef::getImplementation(const string& target, bool resolveNodeGraph) const
6361
{
62+
vector<InterfaceElementPtr> interfaces = getDocument()->getMatchingImplementations(getQualifiedName(getName()));
63+
vector<InterfaceElementPtr> secondary = getDocument()->getMatchingImplementations(getName());
64+
interfaces.insert(interfaces.end(), secondary.begin(), secondary.end());
65+
66+
// If requested, resolve Implementation elements to their linked NodeGraph elements.
67+
if (resolveNodeGraph)
68+
{
69+
for (size_t i = 0; i < interfaces.size(); ++i)
70+
{
71+
ImplementationPtr impl = interfaces[i]->asA<Implementation>();
72+
if (impl && impl->hasNodeGraph())
73+
{
74+
NodeGraphPtr nodeGraph = getDocument()->getNodeGraph(impl->getNodeGraph());
75+
if (nodeGraph)
76+
{
77+
interfaces[i] = nodeGraph;
78+
}
79+
}
80+
}
81+
}
82+
83+
// Return the first implementation when no target is specified.
84+
if (target.empty())
85+
{
86+
return !interfaces.empty() ? interfaces[0] : InterfaceElementPtr();
87+
}
88+
6489
// Get all candidate targets matching the given target,
6590
// taking inheritance into account.
91+
const TargetDefPtr targetDef = getDocument()->getTargetDef(target);
6692
const StringVec candidateTargets = targetDef ? targetDef->getMatchingTargets() : StringVec();
6793

6894
// First, search for a target-specific match.
@@ -91,35 +117,6 @@ InterfaceElementPtr selectImplementation(const vector<InterfaceElementPtr>& inte
91117

92118
return InterfaceElementPtr();
93119
}
94-
};
95-
96-
InterfaceElementPtr NodeDef::getImplementation(const string& target) const
97-
{
98-
vector<InterfaceElementPtr> interfaces = getDocument()->getMatchingImplementations(getQualifiedName(getName()));
99-
vector<InterfaceElementPtr> secondary = getDocument()->getMatchingImplementations(getName());
100-
interfaces.insert(interfaces.end(), secondary.begin(), secondary.end());
101-
102-
if (target.empty())
103-
{
104-
return !interfaces.empty() ? interfaces[0] : InterfaceElementPtr();
105-
}
106-
107-
return selectImplementation(interfaces, getDocument()->getTargetDef(target));
108-
}
109-
110-
InterfaceElementPtr NodeDef::getUnmappedImplementation(const string& target) const
111-
{
112-
vector<InterfaceElementPtr> interfaces = getDocument()->getMatchingUnmappedImplementations(getQualifiedName(getName()));
113-
vector<InterfaceElementPtr> secondary = getDocument()->getMatchingUnmappedImplementations(getName());
114-
interfaces.insert(interfaces.end(), secondary.begin(), secondary.end());
115-
116-
if (target.empty())
117-
{
118-
return !interfaces.empty() ? interfaces[0] : InterfaceElementPtr();
119-
}
120-
121-
return selectImplementation(interfaces, getDocument()->getTargetDef(target));
122-
}
123120

124121
StringMap NodeDef::getInputHints() const
125122
{

‎source/MaterialXCore/Definition.h‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -142,19 +142,12 @@ class MX_CORE_API NodeDef : public InterfaceElement
142142
/// implementations.
143143
/// @param target An optional target name, which will be used to filter
144144
/// the implementations that are considered.
145+
/// @param resolveNodeGraph Allow resolution of Implementation elements
146+
/// to their linked NodeGraph elements. Defaults to true.
145147
/// @return An implementation for this nodedef, or an empty shared pointer
146148
/// if none was found. Note that a node implementation may be either
147149
/// an Implementation element or a NodeGraph element.
148-
InterfaceElementPtr getImplementation(const string& target = EMPTY_STRING) const;
149-
150-
/// Return the first implementation for this nodedef, optionally filtered
151-
/// by the given target name.
152-
/// @param target An optional target name, which will be used to filter
153-
/// the implementations that are considered.
154-
/// @return An implementation for this nodedef, or an empty shared pointer
155-
/// if none was found. Note that a node implementation may be either
156-
/// an Implementation element or a NodeGraph element.
157-
InterfaceElementPtr getUnmappedImplementation(const string& target = EMPTY_STRING) const;
150+
InterfaceElementPtr getImplementation(const string& target = EMPTY_STRING, bool resolveNodeGraph = true) const;
158151

159152
/// @}
160153
/// @name Hints

‎source/MaterialXCore/Document.cpp‎

Lines changed: 6 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,7 @@ class Document::Cache
4444
// Clear the existing cache.
4545
portElementMap.clear();
4646
nodeDefMap.clear();
47-
implementationDirectMap.clear();
48-
implementationIndirectMap.clear();
47+
implementationMap.clear();
4948

5049
// Traverse the document to build a new cache.
5150
for (ElementPtr elem : doc.lock()->traverseTree())
@@ -87,28 +86,9 @@ class Document::Cache
8786
InterfaceElementPtr interface = elem->asA<InterfaceElement>();
8887
if (interface)
8988
{
90-
if (interface->isA<NodeGraph>())
89+
if (interface->isA<Implementation>() || interface->isA<NodeGraph>())
9190
{
92-
implementationDirectMap[interface->getQualifiedName(nodeDefString)].push_back(interface);
93-
implementationIndirectMap[interface->getQualifiedName(nodeDefString)].push_back(interface);
94-
}
95-
ImplementationPtr impl = interface->asA<Implementation>();
96-
if (impl)
97-
{
98-
implementationDirectMap[interface->getQualifiedName(nodeDefString)].push_back(interface);
99-
100-
// Check for implementation which specifies a nodegraph as the implementation
101-
const string& nodeGraphString = impl->getNodeGraph();
102-
if (!nodeGraphString.empty())
103-
{
104-
NodeGraphPtr nodeGraph = impl->getDocument()->getNodeGraph(nodeGraphString);
105-
if (nodeGraph)
106-
implementationIndirectMap[interface->getQualifiedName(nodeDefString)].push_back(nodeGraph);
107-
}
108-
else
109-
{
110-
implementationIndirectMap[interface->getQualifiedName(nodeDefString)].push_back(interface);
111-
}
91+
implementationMap[interface->getQualifiedName(nodeDefString)].push_back(interface);
11292
}
11393
}
11494
}
@@ -124,8 +104,7 @@ class Document::Cache
124104
bool valid;
125105
std::unordered_map<string, std::vector<PortElementPtr>> portElementMap;
126106
std::unordered_map<string, std::vector<NodeDefPtr>> nodeDefMap;
127-
std::unordered_map<string, std::vector<InterfaceElementPtr>> implementationDirectMap;
128-
std::unordered_map<string, std::vector<InterfaceElementPtr>> implementationIndirectMap;
107+
std::unordered_map<string, std::vector<InterfaceElementPtr>> implementationMap;
129108
};
130109

131110
//
@@ -390,28 +369,9 @@ vector<InterfaceElementPtr> Document::getMatchingImplementations(const string& n
390369
_cache->refresh();
391370

392371
// Return all implementations matching the given nodedef string.
393-
if (_cache->implementationIndirectMap.count(nodeDef))
394-
{
395-
matchingImplementations.insert(matchingImplementations.end(), _cache->implementationIndirectMap.at(nodeDef).begin(), _cache->implementationIndirectMap.at(nodeDef).end());
396-
}
397-
398-
return matchingImplementations;
399-
}
400-
401-
vector<InterfaceElementPtr> Document::getMatchingUnmappedImplementations(const string& nodeDef) const
402-
{
403-
// Recurse to data library if present.
404-
vector<InterfaceElementPtr> matchingImplementations = hasDataLibrary() ?
405-
getDataLibrary()->getMatchingUnmappedImplementations(nodeDef) :
406-
vector<InterfaceElementPtr>();
407-
408-
// Refresh the cache.
409-
_cache->refresh();
410-
411-
// Return all implementations matching the given nodedef string.
412-
if (_cache->implementationDirectMap.count(nodeDef))
372+
if (_cache->implementationMap.count(nodeDef))
413373
{
414-
matchingImplementations.insert(matchingImplementations.end(), _cache->implementationDirectMap.at(nodeDef).begin(), _cache->implementationDirectMap.at(nodeDef).end());
374+
matchingImplementations.insert(matchingImplementations.end(), _cache->implementationMap.at(nodeDef).begin(), _cache->implementationMap.at(nodeDef).end());
415375
}
416376

417377
return matchingImplementations;

‎source/MaterialXCore/Document.h‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -546,16 +546,10 @@ class MX_CORE_API Document : public GraphElement
546546
removeChildOfType<Implementation>(name);
547547
}
548548

549-
/// Return a vector of all node implementations that match the given
550-
/// NodeDef string. Note that a node implementation may be either an
551-
/// Implementation element or NodeGraph element. Resolving any inheritance
552-
/// chain present in the implementations.
553-
vector<InterfaceElementPtr> getMatchingImplementations(const string& nodeDef) const;
554-
555549
/// Return a vector of all node implementations that match the given
556550
/// NodeDef string. Note that a node implementation may be either an
557551
/// Implementation element or NodeGraph element.
558-
vector<InterfaceElementPtr> getMatchingUnmappedImplementations(const string& nodeDef) const;
552+
vector<InterfaceElementPtr> getMatchingImplementations(const string& nodeDef) const;
559553

560554
/// @}
561555
/// @name UnitDef Elements

‎source/MaterialXGenOsl/LibsToOso.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ int main(int argc, char* const argv[])
398398
{
399399
// Determine whether or not there's a valid implementation of the current `NodeDef` for the type associated
400400
// to our OSL shader generator, i.e. OSL, and if not, skip it.
401-
mx::InterfaceElementPtr nodeImpl = nodeDef->getUnmappedImplementation(oslShaderGen->getTarget());
401+
mx::InterfaceElementPtr nodeImpl = nodeDef->getImplementation(oslShaderGen->getTarget(), false);
402402

403403
if (!nodeImpl)
404404
{

‎source/PyMaterialX/PyMaterialXCore/PyDefinition.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ void bindPyDefinition(py::module& mod)
2121
.def("setNodeGroup", &mx::NodeDef::setNodeGroup)
2222
.def("hasNodeGroup", &mx::NodeDef::hasNodeGroup)
2323
.def("getNodeGroup", &mx::NodeDef::getNodeGroup)
24-
.def("getImplementation", &mx::NodeDef::getImplementation)
2524
.def("getImplementation", &mx::NodeDef::getImplementation,
26-
py::arg("target") = mx::EMPTY_STRING)
25+
py::arg("target") = mx::EMPTY_STRING,
26+
py::arg("resolveNodeGraph") = true)
2727
.def("isVersionCompatible", &mx::NodeDef::isVersionCompatible)
2828
.def_readonly_static("CATEGORY", &mx::NodeDef::CATEGORY)
2929
.def_readonly_static("NODE_ATTRIBUTE", &mx::NodeDef::NODE_ATTRIBUTE)

0 commit comments

Comments
 (0)