[PATCH 3/4] d3dx10/tests: Test frame data for created texture.

[Ziqing Hui]

Signed-off-by: Ziqing Hui 
---
 dlls/d3dx10_43/tests/d3dx10.c | 208 ++++++++++++++++++++++++++++++++++
 1 file changed, 208 insertions(+)

[Matteo Bruni]

On Mon, Jun 7, 2021 at 8:58 AM Ziqing Hui  wrote:
> Signed-off-by: Ziqing Hui
>
> dlls/d3dx10_43/tests/d3dx10.c | 208 ++++++++++++++++++++++++++++++++++
>  1 file changed, 208 insertions(+)
> +static unsigned int get_bpp_from_format(DXGI_FORMAT format)
> +{
> - switch (format)
> - {
> -
>    case DXGI_FORMAT_R32G32B32A32_TYPELESS:

If you wanted to make the patch smaller you could get rid of all the
cases you don't need for the current tests. I don't mind though.
> +static ID3D10Texture2D *copy_texture(ID3D10Device *device, ID3D10Texture2D *texture)

This could borrow from the other d3d tests (e.g. d3d10core, d3d11) and
become get_texture_readback() or something along those lines.
> +{
> - ID3D10Texture2D *texture_copy;
> - D3D10_TEXTURE2D_DESC desc;
> - HRESULT hr;
> -
> - ID3D10Texture2D_GetDesc(texture, &desc);
> - desc.Usage = D3D10_USAGE_STAGING;
> - desc.BindFlags = 0;
> - desc.CPUAccessFlags = D3D10_CPU_ACCESS_READ | D3D10_CPU_ACCESS_WRITE;

You only want D3D10_CPU_ACCESS_READ here.
> +static void check_frame_data(D3D10_MAPPED_TEXTURE2D *map, unsigned int stride, unsigned int height,
> -
>    unsigned int subresource, unsigned int i, unsigned int line)

I don't think this needs to be a separate function, unless you plan to
add a second caller later on.
> -
>    ok_(__FILE__, line)(line_match, "Test %u: Subresource %u: Data mismatch for line %u.\n", i, subresource, j);

This seems to be a great occasion to make use of the new
winetest_push_context / winetest_pop_context functions. It probably
makes sense to add a patch earlier in the sequence to replace all the
"Test %u: " prefixes in check_resource_info() with a test context.
> +static void check_resource_data(ID3D10Device *device, ID3D10Resource *resource, unsigned int i, unsigned int line)

If you use the test context functions you don't need to pass the test
index anymore, provided that you instead pass the struct test_image
pointer to the function. Which to me looks nicer anyway.
> - stride = (width * get_bpp_from_format(desc.Format) + 7) >> 3;

Nitpick, I'd rather have a "/ 8" here in place of the shift, it's the
same thing of course but it makes it a tiny bit clearer that this is a
bit -> byte count conversion.
> - if (is_block_compressed(desc.Format))
> - {
> -
>    stride <<= 2;
> -
>    height = (height + 3) >> 2;
> - }

Similarly here, except this also assumes that the block side is 4
(which is a fair assumption in the tests, but no reason to make it
more obscure than necessary).