Re: [PATCH v3 6/6] d3d9/tests: Add Fetch4 tests

[Henri Verbeet]

On Sat, 24 Nov 2018 at 23:51, Daniel Ansorregui  wrote:
> - Implemented for texld/texldp/texldd/texldb/texldl
> - In all cases tested on Windows10 + Intel
> Fetch4 enabled always produced same result (like texld)
> Signed-off-by: Daniel Ansorregui
>
> dlls/d3d9/tests/visual.c | 313 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 313 insertions(+)

Since the CheckDeviceFormat() check will prevent this from doing
anything on implementations that don't support DF24, you may as well
send this test as the first patch in the series.
> - struct
> - {
> -
>    float x, y, z;
> -
>    float tu, tv;
> - }

Not a big deal, but we tend to do these as "struct vec3 position" and
"struct vec2 texcoord".
> - struct struct_expected_color
> - {
> -
>    UINT x, y;
> -
>    D3DCOLOR color;
> - };

Why the "struct_" prefix on the structure name? In fact, you don't
really need a structure name at all.
> - static const DWORD texture_data[4] = {0x10111213,

Again minor, but no need for the "4".
> - IDirect3DTexture9 *texture_L8, *texture_A8, *texture_A8R8G8B8;

You'll probably want an array of texture formats you want to test, and
loop over it.
> - /* Create our texture for FETCH4 */
> - hr = IDirect3DDevice9_CreateTexture(device, 4, 4, 1, 0, D3DFMT_L8, D3DPOOL_MANAGED, &texture_L8, NULL);
> - ok(hr == D3D_OK, "Failed to create texture, hr %#x.\n", hr);
> - memset(&lr, 0, sizeof(lr));

The memset() is redundant.
> - hr = IDirect3DTexture9_LockRect(texture_L8, 0, &lr, NULL, 0);
> - ok(hr == D3D_OK, "Failed to lock texture, hr %#x.\n", hr);
> - memcpy(lr.pBits, texture_data, sizeof(texture_data));
> - hr = IDirect3DTexture9_UnlockRect(texture_L8, 0);
> - ok(hr == D3D_OK, "Failed to unlock texture, hr %#x.\n", hr);

This probably works in practice, but you should be using the pitch.
> - /* Render with fetch4 and test if we obtain proper results */
> - for (i=0; i<5; i++)

Formatting.
> - {
> -
>    if (i==0)
> -
>        hr = IDirect3DDevice9_SetPixelShader(device, ps_texld);
> -
>    else if(i==1)
> -
>        hr = IDirect3DDevice9_SetPixelShader(device, ps_texldp);
> -
>    else if(i==2)
> -
>        hr = IDirect3DDevice9_SetPixelShader(device, ps_texldd);
> -
>    else if(i==3)
> -
>        hr = IDirect3DDevice9_SetPixelShader(device, ps_texldb);
> -
>    else
> -
>        hr = IDirect3DDevice9_SetPixelShader(device, ps_texldl);
> -
>    ok(SUCCEEDED(hr), "SetPixelShader failed, hr %#x.\n", hr);

If you create an array like this:

static const DWORD *ps_code[] =
    {
        ps_code_texld,
        ps_code_texldp,
        ...
    };

You can loop over it, and avoid the fairly awkward construction above.