[PATCH 1/1] ddraw: Return the primary legacy ddraw device last.

[Stefan Dösinger]

There are a few things I am wondering about and where the test can be extended:

The existing test
>     /* Test with valid callback parameter and count the number of primary devices */
>     callbackCount = 0;
>     ret = pDirectDrawEnumerateExA(test_count_callbackExA, &callbackCount, 0);
>     ok(ret == DD_OK, "Expected DD_OK, got %d\n", ret);
>     ok(callbackCount == 1, "Expected 1 primary device, got %d\n", callbackCount);
Combined with your change that suggests that without any flag only the NULL guid is enumerated. If so I think that's worth an explicit check.

Is the NULL guid enumerated when DDENUM_DETACHEDSECONDARYDEVICES and/or DDENUM_NONDISPLAYDEVICES are set?

Regarding the specific changes you make:

> dlls/ddraw/tests/ddrawmodes.c |   35 +++++++++++++++++++++++++++++++++++
I'd prefer that this test be moved to ddraw1.c

> +    static char DriverDescription[] = "DirectDraw HAL";
> +    static char DriverName[] = "display";
Please do not use CamelCase or lpFoo in d3d-related code.

Somewhat related, but that can be a separate patch: d3d3_EnumDevices() mentions that some games modify the strings. In your code (and the current code), such modifications would persist for the runtime of the application. It would be interesting to add a test for this.

> +static BOOL WINAPI test_last_callbackExA(GUID *lpGUID, char *lpDriverDescription,
> +        char *lpDriverName, void *lpContext, HMONITOR hm)
You should be able to combine the callbacks. Just declare a structure and put a counter in it.

> +{
> +    GUID **context_guid = (GUID **)lpContext;
> +    static GUID last_guid;
I guess this works, but it is somewhat ugly. If you use a struct you can put a GUID and a BOOL guid_was_null in it. Or a GUID and a GUID *.

Please extend the test to DirectDrawEnumerateA. MSDN suggests it is equivalent to DirectDrawEnumerateExA(DDENUM_NONDISPLAYDEVICES), but I have my doubts here. Comparing the strings of the non-NULL device against "DirectDraw HAL" and "display" (in the Ex and non-Ex case) would also be a good idea.

Am 13.10.2014 um 21:16 schrieb Erich E. Hoover <erich.e.hoover at gmail.com>:

> Fixes bug #37307.  There are a couple of other bugs that are
> preventing Urban Assault from working, but this fixes the problem with
> returning the interfaces in the wrong order.
> <0001-ddraw-Return-the-primary-legacy-ddraw-device-last.patch>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 842 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20141014/2a555d4b/attachment.sig>