[PATCH v2 0/5] MR29: vkd3d-shader/hlsl: Support complex implicit casts, complex explicit casts and complex broadcasts. (PART 1/2)

[Francisco Casas (＠fcasas)]

Hello, this is the first part of a patch series to support:

* Complex broadcasts.
* Complex implicit casts between component-wise equal types.
* Complex explicit casts between component-wise compatible types.

This first part of the series includes mostly tests.

Following patches in: https://gitlab.winehq.org/fcasas/vkd3d/-/commits/complex_broadcasts_2/

This supersedes !16 .

--
  v2: tests: Test explicit casts between types that are component-wise
      tests: Test implicit casts between types that are equal component-wise.
      vkd3d-shader/hlsl: Always go through implicit conversion in assignments.
      tests: Test for invalid complex broadcasts.
      tests: Set ULPs to 2 in normalize() test.

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

[PATCH v2 1/5] tests: Set ULPs to 2 in normalize() test.

[Francisco Casas]

From: Francisco Casas

---

Otherwise the test fails on a NVIDIA GeForce GTX 1050 Ti GPU.

The error was:

shader_runner:535:Section [test], line 9: Test failed: Got {2.72165507e-01, 4.08248246e-01, 5.44331014e-01, 6.80413783e-01}, expected {2.72165537e-01, 4.08248305e-01, 5.44331074e-01, 6.80413842e-01} at (0, 0).
---
 tests/hlsl-normalize.shader_test | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/hlsl-normalize.shader_test b/tests/hlsl-normalize.shader_test
index 71b900e8..359a7590 100644
--- a/tests/hlsl-normalize.shader_test
+++ b/tests/hlsl-normalize.shader_test
@@ -9,7 +9,7 @@ float4 main() : SV_TARGET
 [test]
 uniform 0 float4 2.0 3.0 4.0 5.0
 draw quad
-probe all rgba (0.272165537, 0.408248305, 0.544331074, 0.680413842) 1
+probe all rgba (0.272165537, 0.408248305, 0.544331074, 0.680413842) 2

[pixel shader]
 uniform float3 x;

[PATCH v2 2/5] tests: Test for invalid complex broadcasts.

[Francisco Casas]

From: Francisco Casas

---

While at it, rename structs to "apple" and "banana" on other tests to
make them more readable.
---
 tests/cast-broadcast.shader_test | 56 +++++++++++++++++++++++++++++---
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/tests/cast-broadcast.shader_test b/tests/cast-broadcast.shader_test
index 02d14c0b..f60eb758 100644
--- a/tests/cast-broadcast.shader_test
+++ b/tests/cast-broadcast.shader_test
@@ -1,24 +1,70 @@
 [pixel shader]
-
-struct foo
+struct apple
 {
     float3 aa;
     float4 bb;
 };

-struct bar
+struct banana
 {
-    struct foo aa;
+    struct apple aa;
     int2 bb;
     int4 cc[8];
 };

float4 main() : SV_TARGET
 {
-    struct bar p = (struct bar)42;
+    struct banana p = (struct banana)42;
     return p.aa.bb + p.cc[5];
 }

[test]
 todo draw quad
 todo probe all rgba (84.0, 84.0, 84.0, 84.0)
+
+
+[pixel shader fail todo]
+struct apple
+{
+    float3 aa;
+    float4 bb;
+};
+
+float4 main() : SV_TARGET
+{
+    struct apple f = 31;
+    return f.bb;
+}
+
+
+[pixel shader fail]
+struct apple
+{
+    float3 aa;
+    float4 bb;
+};
+
+float4 fun(struct apple f)
+{
+    return f.bb;
+}
+
+float4 main() : SV_TARGET
+{
+    return fun(31);
+}
+
+
+[pixel shader fail]
+struct apple
+{
+    float4 foo;
+    Texture2D tex;
+};
+
+float4 PSMain() : SV_TARGET
+{
+    struct apple a1;
+    a1 = (struct apple)1;
+    return a1.foo;
+}

[PATCH v2 3/5] vkd3d-shader/hlsl: Always go through implicit conversion in assignments.

[Francisco Casas]

From: Francisco Casas

Otherwise we silently skip some type checks.

---

I think it doesn't hurt in the usual cases since
add_implicit_conversion() just returns the node if
hlsl_types_are_equal(src_type, dst_type).
---
 libs/vkd3d-shader/hlsl.y         | 6 ++----
 tests/cast-broadcast.shader_test | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 60ca09ce..66ca3e3e 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -1636,12 +1636,10 @@ static struct hlsl_ir_node *add_assignment(struct hlsl_ctx *ctx, struct list *in
     }

if (lhs_type->type <= HLSL_CLASS_LAST_NUMERIC)
-    {
         writemask = (1 << lhs_type->dimx) - 1;

-        if (!(rhs = add_implicit_conversion(ctx, instrs, rhs, lhs_type, &rhs->loc)))
-            return NULL;
-    }
+    if (!(rhs = add_implicit_conversion(ctx, instrs, rhs, lhs_type, &rhs->loc)))
+        return NULL;

