[PATCH vkd3d 1/2] tests: Fix shader tests crashes on WARP driver.

[Henri Verbeet]

On Thu, 16 Dec 2021 at 21:12, Nikolay Sivov <nsivov at codeweavers.com> wrote:
> On 12/15/21 20:57, Henri Verbeet wrote:
> > On Sat, 11 Dec 2021 at 20:44, Nikolay Sivov <nsivov at codeweavers.com> wrote:
> >> @@ -419,9 +418,11 @@ static void parse_test_directive(struct shader_context *context, const char *lin
> >>           hr = create_root_signature(context->c.device, &root_signature_desc, &context->c.root_signature);
> >>           ok(hr == S_OK, "Failed to create root signature, hr %#x.\n", hr);
> >>
> >> -        pso = create_pipeline_state(context->c.device, context->c.root_signature,
> >> +        if (context->c.pso)
> >> +            ID3D12PipelineState_Release(context->c.pso);
> >> +        context->c.pso = create_pipeline_state(context->c.device, context->c.root_signature,
> >>                   context->c.render_target_desc.Format, NULL, &ps, NULL);
> >> -        if (!pso)
> >> +        if (!context->c.pso)
> >>               return;
> >>
> > I suppose that's an improvement in the sense that it should work in
> > practice for most of our existing tests, but it doesn't strike me as
> > entirely correct either. In particular, the restriction is that we
> > can't destroy the state object until all command lists referencing it
> > have finished executing on the GPU. That mostly works out in practice
> > because the typical pattern is to do a "draw", followed by a "probe",
> > in which case the "probe" will wait for the command list to finish
> > executing, but I could certainly imagine writing tests where we do
> > multiple draws before probing.
> Would it be correct in terms of test results to flush current command
> list, on repeated "draw"s? I see there is ::Reset() that according to
> docs will reinitialize recording, if last Close() succeeded. Test could
> use "pso != NULL" as a marker that "probe" wasn't used since last draw,
> and close-exec-wait-reset right before starting new recording.
>
> Another brute force option is to simply allow for arbitrary number of
> state objects in test context, accumulated indefinitely until next "probe".

Right, either of those approaches should work. The latter would likely
be a bit more efficient, although I don't think we're overly concerned
about the performance of the tests (yet). In case of the first
approach, note how get_texture_readback_with_command_list() (used by
the "probe" implementation) does this:

  - Close the existing command list with ID3D12GraphicsCommandList_Close().
  - Execute the command list with exec_command_list().
  - Wait for the queue to become idle with wait_queue_idle().

We then call reset_command_list() to restart recording.