KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.91k stars 816 forks source link

Mismatch in GLSL/Vulkan behavior for gl_FragDepth #2918

Open Tobski opened 2 years ago

Tobski commented 2 years ago

As of the 1.3.210 Vulkan spec update, Vulkan clarified that not writing to the FragDepth built-in is undefined behavior - undefined values can lead to undefined behavior, so the difference is somewhat moot.

However, GLSL states:

If a shader statically assigns a value to gl_FragDepth, and there is an execution path through the shader that does not set gl_FragDepth, then the value of the fragment’s depth may be undefined for executions of the shader that take that path. That is, if the set of linked fragment shaders statically contain a write to gl_FragDepth, then it is responsible for always writing it.

Which seems to intend that there's no potential for crashes.

In order to reconcile this, GLSLang should initialise the value of gl_FragDepth any time gl_FragDepth is declared, when generating SPIR-V. Initializing it to gl_FragCoord.z is likely the safest choice, as this is most likely what developers intend when writing such code.

greg-lunarg commented 2 years ago

if the set of linked fragment shaders statically contain a write to gl_FragDepth, then it is responsible for always writing it.

By my reading, "it" is the set of linked fragment shaders, not glslangValidator. This section seems to me to put the burden on the shader author, not glslangValidator, and so does not seem to be requiring glslangValidator to initialize gl_FragDepth.

Am I misunderstanding this? Please explain.

Tobski commented 2 years ago

The bit that is ambiguous is that GLSL/GL suggest that there's no chance of things blowing up with that undefined value; something that can happen with Vulkan at the SPIR-V level. So the same shader on GL would just result in surprise values showing up, whereas in Vulkan it can explode unless we code defensively in glslang.