[PATCH v4 0/5] MR280: vkd3d-shader: sm1 semantic matching.
This is probably the most ugly and controversial bit of API. I don't really know if this is the right approach to solving this. sm4 registers match by register index, such that shaders can mostly be compiled in isolation. sm1 does not—registers may be specified in any order in the vertex and pixel shaders, and will be matched by usage and usage index. By itself this is not much of a problem. Where it gets hairy is that we want to do some degree of caching, as well as pre-compilation, and avoid recompiling either shader every time it's matched with a new one. Wine currently deals with this problem, for GLSL, by generating a "main" GLSL shader for the vertex shader, and then an extra function setup_vs_output(), and linking the two together every time a new pixel shader is used. This could in theory be used for SPIR-V, but it requires the use of extra, probably external, code to link SPIR-V shaders together, which I do not particularly anticipate being well-received. (I'm not sure how Wine deals with this problem in the ARB backend. It seems to take the pixel shader signature into account when generating the vertex shader— cf. init_output_registers()—but it doesn't take it into account when looking up a vertex shader variant? I didn't look too closely at the code, so maybe I'm missing something.) -- The vkd3d parts of this patch are quite straightforward, and looking at them, I think the design is quite intuitive in isolation. There may be some room for internal refactoring (in particular with an eye to not so much overloading the "register_index" field of struct shader_signature_element) but I'm relatively happy with the way it turned out. In isolation, that is. The Wine part is worse. I've uploaded branches for vkd3d and Wine that use this API, and correctly handle shaders with some nontrivial reordering: https://gitlab.winehq.org/zfigura/vkd3d/-/commits/himavant5 https://gitlab.winehq.org/zfigura/wine/-/commits/himavant_cb The test can be run, as before, like so: make tests/shader_runner.exe && WINE_D3D_CONFIG=renderer=vulkan wine tests/shader_runner.exe ../vkd3d/tests/hlsl/sm1-interstage-interface.shader_test The interesting Wine parts are concentrated in a single patch, 5cfb9d930f11e. The patch takes a few shortcuts, partly because I wanted not to block the vkd3d API design questions, but also because while writing it I came up with a couple of problems that I wasn't sure how to fix. There are two main problems I see: (1) This patch has the user pass the signature from the pixel shader when compiling the vertex shader, and looks up register indices already arbitrarily allocated by the pixel shader. This is problematic when trying to use this signature as a cache key, by virtue of it not being clear (or even defined) which fields are key elements and which aren't. It's also not particularly kind to lookup, on account of not being directly comparable with memcmp(). There are a few options I see: (a) Provide an internal function to compare key elements. This feels... odd, like a very special-purpose function, but perhaps workable. (b) Just make the user deal with it, and assert that all fields are key elements. (c) Use some alternative, perhaps shortened structure as a field of vkd3d_shader_next_stage_info. This has the disadvantage that it is not as simple for a hypothetical user to retrieve from the pixel shader, but we would presumably provide a function to generate one from a shader signature. This would probably also be kinder to cache lookup if it's shorter. (d) Make caching vkd3d's responsibility, to some degree. This seems daunting, but the more we optimize, the more difficult it may be to design API that allows for nice caching. (2) Assuming we use signatures, there is a memory management problem that 5cfb9d930f11e spells out. This is probably a matter of "just fix it", but I suppose another option is to take the GLSL or ARB architecture. -- April is the cruellest month, breeding Lilacs out of the dead land, mixing Memory and desire, stirring Dull roots with spring rain. -- v4: vkd3d-shader: Introduce a function to build a varying map between sm1 stages. https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
From: Zebediah Figura <zfigura(a)codeweavers.com> spirv will need this. --- libs/vkd3d-shader/d3dbc.c | 7 ++++++- tests/vkd3d_shader_api.c | 6 +++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 9a147e3d..22e463bf 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -524,6 +524,8 @@ static struct signature_element *find_signature_element_by_register_index( return NULL; } +#define SM1_COLOR_REGISTER_OFFSET 8 + static bool add_signature_element(struct vkd3d_shader_sm1_parser *sm1, bool output, const char *name, unsigned int index, enum vkd3d_shader_sysval_semantic sysval, unsigned int register_index, bool is_dcl, unsigned int mask) @@ -606,7 +608,7 @@ static bool add_signature_element_from_register(struct vkd3d_shader_sm1_parser * return true; } return add_signature_element(sm1, false, "COLOR", register_index, - VKD3D_SHADER_SV_NONE, register_index, is_dcl, mask); + VKD3D_SHADER_SV_NONE, SM1_COLOR_REGISTER_OFFSET + register_index, is_dcl, mask); case VKD3DSPR_TEXTURE: /* For vertex shaders, this is ADDR. */ @@ -633,6 +635,9 @@ static bool add_signature_element_from_register(struct vkd3d_shader_sm1_parser * /* fall through */ case VKD3DSPR_ATTROUT: + return add_signature_element(sm1, true, "COLOR", register_index, + VKD3D_SHADER_SV_NONE, SM1_COLOR_REGISTER_OFFSET + register_index, is_dcl, mask); + case VKD3DSPR_COLOROUT: return add_signature_element(sm1, true, "COLOR", register_index, VKD3D_SHADER_SV_NONE, register_index, is_dcl, mask); diff --git a/tests/vkd3d_shader_api.c b/tests/vkd3d_shader_api.c index 8bd728d9..4893f44f 100644 --- a/tests/vkd3d_shader_api.c +++ b/tests/vkd3d_shader_api.c @@ -505,7 +505,7 @@ static void test_scan_signatures(void) { {"POSITION", 0, 0, VKD3D_SHADER_SV_POSITION, VKD3D_SHADER_COMPONENT_FLOAT, 0, 0xf, 0xf}, {"TEXCOORD", 2, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 2, 0xf, 0xf}, - {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 0, 0xf, 0xf}, + {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 8, 0xf, 0xf}, {"FOG", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 1, 0x1, 0x1}, {"PSIZE", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 2, 0x1, 0x1}, }; @@ -582,7 +582,7 @@ static void test_scan_signatures(void) static const struct vkd3d_shader_signature_element ps2_inputs[] = { {"TEXCOORD", 2, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 2, 0xf, 0xf}, - {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 0, 0xf, 0xf}, + {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 8, 0xf, 0xf}, }; static const struct vkd3d_shader_signature_element ps2_outputs[] = @@ -603,7 +603,7 @@ static void test_scan_signatures(void) static const struct vkd3d_shader_signature_element ps3_inputs[] = { - {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 0, 0xf, 0xf}, + {"COLOR", 0, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 8, 0xf, 0xf}, {"TEXCOORD", 2, 0, VKD3D_SHADER_SV_NONE, VKD3D_SHADER_COMPONENT_FLOAT, 2, 0xf, 0xf}, }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
From: Zebediah Figura <zfigura(a)codeweavers.com> We want to be able to remap input signatures based on the signature index, but signature normalization both reorders the signature, and requires the old register index, so add a new field for this. --- libs/vkd3d-shader/d3dbc.c | 1 + libs/vkd3d-shader/dxbc.c | 1 + libs/vkd3d-shader/spirv.c | 8 ++++---- libs/vkd3d-shader/vkd3d_shader_private.h | 4 ++++ 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/libs/vkd3d-shader/d3dbc.c b/libs/vkd3d-shader/d3dbc.c index 22e463bf..1277251b 100644 --- a/libs/vkd3d-shader/d3dbc.c +++ b/libs/vkd3d-shader/d3dbc.c @@ -557,6 +557,7 @@ static bool add_signature_element(struct vkd3d_shader_sm1_parser *sm1, bool outp element->sysval_semantic = sysval; element->component_type = VKD3D_SHADER_COMPONENT_FLOAT; element->register_index = register_index; + element->target_location = register_index; element->register_count = 1; element->mask = mask; element->used_mask = is_dcl ? 0 : mask; diff --git a/libs/vkd3d-shader/dxbc.c b/libs/vkd3d-shader/dxbc.c index 716b7bdb..cedc3da4 100644 --- a/libs/vkd3d-shader/dxbc.c +++ b/libs/vkd3d-shader/dxbc.c @@ -391,6 +391,7 @@ static int shader_parse_signature(const struct vkd3d_shader_dxbc_section_desc *s read_dword(&ptr, &e[i].sysval_semantic); read_dword(&ptr, &e[i].component_type); read_dword(&ptr, &e[i].register_index); + e[i].target_location = e[i].register_index; e[i].register_count = 1; read_dword(&ptr, &mask); e[i].mask = mask & 0xff; diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index d71f0a69..27d95912 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -4602,7 +4602,7 @@ static uint32_t spirv_compiler_emit_input(struct spirv_compiler *compiler, } else { - unsigned int location = signature_element->register_index; + unsigned int location = signature_element->target_location; input_id = spirv_compiler_emit_array_variable(compiler, &builder->global_stream, storage_class, component_type, input_component_count, array_sizes, 2); @@ -4980,7 +4980,7 @@ static void spirv_compiler_emit_output(struct spirv_compiler *compiler, } else { - unsigned int location = signature_element->register_index; + unsigned int location = signature_element->target_location; if (is_patch_constant) location += shader_signature_next_location(&compiler->output_signature); @@ -4989,10 +4989,10 @@ static void spirv_compiler_emit_output(struct spirv_compiler *compiler, storage_class, component_type, output_component_count, array_sizes, 2); vkd3d_spirv_add_iface_variable(builder, id); - if (is_dual_source_blending(compiler) && signature_element->register_index < 2) + if (is_dual_source_blending(compiler) && location < 2) { vkd3d_spirv_build_op_decorate1(builder, id, SpvDecorationLocation, 0); - vkd3d_spirv_build_op_decorate1(builder, id, SpvDecorationIndex, signature_element->register_index); + vkd3d_spirv_build_op_decorate1(builder, id, SpvDecorationIndex, location); } else { diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index d35f49a6..9f426dbc 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -815,11 +815,15 @@ struct signature_element unsigned int stream_index; enum vkd3d_shader_sysval_semantic sysval_semantic; enum vkd3d_shader_component_type component_type; + /* Before normalization, this is the register index in the source shader. + * After normalization (or always for DXIL), this is the *signature* index. */ unsigned int register_index; unsigned int register_count; unsigned int mask; unsigned int used_mask; enum vkd3d_shader_minimum_precision min_precision; + /* Register index / location in the target shader. */ + unsigned int target_location; }; struct shader_signature -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
From: Zebediah Figura <zfigura(a)codeweavers.com> --- include/vkd3d_shader.h | 78 +++++++++++++++++++++++- libs/vkd3d-shader/ir.c | 71 ++++++++++++++++++++- libs/vkd3d-shader/spirv.c | 2 +- libs/vkd3d-shader/vkd3d_shader_private.h | 3 +- 4 files changed, 150 insertions(+), 4 deletions(-) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index d6653d18..9b027e9f 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -90,6 +90,11 @@ enum vkd3d_shader_structure_type * \since 1.9 */ VKD3D_SHADER_STRUCTURE_TYPE_SCAN_SIGNATURE_INFO, + /** + * The structure is a vkd3d_shader_next_stage_info structure. + * \since 1.9 + */ + VKD3D_SHADER_STRUCTURE_TYPE_NEXT_STAGE_INFO, VKD3D_FORCE_32_BIT_ENUM(VKD3D_SHADER_STRUCTURE_TYPE), }; @@ -1676,6 +1681,76 @@ struct vkd3d_shader_scan_signature_info struct vkd3d_shader_signature patch_constant; }; +/** + * Describes the mapping of a output varying register in a shader stage, + * to an input varying register in the following shader stage. + * + * This structure is used in struct vkd3d_shader_next_stage_info. + */ +struct vkd3d_shader_varying_map +{ + /** + * The signature index (in the output signature) of the output varying. + * If greater than or equal to the number of elements in the output + * signature, signifies that the varying is consumed by the next stage but + * not written by this one. + */ + unsigned int output_signature_index; + /** The register index of the input varying to map this register to. */ + unsigned int input_register_index; + /** The mask consumed by the destination register. */ + unsigned int input_mask; +}; + +/** + * A chained structure which describes the next shader in the pipeline. + * + * This structure is optional, and should only be provided if there is in fact + * another shader in the pipeline. + * However, depending on the input and output formats, this structure may be + * necessary in order to generate shaders which correctly match each other. + * If the structure or its individual fields are not provided, vkd3d-shader + * will generate shaders which may be correct in isolation, but are not + * guaranteed to correctly match each other. + * + * For example, when compiling legacy Direct3D bytecode to SPIR-V, vkd3d-shader + * needs to assign a mapping of Direct3D usages to SPIR-V varying locations, + * and because legacy Direct3D bytecode need not specify locations in the same + * order between stages (unlike e.g. TPF) vkd3d-shader needs to know that + * mapping when compiling linked stages. + * + * This structure is passed to vkd3d_shader_compile() and extends + * vkd3d_shader_compile_info. + * + * This structure contains only input parameters. + * + * \since 1.9 + */ +struct vkd3d_shader_next_stage_info +{ + /** Must be set to VKD3D_SHADER_STRUCTURE_TYPE_NEXT_STAGE_INFO. */ + enum vkd3d_shader_structure_type type; + /** Optional pointer to a structure containing further parameters. */ + const void *next; + + /** + * A mapping of output varyings in this shader stage to input varyings + * in the next shader stage. + * + * This mapping should include exactly one element for each varying + * consumed by the next shader stage. + * If this shader stage outputs a varying that is not consumed by the next + * shader stage, that varying should be absent from this array. + * + * This field must be provided when compiling from legacy Direct3D + * bytecode, if there is another shader stage (i.e. it must be provided for + * vertex shaders). + */ + const struct vkd3d_shader_varying_map *varying_map; + /** The number of registers provided in \ref varying_map. */ + unsigned int varying_count; +}; + #ifdef LIBVKD3D_SHADER_SOURCE # define VKD3D_SHADER_API VKD3D_EXPORT #else @@ -1748,13 +1823,14 @@ VKD3D_SHADER_API const enum vkd3d_shader_target_type *vkd3d_shader_get_supported * * Depending on the source and target types, this function may support the * following chained structures: + * - vkd3d_shader_hlsl_source_info * - vkd3d_shader_interface_info + * - vkd3d_shader_next_stage_info * - vkd3d_shader_scan_descriptor_info * - vkd3d_shader_scan_signature_info * - vkd3d_shader_spirv_domain_shader_target_info * - vkd3d_shader_spirv_target_info * - vkd3d_shader_transform_feedback_info - * - vkd3d_shader_hlsl_source_info * * \param compile_info A chained structure containing compilation parameters. * diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index d74f81af..6589ed24 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -85,6 +85,70 @@ static void shader_instruction_eliminate_phase_instance_id(struct vkd3d_shader_i shader_register_eliminate_phase_addressing((struct vkd3d_shader_register *)&ins->dst[i].reg, instance_id); } +static const struct vkd3d_shader_varying_map *find_varying_map( + const struct vkd3d_shader_next_stage_info *next_stage, unsigned int signature_idx) +{ + unsigned int i; + + for (i = 0; i < next_stage->varying_count; ++i) + { + if (next_stage->varying_map[i].output_signature_index == signature_idx) + return &next_stage->varying_map[i]; + } + + return NULL; +} + +static enum vkd3d_result remap_output_signature(struct shader_signature *signature, + const struct vkd3d_shader_compile_info *compile_info) +{ + const struct vkd3d_shader_next_stage_info *next_stage; + unsigned int i; + + if (!(next_stage = vkd3d_find_struct(compile_info->next, NEXT_STAGE_INFO))) + { + ERR("Next stage information was not provided.\n"); + return VKD3D_ERROR_INVALID_ARGUMENT; + } + + for (i = 0; i < signature->element_count; ++i) + { + const struct vkd3d_shader_varying_map *map = find_varying_map(next_stage, i); + struct signature_element *e = &signature->elements[i]; + + if (map) + { + unsigned int input_mask = map->input_mask; + + e->target_location = map->input_register_index; + + /* It is illegal in Vulkan if the next shader uses the same varying + * location with a different mask. */ + if (input_mask && input_mask != e->mask) + { + FIXME("Output mask %#x does not match input mask %#x.\n", e->mask, input_mask); + return VKD3D_ERROR_NOT_IMPLEMENTED; + } + } + else + { + FIXME("This stage outputs varyings not consumed by the next stage.\n"); + return VKD3D_ERROR_NOT_IMPLEMENTED; + } + } + + for (i = 0; i < next_stage->varying_count; ++i) + { + if (next_stage->varying_map[i].output_signature_index >= signature->element_count) + { + FIXME("The next stage consumes varyings not written by this stage.\n"); + return VKD3D_ERROR_NOT_IMPLEMENTED; + } + } + + return VKD3D_OK; +} + struct hull_flattener { struct vkd3d_shader_instruction_array instructions; @@ -1194,7 +1258,8 @@ static enum vkd3d_result instruction_array_normalise_flat_constants(struct vkd3d return VKD3D_OK; } -enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser) +enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, + const struct vkd3d_shader_compile_info *compile_info) { struct vkd3d_shader_instruction_array *instructions = &parser->instructions; enum vkd3d_result result = VKD3D_OK; @@ -1202,6 +1267,10 @@ enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser) if (parser->shader_desc.is_dxil) return result; + if (parser->shader_version.major < 4 && parser->shader_version.type == VKD3D_SHADER_TYPE_VERTEX + && (result = remap_output_signature(&parser->shader_desc.output_signature, compile_info)) < 0) + return result; + if (parser->shader_version.type == VKD3D_SHADER_TYPE_HULL && (result = instruction_array_flatten_hull_shader_phases(instructions)) >= 0) { diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 27d95912..28825256 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -9542,7 +9542,7 @@ static int spirv_compiler_generate_spirv(struct spirv_compiler *compiler, compiler->location.column = 0; compiler->location.line = 1; - if ((result = vkd3d_shader_normalise(parser)) < 0) + if ((result = vkd3d_shader_normalise(parser, compile_info)) < 0) return result; instructions = parser->instructions; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 9f426dbc..22b82ff4 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -1410,6 +1410,7 @@ void dxbc_writer_add_section(struct dxbc_writer *dxbc, uint32_t tag, const void void dxbc_writer_init(struct dxbc_writer *dxbc); int dxbc_writer_write(struct dxbc_writer *dxbc, struct vkd3d_shader_code *code); -enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser); +enum vkd3d_result vkd3d_shader_normalise(struct vkd3d_shader_parser *parser, + const struct vkd3d_shader_compile_info *compile_info); #endif /* __VKD3D_SHADER_PRIVATE_H */ -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/ir.c | 3 +-- libs/vkd3d-shader/spirv.c | 6 ++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 5 ++++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/libs/vkd3d-shader/ir.c b/libs/vkd3d-shader/ir.c index 6589ed24..f8d31d91 100644 --- a/libs/vkd3d-shader/ir.c +++ b/libs/vkd3d-shader/ir.c @@ -132,8 +132,7 @@ static enum vkd3d_result remap_output_signature(struct shader_signature *signatu } else { - FIXME("This stage outputs varyings not consumed by the next stage.\n"); - return VKD3D_ERROR_NOT_IMPLEMENTED; + e->target_location = SIGNATURE_TARGET_LOCATION_UNUSED; } } diff --git a/libs/vkd3d-shader/spirv.c b/libs/vkd3d-shader/spirv.c index 28825256..2725ed80 100644 --- a/libs/vkd3d-shader/spirv.c +++ b/libs/vkd3d-shader/spirv.c @@ -4978,6 +4978,12 @@ static void spirv_compiler_emit_output(struct spirv_compiler *compiler, spirv_compiler_emit_register_execution_mode(compiler, &dst->reg); } + else if (signature_element->target_location == SIGNATURE_TARGET_LOCATION_UNUSED) + { + storage_class = SpvStorageClassPrivate; + id = spirv_compiler_emit_array_variable(compiler, &builder->global_stream, + storage_class, component_type, output_component_count, array_sizes, 2); + } else { unsigned int location = signature_element->target_location; diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 22b82ff4..2d68a745 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -807,6 +807,8 @@ enum vkd3d_shader_input_sysval_semantic VKD3D_SIV_LINE_DENSITY_TESS_FACTOR = 22, }; +#define SIGNATURE_TARGET_LOCATION_UNUSED (~0u) + struct signature_element { unsigned int sort_index; @@ -822,7 +824,8 @@ struct signature_element unsigned int mask; unsigned int used_mask; enum vkd3d_shader_minimum_precision min_precision; - /* Register index / location in the target shader. */ + /* Register index / location in the target shader. + * If SIGNATURE_TARGET_LOCATION_UNUSED, this element should not be written. */ unsigned int target_location; }; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
From: Zebediah Figura <zfigura(a)codeweavers.com> --- include/vkd3d_shader.h | 31 +++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_main.c | 39 +++++++++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/include/vkd3d_shader.h b/include/vkd3d_shader.h index 9b027e9f..21eb20d5 100644 --- a/include/vkd3d_shader.h +++ b/include/vkd3d_shader.h @@ -1742,6 +1742,8 @@ struct vkd3d_shader_next_stage_info * If this shader stage outputs a varying that is not consumed by the next * shader stage, that varying should be absent from this array. * + * This mapping may be constructed by vkd3d_shader_build_varying_map(). + * * This field must be provided when compiling from legacy Direct3D * bytecode, if there is another shader stage (i.e. it must be provided for * vertex shaders). @@ -2264,6 +2266,35 @@ VKD3D_SHADER_API int vkd3d_shader_serialize_dxbc(size_t section_count, */ VKD3D_SHADER_API void vkd3d_shader_free_scan_signature_info(struct vkd3d_shader_scan_signature_info *info); +/** + * Build a mapping of output varyings in a shader stage to input varyings in + * the following shader stage. + * + * This mapping should be used in struct vkd3d_shader_next_stage_info to + * compile the first shader. + * + * \param output_signature The output signature of the first shader. + * + * \param input_signature The input signature of the second shader. + * + * \param count On output, contains the number of entries written into + * \ref varyings. + * + * \param varyings Pointer to an output array of varyings. + * This must point to space for N varyings, where N is the number of elements + * in the input signature. + * + * \remark Valid legacy Direct3D pixel shaders have at most 12 varying inputs: + * 10 inter-stage varyings, face, and position. + * Therefore, in practice, it is safe to call this function with a + * pre-allocated array with a fixed size of 12. + * + * \since 1.9 + */ +VKD3D_SHADER_API void vkd3d_shader_build_varying_map(const struct vkd3d_shader_signature *output_signature, + const struct vkd3d_shader_signature *input_signature, + unsigned int *count, struct vkd3d_shader_varying_map *varyings); + #endif /* VKD3D_SHADER_NO_PROTOTYPES */ /** Type of vkd3d_shader_get_version(). */ diff --git a/libs/vkd3d-shader/vkd3d_shader_main.c b/libs/vkd3d-shader/vkd3d_shader_main.c index e8259181..3171d2b6 100644 --- a/libs/vkd3d-shader/vkd3d_shader_main.c +++ b/libs/vkd3d-shader/vkd3d_shader_main.c @@ -1891,3 +1891,42 @@ void shader_instruction_array_destroy(struct vkd3d_shader_instruction_array *ins vkd3d_free(instructions->icbs[i]); vkd3d_free(instructions->icbs); } + +void vkd3d_shader_build_varying_map(const struct vkd3d_shader_signature *output_signature, + const struct vkd3d_shader_signature *input_signature, + unsigned int *ret_count, struct vkd3d_shader_varying_map *varyings) +{ + uint32_t unused_signature_idx = output_signature->element_count; + unsigned int count = 0; + unsigned int i; + + TRACE("output_signature %p, input_signature %p, count %u, varyings %p.\n", + output_signature, input_signature, *count, varyings); + + for (i = 0; i < input_signature->element_count; ++i) + { + const struct vkd3d_shader_signature_element *input_element, *output_element; + + input_element = &input_signature->elements[i]; + + if (input_element->sysval_semantic != VKD3D_SHADER_SV_NONE) + continue; + + varyings[count].input_register_index = input_element->register_index; + varyings[count].input_mask = input_element->mask; + + if ((output_element = vkd3d_shader_find_signature_element(output_signature, + input_element->semantic_name, input_element->semantic_index, 0))) + { + varyings[count].output_signature_index = output_element - output_signature->elements; + } + else + { + varyings[count].output_signature_index = unused_signature_idx++; + } + + ++count; + } + + *ret_count = count; +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280
I think I did. It shouldn't be any meaner than "output_signature->element_count + some extra stuff" though; it's a different bound, but it doesn't say anything about the order by itself. I wouldn't necessarily be opposed to specifying this as e.g. a sorted array of "{src_index, dst_index, dst_mask}" either though.
v4 does this, albeit without the sorting. I also went ahead and added a patch to avoid outputting semantics not read by the next shader stage; it ended up being simpler than I was afraid it would be. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280#note_40858
```diff + /* Before normalization, this is the register index in the source shader. + * After normalization (or always for DXIL), this is the *signature* index. */ unsigned int register_index; unsigned int register_count; unsigned int mask; unsigned int used_mask; enum vkd3d_shader_minimum_precision min_precision; + /* Register index / location in the target shader. */ + unsigned int target_location; ```
It's probably out of scope for this series, but it seems like some renaming is in order here; we probably don't want the meaning of "register_index" to change like that.
```diff + if (parser->shader_version.major < 4 && parser->shader_version.type == VKD3D_SHADER_TYPE_VERTEX + && (result = remap_output_signature(&parser->shader_desc.output_signature, compile_info)) < 0) + return result; ```
I don't mind too much, but do we need to limit this like that? I can't say it would necessarily be terribly useful, but if someone wanted to remap e.g. sm4 vertex shader outputs, should we try to stop them?
```diff + else + { + varyings[count].output_signature_index = unused_signature_idx++; + } ```
It's probably inconsequential, but we could just set this to "output_signature->element_count", right? -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/280#note_40865
participants (3)
-
Henri Verbeet (＠hverbeet) -
Zebediah Figura
-
Zebediah Figura (＠zfigura)