[hlsl-out] fix matrix not being declared as transposed

[teoxoy]

HLSL uses inverted col/row notation (relative to GLSL, WGSL, MSL).

i.e. mat3x2 in WGSL = float2x3 in HLSL

Reference: HLSL Matrix Doc

related to original change in #1199

[cwfitzgerald]

I'm confused, this PR appears to be making the hlsl declaration types be the same as wgsl, with column first and then row, but the comments and the PR say that it should be inverted.

[teoxoy]

HLSL's matrix type is of the form matRxC

matrix type in GLSL/WGSL/MSL is of the form matCxR

So passing column first, row second, to HLSL will cause the matrix to be the transposed version of the original.

[teoxoy]

Before this PR, trying to change the access test as in 0d920f5 and passing the resulting HLSL code trough DXC results in the error below (too many elements in vector initialization (expected 4 elements, have 5))

see https://shader-playground.timjones.io/582968a71c9665f384078ed17280d4f3

With the fix in this PR this no longer happens.

[cwfitzgerald]

One comment, but otherwise looks okay

[kvark]

Thank you for the PR!
We need to take a bit of extra thought to review this, since we specifically output all matrices in buffers as row-major.
Our HLSL matrix logic is a bit non-trivial. Maybe it's a good chance to try to document it thoroughly in the code (i.e. at the top of the module) and understand clearly.

[teoxoy]

No problem.

Regarding documentation, there is already this nicely written note

naga/src/back/hlsl/mod.rs

Lines 9 to 21 in 05f050f

	All matrix construction/deconstruction is row based in HLSL. This means that when
	we construct a matrix from column vectors, our matrix will be implicitly transposed.
	The inverse transposition happens when we call `[0]` to get the zeroth column vector.
	Because all of our matrices are implicitly transposed, we flip arguments to `mul`. `mat * vec`
	becomes `vec * mat`, etc. This acts as the inverse transpose making the results identical.
	The only time we don't get this implicit transposition is when reading matrices from Uniforms/Push Constants.
	To deal with this, we add `row_major` to all declarations of matrices in Uniforms/Push Constants.
	Finally because all of our matrices are transposed, if you use `mat3x4`, it'll become `float4x3` in HLSL.
	[hlsl]: https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl

is there anything else you'd like me to add?

[kvark]

So to me this comment reads like the current code is already correct, isn't it?

[teoxoy]

Finally because all of our matrices are transposed, if you use mat3x4, it'll become float4x3 in HLSL.

This line is not right and what this PR is solving. mat3x4 will become a mat4x3 (GLSL/WGSL/MSL notation) but will become a float3x4 in HLSL (due to HLSL's matrix notation being inverted already).

I fixed this doc line in this PR as well.

This whole thing is quite confusing and I got confused as well while trying to fix it.

[teoxoy]

The easiest way for me to visualize this was to compare DXC's SPIR-V output with naga's SPIR-V output since DXC is also transposing the matrix when translating to SPIR-V.

DXC HLSL -> SPIR-V

https://shader-playground.timjones.io/29a832dba447e0ba043258dc84a753e7

naga WGSL -> SPIR-V

https://shader-playground.timjones.io/748b0d2af9ac941580a1278f2a853123

[kvark]

This line is not right and what this PR is solving. mat3x4 will become a mat4x3 (GLSL/WGSL/MSL notation)

Sorry, I'm not following this. Here is the GLSL code for matrix type:

            TypeInner::Matrix {
                columns,
                rows,
                width,
            } => write!(
                self.out,
                "{}mat{}x{}",
                glsl_scalar(crate::ScalarKind::Float, width)?.prefix,
                columns as u8,
                rows as u8
            )?,

Clearly it makes it so mat3x4 in WGSL becomes mat3x4 in GLSL. I.e. there is no difference in notation. The only difference is with HLSL.

[teoxoy]

Sorry I think I made my explanation more confusing than it should have been.

See last comment line below with () added by me.

Finally because all of our matrices are transposed, if you use mat3x4 (3 columns, 4 rows), it'll become float4x3 (3 columns, 4 rows -> issue is here, not transposed) in HLSL.

[kvark]

Thanks for your patience!
I launched a small sanity test in https://github.com/gfx-rs/naga/runs/5714948153?check_suite_focus=true to see if the current algorithm produces invalid HLSL, and indeed that's the case.