Re: [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  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.