Re: [PATCH 1/6] wined3d: Implement constant buffer offsetting for Vulkan.

[Henri Verbeet]

On Tue, 20 Jul 2021 at 05:23, Zebediah Figura  wrote:
> -const VkDescriptorBufferInfo *wined3d_buffer_vk_get_buffer_info(struct wined3d_buffer_vk *buffer_vk)
> +void wined3d_buffer_vk_get_buffer_info(struct wined3d_buffer_vk *buffer_vk, VkDescriptorBufferInfo *buffer_info)
>  {
> - if (buffer_vk->bo_user.valid)
> -
>    return &buffer_vk->buffer_info;
> -
> - buffer_vk->buffer_info.buffer = buffer_vk->bo.vk_buffer;
> - buffer_vk->buffer_info.offset = buffer_vk->bo.buffer_offset;
> - buffer_vk->buffer_info.range = buffer_vk->b.resource.size;
> - buffer_vk->bo_user.valid = true;
> -
> - return &buffer_vk->buffer_info;
> - buffer_info->buffer = buffer_vk->bo.vk_buffer;
> - buffer_info->offset = buffer_vk->bo.buffer_offset;
> - buffer_info->range = buffer_vk->b.resource.size;
>
> }

That seems like somewhat of a gratuitous change. There may be an
argument for it, but it doesn't have a lot to do with constant buffer
offsetting.
> @@ -2713,7 +2713,13 @@ static bool wined3d_context_vk_update_descriptors(struct wined3d_context_vk *con
>          switch (binding->shader_descriptor_type)
>          {
>              case WINED3D_SHADER_DESCRIPTOR_TYPE_CBV:
> -
>            if (!(buffer = state->cb[binding->shader_type][binding->resource_idx].buffer))
> -
>        {
> -
>            enum wined3d_shader_type shader_type = binding->shader_type;
> -
>            size_t resource_idx = binding->resource_idx;
> -
>            const struct wined3d_constant_buffer_state *cb_state = &state->cb[shader_type][resource_idx];
> -
>            VkDescriptorBufferInfo *buffer_info = &buffers[shader_type][resource_idx];
> -
> -
>            if (!(buffer = cb_state->buffer))

We do this in a couple of other places, but I'm generally not a fan of
creating a new scope for case labels like that. As a rule of thumb,
I'd say that in the cases where it's really worth creating a new
scope, introducing a separate function tends to be the better
solution. This specific case may be in that category as well;
wined3d_context_vk_update_descriptors() is already a moderately sized
function before this change, and introducing separate functions for
the different descriptor types may very well end up improving
readability.