Fix MaterialX not working with AMD material lib #2236
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
JGamache-autodesk
commented
Apr 1, 2022
JGamache-autodesk
commented
- Fixed a bug where a folded constant was mistook for a renamed variable
- Fixed a bug where constant nodes had incorrect input name
- Cleaned up unnecessary MaterialX data copied by addInputFromNodeDef
- Fixed a bug where a folded constant was mistook for a renamed variable - Fixed a bug where constant nodes had incorrect input name - Cleaned up unnecessary MaterialX data copied by addInputFromNodeDef
| @@ -484,11 +486,11 @@ void _AddMissingTexcoordReaders(mx::DocumentPtr& mtlxDoc) | |||
| _mtlxTokens->geompropvalue.GetString(), | |||
| _mtlxTokens->ST_reader.GetString(), | |||
| _mtlxTokens->vector2.GetString()); | |||
| mx::ValueElementPtr prpInput | |||
| = stReader->addInputFromNodeDef(_mtlxTokens->geomprop.GetString()); | |||
There was a problem hiding this comment.
addInputFromNodeDef will copy all properties from the NodeDef. We only need name and type.
| passthruNode->addInputFromNodeDef(_mtlxTokens->in.GetString()); | ||
| passthruNode->setConnectedNodeName(_mtlxTokens->in.GetString(), newSource->getName()); | ||
| passthruNode->addInput( | ||
| _mtlxTokens->value.GetString(), _mtlxTokens->vector3.GetString()); |
There was a problem hiding this comment.
Bugfix: the input of a constant node is called "value", not "in".
| MString uniqueName(input.c_str()); | ||
| while (!input.empty() && path.back() != input.back()) { | ||
| input.pop_back(); | ||
| // If a digit was added, we should be able to find the last path element inside the |
There was a problem hiding this comment.
We want to fix N2_color renamed to N2_color1 because color is a reserved keyword. There was a bug where a constant node N3_value would be folded into N6_in1. This caused a crash because we could not fix the name correctly, and we should not even try to fix the name.
JGamache-autodesk
added
ready-for-merge
williamkrick
approved these changes
Apr 4, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.