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

Erich E. Hoover erich.e.hoover at gmail.com
Tue Oct 14 11:22:26 CDT 2014


On Tue, Oct 14, 2014 at 2:32 AM, Stefan Dösinger
<stefandoesinger at gmail.com> wrote:
> 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.

That's correct, for once this also matches the documentation ;)  That
was the point of that test, what kind of check were you thinking?

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

Yes, the NULL/primary device is always enumerated (unless the callback
requests an early stop).

> ...
> Please do not use CamelCase or lpFoo in d3d-related code.

Sorry, I thought that you wanted the variable names maintained based
on some of the other patches I've done here - I'll fix that.

> 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.

Taking a look at that code it looks like we're "protecting" such
applications from having a problem by copying the memory.  I can
easily add a separate patch for exploring this, but are we really
interested in protecting ourselves from such an app here when we've
(to my knowledge) never run into one?  The primary device callback
code has been in place for a long time...

>> +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.

Will do.

>> +{
>> +    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 *.

Can do.

> 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.

This sounds like something that should be done separately, as we don't
currently support the DDENUM_NONDISPLAYDEVICES flag.  None of the
testbots actually return "DirectDraw HAL"/"display", nor do my tests
on a Windows 7 machine.  I'd hesitate to change what we return here to
what Windows returns now (Primary Display Driver), since that's likely
to break QuickTime (according to the comment).

Best,
Erich



More information about the wine-devel mailing list