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 <pgofman(a)codeweavers.com>
> ---
>       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