[PATCH v6 0/34] 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.

--
  v6: vkd3d: Store command list commands in a buffer until executed.
      vkd3d: Store WriteBufferImmediate() arguments in a buffer.
      vkd3d: Store ExecuteIndirect() arguments in a buffer.
      vkd3d: Store SetPredication() arguments in a buffer.
      vkd3d: Store ResolveQueryData() arguments in a buffer.
      vkd3d: Store EndQuery() arguments in a buffer.
      vkd3d: Store BeginQuery() arguments in a buffer.
      vkd3d: Store d3d12_command_list_clear_uav() arguments in a buffer.
      vkd3d: Store ClearRenderTargetView() arguments in a buffer.
      vkd3d: Store ClearDepthStencilView() arguments in a buffer.
      vkd3d: Store OMSetRenderTargets() arguments in a buffer.
      vkd3d: Store SOSetTargets() arguments in a buffer.
      vkd3d: Store IASetVertexBuffers() arguments in a buffer.
      vkd3d: Store IASetIndexBuffer() arguments in a buffer.
      vkd3d: Store d3d12_command_list_set_root_descriptor() arguments in a buffer.
      vkd3d: Store d3d12_command_list_set_root_cbv() arguments in a buffer.
      vkd3d: Store d3d12_command_list_set_root_constants() arguments in a buffer.
      vkd3d: Store d3d12_command_list_set_descriptor_table() arguments in a buffer.
      vkd3d: Store d3d12_command_list_set_root_signature() arguments in a buffer.
      vkd3d: Add an internal refcount to struct d3d12_root_signature.
      vkd3d: Store ResourceBarrier() arguments in a buffer.
      vkd3d: Store SetPipelineState() arguments in a buffer.
      vkd3d: Store OMSetStencilRef() arguments in a buffer.
      vkd3d: Store OMSetBlendFactor() arguments in a buffer.
      vkd3d: Store RSSetScissorRects() arguments in a buffer.
      vkd3d: Store RSSetViewports() arguments in a buffer.
      vkd3d: Store IASetPrimitiveTopology() arguments in a buffer.
      vkd3d: Store ResolveSubresource() arguments in a buffer.
      vkd3d: Store CopyResource() arguments in a buffer.
      vkd3d: Store CopyTextureRegion() arguments in a buffer.
      vkd3d: Store CopyBufferRegion() arguments in a buffer.
      vkd3d: Store Dispatch() arguments in a buffer.
      vkd3d: Store DrawIndexedInstanced() arguments in a buffer.
      vkd3d: Store DrawInstanced() arguments in a buffer.

This merge request has too many patches to be relayed via email.
Please visit the URL below to see the contents of the merge request.
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/294

[Giovanni Mascellani (＠giomasce)]

On Wed Aug 23 02:21:42 2023 +0000, Conor McCarthy wrote:
> This is not the last patch series, and the `list_state` declaration will
> be moved elsewhere. The `cmd_list` parameter to
> `d3d12_command_list_state_flush()` will disappear too.

As Henri said, it's nicer if changes make sense at any point during the commit history, not just after some other commit is added. So I'd rather move `list_state` inside `d3d12_command_list_state_flush()` and rename the function now, and then undo the change is needed in a later patch.

[Henri Verbeet (＠hverbeet)]

> > I'm not sure it makes a difference, but I'm also not sure loading that pointer from the command buffer instead is necessarily better/faster than loading it from a fixed table.
>
> We would need to load the id from the command buffer, so loading the function pointer directly from there is certainly faster, though the difference may not be much. It depends mostly on whether the lookup table ends up uncached by the time Vulkan has handled the command. Also, I would like to avoid an unnecessary extra step, even if the result is not quite consistent with other code. Are there other reasons for doing this? Making that kind of change to all 34 commits is very time-consuming.

Well, I've found the operation IDs helpful before when debugging command buffer issues. In principle a function name could be derived from the function pointer as well, but it's quite a bit more painful than just printing the ID in a trace.

The broader point here is that "well, performance" on its own isn't a terribly compelling argument to justify design decisions; unless it's something particularly obvious, it really helps to have the performance data to back it up. In this particular case, my expectation would be that the hard to predict indirect call itself would be much more problematic than fetching the pointer. I may very well be wrong about that, but as it is we don't know that.