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

[Matteo Bruni]

On Mon, Jun 7, 2021 at 8:58 AM Ziqing Hui <zhui at codeweavers.com> wrote:
>
>
> Signed-off-by: Ziqing Hui <zhui at codeweavers.com>
> ---
>  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).