microsoft / ShaderConductor

ShaderConductor is a tool designed for cross-compiling HLSL to other shading languages
MIT License
1.76k stars 251 forks source link

Options::packMatricesInRowMajor is inverted #41

Closed tklajnscek closed 4 years ago

tklajnscek commented 5 years ago

Describe the bug Options::packMatricesInRowMajor is inverted. Setting it to true, passes in '-Zpc' which makes the compiler generate column major matrices.

To Reproduce Inspect the source code:

        // HLSL matrices are translated into SPIR-V OpTypeMatrixs in a transposed manner,
        // See also https://antiagainst.github.io/post/hlsl-for-vulkan-matrices/
        if (options.packMatricesInRowMajor)
        {
            dxcArgStrings.push_back(L"-Zpc");
        }
        else
        {
            dxcArgStrings.push_back(L"-Zpr");
        }

Expected behavior The values need to be inverted. Reading the comment it seems that this might've been intentionally inverted to fix SPIRV codegen, but my DX12/Vulkan/GLES engine works correctly for all three APIs when I pass in -Zpr and my matrices are row major in memory.

gongminmin commented 5 years ago

How do you use your matrix in shader? If you are writing mul(vector, matrix), a row major matrix on CPU need to be transposed and copy to cbuffer. If it's mul(matrix, vector), you don't need the transpose matries on CPU, but the shader is in lower efficiency.