[PATCH v2 0/2] MR109: vkd3d-shader/hlsl: Improve handling of builtin aliases "matrix" and "vector".

[Nikolay Sivov (＠nsivov)]

--
  v2: vkd3d-shader/hlsl: Improve handling of builtin alias type "vector".
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/109

[PATCH v2 1/2] vkd3d-shader/hlsl: Improve handling of builtin alias type "matrix".

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl.c          |  3 ++-
 tests/hlsl-type-names.shader_test | 32 +++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index d15abe50..b9b37eea 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -610,6 +610,7 @@ static const char * get_case_insensitive_typename(const char *name)
     {
         "dword",
         "float",
+        "matrix",
     };
     unsigned int i;
 
@@ -2841,7 +2842,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx)
         {"dword",           HLSL_CLASS_SCALAR, HLSL_TYPE_UINT,          1, 1},
         {"float",           HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT,         1, 1},
         {"VECTOR",          HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT,         4, 1},
-        {"MATRIX",          HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT,         4, 4},
+        {"matrix",          HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT,         4, 4},
         {"STRING",          HLSL_CLASS_OBJECT, HLSL_TYPE_STRING,        1, 1},
         {"TEXTURE",         HLSL_CLASS_OBJECT, HLSL_TYPE_TEXTURE,       1, 1},
         {"PIXELSHADER",     HLSL_CLASS_OBJECT, HLSL_TYPE_PIXELSHADER,   1, 1},
diff --git a/tests/hlsl-type-names.shader_test b/tests/hlsl-type-names.shader_test
index e88f1cda..69510b34 100644
--- a/tests/hlsl-type-names.shader_test
+++ b/tests/hlsl-type-names.shader_test
@@ -2,6 +2,7 @@
 typedef float2 Dword;
 typedef float3 dWord;
 typedef float2 fLoat;
+typedef float2 mAtrix;
 
 float4 f()
 {
@@ -10,6 +11,9 @@ float4 f()
     dword v1 = {1, 2};
     float v2 = {2, 3};
     DWORD v4 = 4;
+    mAtrix v5 = {1, 2};
+    maTrix v6 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
+    matrix v7 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
     return float4(v1.x, v2.x, v2.y, v4);
 }
 
@@ -55,3 +59,31 @@ float4 main() : sv_target
 {
     return float4(0, 0, 0, 0);
 }
+
+[pixel shader fail]
+typedef float2 matrix;
+
+float4 main() : sv_target
+{
+    return float4(0, 0, 0, 0);
+}
+
+[pixel shader fail]
+float4 f()
+{
+    typedef float2 matrix;
+    return float4(0, 0, 0, 0);
+}
+
+float4 main() : SV_TARGET
+{
+    return f();
+}
+
+[pixel shader fail]
+Matrix<float, 2, 2> m;
+
+float4 main() : sv_target
+{
+    return float4(0, 0, 0, 0);
+}
-- 
GitLab


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

[PATCH v2 2/2] vkd3d-shader/hlsl: Improve handling of builtin alias type "vector".

[Nikolay Sivov]

From: Nikolay Sivov <nsivov(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl.c          |  3 ++-
 tests/hlsl-type-names.shader_test | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index b9b37eea..388f8336 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -611,6 +611,7 @@ static const char * get_case_insensitive_typename(const char *name)
         "dword",
         "float",
         "matrix",
+        "vector",
     };
     unsigned int i;
 
@@ -2841,7 +2842,7 @@ static void declare_predefined_types(struct hlsl_ctx *ctx)
     {
         {"dword",           HLSL_CLASS_SCALAR, HLSL_TYPE_UINT,          1, 1},
         {"float",           HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT,         1, 1},
-        {"VECTOR",          HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT,         4, 1},
+        {"vector",          HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT,         4, 1},
         {"matrix",          HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT,         4, 4},
         {"STRING",          HLSL_CLASS_OBJECT, HLSL_TYPE_STRING,        1, 1},
         {"TEXTURE",         HLSL_CLASS_OBJECT, HLSL_TYPE_TEXTURE,       1, 1},
diff --git a/tests/hlsl-type-names.shader_test b/tests/hlsl-type-names.shader_test
index 69510b34..a8906d5b 100644
--- a/tests/hlsl-type-names.shader_test
+++ b/tests/hlsl-type-names.shader_test
@@ -3,6 +3,7 @@ typedef float2 Dword;
 typedef float3 dWord;
 typedef float2 fLoat;
 typedef float2 mAtrix;
+typedef float2 vEctor;
 
 float4 f()
 {
@@ -14,6 +15,8 @@ float4 f()
     mAtrix v5 = {1, 2};
     maTrix v6 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
     matrix v7 = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16};
+    vector v8 = {1, 2, 3, 4};
+    vEctor v9 = {1, 2};
     return float4(v1.x, v2.x, v2.y, v4);
 }
 
@@ -80,6 +83,18 @@ float4 main() : SV_TARGET
     return f();
 }
 
+[pixel shader fail]
+float4 f()
+{
+    typedef float2 vector;
+    return float4(0, 0, 0, 0);
+}
+
+float4 main() : SV_TARGET
+{
+    return f();
+}
+
 [pixel shader fail]
 Matrix<float, 2, 2> m;
 
@@ -87,3 +102,11 @@ float4 main() : sv_target
 {
     return float4(0, 0, 0, 0);
 }
+
+[pixel shader fail]
+Vector<float, 2> v;
+
+float4 main() : sv_target
+{
+    return float4(0, 0, 0, 0);
+}
-- 
GitLab

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

[Giovanni Mascellani (＠giomasce)]

Unless I'm getting confused somehow, it seems that the tests introduced in 2/2 already pass in 1/2, so they don't really justify the changes in 2/2. More in general, I think it would be useful to first add the failing tests with todo and then, in a separated commit, add the code changes that fix the tests. That makes it easier for the reviewer to see why the commit is useful.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/109#note_25760

[Zebediah Figura (＠zfigura)]

> Unless I'm getting confused somehow, it seems that the tests introduced in 2/2 already pass in 1/2, so they don't really justify the changes in 2/2. More in general, I think it would be useful to first add the failing tests with todo and then, in a separated commit, add the code changes that fix the tests. That makes it easier for the reviewer to see why the commit is useful.

In general, yes, although this series is simple enough it's probably not worth rerolling.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/109#note_25783

[Nikolay Sivov (＠nsivov)]

Is there any actual issues I should address here?

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/109#note_26725

[Giovanni Mascellani (＠giomasce)]

On Wed Mar 22 11:42:36 2023 +0000, Nikolay Sivov wrote:
> Is there any actual issues I should address here?
> Well, for example my comment...

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/109#note_27522