while (lhs->type != HLSL_IR_LOAD)
     {
diff --git a/tests/cast-broadcast.shader_test b/tests/cast-broadcast.shader_test
index f60eb758..e21e65b0 100644
--- a/tests/cast-broadcast.shader_test
+++ b/tests/cast-broadcast.shader_test
@@ -23,7 +23,7 @@ todo draw quad
 todo probe all rgba (84.0, 84.0, 84.0, 84.0)

-[pixel shader fail todo]
+[pixel shader fail]
 struct apple
 {
     float3 aa;

[PATCH v2 4/5] tests: Test implicit casts between types that are equal component-wise.

[Francisco Casas]

From: Francisco Casas

---
 Makefile.am                                |   1 +
 tests/cast-componentwise-equal.shader_test | 181 +++++++++++++++++++++
 2 files changed, 182 insertions(+)
 create mode 100644 tests/cast-componentwise-equal.shader_test

diff --git a/Makefile.am b/Makefile.am
index 110b66f6..438c0158 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -58,6 +58,7 @@ vkd3d_shader_tests = \
    tests/arithmetic-uint.shader_test \
    tests/bitwise.shader_test \
    tests/cast-broadcast.shader_test \
+	tests/cast-componentwise-equal.shader_test \
    tests/cast-to-float.shader_test \
    tests/cast-to-half.shader_test \
    tests/cast-to-int.shader_test \
diff --git a/tests/cast-componentwise-equal.shader_test b/tests/cast-componentwise-equal.shader_test
new file mode 100644
index 00000000..1ff99b88
--- /dev/null
+++ b/tests/cast-componentwise-equal.shader_test
@@ -0,0 +1,181 @@
+[pixel shader fail]
+struct apple
+{
+    float3 aa;
+    float bb;
+};
+
+float4 main() : sv_target
+{
+    float4 f = {1, 2, 3, 4};
+    struct apple a;
+
+    a = f;
+    return a.aa.xyzx;
+}
+
+
+[pixel shader fail]
+struct apple
+{
+    float aa;
+    float bb;
+};
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2};
+    float2 f;
+
+    f = a;
+    return f.xyxy;
+}
+
+
+[pixel shader]
+struct apple
+{
+    float3 aa;
+    float bb;
+};
+
+float4 main() : sv_target
+{
+    float f[4] = {1, 2, 3, 4};
+    struct apple a;
+
+    a = f;
+    return a.aa.xyzx;
+}
+
+
+[test]
+todo draw quad
+todo probe all rgba (1.0, 2.0, 3.0, 1.0)
+
+
+[pixel shader fail]
+struct apple
+{
+    float3 aa;
+    int bb;
+};
+
+float4 main() : sv_target
+{
+    float f[4] = {1, 2, 3, 4};
+    struct apple a;
+
+    a = f;
+    return a.aa.xyzx;
+}
+
+
+[pixel shader]
+Texture2D tex;
+
+struct apple
+{
+    int2 aa;
+    Texture2D bb;
+    float cc;
+};
+
+struct banana
+{
+    int aa[2];
+    Texture2D bb;
+    float cc;
+};
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2, tex, 4};
+    struct banana b;
+
+    b = a;
+    return b.cc;
+}
+
+[test]
+todo draw quad
+todo probe all rgba (4.0, 4.0, 4.0, 4.0)
+
+
+[pixel shader]
+Texture2D tex;
+
+struct apple
+{
+    int2 aa;
+    Texture2D bb;
+    float cc;
+};
+
+struct banana
+{
+    int aa[2];
+    Texture2D bb;
+    float cc;
+};
+
+float4 fun(struct banana b)
+{
+    return b.cc;
+}
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2, tex, 5};
+
+    return fun(a);
+}
+
+[test]
+todo draw quad
+todo probe all rgba (5.0, 5.0, 5.0, 5.0)
+
+
+[pixel shader]
+struct apple
+{
+    float3 xx[2];
+    int4 yy;
+};
+
+struct banana
+{
+    struct apple apples[2];
+    int3 bb[2];
+    int4 cc[3];
+};
+
+struct cherry
+{
+    int2 xx;
+    int yy[3];
+};
+
+struct donut
+{
+    float4 aa;
+    float2 bb;
+    int cc[4];
+    float dd[6];
+    struct cherry cherries[4];
+    int2 ee;
+};
+
+float4 main() : sv_target
+{
+    struct banana b = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
+            21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37};
+    struct donut d;
+
+    d = b;
+    return d.aa + d.cherries[3].yy[2] + d.ee.xyxx;
+}
+
+[test]
+todo draw quad
+todo probe all rgba (71.0, 73.0, 73.0, 74.0)

[PATCH v2 5/5] tests: Test explicit casts between types that are component-wise compatible.

