[PATCH v2 1/2] win32u: Don't pass physdev to wine_get_wgl_driver() driver function.

[Paul Gofman]

On 2/3/22 17:25, Jacek Caban wrote:
> Hi Paul,
>
> On 2/1/22 18:32, Paul Gofman wrote:
>> @@ -1347,13 +1349,29 @@ NTSTATUS WINAPI 
>> NtGdiDdDDICheckVidPnExclusiveOwnership( const D3DKMT_CHECKVIDPNE
>>    */
>>   struct opengl_funcs * CDECL __wine_get_wgl_driver( HDC hdc, UINT 
>> version )
>>   {
>> +    const struct gdi_dc_funcs *driver_funcs[11], **next_funcs;
>>       struct opengl_funcs *ret = NULL;
>>       DC * dc = get_dc_ptr( hdc );
>> +    PHYSDEV physdev;
>>         if (dc)
>>       {
>> -        PHYSDEV physdev = GET_DC_PHYSDEV( dc, wine_get_wgl_driver );
>> -        ret = physdev->funcs->wine_get_wgl_driver( physdev, version );
>> +        next_funcs = driver_funcs;
>> +        physdev = GET_DC_PHYSDEV( dc, wine_get_wgl_driver );
>> +        while (physdev && next_funcs - driver_funcs < 
>> ARRAY_SIZE(driver_funcs) - 1)
>> +        {
>> +            *next_funcs++ = physdev->funcs;
>> +            if (physdev->funcs == &null_driver) break;
>> +            physdev = GET_NEXT_PHYSDEV( physdev, wine_get_wgl_driver );
>> +        }
>> +        *next_funcs = NULL;
>> +        next_funcs = driver_funcs;
>> +        while (*next_funcs)
>> +        {
>> +            ret = (*next_funcs)->wine_get_wgl_driver( version, 
>> &next_funcs );
>> +            if (ret || !next_funcs) break;
>> +            ++next_funcs;
>> +        }
>
>
> If the current PHYSDEV locking does not fit what we need here, it may 
> be indeed a good idea to use something different here. It seems unique 
> in this aspect anyway. However, it doesn't seem like hacking around 
> PHYSDEV pointers like this is really what we need here. You could, for 
> example, move wine_get_wgl from gdi_dc_funcs to user_driver_funcs 
> (similar to what 9596e7f2a4e972c35efdca38b01d068bfc055d36 did for 
> Vulkan) and call it with no PHYSDEV involved. __wine_get_wgl_driver 
> could look roughly like this:
>
>
> if ((hwnd = NtUserWindowFromDC( hdc )) && (ret = 
> user_driver->wine_get_wgl_driver( hwnd, version )) return ret;
>
> if (selected correct bitmap on hdc) return get_osmesa_driver( version );
>
> return (void *)-1;
>
>
> How does it look to you?
>
>
I am not quite sure, but this looks a bit convoluted to me. First in 
terms of getting user hwnd from dc (which can work as I understand but 
brings some dependency on the not directly related locking and logic). 
Then heuristics in __wine_get_wgl_driver() for guessing if osmesa should 
be used while currently it is more straightforward as all that knowledge 
is already incorporated into the driver chain, there is the logic spread 
elsewhere which controls that.

I realize that hacking out driver function pointers indeed does not look 
nice. Maybe if getting hwnd is really not an issue (I assume 
user_callbacks->pWindowFromDC() can be used for that?) we can indicate 
in the DC structure if osmesa should be used (to be set when the dibdrv 
is added to device chain not through windrv)? This way we won't be 
duplicating that logic.