glslang
glslang copied to clipboard
Make gl_HitT proper aliases of gl_RayTmax
In https://github.com/KhronosGroup/glslang/pull/2759, a mechanism to better handle the aliased builtin variables gl_VertexID and gl_VertexIndex (and their Instance counterparts) was committed. There is a similar problem for gl_HitTEXT and gl_RayTmaxEXT (and their NV counterparts), where they end up handled as duplicated variables and SPIR-V generation ends up creating two OpVariables with the same builtin decoration.
This PR simply makes use of the RetargetVariable code from https://github.com/KhronosGroup/glslang/pull/2759, as well as remove some then redundant code for handling gl_HitT.
By my reading, gl_HitTNV and gl_RayTmaxNV are not aliases in GL_NV_ray_tracing. Likewise HitTNV and RayTmaxNV are different things in SPV_NV_ray_tracing. So this should not cause changes for GL_NV_ray_tracing shaders targeting SPV_NV_ray_tracing. Right?
Yet I am seeing, for example, changes for spv.AnyHitShader.rahit test which is a GL_NV_ray_tracing shader targeting SPV_NV_ray_tracing.
Should this be changing? Or am I misunderstanding something here?
I see, indeed the SPIR-V has two distinct decorations for the two in NV, unlike the EXT versions which only has RayTmaxKHR. I'll revert the changes to the NV variables, thanks for pointing it out!
@Max-Andersson do you plan to pick this back up at some point?
@greg-lunarg Looking at the GLSL_NV_ray_tracing spec, it does say "gl_HitTNV is available only in the any-hit and closest-hit shaders. This is an alias of gl_RayTmaxNV added to closest-hit and any-hit shaders for convenience." (line 657 here: https://github.com/KhronosGroup/GLSL/blob/master/extensions/nv/GLSL_NV_ray_tracing.txt). Seems like the specs are somewhat ambivalent on the issue.