[PATCH v3 2/3] wined3d: Don't pass unrelated data to shader_spirv_compile().

[Henri Verbeet]

On Thu, 23 Sept 2021 at 10:43, Jan Sikorski <jsikorski at codeweavers.com> wrote:
> +struct shader_spirv_interface
> +{
> +    const struct vkd3d_shader_resource_binding *bindings;
> +    SIZE_T binding_count;
> +
> +    const struct vkd3d_shader_uav_counter_binding *uav_counters;
> +    SIZE_T uav_counter_count;
> +};
[...]
>  static VkShaderModule shader_spirv_compile(struct wined3d_context_vk *context_vk,
> -        struct wined3d_shader *shader, const struct shader_spirv_compile_arguments *args,
> -        const struct shader_spirv_resource_bindings *bindings, const struct wined3d_stream_output_desc *so_desc)
> +        struct wined3d_shader_desc *shader_desc, enum wined3d_shader_type shader_type,
> +        const struct shader_spirv_compile_arguments *args, const struct shader_spirv_interface *shader_interface,
> +        const struct wined3d_stream_output_desc *so_desc)
>  {
>      struct wined3d_shader_spirv_compile_args compile_args;
>      struct wined3d_shader_spirv_shader_interface iface;
> +    VkShaderModuleCreateInfo shader_create_info;
>      struct vkd3d_shader_compile_info info;
>      const struct wined3d_vk_info *vk_info;
> -    enum wined3d_shader_type shader_type;
> -    VkShaderModuleCreateInfo shader_desc;
>      struct wined3d_device_vk *device_vk;
>      struct vkd3d_shader_code spirv;
>      VkShaderModule module;
> @@ -321,15 +329,14 @@ static VkShaderModule shader_spirv_compile(struct wined3d_context_vk *context_vk
>      VkResult vr;
>      int ret;
>
> -    shader_spirv_init_shader_interface_vk(&iface, shader, bindings, so_desc);
> -    shader_type = shader->reg_maps.shader_version.type;
> +    shader_spirv_init_shader_interface_vk(&iface, shader_interface, so_desc);
>      shader_spirv_init_compile_args(&compile_args, &iface.vkd3d_interface,
>              VKD3D_SHADER_SPIRV_ENVIRONMENT_VULKAN_1_0, shader_type, args);
>
[...]
> @@ -413,7 +422,17 @@ static struct shader_spirv_graphics_program_variant_vk *shader_spirv_find_graphi
>      variant_vk = &program_vk->variants[variant_count];
>      variant_vk->compile_args = args;
>      variant_vk->binding_base = binding_base;
> -    if (!(variant_vk->vk_module = shader_spirv_compile(context_vk, shader, &args, bindings, so_desc)))
> +
> +    shader_interface.bindings = bindings->bindings;
> +    shader_interface.binding_count = bindings->binding_count;
> +    shader_interface.uav_counters = bindings->uav_counters;
> +    shader_interface.uav_counter_count = bindings->uav_counter_count;
> +
> +    shader_desc.byte_code = shader->byte_code;
> +    shader_desc.byte_code_size = shader->byte_code_size;
> +
> +    if (!(variant_vk->vk_module = shader_spirv_compile(context_vk, &shader_desc, shader_type, &args,
> +            &shader_interface, so_desc)))
>          return NULL;

It's not explicitly stated in the patch description, but I guess the
idea here is to allow calling shader_spirv_compile() without passing
it a wined3d_shader object. The shader_spirv_interface structure feels
a little superfluous; it largely corresponds to part of the
shader_spirv_resource_bindings structure that's currently passed in,
and all it's fields are set to NULL for the call introduced in patch
3/3. We could arguably simply pass NULL for the current "bindings"
parameter in patch 3/3, and then handle that NULL in
shader_spirv_init_shader_interface_vk(). Alternatively, we could pass
a vkd3d_shader_interface_info structure to shader_spirv_compile(), in
which case calling shader_spirv_init_shader_interface_vk() would
become the responsibility of the callers of shader_spirv_compile(),
and we could still pass NULL in patch 3/3.

Patch 3/3 makes the "shader_desc" parameter const, but that seems more
appropriate in this patch.