shader-slang / slang

Making it easier to work with shaders
MIT License
1.78k stars 158 forks source link

GLSL matrix row/column major-ness issue #4360

Open jkwak-work opened 1 week ago

jkwak-work commented 1 week ago

Problem description Following four CTS tests are failing as a regression,

dEQP-VK.glsl.matrix.div.uniform.mediump_mat2x3_float_vertex
dEQP-VK.glsl.matrix.div.uniform.mediump_mat2x3_float_fragment
dEQP-VK.glsl.matrix.div.uniform.highp_mat2x3_float_vertex
dEQP-VK.glsl.matrix.div.uniform.highp_mat2x3_float_fragment

This regression is from #4069, and it appears to be related to the matrix row/column major-ness.

Note that there are other tests that are similar but passing,

dEQP-VK.glsl.matrix.div.const.mediump_mat2x3_float_vertex // PASSING
dEQP-VK.glsl.matrix.div.const.mediump_mat2x3_float_fragment // PASSING
dEQP-VK.glsl.matrix.div.const.highp_mat2x3_float_vertex // PASSING
dEQP-VK.glsl.matrix.div.const.highp_mat2x3_float_fragment // PASSING

The issue seems to be limited to the case where the matrix data is fed via uniform. Also note that the tests are failing with both -emit-spirv-directly and -emit-spirv-via-glsl.

Goal It appears to be an issue with matrix-major-ness. We may want to use RowMajor decorator on matrices, but it is unclear why the test is failing.

Repro step The following shaders are used for the CTS test,

// Vertex shader
#version 310 es
layout(location = 0) in highp vec4 a_position;
layout(location = 0) out mediump vec4 v_color;
layout(std140, set = 0, binding = 0) uniform buffer0 { mediump mat2x3 u_in0; };
layout(std140, set = 0, binding = 1) uniform buffer1 { mediump float u_in1; };

void main (void)
{
        gl_Position = a_position;
        mediump mat2x3 res = u_in0 / u_in1;
        v_color = vec4(res[0] + res[1], 1.0);
}
// Fragment shader
#version 310 es
layout(location = 0) out mediump vec4 dEQP_FragColor;
layout(location = 0) in mediump vec4 v_color;

void main (void)
{
        dEQP_FragColor = v_color;
}

The test expect a full white image, but we generate a yellow image. I cannot figure out what values are used for u_in0 and u_in1. But it is probably using the same value as the test below.

As a comparison, the passing test, "dEQP-VK.glsl.matrix.div.const.mediump_mat2x3_float_vertex", uses the following vertex shader.

#version 310 es
layout(location = 0) in highp vec4 a_position;
layout(location = 0) out mediump vec4 v_color;
const mediump mat2x3 in0 = mat2x3(-0.6, -0.7, -0.2, -0.1, -1.2, 0.0);
layout(std140, set = 0, binding = 0) uniform buffer1 { mediump float u_in1; };

void main (void)
{
        gl_Position = a_position;
        mediump mat2x3 res = in0 / u_in1;
        v_color = vec4(res[0] + res[1], 1.0);
}
csyonghe commented 1 week ago

I think this is just because the CTS code isn't properly setting matrix majorness when triggering a slang compile. We should specify column major in the slang api when compiling these shaders.

jkwak-work commented 1 week ago

I see. That sounds simple.