Re: [PATCH 0/6] MR6360: d3dx9: Beginning of pixel format handling changes.

[Matteo Bruni (＠Mystral)]

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
> -
>    check_pixel_float4(&lockrect, 0, 1, -1.0f,      -1.0f,      1.0f, 0.666667f, TRUE);
> -
>    check_pixel_float4(&lockrect, 1, 1, -0.503937f, -0.007874f, 1.0f, 1.0f,      TRUE);
> -
>    hr = IDirect3DSurface9_UnlockRect(surf);
> -
>    ok(hr == D3D_OK, "Failed to unlock surface, hr %#lx.\n", hr);
> -
>    if (SUCCEEDED(hr))
> -
>    {
> -
>        /* The luma value goes into the alpha channel. */
> -
>        hr = IDirect3DSurface9_LockRect(surf, &lockrect, NULL, D3DLOCK_READONLY);
> -
>        ok(hr == D3D_OK, "Failed to lock surface, hr %#lx.\n", hr);
> -
>        check_pixel_float4(&lockrect, 0, 0,  0.0f,       0.377953f, 1.0f, 0.0f,      TRUE);
> -
>        check_pixel_float4(&lockrect, 1, 0,  0.503937f,  1.0f,      1.0f, 0.333333f, TRUE);
> -
>        check_pixel_float4(&lockrect, 0, 1, -1.0f,      -1.0f,      1.0f, 0.666667f, TRUE);
> -
>        check_pixel_float4(&lockrect, 1, 1, -0.503937f, -0.007874f, 1.0f, 1.0f,      TRUE);
> -
>        hr = IDirect3DSurface9_UnlockRect(surf);
> -
>        ok(hr == D3D_OK, "Failed to unlock surface, hr %#lx.\n", hr);
> -
>    }

Is the `if (SUCCEEDED(hr)){}` to avoid spurious successes from the last working `D3DXLoadSurfaceFromMemory()` call? Instead I think I'd either update the last argument of those `check_pixel_float4()` calls as necessary (even if they only pass by chance) or put the `if (SUCCEEDED(hr)){}` in the patch originally introducing the test.