Re: [PATCH 5/6] d3d11: Implement constant buffer offsetting.

[Henri Verbeet]

On Thu, 8 Jul 2021 at 01:13, Zebediah Figura  wrote:
> @@ -573,11 +581,31 @@ static void d3d11_device_context_set_constant_buffers(ID3D11DeviceContext1 *ifac
>          return;
>      }
> - if (!offsets != !counts)
> - {
> -
>    WARN("Got offsets pointer %p but counts pointer %p; ignoring call.\n", offsets, counts);
> -
>    return;
> - }
> - for (i = 0; i < buffer_count; ++i)
>    {
>        struct d3d_buffer *buffer = unsafe_impl_from_ID3D11Buffer(buffers[i]);
> -
>    wined3d_buffers[i] = buffer ? buffer->wined3d_buffer : NULL;
> -
>    if (offsets && (offsets[i] & 15))
> -
>    {
> -
>        WARN("Offset %u is not aligned to a multiple of 16.\n", offsets[i]);
> -
>        return;
> -
>    }
> -
> -
>    if (counts && (counts[i] & 15))
> -
>    {
> -
>        WARN("Count %u is not aligned to a multiple of 16.\n", counts[i]);
> -
>        return;
> -
>    }
> -
> -
>    wined3d_buffers[i].buffer = buffer ? buffer->wined3d_buffer : NULL;
> -
>    wined3d_buffers[i].offset = (offsets ? offsets[i] : 0) * 16;
> -
>    wined3d_buffers[i].size = (counts ? counts[i] : 4096) * 16;
>
> }
>
> wined3d_mutex_lock();

There are two different kinds of "16" above, which probably doesn't
make thing clearer. For one of those, we could use "sizeof(struct
wined3d_vec4)". I think the other one may be
"D3D11_COMMONSHADER_CONSTANT_BUFFER_PARTIAL_UPDATE_EXTENTS_BYTE_ALIGNMENT",
although that's perhaps a little unwieldy. We could still define our
own constant though.
> @@ -2713,7 +2713,7 @@ 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]))
> -
>            if (!(buffer = state->cb[binding->shader_type][binding->resource_idx].buffer))
>            {
>                if (!wined3d_shader_descriptor_writes_vk_add_write(writes, vk_descriptor_set,
>                        binding->binding_idx, VK_DESCRIPTOR_TYPE_UNIFORM_BUFFER,

Note that this patch doesn't implement constant buffer offsetting for
the Vulkan backend. (Which is fine, if perhaps a little unfortunate.)
> @@ -1867,6 +1867,12 @@ static void state_init_default(struct wined3d_state *state, const struct wined3d
>  for (i = 0; i < WINED3D_MAX_STREAMS; ++i)
>      state->streams[i].frequency = 1;
> -
> - for (i = 0; i < WINED3D_SHADER_TYPE_COUNT; ++i)
> - {
> -
>    for (j = 0; j < MAX_CONSTANT_BUFFERS; ++j)
> -
>        state->cb[i][j].size = MAX_IMMEDIATE_CONSTANT_BUFFER_SIZE * 16;
> - }
>
> }

Using MAX_IMMEDIATE_CONSTANT_BUFFER_SIZE here is perhaps convenient,
but it's somewhat of an abuse. Immediate constant buffers are constant
buffers defined inline in the shader bytecode, accessed through the
"icb" register.