[PATCH 4/4] d2d1/tests: Run tests using ID3D11Device too.

[Henri Verbeet]

On Thu, 7 Jan 2021 at 20:45, Rémi Bernon <rbernon at codeweavers.com> wrote:
> @@ -36,13 +37,17 @@ static BOOL use_mt = TRUE;
>  static struct test_entry
>  {
>      void (*test)(void);
> +    void (*d3d1x_test)(BOOL d3d11);
> +    BOOL d3d11;
>  } *mt_tests;
>  size_t mt_tests_size, mt_test_count;
>
The function pointers above should probably be in a union. For
comparison, see struct test_entry and the functions operating on it in
the d3d11 tests.

>  struct resource_readback
>  {
> -    ID3D10Resource *resource;
> -    D3D10_MAPPED_TEXTURE2D map_desc;
> +    ID3D10Resource *d3d10_resource;
> +    ID3D11Resource *d3d11_resource;
> +    D3D10_MAPPED_TEXTURE2D d3d10_map_desc;
> +    D3D11_MAPPED_SUBRESOURCE d3d11_map_desc;
>      unsigned int pitch, width, height;
>      void *data;
>  };
As in patch 3/4 of this series, I think the "d3d10_map_desc" and
"d3d11_map_desc" fields above should be superfluous. The
"d3d10_resource" and "d3d11_resource" fields should probably either be
in a union, or simply stored as an IUnknown pointer.

> @@ -104,7 +109,29 @@ static void queue_test(void (*test)(void))
>          mt_tests_size = max(16, mt_tests_size * 2);
>          mt_tests = heap_realloc(mt_tests, mt_tests_size * sizeof(*mt_tests));
>      }
> -    mt_tests[mt_test_count++].test = test;
> +    mt_tests[mt_test_count].test = test;
> +    mt_tests[mt_test_count++].d3d1x_test = NULL;
> +}
> +
> +static void queue_d3d1x_test(void (*test)(BOOL d3d11))
> +{
> +    if (mt_test_count >= mt_tests_size)
> +    {
> +        mt_tests_size = max(16, mt_tests_size * 2);
> +        mt_tests = heap_realloc(mt_tests, mt_tests_size * sizeof(*mt_tests));
> +    }
> +    mt_tests[mt_test_count].test = NULL;
> +    mt_tests[mt_test_count].d3d1x_test = test;
> +    mt_tests[mt_test_count++].d3d11 = FALSE;
> +
> +    if (mt_test_count >= mt_tests_size)
> +    {
> +        mt_tests_size = max(16, mt_tests_size * 2);
> +        mt_tests = heap_realloc(mt_tests, mt_tests_size * sizeof(*mt_tests));
> +    }
> +    mt_tests[mt_test_count].test = NULL;
> +    mt_tests[mt_test_count].d3d1x_test = test;
> +    mt_tests[mt_test_count++].d3d11 = TRUE;
>  }
>
>  static DWORD WINAPI thread_func(void *ctx)
> @@ -115,7 +142,10 @@ static DWORD WINAPI thread_func(void *ctx)
>      {
>          j = *i;
>          if (InterlockedCompareExchange(i, j + 1, j) == j)
> -            mt_tests[j].test();
> +        {
> +            if (mt_tests[j].test) mt_tests[j].test();
> +            else if (mt_tests[j].d3d1x_test) mt_tests[j].d3d1x_test(mt_tests[j].d3d11);
> +        }
>      }
>
>      return 0;
> @@ -132,7 +162,8 @@ static void run_queued_tests(void)
>      {
>          for (i = 0; i < mt_test_count; ++i)
>          {
> -            mt_tests[i].test();
> +            if (mt_tests[i].test) mt_tests[i].test();
> +            else if (mt_tests[i].d3d1x_test) mt_tests[i].d3d1x_test(mt_tests[i].d3d11);
>          }
>
>          return;
The code above is not the worst thing in the world, but I think the
queue_test_entry()/run_mt_test() solution in the d3d11 tests is nicer.

Also, if the idea is that we'll want to run most tests on both d3d10
and d3d11, queue_d3d1x_test() should probably be the default (i.e.,
queue_test()), and we should introduce e.g. queue_d3d10_test() for
tests that should only be run with d3d10.

> -static void release_resource_readback(struct resource_readback *rb)
> +static void get_surface_readback(IDXGISurface *surface, struct resource_readback *rb, BOOL d3d11)
>  {
> -    ID3D10Texture2D_Unmap((ID3D10Texture2D *)rb->resource, 0);
> -    ID3D10Resource_Release(rb->resource);
> +    if (d3d11) get_d3d11_surface_readback(surface, rb);
> +    else get_d3d10_surface_readback(surface, rb);
> +}
> +
Passing the "d3d11" variable around everywhere is a little ugly. I
think we can simply QueryInterface() for the ID3D11Resource and
ID3D10Resource interfaces here and use the one that succeeds, without
necessarily caring about how the device was created. However, if we
were to pass something around here, it should probably be a
d2d1_test_context structure.