[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@codeweavers.com
-- v2: vkd3d-shader/fx: Add an option to include empty buffers in the effect binary.
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@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)
From: Nikolay Sivov nsivov@codeweavers.com
Signed-off-by: Nikolay Sivov nsivov@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)) {
This merge request was approved by Zebediah Figura.
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.
This merge request was approved by Giovanni Mascellani.
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.
+ " --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.
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Nikolay Sivov
-
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)