[PATCH 0/1] MR9322: winex11: Expose only EGL configs that match the display depth.
From: Elizabeth Figura <zfigura(a)codeweavers.com> --- dlls/win32u/opengl.c | 1 + dlls/winex11.drv/opengl.c | 23 +++++++++++++++++++++++ include/wine/opengl_driver.h | 1 + 3 files changed, 25 insertions(+) diff --git a/dlls/win32u/opengl.c b/dlls/win32u/opengl.c index fc0be5795d4..2f3645bcf28 100644 --- a/dlls/win32u/opengl.c +++ b/dlls/win32u/opengl.c @@ -469,6 +469,7 @@ static UINT egldrv_init_pixel_formats( UINT *onscreen_count ) for (i = 0, j = 0; i < count; i++) { funcs->p_eglGetConfigAttrib( egl->display, configs[i], EGL_RENDERABLE_TYPE, &render ); + if (egl->filter_config && !egl->filter_config( egl, configs[i] )) continue; if (render & EGL_OPENGL_BIT) configs[j++] = configs[i]; } count = j; diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index c4cc2b1fe1f..f02c6a7a6e8 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -463,10 +463,33 @@ done: return ret; } +static BOOL x11drv_egl_filter_config( struct egl_platform *egl, EGLConfig config ) +{ + XVisualInfo *visuals; + XVisualInfo visual; + int count; + + funcs->p_eglGetConfigAttrib( egl->display, config, EGL_NATIVE_VISUAL_ID, (EGLint *)&visual.visualid ); + if (!(visuals = XGetVisualInfo( gdi_display, VisualIDMask, &visual, &count ))) return FALSE; + + /* Reject formats whose depth does not match the screen depth so that we + * can copy child windows on-screen using XCopyArea(). + * See x11drv_init_pixel_formats() for the same logic with GLX. */ + if (visuals->depth != default_visual.depth) + { + XFree( visuals ); + return FALSE; + } + + XFree( visuals ); + return TRUE; +} + static void x11drv_init_egl_platform( struct egl_platform *platform ) { platform->type = EGL_PLATFORM_X11_KHR; platform->native_display = gdi_display; + platform->filter_config = x11drv_egl_filter_config; egl = platform; } diff --git a/include/wine/opengl_driver.h b/include/wine/opengl_driver.h index e9052392471..60117e03cc9 100644 --- a/include/wine/opengl_driver.h +++ b/include/wine/opengl_driver.h @@ -134,6 +134,7 @@ struct egl_platform EGLenum type; EGLNativeDisplayType native_display; BOOL force_pbuffer_formats; + BOOL (*filter_config)(struct egl_platform *platform, EGLConfig config); /* filled by win32u after init_egl_platform */ EGLDeviceEXT device; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9322
Shouldn't this be filtered on the opengl32 level (might be the case depending on the comments below)?
It's an x11 restriction. I don't even know if other display drivers expose this information.
Layered windows for instance use 32bpp visual while the default visual is often 24bpp. I'm not sure how or why this currently works for toplevel windows, but I think that what we need here is for the child window to match its toplevel bpp, not necessarily the screen default bpp.
I'm also curious whether there are some restrictions on Windows between parent and child bpp and pixel format enumeration, maybe it would be worth testing?
This is about the visual depth, not the buffer depth. You can have a 32-bit RGBA buffer that only has a 24-bit visual depth. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9322#note_120909
Hmm, okay. I guess we have the same issue with GLX with layered windows, they are probably broken with child window already anyway. Would it be enough to remove the `PFD_DRAW_TO_WINDOW` flag from the pixel format descriptor? I'm thinking this could perhaps be done entirely on the winex11 side, and avoid completely removing the undesired configs including for non-onscreen usage, by hooking the `describe_pixel_format` driver func like this: ```diff diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index c1e4ca8d5af6..570c011aabf6 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -216,6 +216,7 @@ enum glx_swap_control_method static struct glx_pixel_format *pixel_formats; static int nb_pixel_formats, nb_onscreen_formats; static const struct egl_platform *egl; +static BOOL (*p_egl_describe_pixel_format)( int format, struct wgl_pixel_format *pf ); /* Selects the preferred GLX swap control method for use by wglSwapIntervalEXT */ static enum glx_swap_control_method swap_control_method = GLX_SWAP_CONTROL_NONE; @@ -506,6 +507,22 @@ BOOL visual_from_pixel_format( int format, XVisualInfo *visual ) } } +static BOOL x11drv_egl_describe_pixel_format( int format, struct wgl_pixel_format *pf ) +{ + XVisualInfo visual; + + if (!p_egl_describe_pixel_format( format, pf )) return FALSE; + if (!visual_from_pixel_format( format, &visual ) || visual.depth != default_visual.depth) + { + /* Forbid drawing to windows with formats whose depth does not match the screen depth + * so that we can copy child windows on-screen using XCopyArea(). + * See x11drv_init_pixel_formats() for the same logic with GLX. */ + pf->pfd.dwFlags &= ~PFD_DRAW_TO_WINDOW; + } + + return TRUE; +} + static BOOL x11drv_egl_surface_create( HWND hwnd, int format, struct opengl_drawable **drawable ) { struct opengl_drawable *previous; @@ -561,6 +578,8 @@ UINT X11DRV_OpenGLInit( UINT version, const struct opengl_funcs *opengl_funcs, c x11drv_driver_funcs = **driver_funcs; x11drv_driver_funcs.p_init_egl_platform = x11drv_init_egl_platform; x11drv_driver_funcs.p_surface_create = x11drv_egl_surface_create; + x11drv_driver_funcs.p_describe_pixel_format = x11drv_egl_describe_pixel_format; + p_egl_describe_pixel_format = (*driver_funcs)->p_describe_pixel_format; *driver_funcs = &x11drv_driver_funcs; return STATUS_SUCCESS; } ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9322#note_120925
Hmm, okay. I guess we have the same issue with GLX with layered windows, they are probably broken with child window already anyway.
Would it be enough to remove the `PFD_DRAW_TO_WINDOW` flag from the pixel format descriptor?
I'm thinking this could perhaps be done entirely on the winex11 side, and avoid completely removing the undesired configs including for non-onscreen usage, by hooking the `describe_pixel_format` driver func like this:
Sorry I missed this response. It would work, I think, and it looks correct, but I can't say I'm familiar enough with the intent of the API design to say that it is correct. Since you've more or less written the patch there, do you want to submit it? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9322#note_121958
I don't really mind, feel free to author it and update the MR, unless you prefer not to. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9322#note_121959
This merge request was closed by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9322
participants (3)
-
Elizabeth Figura -
Elizabeth Figura (＠zfigura)
-
Rémi Bernon