[PATCH 0/5] MR98: hlsl: Matrix majority, 2023 edition.
From: Zebediah Figura <zfigura(a)codeweavers.com> --- tests/hlsl-majority-pragma.shader_test | 28 ++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/hlsl-majority-pragma.shader_test b/tests/hlsl-majority-pragma.shader_test index e7fc75cd..d269e859 100644 --- a/tests/hlsl-majority-pragma.shader_test +++ b/tests/hlsl-majority-pragma.shader_test @@ -19,3 +19,31 @@ uniform 8 float4 0.1 0.3 0.0 0.0 uniform 12 float4 0.2 0.4 0.0 0.0 draw quad probe all rgba (0.17, 0.39, 0.17, 0.39) 1 + +% The documentation claims these strings are subject to macro expansion. +% They are not. + +[pixel shader] + +#define KEY pack_matrix +#pragma KEY(row_major) +#define VALUE row_major +#pragma pack_matrix(VALUE) +#define PRAGMA pack_matrix(row_major) +#pragma PRAGMA + +uniform float2x2 r; + +float4 main() : sv_target +{ + float4 ret; + ret.xy = mul(r, float2(0.5, 0.6)); + ret.zw = 0.5; + return ret; +} + +[test] +uniform 0 float4 0.1 0.2 0.0 0.0 +uniform 4 float4 0.3 0.4 0.0 0.0 +draw quad +probe all rgba (0.23, 0.34, 0.5, 0.5) 1 -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl-invalid.shader_test | 18 ----- tests/majority-syntax.shader_test | 108 ++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 18 deletions(-) create mode 100644 tests/majority-syntax.shader_test diff --git a/Makefile.am b/Makefile.am index f9199472..cab5c63f 100644 --- a/Makefile.am +++ b/Makefile.am @@ -116,6 +116,7 @@ vkd3d_shader_tests = \ tests/hlsl-vector-indexing.shader_test \ tests/hlsl-vector-indexing-uniform.shader_test \ tests/logic-operations.shader_test \ + tests/majority-syntax.shader_test \ tests/math.shader_test \ tests/matrix-semantics.shader_test \ tests/max.shader_test \ diff --git a/tests/hlsl-invalid.shader_test b/tests/hlsl-invalid.shader_test index e56205cb..ad062652 100644 --- a/tests/hlsl-invalid.shader_test +++ b/tests/hlsl-invalid.shader_test @@ -86,15 +86,6 @@ uniform float4 main() : sv_target return 0; } -[pixel shader fail] -typedef row_major float4x4 matrix_t; -typedef column_major matrix_t matrix2_t; - -float4 main() : sv_target -{ - return 0; -} - [pixel shader fail] float4 main() { @@ -228,15 +219,6 @@ float4 main() : sv_target return float4(0, 0, 0, 0); } -[pixel shader fail] -typedef row_major float4x4 mat_t; -column_major mat_t m; - -float4 main() : sv_target -{ - return float4(0, 0, 0, 0); -} - [pixel shader fail] typedef struct apple { diff --git a/tests/majority-syntax.shader_test b/tests/majority-syntax.shader_test new file mode 100644 index 00000000..63ba18f3 --- /dev/null +++ b/tests/majority-syntax.shader_test @@ -0,0 +1,108 @@ +[pixel shader] +typedef row_major float2x2 mat_t; + +row_major mat_t m; + +float4 main() : sv_target +{ + return float4(m[0], m[1]); +} + +[test] +uniform 0 float4 0.1 0.3 0.0 0.0 +uniform 4 float4 0.2 0.4 0.0 0.0 +draw quad +probe all rgba (0.1, 0.3, 0.2, 0.4) + +[pixel shader fail] +row_major row_major float4x4 m; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader fail] +typedef row_major float4x4 mat_t; +column_major mat_t m; + +float4 main() : sv_target +{ + return float4(0, 0, 0, 0); +} + +[pixel shader fail] +typedef row_major float4x4 matrix_t; +typedef column_major matrix_t matrix2_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +typedef column_major float4 myfloat_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +column_major float4 f; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail] +struct apple +{ + float4x4 m; +}; +typedef column_major struct apple apple_t; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +struct apple +{ + float4x4 m; +}; +column_major struct apple a; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +typedef float4x4 myarray_t[2]; + +row_major myarray_t a; + +float4 main() : sv_target +{ + return 0; +} + +[pixel shader fail todo] +float4 main(uniform row_major float4 f) : sv_target +{ + return 0; +} + +[pixel shader fail todo] +struct apple +{ + row_major float4 f; +}; + +float4 main() : sv_target +{ + return 0; +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 8 +++++--- tests/majority-syntax.shader_test | 10 +++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 76b0eae7..576d6b3f 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -903,6 +903,11 @@ static struct hlsl_type *apply_type_modifiers(struct hlsl_ctx *ctx, struct hlsl_ else default_majority = HLSL_MODIFIER_ROW_MAJOR; } + else if (type->type != HLSL_CLASS_MATRIX && (*modifiers & HLSL_MODIFIERS_MAJORITY_MASK)) + { + hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, + "'row_major' and 'column_major' modifiers are only allowed for matrices."); + } if (!default_majority && !(*modifiers & HLSL_TYPE_MODIFIERS_MASK)) return type; @@ -2041,9 +2046,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t } vkd3d_free(v->arrays.sizes); - if (type->type != HLSL_CLASS_MATRIX) - check_invalid_matrix_modifiers(ctx, modifiers, v->loc); - if (modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)) { struct vkd3d_string_buffer *string; diff --git a/tests/majority-syntax.shader_test b/tests/majority-syntax.shader_test index 63ba18f3..4ab385a5 100644 --- a/tests/majority-syntax.shader_test +++ b/tests/majority-syntax.shader_test @@ -48,7 +48,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] column_major float4 f; float4 main() : sv_target @@ -68,7 +68,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] struct apple { float4x4 m; @@ -80,7 +80,7 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] typedef float4x4 myarray_t[2]; row_major myarray_t a; @@ -90,13 +90,13 @@ float4 main() : sv_target return 0; } -[pixel shader fail todo] +[pixel shader fail] float4 main(uniform row_major float4 f) : sv_target { return 0; } -[pixel shader fail todo] +[pixel shader fail] struct apple { row_major float4 f; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> We will warn subsequently in apply_type_modifiers() or add_typedef(). --- libs/vkd3d-shader/hlsl.y | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 576d6b3f..4c631313 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -408,12 +408,6 @@ static DWORD add_modifiers(struct hlsl_ctx *ctx, DWORD modifiers, DWORD mod, con hlsl_release_string_buffer(ctx, string); return modifiers; } - if ((mod & HLSL_MODIFIERS_MAJORITY_MASK) && (modifiers & HLSL_MODIFIERS_MAJORITY_MASK)) - { - hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, - "'row_major' and 'column_major' modifiers are mutually exclusive."); - return modifiers; - } return modifiers | mod; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 4c631313..3a4b2d5d 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -2040,16 +2040,6 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t } vkd3d_free(v->arrays.sizes); - if (modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)) - { - struct vkd3d_string_buffer *string; - - if ((string = hlsl_modifiers_to_string(ctx, modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)))) - hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, - "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); - hlsl_release_string_buffer(ctx, string); - } - if (!(var = hlsl_new_var(ctx, v->name, type, v->loc, &v->semantic, modifiers, &v->reg_reservation))) { free_parse_variable_def(v); @@ -3794,8 +3784,9 @@ preproc_directive: struct_declaration: var_modifiers struct_spec variables_def_optional ';' { - struct hlsl_type *type; + unsigned int invalid_modifiers; unsigned int modifiers = $1; + struct hlsl_type *type; if (!$3) { @@ -3809,6 +3800,18 @@ struct_declaration: if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1))) YYABORT; + + invalid_modifiers = modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT); + if (invalid_modifiers) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_modifiers_to_string(ctx, invalid_modifiers))) + hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, + "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); + hlsl_release_string_buffer(ctx, string); + } + $$ = declare_vars(ctx, type, modifiers, $3); } @@ -4527,11 +4530,24 @@ type_spec: declaration: var_modifiers type variables_def ';' { - struct hlsl_type *type; + unsigned int invalid_modifiers; unsigned int modifiers = $1; + struct hlsl_type *type; if (!(type = apply_type_modifiers(ctx, $2, &modifiers, @1))) YYABORT; + + invalid_modifiers = modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT); + if (invalid_modifiers) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_modifiers_to_string(ctx, invalid_modifiers))) + hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, + "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); + hlsl_release_string_buffer(ctx, string); + } + $$ = declare_vars(ctx, type, modifiers, $3); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
else default_majority = HLSL_MODIFIER_ROW_MAJOR; } + else if (type->type != HLSL_CLASS_MATRIX && (*modifiers & HLSL_MODIFIERS_MAJORITY_MASK)) + { + hlsl_error(ctx, &loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, + "'row_major' and 'column_major' modifiers are only allowed for matrices."); + }
Any reason for not reusing `check_invalid_matrix_modifiers()`? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24540
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/hlsl.y:
} vkd3d_free(v->arrays.sizes);
- if (modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)) - { - struct vkd3d_string_buffer *string; - - if ((string = hlsl_modifiers_to_string(ctx, modifiers & (HLSL_STORAGE_IN | HLSL_STORAGE_OUT)))) - hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_MODIFIER, - "Modifiers '%s' are not allowed on non-parameter variables.", string->buffer); - hlsl_release_string_buffer(ctx, string); - } -
Can't you just move this check before the loop in `declare_vars()`, instead of duplicating it before each call to `declare_vars()`? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24541
On Thu Feb 16 15:58:13 2023 +0000, Giovanni Mascellani wrote:
Any reason for not reusing `check_invalid_matrix_modifiers()`? At that point, no, no good reason.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24556
On Thu Feb 16 15:58:14 2023 +0000, Giovanni Mascellani wrote:
Can't you just move this check before the loop in `declare_vars()`, instead of duplicating it before each call to `declare_vars()`? I think I did it this way to use the modifier location rather than the variable location. There's probably a better way to do that, though...
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24557
On Thu Feb 16 20:09:32 2023 +0000, Zebediah Figura wrote:
I think I did it this way to use the modifier location rather than the variable location. There's probably a better way to do that, though... That makes sense, but I think it would be sensible to at least deduplicate that code using a helper.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/98#note_24558
participants (3)
-
Giovanni Mascellani (＠giomasce) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)