On 30 June 2014 17:42, Patrick Rudolph <siro(a)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.