On 2/3/22 18:41, Paul Gofman wrote:
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.
I've got another idea. Given we don't actually need anything from the DC when getting GL functions from the drivers besides the DC ptr don't get freed, can't we just GDI_inc_ref_count() on the DC handle and proceed to physdev->funcs->wine_get_wgl_driver() without holding any locks and without changing anything in the current physdev drivers?