Fix geomPropValue for GLSL to output varying vs unifom

autodesk-forks / MaterialX

MaterialX C++ and Python libraries

http://www.materialx.org/

Apache License 2.0

107 stars 23 forks source link

Fix geomPropValue for GLSL to output varying vs unifom #1042

Closed bernardkwok closed 3 years ago

bernardkwok commented 4 years ago

Currently the custom GLSL code generation node (MaterialXGenGlsl\Nodes\GeomPropValueNodeGlsl.cpp) is outputting a uniform instead of a varying parameter. This should be changed to output a varying to match the current specification (as of v1.38).

Work:

Fixes to GeomPropValueNodeGLSL.cpp
Testing for raw glsl, ogsfx, and ogsxml variants.

Follow-up:

Checking that this will still work with SPIR-V cross-compilation.

Whether to extend geomPropValue to indicate that the input is a uniform is a separate issue beyond the scope of this fix. A placeholder item can be found here: #1051.

JGamache-autodesk commented 4 years ago

Testing raises some questions about handling invalid types in GLSL. What should be the GLSL code output for types bool and string? Also there is a requirement to add the keyword flat to integer varying inputs. Easy to implement, but might trip the HLSL converter.

bernardkwok commented 4 years ago

For the first point, as you can't have boolean or string vertex streams we could leave these as uniforms. We are no worse off than before. Any language which does not support "string" we output integers instead and leave it up the the binding process to determine the string->integer mapping.
As for integer streams, I'm not sure of the frequency of these so maybe we could also defer this for now and also just keep them as uniforms.
Which leaves us with 2-4 channel float support.

bernardkwok commented 3 years ago

Pulling out a note from PR as there was a possible concern with adding in the "flat" qualifier for integer varyings. Seems this is a non-issue (See https://github.com/KhronosGroup/glslang/issues/940)