Re: [PATCH 1/2] d3dx9_36/tests: Add D3DXSaveSurfaceToFileInMemory D3DXIFF_DDS tests

[Matteo Bruni]

2015-12-03 7:55 GMT+01:00 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>:
> Also updates the structure dds_header to be the same as d3dx9_36/surface.c.
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> ---
>  dlls/d3dx9_36/tests/surface.c | 38 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c
> index 1697a03..1c85ca3 100644
> --- a/dlls/d3dx9_36/tests/surface.c
> +++ b/dlls/d3dx9_36/tests/surface.c
> @@ -211,7 +211,9 @@ struct dds_header
>      struct dds_pixel_format pixel_format;
>      DWORD caps;
>      DWORD caps2;
> -    DWORD reserved2[3];
> +    DWORD caps3;
> +    DWORD caps4;
> +    DWORD reserved2;
>  };
>
> /* fills dds_header with reasonable default values */
> @@ -1231,6 +1233,7 @@ static void test_D3DXSaveSurfaceToFileInMemory(IDirect3DDevice9 *device)
>      RECT rect;
>      ID3DXBuffer *buffer;
>      IDirect3DSurface9 *surface;
> +    struct dds_header *header;
>
> hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 4, 4, D3DFMT_A8R8G8B8, D3DPOOL_SCRATCH, &surface, NULL);
>      if (FAILED(hr)) {
> @@ -1248,6 +1251,39 @@ static void test_D3DXSaveSurfaceToFileInMemory(IDirect3DDevice9 *device)
>          ID3DXBuffer_Release(buffer);
>      }
>
> +    SetRect(&rect, 0, 0, 0, 0);
> +    hr = D3DXSaveSurfaceToFileInMemory(&buffer, D3DXIFF_DDS, surface, NULL, &rect);
> +    todo_wine ok(hr == D3D_OK || broken(hr == D3DERR_INVALIDCALL), "D3DXSaveSurfaceToFileInMemory returned %#x, expected %#x\n", hr, D3D_OK);

If the broken() predicate is there for the same reason as the previous
test (i.e. failure with the debug d3d9 runtime) I think it should be
dropped. If that's not the case, where does this fail for you?
Please put a period at the end of the ok() string.
> +    if (SUCCEEDED(hr)) {

'{' on the next line please.
> +        header = ID3DXBuffer_GetBufferPointer(buffer);
> +
> +        ok(header->magic == MAKEFOURCC('D','D','S',' '), "Invalid DDS signature\n");
> +        ok(header->size == 124, "Invalid DDS size %d\n", header->size);
> +        ok(header->height == 0, "Wrong height %d\n", header->height);

We usually do something like "!header->height" in d3d code in similar cases.
> +        ok(header->width == 0, "Wrong width %d\n", header->width);
> +        ok(header->depth == 0, "Wrong depth %d\n", header->depth);
> +        ok(header->miplevels == 0, "Wrong miplevels %d\n", header->miplevels);
> +        ok(header->flags == (DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PIXELFORMAT),
> +           "Wrong flags %x\n", header->flags);

Please use 8 spaces indents for line continuations.
> +        ID3DXBuffer_Release(buffer);
> +    }
> +
> +    hr = D3DXSaveSurfaceToFileInMemory(&buffer, D3DXIFF_DDS, surface, NULL, NULL);
> +    ok(hr == D3D_OK || broken(hr == D3DERR_INVALIDCALL), "D3DXSaveSurfaceToFileInMemory returned %#x, expected %#x\n", hr, D3D_OK);
> +    if (SUCCEEDED(hr)) {

You can avoid the 'if' here if you don't need the broken() above.
> +        header = ID3DXBuffer_GetBufferPointer(buffer);
> +
> +        ok(header->magic == MAKEFOURCC('D','D','S',' '), "Invalid DDS signature\n");
> +        todo_wine ok(header->size == 124, "Invalid DDS size %d\n", header->size);
> +        ok(header->height == 4, "Wrong height %d\n", header->height);
> +        ok(header->width == 4, "Wrong width %d\n", header->width);
> +        todo_wine ok(header->depth == 0, "Wrong depth %d\n", header->depth);
> +        todo_wine ok(header->miplevels == 0, "Wrong miplevels %d\n", header->miplevels);
> +        todo_wine ok(header->flags == (DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PIXELFORMAT),
> +                     "Wrong flags %x\n", header->flags);
> +        ID3DXBuffer_Release(buffer);
> +    }
> +
>      IDirect3DSurface9_Release(surface);
>  }
>
> --
> 1.9.1