[PATCH 0/33] MR294: vkd3d: Store command list commands in a buffer until executed.

[Conor McCarthy (＠cmccarthy)]

It's no problem to send fewer of these per MR. I have included the complete set because all but the last introduce no functional changes, and upstreaming a smaller set would leave the changes in a half-done state with unnecessary buffering.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d/command.c:
>  return impl_from_ID3D12CommandAllocator(iface);
>
> }
>
> +struct draw_instanced_args
> +{
> - unsigned int vertex_count_per_instance;

If you add a `struct d3d12_command_header header` field at the beginning of the struct representing each call (and also in `union d3d12_command_args`) you probably have less bookkeeping to do in `d3d12_command_heap_reserve()`, and maybe it's also clearer what you want to do. You don't even have to care that the size of the header is a multiple of the stride, I think.

[Giovanni Mascellani (＠giomasce)]

Giovanni Mascellani (@giomasce) commented about libs/vkd3d/vkd3d_private.h:
>  VKD3D_PIPELINE_BIND_POINT_COUNT = 0x2,
>
> };
>
> +struct d3d12_command_heap
> +{
> - size_t *commands;

Aligning the commands in the array to pointers makes totally sense (in C11 it would make even more sense to align to `max_align_t`, just to be sure), but I would still define the array as `void *`. Since you're going to type pun it a lot anyway, using `size_t *` just feels misleading towards the reader to me.

[Giovanni Mascellani (＠giomasce)]

I still have to work my way through the last patch, so I'm sending a couple of preliminary comments for the moment.

Also, after the last commit four tests fail here (on RADV and llvmpipe) with this assertion:
```
shader_runner: ../vkd3d/libs/vkd3d/command.c:434: root_signature_get_descriptor_table: Assertion `p->parameter_type == D3D12_ROOT_PARAMETER_TYPE_DESCRIPTOR_TABLE' failed.
```

The tests are:
* `compute.shader_test`
* `numthreads.shader_test`
* `uav-out-param.shader_test`
* `uav-load.shader_test`

[Henri Verbeet (＠hverbeet)]

Perhaps unsurprisingly, I'm seeing a lot of similarities between this and wined3d_command_list/wined3d_deferred_context. That code has the advantage of having seen a fair amount of production testing, as well as being familiar to existing wined3d contributors. Unless there's a compelling argument to deviate from that setup, could we please try to keep the vkd3d implementation similar to that one?