Re: [PATCH 6/8] d3d11: Implement ID3D11DeviceContext::FinishCommandList().

[Henri Verbeet]

On Wed, 26 May 2021 at 07:23, Zebediah Figura  wrote:
> +struct d3d11_command_list
> +{
> - ID3D11CommandList ID3D11CommandList_iface;
> - LONG refcount;
> -
> - ID3D11Device *device;
> - struct wined3d_command_list *wined3d_list;
> - struct wined3d_private_store private_store;
>
> +};

[...]
> +static void STDMETHODCALLTYPE d3d11_command_list_GetDevice(ID3D11CommandList *iface, ID3D11Device **device)
> +{
> - struct d3d11_command_list *list = impl_from_ID3D11CommandList(iface);
> -
> - TRACE("iface %p, device %p.\n", iface, device);
> -
> - *device = (ID3D11Device *)list->device;
> - ID3D11Device_AddRef(*device);
>
> +}

The cast seems superfluous, but an argument could also be made for
storing an ID3D11Device2 pointer in struct d3d11_command_list, like we
do for most other d3d11 objects.
> @@ -2629,9 +2746,44 @@ static UINT STDMETHODCALLTYPE d3d11_device_context_GetContextFlags(ID3D11DeviceC
>  static HRESULT STDMETHODCALLTYPE d3d11_device_context_FinishCommandList(ID3D11DeviceContext1 *iface,
>          BOOL restore, ID3D11CommandList **command_list)
>  {
> - struct d3d11_device_context *context = impl_from_ID3D11DeviceContext1(iface);
> - struct d3d11_command_list *object;
> - HRESULT hr;
> - TRACE("iface %p, restore %#x, command_list %p.\n", iface, restore, command_list);
> - return DXGI_ERROR_INVALID_CALL;
> - if (context->type == D3D11_DEVICE_CONTEXT_IMMEDIATE)
> - {
> -
>    WARN("Attempt to record command list on an immediate context; returning DXGI_ERROR_INVALID_CALL.\n");
> -
>    return DXGI_ERROR_INVALID_CALL;
> - }
> -
> - if (!(object = heap_alloc_zero(sizeof(*object))))
> -
>    return E_OUTOFMEMORY;
> -
> - object->ID3D11CommandList_iface.lpVtbl = &d3d11_command_list_vtbl;
> - object->refcount = 1;
> - object->device = (ID3D11Device *)&context->device->ID3D11Device2_iface;
> - wined3d_private_store_init(&object->private_store);
> -

... in which case we could get rid of the cast above instead.
> - wined3d_mutex_lock();
> -
> - if (FAILED(hr = wined3d_deferred_context_record_command_list(context->wined3d_context,
> -
>        restore, &object->wined3d_list)))
> - {
> -
>    WARN("Failed to record wined3d command list, hr %#x.\n", hr);
> -
>    heap_free(object);
> -
>    return hr;
> - }
> -

That's missing a wined3d_private_store_cleanup(), strictly speaking.
It wouldn't do anything in this case, but that's an implementation
detail, and subject to future change.
> - ID3D11Device2_AddRef(&context->device->ID3D11Device2_iface);
> -

That should probably be "ID3D11Device_AddRef(object->device);".
> +HRESULT CDECL wined3d_deferred_context_record_command_list(struct wined3d_device_context *context,
> -
>    BOOL restore, struct wined3d_command_list **list)
>
> +{

bool, probably.