[PATCH v2 0/2] MR742: vkd3d-shader/fx: Add an option to include empty buffers in the effect binary.
Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> -- v2: vkd3d-shader/fx: Add an option to include empty buffers in the effect binary. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/fx.c | 3 ++- libs/vkd3d-shader/hlsl.c | 8 +++++--- libs/vkd3d-shader/hlsl.h | 5 ++++- libs/vkd3d-shader/hlsl.y | 4 ++-- tests/hlsl/cbuffer.shader_test | 18 ++++++++++++++++++ 5 files changed, 31 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/fx.c b/libs/vkd3d-shader/fx.c index 9424a5685..5fcd3eacf 100644 --- a/libs/vkd3d-shader/fx.c +++ b/libs/vkd3d-shader/fx.c @@ -1038,7 +1038,8 @@ static void write_fx_4_buffer(struct hlsl_buffer *b, struct fx_write_context *fx put_u32(buffer, bind_point); /* Bind point */ put_u32(buffer, 0); /* Annotations count */ - /* FIXME: write annotations */ + if (b->annotations) + hlsl_fixme(ctx, &b->loc, "Writing annotations for buffers is not implemented."); count = 0; size = 0; diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index cba954c98..174589630 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -2031,7 +2031,8 @@ struct hlsl_ir_function_decl *hlsl_new_func_decl(struct hlsl_ctx *ctx, } struct hlsl_buffer *hlsl_new_buffer(struct hlsl_ctx *ctx, enum hlsl_buffer_type type, const char *name, - uint32_t modifiers, const struct hlsl_reg_reservation *reservation, const struct vkd3d_shader_location *loc) + uint32_t modifiers, const struct hlsl_reg_reservation *reservation, struct hlsl_scope *annotations, + const struct vkd3d_shader_location *loc) { struct hlsl_buffer *buffer; @@ -2042,6 +2043,7 @@ struct hlsl_buffer *hlsl_new_buffer(struct hlsl_ctx *ctx, enum hlsl_buffer_type buffer->modifiers = modifiers; if (reservation) buffer->reservation = *reservation; + buffer->annotations = annotations; buffer->loc = *loc; list_add_tail(&ctx->buffers, &buffer->entry); return buffer; @@ -3586,10 +3588,10 @@ static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct vkd3d_shader_compil list_init(&ctx->buffers); if (!(ctx->globals_buffer = hlsl_new_buffer(ctx, HLSL_BUFFER_CONSTANT, - hlsl_strdup(ctx, "$Globals"), 0, NULL, &ctx->location))) + hlsl_strdup(ctx, "$Globals"), 0, NULL, NULL, &ctx->location))) return false; if (!(ctx->params_buffer = hlsl_new_buffer(ctx, HLSL_BUFFER_CONSTANT, - hlsl_strdup(ctx, "$Params"), 0, NULL, &ctx->location))) + hlsl_strdup(ctx, "$Params"), 0, NULL, NULL, &ctx->location))) return false; ctx->cur_buffer = ctx->globals_buffer; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 64111f3fc..4f9f04c7b 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -806,6 +806,8 @@ struct hlsl_buffer * If provided, it should be of type 'b' if type is HLSL_BUFFER_CONSTANT and 't' if type is * HLSL_BUFFER_TEXTURE. */ struct hlsl_reg_reservation reservation; + /* Scope that contains annotations for this buffer. */ + struct hlsl_scope *annotations; /* Item entry for hlsl_ctx.buffers */ struct list entry; @@ -1228,7 +1230,8 @@ struct hlsl_ir_node *hlsl_new_binary_expr(struct hlsl_ctx *ctx, enum hlsl_ir_exp struct hlsl_ir_node *arg2); struct hlsl_ir_node *hlsl_new_bool_constant(struct hlsl_ctx *ctx, bool b, const struct vkd3d_shader_location *loc); struct hlsl_buffer *hlsl_new_buffer(struct hlsl_ctx *ctx, enum hlsl_buffer_type type, const char *name, - uint32_t modifiers, const struct hlsl_reg_reservation *reservation, const struct vkd3d_shader_location *loc); + uint32_t modifiers, const struct hlsl_reg_reservation *reservation, struct hlsl_scope *annotations, + const struct vkd3d_shader_location *loc); struct hlsl_ir_node *hlsl_new_call(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *decl, const struct vkd3d_shader_location *loc); struct hlsl_ir_node *hlsl_new_cast(struct hlsl_ctx *ctx, struct hlsl_ir_node *node, struct hlsl_type *type, diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 52c217654..981af7087 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -5673,12 +5673,12 @@ effect_group: } buffer_declaration: - var_modifiers buffer_type any_identifier colon_attribute + var_modifiers buffer_type any_identifier colon_attribute annotations_opt { if ($4.semantic.name) hlsl_error(ctx, &@4, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC, "Semantics are not allowed on buffers."); - if (!(ctx->cur_buffer = hlsl_new_buffer(ctx, $2, $3, $1, &$4.reg_reservation, &@3))) + if (!(ctx->cur_buffer = hlsl_new_buffer(ctx, $2, $3, $1, &$4.reg_reservation, $5, &@3))) YYABORT; } diff --git a/tests/hlsl/cbuffer.shader_test b/tests/hlsl/cbuffer.shader_test index 690491b79..2140e1b88 100644 --- a/tests/hlsl/cbuffer.shader_test +++ b/tests/hlsl/cbuffer.shader_test @@ -16,6 +16,24 @@ uniform 0 float4 1.0 2.0 3.0 4.0 todo(glsl) draw quad probe all rgba (1.0, 2.0, 3.0, 4.0) +[pixel shader fail(sm>=6)] +// Annotations +cbuffer cb : register(b1) < int i = 1; > +{ + float4 m1; +}; + +cbuffer cb < int i = 2; > +{ + float4 m2; +}; + +float4 main() : sv_target +{ + return m1 + m2; +} + + [pixel shader] // Test empty constant buffer. cbuffer Constants : register(b1) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- include/vkd3d_shader.h | 8 ++++++++ libs/vkd3d-shader/fx.c | 6 +++++- libs/vkd3d-shader/hlsl.c | 4 ++++ libs/vkd3d-shader/hlsl.h | 1 + programs/vkd3d-compiler/main.c | 9 +++++++++ 5 files changed, 27 insertions(+), 1 deletion(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 0ce2ef67b..a2ccf45ed 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -321,6 +321,14 @@ enum vkd3d_shader_compile_option_name * \since 1.12 */ VKD3D_SHADER_COMPILE_OPTION_WARN_IMPLICIT_TRUNCATION = 0x0000000c, + /** + * If \a value is nonzero, empty constant buffers descriptions are + * written out in the output effect binary. This option applies only + * to fx_4_0 and fx_4_1 profiles and is otherwise ignored. + * + * \since 1.12 + */ + VKD3D_SHADER_COMPILE_OPTION_INCLUDE_EMPTY_BUFFERS_IN_EFFECTS = 0x0000000d, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_NAME), }; diff --git a/libs/vkd3d-shader/fx.c b/libs/vkd3d-shader/fx.c index 5fcd3eacf..466908cd8 100644 --- a/libs/vkd3d-shader/fx.c +++ b/libs/vkd3d-shader/fx.c @@ -90,6 +90,7 @@ struct fx_write_context int status; bool child_effect; + bool include_empty_buffers; const struct fx_write_context_ops *ops; }; @@ -191,6 +192,7 @@ static void fx_write_context_init(struct hlsl_ctx *ctx, const struct fx_write_co list_init(&fx->types); fx->child_effect = fx->ops->are_child_effects_supported && ctx->child_effect; + fx->include_empty_buffers = version == 4 && ctx->include_empty_buffers; hlsl_block_init(&block); hlsl_prepend_global_uniform_copy(fx->ctx, &block); @@ -1065,7 +1067,9 @@ static void write_buffers(struct fx_write_context *fx) LIST_FOR_EACH_ENTRY(buffer, &fx->ctx->buffers, struct hlsl_buffer, entry) { - if (!buffer->size) + if (!buffer->size && !fx->include_empty_buffers) + continue; + if (!strcmp(buffer->name, "$Params")) continue; write_fx_4_buffer(buffer, fx); diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 174589630..5638a03a8 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -3622,6 +3622,10 @@ static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct vkd3d_shader_compil ctx->warn_implicit_truncation = option->value; break; + case VKD3D_SHADER_COMPILE_OPTION_INCLUDE_EMPTY_BUFFERS_IN_EFFECTS: + ctx->include_empty_buffers = option->value; + break; + default: break; } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 4f9f04c7b..16ddbf3cf 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -927,6 +927,7 @@ struct hlsl_ctx bool semantic_compat_mapping; bool child_effect; + bool include_empty_buffers; bool warn_implicit_truncation; }; diff --git a/programs/vkd3d-compiler/main.c b/programs/vkd3d-compiler/main.c index 245052e97..b0fcdeeea 100644 --- a/programs/vkd3d-compiler/main.c +++ b/programs/vkd3d-compiler/main.c @@ -42,6 +42,7 @@ enum OPTION_CHILD_EFFECT, OPTION_ENTRY, OPTION_FRAGMENT_COORDINATE_ORIGIN, + OPTION_INCLUDE_EMPTY_BUFFERS, OPTION_MATRIX_STORAGE_ORDER, OPTION_OUTPUT, OPTION_PRINT_SOURCE_TYPES, @@ -202,6 +203,9 @@ static void print_usage(const char *program_name) " targets. Valid values are 'upper-left' (default) and\n" " 'lower-left'. The only value supported by Vulkan\n" " environments is 'upper-left'.\n" + " --include-empty-buffers\n" + " Write empty constant buffers descriptions. This option is only meaningful\n" + " for fx_4_0 and fx_4_1 profiles.\n" " --matrix-storage-order=<order>\n" " Specify the default matrix storage order. Valid values\n" " are 'column' (default) and 'row'.\n" @@ -485,6 +489,7 @@ static bool parse_command_line(int argc, char **argv, struct options *options) {"child-effect", no_argument, NULL, OPTION_CHILD_EFFECT}, {"entry", required_argument, NULL, OPTION_ENTRY}, {"fragment-coordinate-origin", required_argument, NULL, OPTION_FRAGMENT_COORDINATE_ORIGIN}, + {"include-empty-buffers", no_argument, NULL, OPTION_INCLUDE_EMPTY_BUFFERS}, {"matrix-storage-order", required_argument, NULL, OPTION_MATRIX_STORAGE_ORDER}, {"output", required_argument, NULL, OPTION_OUTPUT}, {"formatting", required_argument, NULL, OPTION_TEXT_FORMATTING}, @@ -559,6 +564,10 @@ static bool parse_command_line(int argc, char **argv, struct options *options) options->output_filename = optarg; break; + case OPTION_INCLUDE_EMPTY_BUFFERS: + add_compile_option(options, VKD3D_SHADER_COMPILE_OPTION_INCLUDE_EMPTY_BUFFERS_IN_EFFECTS, 1); + break; + case OPTION_MATRIX_STORAGE_ORDER: if (!parse_matrix_storage_order(&pack_matrix_order, optarg)) { -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742
Giovanni Mascellani (@giomasce) commented about programs/vkd3d-compiler/main.c:
" targets. Valid values are 'upper-left' (default) and\n" " 'lower-left'. The only value supported by Vulkan\n" " environments is 'upper-left'.\n" + " --include-empty-buffers\n" + " Write empty constant buffers descriptions. This option is only meaningful\n" + " for fx_4_0 and fx_4_1 profiles.\n"
Would it make sense to start namespacing options that only apply to some shader types or profiles? For example using `--fx-include-empty-buffers` or even `--fx4-include-empty-buffers`. Not necessarily this time, but maybe start thinking about it. Also for HLSL options, for example: `--hlsl-profile` and `--hlsl-semantic-compat-map` instead of what we have now. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742#note_66473
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742
Would it make sense to start namespacing options that only apply to some shader types or profiles? For example using `--fx-include-empty-buffers` or even `--fx4-include-empty-buffers`. Not necessarily this time, but maybe start thinking about it. Also for HLSL options, for example: `--hlsl-profile` and `--hlsl-semantic-compat-map` instead of what we have now.
Broadly yes, although I don't think we should just remove the existing options; at best we could deprecate them and remove them from --help.
```diff + " --include-empty-buffers\n" + " Write empty constant buffers descriptions. This option is only meaningful\n" + " for fx_4_0 and fx_4_1 profiles.\n" ```
That line is a bit long; we try not to go over 80 columns in the --help output. We could potentially do line-breaking at run-time though, that would also allow us to eventually use the actual terminal width, which may very well be a bit wider than 80 columns. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/742#note_66668
participants (5)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)