[1/3] ddraw/tests: Add more tests for IDirectDraw7::EnumSurfaces.

Tue Aug 25 15:22:59 CDT 2015

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Am 2015-08-25 um 11:15 schrieb Sebastian Lackner:
>  dlls/ddraw/ddraw.c     |    3 
>  dlls/ddraw/tests/d3d.c |  177 ++++++++++++++++++++++++++++++++++++++++++++++---
Please add the test to ddraw{1-7}.c, testing each interface version.
This is the preferred place for new tests. Feel free to remove the
lone EnumSurfaces test from CreateDirect3D. Arguably it's poorly
placed there because CreateDirect3D is more of a helper function than
a real test function.

> +typedef struct
> +{
> +    int found;
> +    int surfaces;
> +} EnumSurfaceTest;
You shouldn't need the typedef, and the counters can be unsigned. In
ddraw*.c enum_surfaces_test matches the naming convention.

> +    memset(&ddsd2, 0, sizeof(ddsd2));
> +    ddsd2.dwSize = sizeof(ddsd2);
> +    ddsd2.dwFlags = DDSD_WIDTH | DDSD_HEIGHT;
> +    ddsd2.dwWidth = 256;
> +    ddsd2.dwHeight = 256;
The test will be more meaningful if there are more than 2 surfaces
that can be enumerated, and if they have different sizes.

> +    count.found = count.surfaces = 0;
> +    rc = IDirectDraw7_EnumSurfaces(lpDD, DDENUMSURFACES_CANBECREATED | DDENUMSURFACES_MATCH,
> +            &ddsd, &count, SurfaceCounter);
> +    ok(rc == DD_OK, "Expected DD_OK, got %x\n", rc);
> +    todo_wine ok(count.found == 1, "Has %d surface descriptions, expected 1\n", count.found);
> +    ok(count.surfaces == 0, "Has %d surfaces, expected 0\n", count.surfaces);
What does the enumerated surface description look like? Is it just
your input description, or does ddraw set additional flags like
DDSCAPS_OFFSCREENPLAIN? If it looks like it is your surface desc you
can try to set some nonsense fields before the call, e.g. set
lpSurface = (void *)0xdeadbeef without setting DDSD_LPSURFACE. If
ddraw adds extra flags please check for them. If you call
CreateSurface inside the callback with the enumerated desc, does it
successfully create a surface?

Another thing to test is DDSCAPS_PRIMARYSURFACE, both without an
existing primary and without one that's already created. This should
show if calling CreateSurface to try to create a test surface is the
right solution.

There are a few more things you could check to satisfy my curiosity,
but I don't think we necessarily need this: In your implementation
calling EnumSurfaces with DDENUMSURFACES_ALL |
DDENUMSURFACES_DOESEXIST from the enumeration callback will find the
temporary surface and you could e.g. AddRef it and keep it. I doubt
this is the case with Microsoft's implementation, otherwise they could
give you their test surface in the callback.