Re: [PATCH] win32u: Don't use get_dc_ptr() in __wine_get_wgl_driver().

[Jacek Caban]

Hi Paul,

On 2/21/22 16:37, Paul Gofman wrote:
> Signed-off-by: Paul Gofman
>   Supersedes 227402.
>   Changes:
>   - use gdi object type for detecting memory DC;
>   - rename dibdrv_wine_get_wgl_driver to dibdrv_get_wgl_driver and move version check to __wine_get_wgl_driver();
>   - remove the caching part;
>   - set wine_get_wgl_driver in __wine_set_display_driver instead of checking for NULL pwine_get_wgl_driver;
>   - also point lazy_load_driver.pwine_get_wgl_driver to nulldrv_wine_get_wgl_driver;
>   - increment WINE_GDI_DRIVER_VERSION;
>   - place pwine_get_wgl_driver next to pwine_get_vulkan_driver.

Thanks, it looks better now.
> @@ -1373,14 +1377,26 @@ NTSTATUS WINAPI NtGdiDdDDICheckVidPnExclusiveOwnership( const D3DKMT_CHECKVIDPNE
>    */
>   struct opengl_funcs * CDECL __wine_get_wgl_driver( HDC hdc, UINT version )
>   {
> - struct opengl_funcs *ret = NULL;
> - DC * dc = get_dc_ptr( hdc );
> - BOOL dibdrv;
> - DC * dc;
> - if (dc)
> - if (!(dc = get_dc_obj( hdc ))) return NULL;
> - if (dc->attr->disabled)
>  {
> -
>    PHYSDEV physdev = GET_DC_PHYSDEV( dc, wine_get_wgl_driver );
> -
>    ret = physdev->funcs->wine_get_wgl_driver( physdev, version );
> -
>    release_dc_ptr( dc );
> -
>    GDI_ReleaseObj( hdc );
> -
>    return NULL;
> }
> - return ret;
> - dibdrv = (get_gdi_object_type( hdc ) == NTGDI_OBJ_MEMDC);
> - GDI_ReleaseObj( hdc );
> -
> - if (dibdrv)
> - {
> -
>    if (version != WINE_WGL_DRIVER_VERSION)
> -
>    {
> -
>        ERR( "version mismatch, opengl32 wants %u but dibdrv has %u\n", version, WINE_WGL_DRIVER_VERSION );
> -
>        return NULL;
> -
>    }
> -
>    return dibdrv_get_wgl_driver();
> - }
> - return load_driver()->pwine_get_wgl_driver( version );
> }

This should be "user_driver->" instead of load_driver().

Looking closer at this, we also probably shouldn't use that for EMFs and 
printer DCs. We don't preserve NtGdiOpenDCW's is_display precisely yet 
(only when a display string is provided). Maybe we should set a default 
display string there (so that you could just check 'is_display = 
dc->display[0] != 0;' here). The alternative would be to store 
is_display flag.
> diff --git a/dlls/win32u/ntgdi_private.h b/dlls/win32u/ntgdi_private.h
> index bfeb4557da7..a75d97c9d1a 100644
> --- a/dlls/win32u/ntgdi_private.h
> +++ b/dlls/win32u/ntgdi_private.h
> @@ -206,6 +206,7 @@ extern UINT get_dib_dc_color_table( HDC hdc, UINT startpos, UINT entries,
>   extern UINT set_dib_dc_color_table( HDC hdc, UINT startpos, UINT entries,
>                                       const RGBQUAD *colors ) DECLSPEC_HIDDEN;
>   extern void dibdrv_set_window_surface( DC *dc, struct window_surface *surface ) DECLSPEC_HIDDEN;
> +extern struct opengl_funcs *dibdrv_get_wgl_driver(void) DECLSPEC_HIDDEN;
>
> /* driver.c */
>   extern const struct gdi_dc_funcs null_driver DECLSPEC_HIDDEN;
> @@ -660,6 +661,25 @@ static inline void copy_bitmapinfo( BITMAPINFO *dst, const BITMAPINFO *src )
>       memcpy( dst, src, get_dib_info_size( src, DIB_RGB_COLORS ));
>   }
>
> +static inline DC *get_dc_obj( HDC hdc )
> +{
> - DWORD type;
> - DC *dc = get_any_obj_ptr( hdc, &type );
> - if (!dc) return NULL;
> -
> - switch (type)
> - {
> - case NTGDI_OBJ_DC:
> - case NTGDI_OBJ_MEMDC:
> - case NTGDI_OBJ_ENHMETADC:
> -
>    return dc;
> - default:
> -
>    GDI_ReleaseObj( hdc );
> -
>    SetLastError( ERROR_INVALID_HANDLE );
> -
>    return NULL;
> - }
>
> +}
> -

We could just move __wine_get_wgl_driver to dc.c and avoid touching headers.

Thanks,

Jacek