Re: [PATCH v4] win32u: Cache GL driver functions in DC.

[Jacek Caban]

Hi Paul,

On 2/18/22 16:52, Paul Gofman wrote:
> @@ -816,6 +834,8 @@ HDC WINAPI NtGdiCreateCompatibleDC( HDC hdc )
>           free_dc_ptr( dc );
>           return 0;
>       }
> - dc->dibdrv = TRUE;
> -
> physDev = GET_DC_PHYSDEV( dc, pSelectBitmap );
> physDev->funcs->pSelectBitmap( physDev, dc->hBitmap );

That's not pretty, you could use something like get_gdi_object_type(...) 
== NTGDI_OBJ_METADC instead.
> diff --git a/dlls/win32u/dibdrv/dc.c b/dlls/win32u/dibdrv/dc.c
> index a96b613d0a3..a91cd76f936 100644
> --- a/dlls/win32u/dibdrv/dc.c
> +++ b/dlls/win32u/dibdrv/dc.c
> @@ -605,7 +605,7 @@ static struct opengl_funcs opengl_funcs =
>   /**********************************************************************
> -
>     dibdrv_wine_get_wgl_driver
>
> */
> -static struct opengl_funcs * CDECL dibdrv_wine_get_wgl_driver( PHYSDEV dev, UINT version )
> +struct opengl_funcs *dibdrv_wine_get_wgl_driver( UINT version )
>   {
>       if (version != WINE_WGL_DRIVER_VERSION)
>       {

Now that it's internal win32u function, I'd suggest to skip "wine_" part 
of the name (and maybe move version check to __wine_get_wgl_driver).
> struct opengl_funcs * CDECL __wine_get_wgl_driver( HDC hdc, UINT version )
>   {
> - struct opengl_funcs *ret = NULL;
> - DC * dc = get_dc_ptr( hdc );
> - struct opengl_funcs *ret;
> - PHYSDEV physdev;
> - BOOL dibdrv;
> - DC * dc;
> -
> - if (!(dc = get_dc_obj( hdc ))) return NULL;
> - if (dc->attr->disabled)
> - {
> -
>    GDI_ReleaseObj( hdc );
> -
>    return NULL;
> - }
> - ret = dc->gl_funcs;
> - dibdrv = dc->dibdrv;
> - GDI_ReleaseObj( hdc );
> -
> - if (ret) return ret;

Do we still need caching part of the patch?
> - if (dc)
> - if (dibdrv) ret = dibdrv_wine_get_wgl_driver( version );
> - else
>  {
> -
>    PHYSDEV physdev = GET_DC_PHYSDEV( dc, wine_get_wgl_driver );
> -
>    ret = physdev->funcs->wine_get_wgl_driver( physdev, version );
> -
>    release_dc_ptr( dc );
> -
>    if (user_driver->pwine_get_wgl_driver) ret = user_driver->pwine_get_wgl_driver( version );
> -
>    else                                   ret = (void *)-1;

You shouldn't need to check for null here. wine_get_wgl_driver should 
have an entry in lazy_load_driver and __wine_set_display_driver, see how 
other functions are handled. In this case, lazy_load_driver could 
probably just point to null driver.
> diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c
> index 5b31c352a23..35bf47c2e74 100644
> --- a/dlls/winex11.drv/init.c
> +++ b/dlls/winex11.drv/init.c
> @@ -312,16 +312,9 @@ static INT CDECL X11DRV_ExtEscape( PHYSDEV dev, INT escape, INT in_count, LPCVOI
>   /**********************************************************************
> -
>       X11DRV_wine_get_wgl_driver
>
> */
> -static struct opengl_funcs * CDECL X11DRV_wine_get_wgl_driver( PHYSDEV dev, UINT version )
> +static struct opengl_funcs * CDECL X11DRV_wine_get_wgl_driver( UINT version )
>   {
> - struct opengl_funcs *ret;
> -
> - if (!(ret = get_glx_driver( version )))
> - {
> -
>    dev = GET_NEXT_PHYSDEV( dev, wine_get_wgl_driver );
> -
>    ret = dev->funcs->wine_get_wgl_driver( dev, version );
> - }
> - return ret;
> - return get_glx_driver( version );
> }

I would consider merging X11DRV_wine_get_wgl_driver get_glx_driver (not 
really important, through).
> /**********************************************************************
> @@ -377,7 +370,6 @@ static const struct user_driver_funcs x11drv_funcs =
>       .dc_funcs.pUnrealizePalette = X11DRV_UnrealizePalette,
>       .dc_funcs.pD3DKMTCheckVidPnExclusiveOwnership = X11DRV_D3DKMTCheckVidPnExclusiveOwnership,
>       .dc_funcs.pD3DKMTSetVidPnSourceOwner = X11DRV_D3DKMTSetVidPnSourceOwner,
> -
>
> .dc_funcs.wine_get_wgl_driver = X11DRV_wine_get_wgl_driver,
>  .dc_funcs.priority = GDI_PRIORITY_GRAPHICS_DRV,
>
> .pActivateKeyboardLayout = X11DRV_ActivateKeyboardLayout,
>
> @@ -418,6 +410,7 @@ static const struct user_driver_funcs x11drv_funcs =
>       .pWindowPosChanging = X11DRV_WindowPosChanging,
>       .pWindowPosChanged = X11DRV_WindowPosChanged,
>       .pSystemParametersInfo = X11DRV_SystemParametersInfo,
> - .pwine_get_wgl_driver = X11DRV_wine_get_wgl_driver,
>  .pwine_get_vulkan_driver = X11DRV_wine_get_vulkan_driver,
>  .pThreadDetach = X11DRV_ThreadDetach,
> };
>
> diff --git a/dlls/winex11.drv/xrender.c b/dlls/winex11.drv/xrender.c
> index 92dc35fa9e1..5ede97fb026 100644
> --- a/dlls/winex11.drv/xrender.c
> +++ b/dlls/winex11.drv/xrender.c
> @@ -2246,7 +2246,6 @@ static const struct gdi_dc_funcs xrender_funcs =
>       NULL,                               /* pUnrealizePalette */
>       NULL,                               /* pD3DKMTCheckVidPnExclusiveOwnership */
>       NULL,                               /* pD3DKMTSetVidPnSourceOwner */
> - NULL,                               /* wine_get_wgl_driver */
>  GDI_PRIORITY_GRAPHICS_DRV + 10      /* priority */
> };
>
> diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h
> index 567a6c21608..24a4bba1c8b 100644
> --- a/include/wine/gdi_driver.h
> +++ b/include/wine/gdi_driver.h
> @@ -159,7 +159,6 @@ struct gdi_dc_funcs
>       BOOL     (CDECL *pUnrealizePalette)(HPALETTE);
>       NTSTATUS (CDECL *pD3DKMTCheckVidPnExclusiveOwnership)(const D3DKMT_CHECKVIDPNEXCLUSIVEOWNERSHIP *);
>       NTSTATUS (CDECL *pD3DKMTSetVidPnSourceOwner)(const D3DKMT_SETVIDPNSOURCEOWNER *);
> -
>
> struct opengl_funcs * (CDECL *wine_get_wgl_driver)(PHYSDEV,UINT);
>
> /* priority order for the driver on the stack */
>  UINT       priority;
>
> @@ -323,6 +322,8 @@ struct user_driver_funcs
>       const struct vulkan_funcs * (CDECL *pwine_get_vulkan_driver)(UINT);
>       /* thread management */
>       void    (CDECL *pThreadDetach)(void);
> -
>
> /* opengl support */
> -
>
> struct opengl_funcs * (CDECL *pwine_get_wgl_driver)(UINT);
> };
>
> extern void CDECL __wine_set_user_driver( const struct user_driver_funcs *funcs, UINT version );

This should also increment WINE_GDI_DRIVER_VERSION. Also, please place 
pwine_get_wgl_driver next to pwine_get_vulkan_driver.