[PATCH v3 0/1] MR329: vkd3d-shader/hlsl: Add determinant() function.

[Nikolay Sivov (＠nsivov)]

I wanted to use ._xy element access to make it shorter, but that attempt was crushed by the reality - such indexing apparently compiles but does not produce correct element access loads. I'm going to update once this is fixed.

--
  v3: vkd3d-shader/hlsl: Add determinant() function.

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/329

[PATCH v3 1/1] vkd3d-shader/hlsl: Add determinant() function.

[Nikolay Sivov]

From: Nikolay Sivov

Signed-off-by: Nikolay Sivov 
---
 Makefile.am                        |   1 +
 libs/vkd3d-shader/hlsl.y           |  62 ++++++++++++++++++
 tests/hlsl/determinant.shader_test | 100 +++++++++++++++++++++++++++++
 3 files changed, 163 insertions(+)
 create mode 100644 tests/hlsl/determinant.shader_test

diff --git a/Makefile.am b/Makefile.am
index b36358b2..eebc359e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -78,6 +78,7 @@ vkd3d_shader_tests = \
    tests/hlsl/cross.shader_test \
    tests/hlsl/d3dcolor-to-ubyte4.shader_test \
    tests/hlsl/ddxddy.shader_test \
+	tests/hlsl/determinant.shader_test \
    tests/hlsl/discard.shader_test \
    tests/hlsl/distance.shader_test \
    tests/hlsl/dot.shader_test \
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 161d1ab4..44cfc46b 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -2839,6 +2839,67 @@ static bool intrinsic_ddy_fine(struct hlsl_ctx *ctx,
     return !!add_unary_arithmetic_expr(ctx, params->instrs, HLSL_OP1_DSY_FINE, arg, loc);
 }

