HLSL: Support Y flip for position outputs from VS/GS

[zeux]

This was previously discussed in #494 but I want to bring this up because the solution there is insufficient in practice.

APIs that natively support HLSL all have NDC Y pointing up. Because of this, any HLSL code base would assume this when outputting SV_Position. glslang currently compiles code like this as is, which, when used in Vulkan, would result in flipped rendering.

It's possible to solve this on the shader side, but that requires modifying every single vertex/geometry shader, which is obviously fragile.
The outcome of #494 seems to have been the VK_KHR_maintenance1 extension which allows you to flip the viewport height. Unfortunately, the support of this extensions remains non-existent on Android: https://vulkan.gpuinfo.org/listreports.php?extension=VK_KHR_maintenance1&option=not

An option that seems user-friendly is to flip position Y in glslang. This needs to be flagged in some way so that the caller can optionally specify it (it would also need to be eventually implemented in DXC...).

One alternative is to make a SPIRV-SPIRV pass but that seems somewhat more fragile (?).

#494 has some concern around which stage to flip the position in - I feel like it's sufficient for glslang to always flip gl_Position, and then the caller can appropriately pass this value if necessary (I'm not clear if it's valid to output gl_Position in VS and read it later in GS, which is the only scenario where you'd need to disable Y flip in VS).

[danginsburg]

I faced this problem as well. We use HLSL and VK_KHR_maintenance1 in order to get y direction matching DX11. However, since some implementations still don't support that extension, I ended up writing a SPIR-V -> SPIR-V translator that patched the y: https://github.com/danginsburg/spirv-utils/blob/master/source/invert_position_y.cpp

There were other options I could have taken, for example modifying all the HLSL to invert Y in shader code and not rely on VK_KHR_maintenance1, but that is error prone and would have touched almost every one of our materials. Another option would have been to use a specialization constant (I think, assuming that works in HLSL in glslang?), but there too I still would have had to modify shader code.

I think if glslang had an option to flip for me, I would simply take that route and remove the reliance on VK_KHR_maintenance1. This seems like a good feature to add to glslang.

[redorav]

Isn't it a question of modifying the projection matrix so it takes this into account? You wouldn't need any shader patching, it would require having different projection matrices per API. We do this for OpenGL and its [-1, 1] z range.

[kayru]

@redorav patching the projection matrix certainly works for cases where it is used. However, there are cases (such as 2d rendering) where you just don't have a transform that you can patch from the CPU side or patching it would be quite fragile / error prone.

[danginsburg]

Yes, what @kayru said, and also engines often have a desire to keep matrices identical between APIs. For example, our engine stores constant buffers in a shared identical format across rendering APIs and does not need to know about coordinate system internals of the underlying rendering API. I think that's a common pattern.

[zeux]

We're in the same situation as well. For both OpenGL and Vulkan, we currently use hlslparser to do HLSL->GLSL conversion, and it injects the appropriate transformation at the end of the vertex shader. This proved to be the simplest option that automatically handles the conversion regardless of what the shader is doing - while in OpenGL you have other coordinate system adjustments you need to make because of depth range and, more importantly, texture Y direction, for Vulkan this is the only thing you need to do to have D3D shaders work.

[ghost]

@danginsburg noted the inversion should happen for more than just the VS.

It doesn't apply for PS/CS. I don't think it should happen for HS, but I'm not certain about that.

Is VS/GS/DS the right set?

[ghost]

If there's no other feedback on the stage list, then I'll remove the WIP... the current incantation of #1174 will apply to VS/GS/DS.

[zeux]

@loopdawg FWIW I believe it's the right set - you need to flip at any stage that can directly connect to rasterizer. If you are using both VS and GS in your pipeline you need to flip in GS (but then VS might not even output gl_Position, or the caller can anticipate that VS isn't feeding data to rasterizer in this setup); if you are using domain shader but aren't using GS you need to flip in DS; if you are using both DS & GS you still need to flip in GS since it's the last stage before the rasterizer.

[ghost]

@zeux - ok thanks!

Of course it is still gated on the compilation option as well.

[danginsburg]

I was able to test y flipping and verify that it worked for our shaders, including GS/DS. Thanks!