[PATCH 0/4] MR692: vkd3d-shader/fx: Add "child-effect" compilation option.
From: Nikolay Sivov <nsivov(a)codeweavers.com> So that it could be used as-is when writing the types. Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 45d02ce2b..2302d21fa 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -3398,8 +3398,8 @@ static void declare_predefined_types(struct hlsl_ctx *ctx) {"pass", HLSL_CLASS_OBJECT, HLSL_TYPE_PASS, 1, 1}, {"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}, - {"vertexshader", HLSL_CLASS_OBJECT, HLSL_TYPE_VERTEXSHADER, 1, 1}, + {"PixelShader", HLSL_CLASS_OBJECT, HLSL_TYPE_PIXELSHADER, 1, 1}, + {"VertexShader", HLSL_CLASS_OBJECT, HLSL_TYPE_VERTEXSHADER, 1, 1}, {"RenderTargetView",HLSL_CLASS_OBJECT, HLSL_TYPE_RENDERTARGETVIEW, 1, 1}, {"DepthStencilView",HLSL_CLASS_OBJECT, HLSL_TYPE_DEPTHSTENCILVIEW, 1, 1}, }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/fx.c | 39 ++++++++++++++++++++++++++++++++------- libs/vkd3d-shader/hlsl.y | 8 ++++++++ 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/libs/vkd3d-shader/fx.c b/libs/vkd3d-shader/fx.c index fd5c84432..6e1d96c10 100644 --- a/libs/vkd3d-shader/fx.c +++ b/libs/vkd3d-shader/fx.c @@ -84,6 +84,7 @@ struct fx_write_context uint32_t buffer_count; uint32_t numeric_variable_count; uint32_t object_variable_count; + uint32_t shader_variable_count; int status; const struct fx_write_context_ops *ops; @@ -424,6 +425,8 @@ static uint32_t write_fx_4_type(const struct hlsl_type *type, struct fx_write_co { static const uint32_t object_type[] = { + [HLSL_TYPE_PIXELSHADER] = 5, + [HLSL_TYPE_VERTEXSHADER] = 6, [HLSL_TYPE_RENDERTARGETVIEW] = 19, [HLSL_TYPE_DEPTHSTENCILVIEW] = 20, }; @@ -454,7 +457,9 @@ static uint32_t write_fx_4_type(const struct hlsl_type *type, struct fx_write_co switch (type->base_type) { case HLSL_TYPE_DEPTHSTENCILVIEW: + case HLSL_TYPE_PIXELSHADER: case HLSL_TYPE_RENDERTARGETVIEW: + case HLSL_TYPE_VERTEXSHADER: put_u32_unaligned(buffer, object_type[type->base_type]); break; case HLSL_TYPE_TEXTURE: @@ -695,9 +700,12 @@ static void write_fx_4_numeric_variable(struct hlsl_ir_var *var, struct fx_write static void write_fx_4_object_variable(struct hlsl_ir_var *var, struct fx_write_context *fx) { + const struct hlsl_type *type = hlsl_get_multiarray_element_type(var->data_type); + uint32_t elements_count = hlsl_get_multiarray_size(var->data_type); struct vkd3d_bytecode_buffer *buffer = &fx->structured; uint32_t semantic_offset, bind_point = ~0u; - uint32_t name_offset, type_offset; + uint32_t name_offset, type_offset, i; + struct hlsl_ctx *ctx = fx->ctx; if (var->reg_reservation.reg_type) bind_point = var->reg_reservation.reg_index; @@ -712,8 +720,29 @@ static void write_fx_4_object_variable(struct hlsl_ir_var *var, struct fx_write_ semantic_offset = put_u32(buffer, semantic_offset); /* Semantic */ put_u32(buffer, bind_point); /* Explicit bind point */ + /* Initializer */ + switch (type->base_type) + { + case HLSL_TYPE_TEXTURE: + case HLSL_TYPE_UAV: + case HLSL_TYPE_RENDERTARGETVIEW: + break; + case HLSL_TYPE_PIXELSHADER: + case HLSL_TYPE_VERTEXSHADER: + /* FIXME: write shader blobs, once parser support works. */ + for (i = 0; i < elements_count; ++i) + put_u32(buffer, 0); + ++fx->shader_variable_count; + break; + default: + hlsl_fixme(ctx, &ctx->location, "Writing initializer for object type %u is not implemented.\n", + type->base_type); + } + put_u32(buffer, 0); /* Annotations count */ /* FIXME: write annotations */ + + ++fx->object_variable_count; } static void write_fx_4_buffer(struct hlsl_buffer *b, struct fx_write_context *fx) @@ -809,7 +838,6 @@ static bool is_object_variable(const struct hlsl_ir_var *var) static void write_objects(struct fx_write_context *fx) { struct hlsl_ir_var *var; - uint32_t count = 0; LIST_FOR_EACH_ENTRY(var, &fx->ctx->extern_vars, struct hlsl_ir_var, extern_entry) { @@ -817,10 +845,7 @@ static void write_objects(struct fx_write_context *fx) continue; write_fx_4_object_variable(var, fx); - ++count; } - - fx->object_variable_count += count; } static int hlsl_fx_4_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) @@ -857,7 +882,7 @@ static int hlsl_fx_4_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) put_u32(&buffer, 0); /* Sampler state count. */ put_u32(&buffer, 0); /* Rendertarget view count. */ put_u32(&buffer, 0); /* Depth stencil view count. */ - put_u32(&buffer, 0); /* Shader count. */ + put_u32(&buffer, fx.shader_variable_count); /* Shader count. */ put_u32(&buffer, 0); /* Inline shader count. */ set_u32(&buffer, size_offset, fx.unstructured.size); @@ -915,7 +940,7 @@ static int hlsl_fx_5_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) put_u32(&buffer, 0); /* Sampler state count. */ put_u32(&buffer, 0); /* Rendertarget view count. */ put_u32(&buffer, 0); /* Depth stencil view count. */ - put_u32(&buffer, 0); /* Shader count. */ + put_u32(&buffer, fx.shader_variable_count); /* Shader count. */ put_u32(&buffer, 0); /* Inline shader count. */ put_u32(&buffer, fx.group_count); /* Group count. */ put_u32(&buffer, 0); /* UAV count. */ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 8cdc82263..4883d6485 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -6453,6 +6453,14 @@ type_no_void: { $$ = hlsl_get_type(ctx->cur_scope, "DepthStencilView", true, true); } + | KW_VERTEXSHADER + { + $$ = hlsl_get_type(ctx->cur_scope, "VertexShader", true, true); + } + | KW_PIXELSHADER + { + $$ = hlsl_get_type(ctx->cur_scope, "PixelShader", true, true); + } type: type_no_void -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- include/vkd3d_shader.h | 20 ++++++++++++++++++++ libs/vkd3d-shader/hlsl.c | 4 ++++ libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-utils/vkd3d_utils_main.c | 11 +++++++++-- programs/vkd3d-compiler/main.c | 11 ++++++++++- 5 files changed, 44 insertions(+), 3 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 9e663919c..27cc1aabc 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -212,6 +212,18 @@ enum vkd3d_shader_compile_option_feature_flags VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_FEATURE_FLAGS), }; +/** Compilation options specific to effects targets. \since 1.12 */ +enum vkd3d_shader_compile_option_effect_options +{ + /** Compilation will produce child effect using shared object descriptions, + * as instructed by the "shared" modifier. Child effects are supported + * with fx_2_0, fx_4_0, and fx_4_1. This option, and "shared" modifiers are ignored + * for fx_5_0 profile. */ + VKD3D_SHADER_COMPILE_OPTION_EFFECT_CHILD_EFFECT = 0x00000001, + + VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_EFFECT_OPTIONS), +}; + enum vkd3d_shader_compile_option_name { /** @@ -279,6 +291,14 @@ enum vkd3d_shader_compile_option_name * \since 1.11 */ VKD3D_SHADER_COMPILE_OPTION_FEATURE = 0x0000000a, + /** + * This option specifies compilation options specific to effects targets. + * + * \a value is a member of enum vkd3d_shader_compile_option_effect_options. + * + * \since 1.12 + */ + VKD3D_SHADER_COMPILE_OPTION_EFFECT_OPTIONS = 0x0000000b, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_COMPILE_OPTION_NAME), }; diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 2302d21fa..48fa0f30b 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -3596,6 +3596,10 @@ static bool hlsl_ctx_init(struct hlsl_ctx *ctx, const struct vkd3d_shader_compil { ctx->semantic_compat_mapping = option->value & VKD3D_SHADER_COMPILE_OPTION_BACKCOMPAT_MAP_SEMANTIC_NAMES; } + else if (option->name == VKD3D_SHADER_COMPILE_OPTION_EFFECT_OPTIONS) + { + ctx->child_effect = option->value & VKD3D_SHADER_COMPILE_OPTION_EFFECT_CHILD_EFFECT; + } } return true; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index aad9b3c7b..9c55d9dfb 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -921,6 +921,7 @@ struct hlsl_ctx uint32_t found_numthreads : 1; bool semantic_compat_mapping; + bool child_effect; }; struct hlsl_resource_load_params diff --git a/libs/vkd3d-utils/vkd3d_utils_main.c b/libs/vkd3d-utils/vkd3d_utils_main.c index c90aad45a..82f492099 100644 --- a/libs/vkd3d-utils/vkd3d_utils_main.c +++ b/libs/vkd3d-utils/vkd3d_utils_main.c @@ -244,7 +244,7 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen { struct vkd3d_shader_preprocess_info preprocess_info; struct vkd3d_shader_hlsl_source_info hlsl_info; - struct vkd3d_shader_compile_option options[4]; + struct vkd3d_shader_compile_option options[5]; struct vkd3d_shader_compile_info compile_info; struct vkd3d_shader_compile_option *option; struct vkd3d_shader_code byte_code; @@ -262,7 +262,7 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen if (flags & ~(D3DCOMPILE_DEBUG | D3DCOMPILE_PACK_MATRIX_ROW_MAJOR | D3DCOMPILE_PACK_MATRIX_COLUMN_MAJOR)) FIXME("Ignoring flags %#x.\n", flags); - if (effect_flags) + if (effect_flags & ~D3DCOMPILE_EFFECT_CHILD_EFFECT) FIXME("Ignoring effect flags %#x.\n", effect_flags); if (secondary_flags) FIXME("Ignoring secondary flags %#x.\n", secondary_flags); @@ -330,6 +330,13 @@ HRESULT WINAPI D3DCompile2(const void *data, SIZE_T data_size, const char *filen option->value = VKD3D_SHADER_COMPILE_OPTION_BACKCOMPAT_MAP_SEMANTIC_NAMES; } + if (effect_flags & D3DCOMPILE_EFFECT_CHILD_EFFECT) + { + option = &options[compile_info.option_count++]; + option->name = VKD3D_SHADER_COMPILE_OPTION_EFFECT_OPTIONS; + option->value = VKD3D_SHADER_COMPILE_OPTION_EFFECT_CHILD_EFFECT; + } + ret = vkd3d_shader_compile(&compile_info, &byte_code, &messages); if (messages && messages_blob) diff --git a/programs/vkd3d-compiler/main.c b/programs/vkd3d-compiler/main.c index 2a2c6b961..79b88030c 100644 --- a/programs/vkd3d-compiler/main.c +++ b/programs/vkd3d-compiler/main.c @@ -39,6 +39,7 @@ enum { OPTION_HELP = CHAR_MAX + 1, OPTION_BUFFER_UAV, + OPTION_CHILD_EFFECT, OPTION_ENTRY, OPTION_FRAGMENT_COORDINATE_ORIGIN, OPTION_MATRIX_STORAGE_ORDER, @@ -470,13 +471,14 @@ static bool parse_command_line(int argc, char **argv, struct options *options) enum vkd3d_shader_compile_option_pack_matrix_order pack_matrix_order; enum vkd3d_shader_compile_option_fragment_coordinate_origin origin; enum vkd3d_shader_compile_option_buffer_uav buffer_uav; - unsigned int compat_options = 0; + unsigned int compat_options = 0, effect_options = 0; int option; static struct option long_options[] = { {"help", no_argument, NULL, OPTION_HELP}, {"buffer-uav", required_argument, NULL, OPTION_BUFFER_UAV}, + {"child-effect", no_argument, NULL, OPTION_CHILD_EFFECT}, {"entry", required_argument, NULL, OPTION_ENTRY}, {"fragment-coordinate-origin", required_argument, NULL, OPTION_FRAGMENT_COORDINATE_ORIGIN}, {"matrix-storage-order", required_argument, NULL, OPTION_MATRIX_STORAGE_ORDER}, @@ -521,6 +523,10 @@ static bool parse_command_line(int argc, char **argv, struct options *options) add_compile_option(options, VKD3D_SHADER_COMPILE_OPTION_BUFFER_UAV, buffer_uav); break; + case OPTION_CHILD_EFFECT: + effect_options |= VKD3D_SHADER_COMPILE_OPTION_EFFECT_CHILD_EFFECT; + break; + case OPTION_ENTRY: case 'e': options->entry_point = optarg; @@ -605,6 +611,9 @@ static bool parse_command_line(int argc, char **argv, struct options *options) if (compat_options) add_compile_option(options, VKD3D_SHADER_COMPILE_OPTION_BACKWARD_COMPATIBILITY, compat_options); + if (effect_options) + add_compile_option(options, VKD3D_SHADER_COMPILE_OPTION_EFFECT_OPTIONS, effect_options); + if (optind < argc) options->filename = argv[argc - 1]; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692
From: Nikolay Sivov <nsivov(a)codeweavers.com> Signed-off-by: Nikolay Sivov <nsivov(a)codeweavers.com> --- libs/vkd3d-shader/fx.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/fx.c b/libs/vkd3d-shader/fx.c index 6e1d96c10..630736732 100644 --- a/libs/vkd3d-shader/fx.c +++ b/libs/vkd3d-shader/fx.c @@ -64,6 +64,7 @@ struct fx_write_context_ops uint32_t (*write_type)(const struct hlsl_type *type, struct fx_write_context *fx); void (*write_technique)(struct hlsl_ir_var *var, struct fx_write_context *fx); void (*write_pass)(struct hlsl_ir_var *var, struct fx_write_context *fx); + bool are_child_effects_supported; }; struct fx_write_context @@ -84,9 +85,12 @@ struct fx_write_context uint32_t buffer_count; uint32_t numeric_variable_count; uint32_t object_variable_count; + uint32_t shared_object_count; uint32_t shader_variable_count; int status; + bool child_effect; + const struct fx_write_context_ops *ops; }; @@ -175,6 +179,8 @@ static void fx_write_context_init(struct hlsl_ctx *ctx, const struct fx_write_co rb_init(&fx->strings, string_storage_compare); list_init(&fx->types); + + fx->child_effect = fx->ops->are_child_effects_supported && ctx->child_effect; } static int fx_write_context_cleanup(struct fx_write_context *fx) @@ -666,6 +672,7 @@ static const struct fx_write_context_ops fx_4_ops = .write_type = write_fx_4_type, .write_technique = write_fx_4_technique, .write_pass = write_fx_4_pass, + .are_child_effects_supported = true, }; static void write_fx_4_numeric_variable(struct hlsl_ir_var *var, struct fx_write_context *fx) @@ -720,6 +727,12 @@ static void write_fx_4_object_variable(struct hlsl_ir_var *var, struct fx_write_ semantic_offset = put_u32(buffer, semantic_offset); /* Semantic */ put_u32(buffer, bind_point); /* Explicit bind point */ + if (fx->child_effect && var->storage_modifiers & HLSL_STORAGE_SHARED) + { + ++fx->shared_object_count; + return; + } + /* Initializer */ switch (type->base_type) { @@ -835,15 +848,21 @@ static bool is_object_variable(const struct hlsl_ir_var *var) } } -static void write_objects(struct fx_write_context *fx) +static void write_objects(struct fx_write_context *fx, bool shared) { struct hlsl_ir_var *var; + if (shared && !fx->child_effect) + return; + LIST_FOR_EACH_ENTRY(var, &fx->ctx->extern_vars, struct hlsl_ir_var, extern_entry) { if (!is_object_variable(var)) continue; + if (fx->child_effect && (shared != !!(var->storage_modifiers & HLSL_STORAGE_SHARED))) + continue; + write_fx_4_object_variable(var, fx); } } @@ -859,9 +878,9 @@ static int hlsl_fx_4_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) put_u32(&fx.unstructured, 0); /* Empty string placeholder. */ write_buffers(&fx); - write_objects(&fx); + write_objects(&fx, false); /* TODO: shared buffers */ - /* TODO: shared objects */ + write_objects(&fx, true); write_techniques(ctx->globals, &fx); @@ -871,7 +890,7 @@ static int hlsl_fx_4_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) put_u32(&buffer, fx.object_variable_count); /* Object variable count. */ put_u32(&buffer, 0); /* Pool buffer count. */ put_u32(&buffer, 0); /* Pool variable count. */ - put_u32(&buffer, 0); /* Pool object count. */ + put_u32(&buffer, fx.shared_object_count); /* Shared object count. */ put_u32(&buffer, fx.technique_count); size_offset = put_u32(&buffer, 0); /* Unstructured size. */ put_u32(&buffer, 0); /* String count. */ @@ -918,7 +937,7 @@ static int hlsl_fx_5_write(struct hlsl_ctx *ctx, struct vkd3d_shader_code *out) put_u32(&fx.unstructured, 0); /* Empty string placeholder. */ write_buffers(&fx); - write_objects(&fx); + write_objects(&fx, false); /* TODO: interface variables */ write_groups(&fx); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692
+ case HLSL_TYPE_PIXELSHADER: + case HLSL_TYPE_VERTEXSHADER: + /* FIXME: write shader blobs, once parser support works. */ + for (i = 0; i < elements_count; ++i) + put_u32(buffer, 0); + ++fx->shader_variable_count; + break;
We shouldn't be returning success if we're not going to output correct shader code. The lack of tests here is also concerning. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63866
Is there a point of making "effect_options" a bitmask, instead of making them top-level options? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63867
On Thu Mar 7 18:26:32 2024 +0000, Zebediah Figura wrote:
+ case HLSL_TYPE_PIXELSHADER: + case HLSL_TYPE_VERTEXSHADER: + /* FIXME: write shader blobs, once parser support works. */ + for (i = 0; i < elements_count; ++i) + put_u32(buffer, 0); + ++fx->shader_variable_count; + break; We shouldn't be returning success if we're not going to output correct shader code. The lack of tests here is also concerning. We are not going to get there, because shader object initializers are not supported in the parser.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63869
On Thu Mar 7 18:33:04 2024 +0000, Zebediah Figura wrote:
Is there a point of making "effect_options" a bitmask, instead of making them top-level options? The point is to have effects flags together, that's how effect flags are handled in D3DCompile() arguments.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63870
On Thu Mar 7 18:33:04 2024 +0000, Nikolay Sivov wrote:
The point is to have effects flags together, that's how effect flags are handled in D3DCompile() arguments. But it's not like we're grouping any other flags together, and in general vkd3d-shader compile options API design is not really informed by d3dcompiler. I'd assert it should be consistent with the way other API options work.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63871
On Thu Mar 7 18:26:32 2024 +0000, Nikolay Sivov wrote:
We are not going to get there, because shader object initializers are not supported in the parser. Then it seems very odd that we're adding this code now? It's effectively dead.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63872
On Thu Mar 7 18:40:47 2024 +0000, Zebediah Figura wrote:
Then it seems very odd that we're adding this code now? It's effectively dead. Would it be less dead if I had a test that runs through this path?
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63873
On Thu Mar 7 18:40:21 2024 +0000, Zebediah Figura wrote:
But it's not like we're grouping any other flags together, and in general vkd3d-shader compile options API design is not really informed by d3dcompiler. I'd assert it should be consistent with the way other API options work. Alright.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/692#note_63874
participants (3)
-
Nikolay Sivov -
Nikolay Sivov (＠nsivov)
-
Zebediah Figura (＠zfigura)