[PATCH 0/4] MR358: vkd3d-shader/hlsl: Parse 'centroid' and 'noperspective' modifiers.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 6 +++++- libs/vkd3d-shader/hlsl.h | 30 ++++++++++++++++-------------- libs/vkd3d-shader/hlsl.l | 2 ++ libs/vkd3d-shader/hlsl.y | 14 ++++++++++++-- libs/vkd3d-shader/tpf.c | 2 +- 5 files changed, 36 insertions(+), 18 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 5fe9047bf..1841f1298 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2191,8 +2191,12 @@ struct vkd3d_string_buffer *hlsl_modifiers_to_string(struct hlsl_ctx *ctx, unsig if (modifiers & HLSL_STORAGE_EXTERN) vkd3d_string_buffer_printf(string, "extern "); - if (modifiers & HLSL_STORAGE_NOINTERPOLATION) + if (modifiers & HLSL_MODIFIER_NOINTERPOLATION) vkd3d_string_buffer_printf(string, "nointerpolation "); + if (modifiers & HLSL_MODIFIER_CENTROID) + vkd3d_string_buffer_printf(string, "centroid "); + if (modifiers & HLSL_MODIFIER_NOPERSPECTIVE) + vkd3d_string_buffer_printf(string, "noperspective "); if (modifiers & HLSL_MODIFIER_PRECISE) vkd3d_string_buffer_printf(string, "precise "); if (modifiers & HLSL_STORAGE_SHARED) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 44cebaaf6..98edc56dd 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -343,20 +343,22 @@ struct hlsl_attribute struct hlsl_src args[]; }; -#define HLSL_STORAGE_EXTERN 0x00000001 -#define HLSL_STORAGE_NOINTERPOLATION 0x00000002 -#define HLSL_MODIFIER_PRECISE 0x00000004 -#define HLSL_STORAGE_SHARED 0x00000008 -#define HLSL_STORAGE_GROUPSHARED 0x00000010 -#define HLSL_STORAGE_STATIC 0x00000020 -#define HLSL_STORAGE_UNIFORM 0x00000040 -#define HLSL_MODIFIER_VOLATILE 0x00000080 -#define HLSL_MODIFIER_CONST 0x00000100 -#define HLSL_MODIFIER_ROW_MAJOR 0x00000200 -#define HLSL_MODIFIER_COLUMN_MAJOR 0x00000400 -#define HLSL_STORAGE_IN 0x00000800 -#define HLSL_STORAGE_OUT 0x00001000 -#define HLSL_MODIFIER_INLINE 0x00002000 +#define HLSL_STORAGE_EXTERN 0x00000001 +#define HLSL_MODIFIER_NOINTERPOLATION 0x00000002 +#define HLSL_MODIFIER_PRECISE 0x00000004 +#define HLSL_STORAGE_SHARED 0x00000008 +#define HLSL_STORAGE_GROUPSHARED 0x00000010 +#define HLSL_STORAGE_STATIC 0x00000020 +#define HLSL_STORAGE_UNIFORM 0x00000040 +#define HLSL_MODIFIER_VOLATILE 0x00000080 +#define HLSL_MODIFIER_CONST 0x00000100 +#define HLSL_MODIFIER_ROW_MAJOR 0x00000200 +#define HLSL_MODIFIER_COLUMN_MAJOR 0x00000400 +#define HLSL_STORAGE_IN 0x00000800 +#define HLSL_STORAGE_OUT 0x00001000 +#define HLSL_MODIFIER_INLINE 0x00002000 +#define HLSL_MODIFIER_CENTROID 0x00004000 +#define HLSL_MODIFIER_NOPERSPECTIVE 0x00008000 #define HLSL_TYPE_MODIFIERS_MASK (HLSL_MODIFIER_PRECISE | HLSL_MODIFIER_VOLATILE | \ HLSL_MODIFIER_CONST | HLSL_MODIFIER_ROW_MAJOR | \ diff --git a/libs/vkd3d-shader/hlsl.l b/libs/vkd3d-shader/hlsl.l index e9ae3ccf3..90abd64a3 100644 --- a/libs/vkd3d-shader/hlsl.l +++ b/libs/vkd3d-shader/hlsl.l @@ -74,6 +74,7 @@ BlendState {return KW_BLENDSTATE; } break {return KW_BREAK; } Buffer {return KW_BUFFER; } cbuffer {return KW_CBUFFER; } +centroid {return KW_CENTROID; } compile {return KW_COMPILE; } const {return KW_CONST; } continue {return KW_CONTINUE; } @@ -95,6 +96,7 @@ inout {return KW_INOUT; } matrix {return KW_MATRIX; } namespace {return KW_NAMESPACE; } nointerpolation {return KW_NOINTERPOLATION; } +noperspective {return KW_NOPERSPECTIVE; } out {return KW_OUT; } packoffset {return KW_PACKOFFSET; } pass {return KW_PASS; } diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index fb6d485ea..ed49d57bc 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4588,6 +4588,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %token KW_BREAK %token KW_BUFFER %token KW_CBUFFER +%token KW_CENTROID %token KW_COLUMN_MAJOR %token KW_COMPILE %token KW_CONST @@ -4610,6 +4611,7 @@ static void validate_texture_format_type(struct hlsl_ctx *ctx, struct hlsl_type %token KW_MATRIX %token KW_NAMESPACE %token KW_NOINTERPOLATION +%token KW_NOPERSPECTIVE %token KW_OUT %token KW_PACKOFFSET %token KW_PASS @@ -4952,7 +4954,7 @@ field: if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1))) YYABORT; - if (modifiers & ~HLSL_STORAGE_NOINTERPOLATION) + if (modifiers & ~HLSL_MODIFIER_NOINTERPOLATION) { struct vkd3d_string_buffer *string; @@ -5899,7 +5901,15 @@ var_modifiers: } | KW_NOINTERPOLATION var_modifiers { - $$ = add_modifiers(ctx, $2, HLSL_STORAGE_NOINTERPOLATION, &@1); + $$ = add_modifiers(ctx, $2, HLSL_MODIFIER_NOINTERPOLATION, &@1); + } + | KW_CENTROID var_modifiers + { + $$ = add_modifiers(ctx, $2, HLSL_MODIFIER_CENTROID, &@1); + } + | KW_NOPERSPECTIVE var_modifiers + { + $$ = add_modifiers(ctx, $2, HLSL_MODIFIER_NOPERSPECTIVE, &@1); } | KW_PRECISE var_modifiers { diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index 045fb6c5f..d089beeae 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -4169,7 +4169,7 @@ static void write_sm4_dcl_semantic(const struct tpf_writer *tpf, const struct hl { enum vkd3d_shader_interpolation_mode mode = VKD3DSIM_LINEAR; - if ((var->storage_modifiers & HLSL_STORAGE_NOINTERPOLATION) || type_is_integer(var->data_type)) + if ((var->storage_modifiers & HLSL_MODIFIER_NOINTERPOLATION) || type_is_integer(var->data_type)) mode = VKD3DSIM_CONSTANT; instr.opcode |= mode << VKD3D_SM4_INTERPOLATION_MODE_SHIFT; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 3 +++ libs/vkd3d-shader/hlsl.y | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 98edc56dd..d9243b1f2 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -364,6 +364,9 @@ struct hlsl_attribute HLSL_MODIFIER_CONST | HLSL_MODIFIER_ROW_MAJOR | \ HLSL_MODIFIER_COLUMN_MAJOR) +#define HLSL_INTERPOLATION_MODIFIERS_MASK (HLSL_MODIFIER_NOINTERPOLATION | HLSL_MODIFIER_CENTROID | \ + HLSL_MODIFIER_NOPERSPECTIVE) + #define HLSL_MODIFIERS_MAJORITY_MASK (HLSL_MODIFIER_ROW_MAJOR | HLSL_MODIFIER_COLUMN_MAJOR) #define HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT 0 diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index ed49d57bc..eb1090c3e 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -4954,7 +4954,7 @@ field: if (!(type = apply_type_modifiers(ctx, $2, &modifiers, true, &@1))) YYABORT; - if (modifiers & ~HLSL_MODIFIER_NOINTERPOLATION) + if (modifiers & ~HLSL_INTERPOLATION_MODIFIERS_MASK) { struct vkd3d_string_buffer *string; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index be0248421..759c8b6ca 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -377,6 +377,8 @@ static void prepend_input_copy_recurse(struct hlsl_ctx *ctx, struct hlsl_block * for (i = 0; i < hlsl_type_element_count(type); ++i) { + unsigned int element_modifiers = modifiers; + if (type->class == HLSL_CLASS_ARRAY) { elem_semantic_index = semantic_index @@ -391,6 +393,7 @@ static void prepend_input_copy_recurse(struct hlsl_ctx *ctx, struct hlsl_block * semantic = &field->semantic; elem_semantic_index = semantic->index; loc = &field->loc; + element_modifiers |= field->storage_modifiers; } if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc))) @@ -402,7 +405,7 @@ static void prepend_input_copy_recurse(struct hlsl_ctx *ctx, struct hlsl_block * return; list_add_after(&c->entry, &element_load->node.entry); - prepend_input_copy_recurse(ctx, block, element_load, modifiers, semantic, elem_semantic_index); + prepend_input_copy_recurse(ctx, block, element_load, element_modifiers, semantic, elem_semantic_index); } } else -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/tpf.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/libs/vkd3d-shader/tpf.c b/libs/vkd3d-shader/tpf.c index d089beeae..4cd5ddfd4 100644 --- a/libs/vkd3d-shader/tpf.c +++ b/libs/vkd3d-shader/tpf.c @@ -4170,7 +4170,33 @@ static void write_sm4_dcl_semantic(const struct tpf_writer *tpf, const struct hl enum vkd3d_shader_interpolation_mode mode = VKD3DSIM_LINEAR; if ((var->storage_modifiers & HLSL_MODIFIER_NOINTERPOLATION) || type_is_integer(var->data_type)) + { mode = VKD3DSIM_CONSTANT; + } + else + { + static const struct + { + unsigned int modifiers; + enum vkd3d_shader_interpolation_mode mode; + } + modes[] = + { + { HLSL_MODIFIER_CENTROID | HLSL_MODIFIER_NOPERSPECTIVE, VKD3DSIM_LINEAR_NOPERSPECTIVE_CENTROID }, + { HLSL_MODIFIER_NOPERSPECTIVE, VKD3DSIM_LINEAR_NOPERSPECTIVE }, + { HLSL_MODIFIER_CENTROID, VKD3DSIM_LINEAR_CENTROID }, + }; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(modes); ++i) + { + if ((var->storage_modifiers & modes[i].modifiers) == modes[i].modifiers) + { + mode = modes[i].mode; + break; + } + } + } instr.opcode |= mode << VKD3D_SM4_INTERPOLATION_MODE_SHIFT; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl.c:
if (modifiers & HLSL_STORAGE_EXTERN) vkd3d_string_buffer_printf(string, "extern "); - if (modifiers & HLSL_STORAGE_NOINTERPOLATION) + if (modifiers & HLSL_MODIFIER_NOINTERPOLATION)
I don't think renaming this makes sense. Currently the difference is whether they are modifiers that can be applied to a type, i.e. whether they're legal on typedefs. Interpolation modifiers aren't. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46222
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
semantic = &field->semantic; elem_semantic_index = semantic->index; loc = &field->loc; + element_modifiers |= field->storage_modifiers;
This logic needs tests, I think. It should be easy to add tests for nointerpolation. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46223
Interpolation doesn't matter for outputs in sm4, but it does in sm6 and GLSL, so can you please add an equivalent of 3/4 for outputs? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46224
On Thu Sep 21 18:15:46 2023 +0000, Zebediah Figura wrote:
I don't think renaming this makes sense. Currently the difference is whether they are modifiers that can be applied to a type, i.e. whether they're legal on typedefs. Interpolation modifiers aren't. Ok. I find 'nointerpolatoin' categorized as storage modifier confusing. It seems easier to call them all as 'modifiers', including ir_var field. Things like 'extern' and 'static' could remain as storage ones. But I don't care enough, I'll revert it.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46226
On Thu Sep 21 18:23:43 2023 +0000, Nikolay Sivov wrote:
Ok. I find 'nointerpolatoin' categorized as storage modifier confusing. It seems easier to call them all as 'modifiers', including ir_var field. Things like 'extern' and 'static' could remain as storage ones. But I don't care enough, I'll revert it. The thing about it is that some of the flags go on the hlsl_type, and the rest go on hlsl_ir_var/hlsl_struct_field. We started separating the naming to make it clear which is which. Interpolation modifiers fall in the latter category.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46227
On Thu Sep 21 18:32:07 2023 +0000, Zebediah Figura wrote:
The thing about it is that some of the flags go on the hlsl_type, and the rest go on hlsl_ir_var/hlsl_struct_field. We started separating the naming to make it clear which is which. Interpolation modifiers fall in the latter category. Also, even when appropriate, changing an identifier is usually something that can stay in its own commit.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46321
Giovanni Mascellani (@giomasce) commented about libs/vkd3d-shader/tpf.c:
+ { + { HLSL_MODIFIER_CENTROID | HLSL_MODIFIER_NOPERSPECTIVE, VKD3DSIM_LINEAR_NOPERSPECTIVE_CENTROID }, + { HLSL_MODIFIER_NOPERSPECTIVE, VKD3DSIM_LINEAR_NOPERSPECTIVE }, + { HLSL_MODIFIER_CENTROID, VKD3DSIM_LINEAR_CENTROID }, + }; + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(modes); ++i) + { + if ((var->storage_modifiers & modes[i].modifiers) == modes[i].modifiers) + { + mode = modes[i].mode; + break; + } + } + } This looks longer and less readable than what you'd get by simply chaining the three cases with `else if`. I can live with it, though.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46322
On Fri Sep 22 09:16:43 2023 +0000, Giovanni Mascellani wrote:
This looks longer and less readable than what you'd get by simply chaining the three cases with `else if`. I can live with it, though. There will be at least two more entries there, so I don't think so.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46325
On Thu Sep 21 18:15:47 2023 +0000, Zebediah Figura wrote:
This logic needs tests, I think. It should be easy to add tests for nointerpolation. Could you suggest a good reference about this feature? I can easily check compiler output, but looking at nointerpolation test, it requires more than that.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46326
Could you suggest a good reference about this feature? I can easily check compiler output, but looking at nointerpolation test, it requires more than that.
The GLSL spec at https://registry.khronos.org/OpenGL/specs/gl/GLSLangSpec.4.60.html#interpola... is fairly decent. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/358#note_46343
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)