-
Notifications
You must be signed in to change notification settings - Fork 203
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
MaterialX 1.39.x support #4134
base: dev
Are you sure you want to change the base?
MaterialX 1.39.x support #4134
Conversation
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.
That will allow compiling. I do have doubts about the shadergen fully working with 1.39 out of the box. There are a few 1.39 backports that are no longer required.
@@ -161,7 +161,11 @@ const string& HwSpecularEnvironmentSamples::name() | |||
|
|||
string GlslFragmentSyntax::getVariableName( | |||
const string& name, | |||
#if MATERIALX_MAJOR_VERSION == 1 && MATERIALX_MINOR_VERSION <= 38 |
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.
Please use the combined syntax:
#if MX_COMBINED_VERSION < 13900
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.
Done.
void GlslFragmentGenerator::toVec3(const TypeDesc* type, string& variable) | ||
{ | ||
if (type->isFloat2()) { | ||
variable = "vec3(" + variable + ", 0.0)"; | ||
} else if (type->isFloat4()) { | ||
variable = variable + ".xyz"; | ||
#else | ||
void GlslFragmentGenerator::toVec3(const TypeDesc type, string& variable) |
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.
Please use const TypeDesc&
?
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.
Done.
std::string channelInfo = ngInput.getChannels(); | ||
#else |
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.
Remove the #else
part. Channels are gone, so we should not see any here. And if we see some, it will be because we forgot to upgrade the document from 1.38 to 1.39, which might be needed in a few places.
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.
For this one, do you think gatherChannels
could just be bypassed entirely, or return an empty string?
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.
Yes, please bypass it entirely on 1.39.
ec6ae9a
to
ebafe82
Compare
Hi,
This is a simple modification to work with MaterialX 1.39.x so that it can compile against the changes that were made from the 1.38.x version, all pre-processor modifications eg.
I've not made extensive testing with this to see if everything is working, it merely lets you compile. The channels API was removed so this probably needs to most attention to see if the behaviour has changed or broke things.
Addresses #4129