-
Notifications
You must be signed in to change notification settings - Fork 358
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for custom MDL nodes #2125
Add support for custom MDL nodes #2125
Conversation
@jstone-lucasfilm I believe this PR does not really add something new. It just allows to use standard MaterialX functionality provided by the node. |
This looks very promising, thanks @krohmerNV. One concern that I have is the addition of language-specific examples to Materials/TestSuite, where previously all of our test suite examples have been language-independent. Since the introduction of that new pattern may take more design thought, I'm thinking that we may want to organize this contribution into two separate steps, where the first step introduces the new logic in MaterialXGenMdl that supports the usage of custom MDL nodes, and the second step is focused on a new design for our test suite that will accommodate language-specific tests. What are your thoughts? |
That's a reasonable approach, yes. |
@krohmerNV Sounds good! |
…m node implementations
- sourcecode nodes always have to prefix to make it easy for authors without knowledge of reserved words in mdl - file/function nodes use the prefix only for names that collide with reserved words in order to support existing mdl modules without change
3d3ed4c
to
77ad9c2
Compare
done |
This may be a useful change in the future, but let's omit it from this specific proposal for simplicity. Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Thank you @jstone-lucasfilm for all the fixes! |
pass string parameter by reference Signed-off-by: krohmerNV <42233792+krohmerNV@users.noreply.github.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
Signed-off-by: Jonathan Stone <jstone@lucasfilm.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me, thanks @krohmerNV!
94ce95d
into
AcademySoftwareFoundation:main
The MDL generator is limited in a way that it only supported the mappings defined in the libraries/***lib/genmdl files.
With this pull request we now fully support nodes.
This comes in two flavors:
We allow to specify an MDL module located in an MDL search path using the
file
attribute. In combination with thefunction
attribute a particular exported function can be selected. There is one aspect that needs to be considered when using this approach:mxp_
prefix in case of such a conflict. The prefix is not applied in general because we want to be able to reference MDL modules without modifying them.By specifying the
sourcecode
attribute, MDL code can be added directly. The generator will inline this code into the generated MDL code, based on the interface of the corresponding node definition. Here, more aspects need to be considered:mxp_
to keep things simple for authors that otherwise would need knowledge about the keywords in MDL.//
. Instead the/* comment */
syntax can be used.Both variants support multiple outputs and for both variants tests have been added to illustrate and validate.
Additionally one more complex procedural material was added that makes use of the inline source code.