+static bool intrinsic_determinant(struct hlsl_ctx *ctx,
+        const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
+{
+    static const char determinant2x2[] =
+            "float determinant(float2x2 m)\n"
+            "{\n"
+            "    return m[0][0] * m[1][1] - m[0][1] * m[1][0];\n"
+            "}";
+    static const char determinant3x3[] =
+            "float determinant(float3x3 m)\n"
+            "{\n"
+            "    float2x2 m1 = { m[1][1], m[1][2], m[2][1], m[2][2] };\n"
+            "    float2x2 m2 = { m[1][0], m[1][2], m[2][0], m[2][2] };\n"
+            "    float2x2 m3 = { m[1][0], m[1][1], m[2][0], m[2][1] };\n"
+            "    float3 v1 = { m[0][0], -m[0][1], m[0][2] };\n"
+            "    float3 v2 = { determinant(m1), determinant(m2), determinant(m3) };\n"
+            "    return dot(v1, v2);\n"
+            "}";
+    static const char determinant4x4[] =
+            "float determinant(float4x4 m)\n"
+            "{\n"
+            "    float3x3 m1 = { m[1][1], m[1][2], m[1][3], m[2][1], m[2][2], m[2][3], m[3][1], m[3][2], m[3][3] };\n"
+            "    float3x3 m2 = { m[1][0], m[1][2], m[1][3], m[2][0], m[2][2], m[2][3], m[3][0], m[3][2], m[3][3] };\n"
+            "    float3x3 m3 = { m[1][0], m[1][1], m[1][3], m[2][0], m[2][1], m[2][3], m[3][0], m[3][1], m[3][3] };\n"
+            "    float3x3 m4 = { m[1][0], m[1][1], m[1][2], m[2][0], m[2][1], m[2][2], m[3][0], m[3][1], m[3][2] };\n"
+            "    float4 v1 = { m[0][0], -m[0][1], m[0][2], -m[0][3] };\n"
+            "    float4 v2 = { determinant(m1), determinant(m2), determinant(m3), determinant(m4) };\n"
+            "    return dot(v1, v2);\n"
+            "}";
+    static const char *bodies[] =
+    {
+        [2] = determinant2x2,
+        [3] = determinant3x3,
+        [4] = determinant4x4,
+    };
+
+    struct hlsl_ir_node *arg = params->args[0];
+    const struct hlsl_type *type = arg->data_type;
+    struct hlsl_ir_function_decl *func;
+    unsigned int dim;
+
+    if (type->class != HLSL_CLASS_SCALAR && type->class != HLSL_CLASS_MATRIX)
+    {
+        hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Invalid argument type.");
+        return false;
+    }
+
+    dim = min(type->dimx, type->dimy);
+    if (dim == 1)
+    {
+        if (!(arg = intrinsic_float_convert_arg(ctx, params, arg, loc)))
+            return false;
+        return hlsl_add_load_component(ctx, params->instrs, arg, 0, loc);
+    }
+
+    if (!(func = hlsl_compile_internal_function(ctx, "determinant", bodies[dim])))
+        return false;
+
+    return add_user_call(ctx, func, params, loc);
+}
+
 static bool intrinsic_distance(struct hlsl_ctx *ctx,
         const struct parse_initializer *params, const struct vkd3d_shader_location *loc)
 {
@@ -3624,6 +3685,7 @@ intrinsic_functions[] =
     {"ddy",                                 1, true,  intrinsic_ddy},
     {"ddy_coarse",                          1, true,  intrinsic_ddy_coarse},
     {"ddy_fine",                            1, true,  intrinsic_ddy_fine},
+    {"determinant",                         1, true,  intrinsic_determinant},
     {"distance",                            2, true,  intrinsic_distance},
     {"dot",                                 2, true,  intrinsic_dot},
     {"exp",                                 1, true,  intrinsic_exp},
diff --git a/tests/hlsl/determinant.shader_test b/tests/hlsl/determinant.shader_test
new file mode 100644
index 00000000..d4a77960
--- /dev/null
+++ b/tests/hlsl/determinant.shader_test
@@ -0,0 +1,100 @@
+[pixel shader]
+float s;
+
+float4 main() : sv_target
+{
+    return determinant(s);
+}
+
+[test]
+uniform 0 float4 9.0 2.0 3.0 4.0
+draw quad
+probe all rgba (9.0, 9.0, 9.0, 9.0)
+
+[pixel shader]
+float1x1 m;
+
+float4 main() : sv_target
+{
+    return determinant(m);
+}
+
+[test]
+uniform 0 float4 1.0 2.0 3.0 4.0
+draw quad
+probe all rgba (1.0, 1.0, 1.0, 1.0)
+
+[pixel shader]
+float2x2 m;
+
+float4 main() : sv_target
+{
+    return determinant(m);
+}
+
+[test]
+uniform 0 float4 1.0 2.0 3.0 4.0
+uniform 4 float4 5.0 6.0 7.0 8.0
+draw quad
+probe all rgba (-4.0, -4.0, -4.0, -4.0)
+
+[pixel shader]
+float2x1 m;
+
+float4 main() : sv_target
+{
+    return determinant(m);
+}
+
+[test]
+uniform 0 float4 1.0 2.0 3.0 4.0
+uniform 4 float4 5.0 6.0 7.0 8.0
+draw quad
+probe all rgba (1.0, 1.0, 1.0, 1.0)
+
+[pixel shader]
+float3x3 m;
+
+float4 main() : sv_target
+{
+    return determinant(m);
+}
+
+[test]
+uniform 0 float4 1.0 2.0 3.0 4.0
+uniform 4 float4 5.0 -6.0 7.0 8.0
+uniform 8 float4 9.0 10.0 11.0 12.0
+draw quad
+probe all rgba (192.0, 192.0, 192.0, 192.0)
+
+[pixel shader]
+float4x4 m;
+
+float4 main() : sv_target
+{
+    return determinant(m);
+}
+
+[test]
+uniform  0 float4  1.0 -2.0  3.0  4.0
+uniform  4 float4  5.0  6.0 -7.0  8.0
+uniform  8 float4  9.0 10.0 11.0 12.0
+uniform 12 float4 13.0 14.0 15.0 16.0
+draw quad
+probe all rgba (-672.0, -672.0, -672.0, -672.0)
+
+[pixel shader fail]
+float1 v;
+
+float4 main() : sv_target
+{
+    return determinant(v);
+}
+
+[pixel shader fail]
+float2 v;
+
+float4 main() : sv_target
+{
+    return determinant(v);
+}

[Nikolay Sivov (＠nsivov)]

Turned out explicit conversion is already happening when function call is added, so it's again removed.

[PATCH v3 0/1] MR329: vkd3d-shader/hlsl: Add determinant() function. - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.

[Francisco Casas (＠fcasas)]

Nice!

[PATCH v3 0/1] MR329: vkd3d-shader/hlsl: Add determinant() function. - approved

[Francisco Casas (＠fcasas)]

This merge request was approved by Francisco Casas.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
> - };
> -
> - struct hlsl_ir_node *arg = params->args[0];
> - const struct hlsl_type *type = arg->data_type;
> - struct hlsl_ir_function_decl *func;
> - unsigned int dim;
> -
> - if (type->class != HLSL_CLASS_SCALAR && type->class != HLSL_CLASS_MATRIX)
> - {
> -
>    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Invalid argument type.");
> -
>    return false;
> - }
> -
> - dim = min(type->dimx, type->dimy);
> - if (dim == 1)
> - {

Not a big deal, but the condition could have just been `if (type->dimx == 1 || type->dimy == 1)`. You don't really need `dim`, apparently.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
> - if (type->class != HLSL_CLASS_SCALAR && type->class != HLSL_CLASS_MATRIX)
> - {
> -
>    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, "Invalid argument type.");
> -
>    return false;
> - }
> -
> - dim = min(type->dimx, type->dimy);
> - if (dim == 1)
> - {
> -
>    if (!(arg = intrinsic_float_convert_arg(ctx, params, arg, loc)))
> -
>        return false;
> -
>    return hlsl_add_load_component(ctx, params->instrs, arg, 0, loc);
> - }
> -
> - if (!(func = hlsl_compile_internal_function(ctx, "determinant", bodies[dim])))
> -
>    return false;

I would guess that `intrinsic_float_convert_arg()` is needed here too. Or do you have evidence that the determinant of a matrix of halves is float rather than half?

[Nikolay Sivov (＠nsivov)]

On Fri Sep  8 15:17:30 2023 +0000, Giovanni Mascellani wrote:
> Not a big deal, but the condition could have just been `if (type->dimx
> == 1 || type->dimy == 1)`. You don't really need `dim`, apparently.

This variable is used couple of lines later.

[Nikolay Sivov (＠nsivov)]

On Fri Sep  8 15:17:31 2023 +0000, Giovanni Mascellani wrote:
> I would guess that `intrinsic_float_convert_arg()` is needed here too.
> Or do you have evidence that the determinant of a matrix of halves is
> float rather than half?

How do you suggest I verify that?

[Giovanni Mascellani (＠giomasce)]

On Fri Sep  8 15:42:58 2023 +0000, Nikolay Sivov wrote:
> This variable is used couple of lines later.

Ugh, I even used the search tool... Sorry, I didn't sleep very well tonight...

[Giovanni Mascellani (＠giomasce)]

On Fri Sep  8 15:43:25 2023 +0000, Nikolay Sivov wrote:
> How do you suggest I verify that?

The trick I know takes advantage of function overloading:
```
float test(float x) { return 1.0; }
float test(half x) { return 2.0; }
float main() { return test(determinant(x)); }
```

[Giovanni Mascellani (＠giomasce)]

Also, could you please add a test like this one:
```c
[pixel shader]
float3x4 m;

float4 main() : sv_target
{
    return determinant(m);
}

[test]
uniform  0 float4  1.0  2.0  3.0 0.0
uniform  4 float4  5.0 -6.0  7.0 0.0
uniform  8 float4  9.0 10.0 11.0 0.0
uniform 12 float4  0.0  0.0  0.0 0.0
draw quad
probe all rgba (192.0, 192.0, 192.0, 192.0)
```

I.e., I want to check what happens for a non-square matrix whose smallest dimension is not 1.