[2/2] ddraw: add dsurface dimension tests, try 11

[Henri Verbeet]

On 30 June 2014 17:42, Patrick Rudolph <siro at das-labor.org> wrote:
> +    const static struct
"static const"

> +                {16}, {0x0000F800}, {0x000007E0}, {0x0000001F}, {0x00000000}
Please use lowercase hex constants.

> +    static struct
> +    {
> +        DWORD dwFlags;
> +        DWORD dwCaps;
> +        DWORD dwWidth;
> +        DWORD dwHeight;
> +        HRESULT hr;
> +    }
Please drop the dw prefixes and capitals from the field names here.

> +        /* 6: Test maximum surface height */
> +        {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN, 1, 0x10000, DDERR_INVALIDPARAMS},
> +        /* 7: Test maximum surface width */
> +        {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN, 0x10000, 1, DDERR_INVALIDPARAMS},
It would perhaps also be useful to verify that e.g. a 1 by 0xffff
surface actually is allowed.

> +        /* 8: Test OUTOFVIDEOMEMORY */
> +        {DDSD_CAPS | DDSD_WIDTH | DDSD_HEIGHT, DDSCAPS_OFFSCREENPLAIN | DDSCAPS_VIDEOMEMORY,
> +            0xffff, 0xffff, DDERR_OUTOFVIDEOMEMORY},
Double indentation here, please.

> +    ddcaps.dwSize = sizeof(DDCAPS);
ddcaps.dwSize = sizeof(ddcaps);

> +    /* expect an error if the hardware doesn't support videomemory */
> +    if (!(ddcaps.ddsCaps.dwCaps & DDSCAPS_VIDEOMEMORY))
> +        tests[8].hr = DDERR_NODIRECTDRAWHW;
You can just handle that when testing the expected result. (And then
"tests" can just be const.) E.g.:

hr = IDirectDraw_CreateSurface(...);
if (tests[i].caps & DDSCAPS_VIDEOMEMORY && !(ddcaps.ddsCaps.dwCaps &
DDSCAPS_VIDEOMEMORY))
    ok(hr == DDERR_NODIRECTDRAWHW, ...);
else
    ok(hr == tests[i].hr, ...);

> +            surfacedesc.dwFlags = tests[i].dwFlags | DDSD_PIXELFORMAT;
It seems arbitrary to put DDSD_CAPS in the table, but not
DDSD_PIXELFORMAT. IMO either both should be added here, or both should
be in the table.

> +            hr = IDirectDraw_CreateSurface(ddraw, &surfacedesc, &surface, NULL);
> +            ok(hr == tests[i].hr, "Pixelformat %s, test %d returned %08x\n", formats[j].name, i, hr);
> +
> +            if (surface)
> +            {
> +                IDirectDrawSurface_Release(surface);
> +                surface = NULL;
> +            }
hr = IDirectDraw_CreateSurface(...);
...
if (FAILED(hr))
    continue;
IDirectDrawSurface_Release(surface);

> +    ok(SUCCEEDED(hr), "CreateSurface returned %08x\n", hr);
> +    if (surface)
The if is redundant here, since you're claiming that creating the
surface should always succeed a line earlier.

> +        surface = NULL;
This doesn't really do anything, since "surface" is never used afterwards.