Update token support for filename resolve and code generation #1171
Conversation
This allows for Interface elements such as nodes and nodegraphs to add any child tokens in the same manner as geominfo elements can. Currently this is only traversing up to the parent. Potentially UDIM replacement could also be performed in the same manner as this would be Document level tokens.
Add in nodegraph test with tokens.
| // Apply any interface tokens to the filename | ||
| for (auto token : getActiveTokens()) | ||
| { | ||
| string key = "[" + token->getName() + "]"; |
There was a problem hiding this comment.
Interface tokens use [] delmiters vs <>.
source/MaterialXCore/Element.h
Outdated
| /// Add tokens to string resolver. Derived classes can override this | ||
| /// method to add per-class type tokens as desired. The default is to not | ||
| /// add any tokens. | ||
| virtual void addTokens(StringResolverPtr& /*resolver*/) const; |
There was a problem hiding this comment.
addTokens() is a new virtual to allow each class derived from Element to add tokens to a string resolve as desired.
I used Element since it's the only common base class for InterfaceElement (ports) and GeomInfo.
source/MaterialXCore/Element.cpp
Outdated
| } | ||
| } | ||
|
|
||
| // Check for any parent tokens | ||
| ConstElementPtr parent = getParent(); |
There was a problem hiding this comment.
This will look for elements which are siblings.
|
Should you add a test case for this? |
| <materialx version="1.38"> | ||
| <nodegraph name="Tokenized_Image" > | ||
| <token name="Image_Resolution" type="string" uniform="true" value="2k" uiname="Image Resolution" /> | ||
| <token name="Image_Extension" type="string" uniform="true" value="png" uiname="Image Extension" /> |
There was a problem hiding this comment.
Nitpick: I guess the uniform flag is not used for tokens.
There was a problem hiding this comment.
Will remove. It was actually done on save of this file.
There was a problem hiding this comment.
Ah yes since the tokens are represented as inputs it writes all input attributes to the file. Not a big issue but we could fix it by adding an attribute ignore list for tokens.
| doc->validate(); | ||
|
|
||
| mx::NodeGraphPtr graph = doc->getNodeGraph("Tokenized_Image"); | ||
| if (graph) |
There was a problem hiding this comment.
Nitpick: Maybe call REQUIRE(graph) here instead, since the test can't run if the test graph cannot be found.
|
|
||
| // Test file name substitution creation. | ||
| mx::StringResolverPtr resolver = input->createStringResolver(); | ||
| mx::StringMap substitutions = resolver->getFilenameSubstitutions(); |
There was a problem hiding this comment.
Nitpick: Maybe use const ref to avoid copy the StringMap.
…okens(). Here interface Inputs have precendence. Update test to check this confguration as well as non-interfacename inputs.
| @@ -317,6 +317,27 @@ InputPtr Input::getInterfaceInput() const | |||
| return nullptr; | |||
| } | |||
|
|
|||
| void Input::addTokens(StringResolverPtr& resolver) const | |||
There was a problem hiding this comment.
Need to customize inputs so that interfaces are taken into consideration. If there is an interface than use tokens at that level.
source/MaterialXCore/Interface.cpp
Outdated
| if (parent) | ||
| { | ||
| parent->addTokens(resolver); | ||
| } |
There was a problem hiding this comment.
Nitpick: You could call the base class method instead of duplicating that code here, in case it changes in the future.
Update #1159
Generalize token addition to a string resolver based on Element type.
<node>and<nodegraph>to add child tokens in the same manner as<geomInfo>elements can via a StringResolver.<token>Elements. However if the Element is an<input>and it has an interface then check tokens against that input instead.Example:
The following is a test graph with two tokens "exposed" as part of a compound
<nodegraph>with corresponding MTLX:
The resolved name would be
resources/images/cloth_2k.pngThe GLSL / Vanilla OSL unit tests results look like this:
