DirectXShaderCompiler icon indicating copy to clipboard operation
DirectXShaderCompiler copied to clipboard

No warning/error when SV_Position semantic is missing from Vert/Domain/Geom output and POSITION is used instead

Open Dredhog opened this issue 4 years ago • 5 comments

Title

No warning/error when SV_Position semantic is missing from Vert/Domain/Geom output and POSITION is used instead

Functional impact

Shaders written for FXC which use the d3d9 style POSITION semantic for the vertex pipeline output will compile with DXC but will produce no output rendering. The POSITION semantic is just interpreted as a user semantic but the fact that SV_Position is not written out is not reported, which is most often a user error. P.S. this gotcha is also not documented in the Porting shaders from FXC to DXC wiki

Minimal repro steps

  1. Compile the following shader with dxc.exe -T vs_6_0 -E Vert repro_shader.txt (or with dxc.exe -T vs_6_0 -E Vert -spirv repro_shader.txt for the SPIR-V backend)
float4 Vert(float4 position : POS) : POSITION
{
	return position;
}

Expected result

An error or at least a warning is printed when there is no SV_Position in the Vertex/Domain/Geometry shader output, possibly pointing to the user defined POSITION semantic as the place for user correction. Similar to what DXC does for uses of the legacy COLOR or DEPTH semantics in the pixel shader output.

Actual result

Shader compiles without any POS SysValue (for the DXIL backend) and no gl_Position (for the SPIR-V backend) in the output. The resulting shader produces no rendering at runtime.

Further technical details

N.B. To my understanding DXC should probably only not warn/error when using the stream output stage in the vertex pipeline but we at Unity don't ever use stream output and would prefer if DXC had a compiler flag to enable this warning when we know before compilation that the shader will never be used for stream output. Used DXC release v1.6.2104 (e09a454)

Dredhog avatar May 04 '21 07:05 Dredhog

@pow2clk @vcsharma It looks similar to #3626. I am considering a warning emission for POSITION. What do you think?

jaebaek avatar May 07 '21 14:05 jaebaek

Hello!

Thanks for the report! From reading the HLSL documentation, it seems we shouldn't error nor warn, but handle POSITION as SV_Position. Seems like both backends are doing that mistake.

@llvm-beanz: is that true?

From the MSDN documentation (https://learn.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-semantics) it seems the POSITION[n] semantic is supported from DX9 and forward, and in the D3D9 -> D3D10 mapping, it says "POSITION0 (and its variant names) is treated as SV_Position".

Only the COLOR semantic seems to be deprecated, as said in the doc:

The COLOR semantic is only valid in shader compatibility mode (that is, when the shader is created using D3D10_SHADER_ENABLE_BACKWARDS_COMPATIBILITY).

Keenuts avatar Feb 29 '24 14:02 Keenuts

Or maybe @pow2clk ?

Keenuts avatar Mar 05 '24 15:03 Keenuts

@tex3d would probably know the history better than me. Observationally it looks like we translate POSITION as SV_POSITION (something we should really deprecate and remove), and we treat COLOR as a user defined semantic rather than a system value.

llvm-beanz avatar Mar 05 '24 16:03 llvm-beanz

Right. If we want to deprecate the use of POSITION as an alias for SV_Position, shall the MSDN docs also be updated? (Suggested change is https://github.com/MicrosoftDocs/win32/pull/1813 )

On the SPIR-V side, we don't support a lot of old behaviors, so I believe it would be safe to consider those old semantics to always be user-defined semantics, and possibly add a warning when such shader with no output is compiled.

Keenuts avatar Mar 07 '24 13:03 Keenuts