[Francisco Casas]

From: Francisco Casas

---
 Makefile.am                                   |   1 +
 .../cast-componentwise-compatible.shader_test | 108 ++++++++++++++++++
 2 files changed, 109 insertions(+)
 create mode 100644 tests/cast-componentwise-compatible.shader_test

diff --git a/Makefile.am b/Makefile.am
index 438c0158..953abaef 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -59,6 +59,7 @@ vkd3d_shader_tests = \
    tests/bitwise.shader_test \
    tests/cast-broadcast.shader_test \
    tests/cast-componentwise-equal.shader_test \
+	tests/cast-componentwise-compatible.shader_test \
    tests/cast-to-float.shader_test \
    tests/cast-to-half.shader_test \
    tests/cast-to-int.shader_test \
diff --git a/tests/cast-componentwise-compatible.shader_test b/tests/cast-componentwise-compatible.shader_test
new file mode 100644
index 00000000..764b56d6
--- /dev/null
+++ b/tests/cast-componentwise-compatible.shader_test
@@ -0,0 +1,108 @@
+[pixel shader]
+struct apple
+{
+    float3 aa;
+    int bb;
+};
+
+float4 main() : sv_target
+{
+    float f[4] = {1, 2, 3, 4};
+    struct apple a;
+
+    a = (struct apple) f;
+    return a.aa.xyzx;
+}
+
+
+[test]
+todo draw quad
+todo probe all rgba (1.0, 2.0, 3.0, 1.0)
+
+
+[pixel shader]
+Texture2D tex;
+
+struct apple
+{
+    int2 aa;
+    Texture2D bb;
+    float cc;
+};
+
+struct banana
+{
+    float aa[2];
+    Texture2D bb;
+    int cc;
+};
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2, tex, 7.2};
+    struct banana b;
+
+    b = (struct banana) a;
+    return b.cc;
+}
+
+[test]
+todo draw quad
+todo probe all rgba (7.0, 7.0, 7.0, 7.0)
+
+
+[pixel shader]
+Texture2D tex;
+
+struct apple
+{
+    int2 aa;
+    Texture2D bb;
+    float cc;
+    float4 dd;
+    Texture2D ee;
+};
+
+struct banana
+{
+    float aa[2];
+    Texture2D bb;
+    int cc;
+};
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2, tex, 3, 4, 5, 6, 7, tex};
+    struct banana b;
+
+    b = (struct banana) a;
+    return b.cc;
+}
+
+[test]
+todo draw quad
+todo probe all rgba (3.0, 3.0, 3.0, 3.0)
+
+
+[pixel shader fail]
+struct apple
+{
+    int2 aa;
+    float bb;
+};
+
+struct banana
+{
+    int2 aa;
+    float bb;
+    float cc;
+};
+
+float4 main() : sv_target
+{
+    struct apple a = {1, 2, 3};
+    struct banana b;
+
+    b = (struct banana) a;
+    return b.bb;
+}

[PATCH v2 0/5] MR29: vkd3d-shader/hlsl: Support complex implicit casts, complex explicit casts and complex broadcasts. (PART 1/2) - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.

[Giovanni Mascellani (＠giomasce)]

In a couple of commit messages you put a `---` marker. I guess that means that you don't want the message below that point to be committed. That used to work with the previous workflow because AJ imported the patch with `git am`, but I am not sure it works any more. I *think* you should remove from the `---` onward if you don't want it to be committed. @julliard, could you please comment on that?

BTW, I am generally in favor of verbose commit messages rather than not, so I think that some of the messages that you want to see discarded are instead sensible to keep in the git history. For example the note on the video card on which the test fails.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about tests/cast-broadcast.shader_test:
> [pixel shader]
> -
>
> -struct foo
> +struct apple

The "While at it" expression suggests that you're doing two different things in the same patch. I think it's better to split it up, even if this means creating a very silly patch just renaming test structs.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about tests/cast-componentwise-compatible.shader_test:
> - int bb;
>
> +};
> -
>
> +float4 main() : sv_target
> +{
> - float f[4] = {1, 2, 3, 4};
> - struct apple a;
> -
> - a = (struct apple) f;
> - return a.aa.xyzx;
>
> +}
> -
> -
>
> +[test]
> +todo draw quad
> +todo probe all rgba (1.0, 2.0, 3.0, 1.0)

I think I might already have told you, so maybe that's on purpose (and in that case it's not a problem), but when `draw quad` fails there is no need to `todo` the `probe` commands.

[Alexandre Julliard (＠julliard)]

> In a couple of commit messages you put a `---` marker. I guess that means that you don't want the message below that point to be committed. That used to work with the previous workflow because AJ imported the patch with `git am`, but I am not sure it works any more. I *think* you should remove from the `---` onward if you don't want it to be committed. @julliard, could you please comment on that?

It still works, though ideally comments that are not meant to be committed would be put in the MR description instead.