[PATCH v2 2/3] d3d11: Don't grab wined3d lock for thread safe procedures.

[Henri Verbeet]

On Tue, 12 Oct 2021 at 14:05, Jan Sikorski <jsikorski at codeweavers.com> wrote:
> ---
>  dlls/d3d11/async.c       |  10 ---
>  dlls/d3d11/buffer.c      |   8 --
>  dlls/d3d11/device.c      | 164 ---------------------------------------
>  dlls/d3d11/inputlayout.c |   6 --
>  dlls/d3d11/shader.c      |  33 --------
>  dlls/d3d11/state.c       |  22 ------
>  dlls/d3d11/view.c        |  24 ------
>  7 files changed, 267 deletions(-)
>
It may be helpful to split this. E.g., _incref() functions except
wined3d_texture_incref(), _decref() functions, device context setters,
etc. Also, note that some of these changes are applicable to the other
Direct3D versions as well, and we might as well do those while we're
at it.

> @@ -1128,11 +1088,9 @@ static void STDMETHODCALLTYPE d3d11_device_context_OMSetRenderTargets(ID3D11Devi
>
>      dsv = unsafe_impl_from_ID3D11DepthStencilView(depth_stencil_view);
>
> -    wined3d_mutex_lock();
>      wined3d_device_context_set_rendertarget_views(context->wined3d_context, 0,
>              ARRAY_SIZE(wined3d_rtvs), wined3d_rtvs, FALSE);
>      wined3d_device_context_set_depth_stencil_view(context->wined3d_context, dsv ? dsv->wined3d_view : NULL);
> -    wined3d_mutex_unlock();
>  }
>
Is that safe? I.e., is OMSetRenderTargets() supposed to be atomic in
the sense that if two of them were to race the resulting state may be
as if either one of them had been called, or is the result allowed to
be inconsistent, with e.g. the RTVs from one call being applied and
the DSVs from the other?

> @@ -1176,10 +1134,8 @@ static void STDMETHODCALLTYPE d3d11_device_context_OMSetRenderTargetsAndUnordere
>              wined3d_initial_counts[uav_start_idx + i] = initial_counts ? initial_counts[i] : ~0u;
>          }
>
> -        wined3d_mutex_lock();
>          wined3d_device_context_set_unordered_access_views(context->wined3d_context, WINED3D_PIPELINE_GRAPHICS,
>                  0, ARRAY_SIZE(wined3d_views), wined3d_views, wined3d_initial_counts);
> -        wined3d_mutex_unlock();
>      }
>  }
>
That applies here as well, although in this case that's an existing issue.

> @@ -2782,8 +2683,6 @@ static HRESULT STDMETHODCALLTYPE d3d11_device_context_FinishCommandList(ID3D11De
>      if (!(object = heap_alloc_zero(sizeof(*object))))
>          return E_OUTOFMEMORY;
>
> -    wined3d_mutex_lock();
> -
>      if (FAILED(hr = wined3d_deferred_context_record_command_list(context->wined3d_context,
>              !!restore, &object->wined3d_list)))
>      {
> @@ -2792,8 +2691,6 @@ static HRESULT STDMETHODCALLTYPE d3d11_device_context_FinishCommandList(ID3D11De
>          return hr;
>      }
>
> -    wined3d_mutex_unlock();
> -
Not dropping the lock on the error path is an existing issue in this
code, actually...

> @@ -4974,12 +4832,10 @@ static void STDMETHODCALLTYPE d3d10_device_OMSetRenderTargets(ID3D10Device1 *ifa
>
>      dsv = unsafe_impl_from_ID3D10DepthStencilView(depth_stencil_view);
>
> -    wined3d_mutex_lock();
>      wined3d_device_context_set_rendertarget_views(device->immediate_context.wined3d_context, 0,
>              ARRAY_SIZE(wined3d_rtvs), wined3d_rtvs, FALSE);
>      wined3d_device_context_set_depth_stencil_view(device->immediate_context.wined3d_context,
>              dsv ? dsv->wined3d_view : NULL);
> -    wined3d_mutex_unlock();
>  }
>
Like d3d11_device_context_OMSetRenderTargets() above.

> @@ -88,9 +86,7 @@ static ULONG STDMETHODCALLTYPE d3d11_vertex_shader_Release(ID3D11VertexShader *i
>      {
>          ID3D11Device2 *device = shader->device;
>
> -        wined3d_mutex_lock();
>          wined3d_shader_decref(shader->wined3d_shader);
> -        wined3d_mutex_unlock();
>          /* Release the device last, it may cause the wined3d device to be
>           * destroyed. */
>          ID3D11Device2_Release(device);

Patch 1/3 didn't make wined3d_shader_decref() thread-safe yet.