Skip to content

Commit

Permalink
Fix output for matrix multiple in GLSL code (#228)
Browse files Browse the repository at this point in the history
When Slang sees a matrix multiplication `M * v` in GLSL code it should (obviously) output GLSL code that also does `M * v`, but there was a bug introduced where the type-checker manages to resolve the operation and recognize it as a matrix-vector multiply, and then the code-generation logic says "oh, I'm generating output for GLSL, and that is reversed from HLSL/Slang, so I'd better reverse these operands!" and outputs `v * M`... which isn't what we want.

I've fixed the problem in an expedient way, by having the front-end resolve the operation to what it believes is an intrinsic multiply operation, rather than a matrix-vector operation. If we ever support cross compilation *from* GLSL (unlikely), we've need to fix this up so that we have both real matrix-vector multiplies and "reversed" multiplies where the operands folow the GLSL convention).

I've added two tests here to confirm the fix. The one under `tests/bugs` catches the actual issue described above, and confirms the fix. The other one under `tests/cross-compile` is just to make sure that we *do* properly reverse the operands to a matrix-vector product when converting from Slang to GLSL.
  • Loading branch information
Tim Foley authored Oct 23, 2017
1 parent 624d122 commit ab64cf2
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 6 deletions.
6 changes: 3 additions & 3 deletions source/slang/glsl.meta.slang
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,13 @@ sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M :int> __intrins
sb << "__generic<T : __BuiltinArithmeticType, let N : int> __intrinsic_op(dot) T operator*(vector<T,N> x, vector<T,N> y);\n";

// vector-matrix
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mulVectorMatrix) vector<T,M> operator*(vector<T,N> x, matrix<T,N,M> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mul) vector<T,M> operator*(vector<T,N> x, matrix<T,N,M> y);\n";

// matrix-vector
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mulMatrixVector) vector<T,N> operator*(matrix<T,N,M> x, vector<T,M> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mul) vector<T,N> operator*(matrix<T,N,M> x, vector<T,M> y);\n";

// matrix-matrix
sb << "__generic<T : __BuiltinArithmeticType, let R : int, let N : int, let C : int> __intrinsic_op(mulMatrixMatrix) matrix<T,R,C> operator*(matrix<T,R,N> x, matrix<T,N,C> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let R : int, let N : int, let C : int> __intrinsic_op(mul) matrix<T,R,C> operator*(matrix<T,R,N> x, matrix<T,N,C> y);\n";



Expand Down
6 changes: 3 additions & 3 deletions source/slang/glsl.meta.slang.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M :int> __intrins
sb << "__generic<T : __BuiltinArithmeticType, let N : int> __intrinsic_op(dot) T operator*(vector<T,N> x, vector<T,N> y);\n";

// vector-matrix
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mulVectorMatrix) vector<T,M> operator*(vector<T,N> x, matrix<T,N,M> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mul) vector<T,M> operator*(vector<T,N> x, matrix<T,N,M> y);\n";

// matrix-vector
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mulMatrixVector) vector<T,N> operator*(matrix<T,N,M> x, vector<T,M> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let N : int, let M : int> __intrinsic_op(mul) vector<T,N> operator*(matrix<T,N,M> x, vector<T,M> y);\n";

// matrix-matrix
sb << "__generic<T : __BuiltinArithmeticType, let R : int, let N : int, let C : int> __intrinsic_op(mulMatrixMatrix) matrix<T,R,C> operator*(matrix<T,R,N> x, matrix<T,N,C> y);\n";
sb << "__generic<T : __BuiltinArithmeticType, let R : int, let N : int, let C : int> __intrinsic_op(mul) matrix<T,R,C> operator*(matrix<T,R,N> x, matrix<T,N,C> y);\n";



Expand Down
28 changes: 28 additions & 0 deletions tests/bugs/matrix-mult.glsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//TEST:COMPARE_GLSL:-profile glsl_fragment_450 -no-checking
// matrix-mult.glsl
#version 450

// Confirm that we don't exchange the operands
// to a matrix-vector multiply when we recognize
// one in "raw" GLSL code.

#ifdef __SLANG__
__import empty;
#endif

layout(binding = 0)
uniform C
{
mat4x4 m;
};

layout(location = 0)
in vec4 v;

layout(location = 0)
out vec4 r;

void main()
{
r = m * v;
}
14 changes: 14 additions & 0 deletions tests/cross-compile/matrix-mult.slang
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//TEST:CROSS_COMPILE: -profile ps_5_0 -entry main -target spirv-assembly

// Confirm that order of arguments to matrix-vector
// multiplication gets reversed when generating GLSL.

cbuffer C
{
float4x3 m;
};

float4 main(float3 v) : SV_Target
{
return mul(m, v);
}
25 changes: 25 additions & 0 deletions tests/cross-compile/matrix-mult.slang.glsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//TEST_IGNORE_FILE:

layout(binding = 0)
uniform C
{
mat4x3 m;
};

vec4 main_(vec3 v)
{
return v * m;
}

layout(location = 0)
in vec3 SLANG_in_v;

layout(location = 0)
out vec4 SLANG_out_main_result;

void main()
{
vec3 v = SLANG_in_v;
vec4 main_result = main_(v);
SLANG_out_main_result = main_result;
}

0 comments on commit ab64cf2

Please sign in to comment.