[PATCH v2 0/2] MR9093: winex11: Create client windows with a colormap for EGL too.
@whrvt I think that's what is necessary for EGL to work better on NVIDIA right? -- v2: winex11: Lookup visual from EGL config EGL_NATIVE_VISUAL_ID. winex11: Move colormap ownership to x11drv_client_surface. https://gitlab.winehq.org/wine/wine/-/merge_requests/9093
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/init.c | 34 ++++++++++++++++--------- dlls/winex11.drv/opengl.c | 53 ++++++++++++++++++++++----------------- dlls/winex11.drv/vulkan.c | 2 +- dlls/winex11.drv/x11drv.h | 4 ++- 4 files changed, 56 insertions(+), 37 deletions(-) diff --git a/dlls/winex11.drv/init.c b/dlls/winex11.drv/init.c index 039c4b0fefd..61ea5078e5b 100644 --- a/dlls/winex11.drv/init.c +++ b/dlls/winex11.drv/init.c @@ -261,6 +261,7 @@ static const struct client_surface_funcs x11drv_client_surface_funcs; struct x11drv_client_surface { struct client_surface client; + Colormap colormap; Window window; RECT rect; @@ -280,6 +281,7 @@ static void x11drv_client_surface_destroy( struct client_surface *client ) TRACE( "%s\n", debugstr_client_surface( client ) ); + if (surface->colormap != default_colormap) XFreeColormap( gdi_display, surface->colormap ); if (surface->window) destroy_client_window( hwnd, surface->window ); if (surface->hdc_dst) NtGdiDeleteObjectApp( surface->hdc_dst ); if (surface->hdc_src) NtGdiDeleteObjectApp( surface->hdc_src ); @@ -426,25 +428,33 @@ static const struct client_surface_funcs x11drv_client_surface_funcs = .present = X11DRV_client_surface_present, }; -Window x11drv_client_surface_create( HWND hwnd, const XVisualInfo *visual, Colormap colormap, struct client_surface **client ) +Window x11drv_client_surface_create( HWND hwnd, int format, struct client_surface **client ) { struct x11drv_client_surface *surface; + XVisualInfo visual = default_visual; + Colormap colormap; - if (!(surface = client_surface_create( sizeof(*surface), &x11drv_client_surface_funcs, hwnd ))) return None; - if (!(surface->window = create_client_window( hwnd, visual, colormap ))) - { - client_surface_release( &surface->client ); - return None; - } - if (!NtUserGetClientRect( hwnd, &surface->rect, NtUserGetDpiForWindow( hwnd ) )) - { - client_surface_release( &surface->client ); - return None; - } + if (format && !visual_from_pixel_format( format, &visual )) return None; + + if (visual.visualid == default_visual.visualid) colormap = default_colormap; + else colormap = XCreateColormap( gdi_display, get_dummy_parent(), visual.visual, + (visual.class == PseudoColor || visual.class == GrayScale || + visual.class == DirectColor) ? AllocAll : AllocNone ); + if (!colormap) return None; + + if (!(surface = client_surface_create( sizeof(*surface), &x11drv_client_surface_funcs, hwnd ))) goto failed; + if (!(surface->window = create_client_window( hwnd, &visual, colormap ))) goto failed; + if (!NtUserGetClientRect( hwnd, &surface->rect, NtUserGetDpiForWindow( hwnd ) )) goto failed; + surface->colormap = colormap; TRACE( "Created %s for client window %lx\n", debugstr_client_surface( &surface->client ), surface->window ); *client = &surface->client; return surface->window; + +failed: + if (surface) client_surface_release( &surface->client ); + XFreeColormap( gdi_display, colormap ); + return None; } /********************************************************************** diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index b810689ff3d..64640fb798c 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -198,7 +198,6 @@ struct gl_drawable { struct opengl_drawable base; GLXDrawable drawable; /* drawable for rendering with GL */ - Colormap colormap; /* colormap for the client window */ }; static struct gl_drawable *impl_from_opengl_drawable( struct opengl_drawable *base ) @@ -471,13 +470,34 @@ static void x11drv_init_egl_platform( struct egl_platform *platform ) egl = platform; } -static inline EGLConfig egl_config_for_format(int format) +static EGLConfig egl_config_for_format( int format ) { assert(format > 0 && format <= 2 * egl->config_count); if (format <= egl->config_count) return egl->configs[format - 1]; return egl->configs[format - egl->config_count - 1]; } +static struct glx_pixel_format *glx_pixel_format_from_format( int format ) +{ + assert( format > 0 && format <= nb_pixel_formats ); + return &pixel_formats[format - 1]; +} + +BOOL visual_from_pixel_format( int format, XVisualInfo *visual ) +{ + if (use_egl) + { + *visual = default_visual; + return TRUE; + } + else + { + struct glx_pixel_format *fmt = glx_pixel_format_from_format( format ); + *visual = *fmt->visual; + return TRUE; + } +} + static BOOL x11drv_egl_surface_create( HWND hwnd, int format, struct opengl_drawable **drawable ) { struct opengl_drawable *previous; @@ -489,7 +509,7 @@ static BOOL x11drv_egl_surface_create( HWND hwnd, int format, struct opengl_draw if ((previous = *drawable) && previous->format == format) return TRUE; NtUserGetClientRect( hwnd, &rect, NtUserGetDpiForWindow( hwnd ) ); - if (!(window = x11drv_client_surface_create( hwnd, &default_visual, default_colormap, &client ))) return FALSE; + if (!(window = x11drv_client_surface_create( hwnd, 0, &client ))) return FALSE; gl = opengl_drawable_create( sizeof(*gl), &x11drv_egl_surface_funcs, format, client ); client_surface_release( client ); if (!gl) return FALSE; @@ -850,12 +870,6 @@ static UINT x11drv_init_pixel_formats( UINT *onscreen_count ) return size; } -static struct glx_pixel_format *glx_pixel_format_from_format( int format ) -{ - assert( format > 0 && format <= nb_pixel_formats ); - return &pixel_formats[format - 1]; -} - static void x11drv_surface_destroy( struct opengl_drawable *base ) { struct gl_drawable *gl = impl_from_opengl_drawable( base ); @@ -863,7 +877,6 @@ static void x11drv_surface_destroy( struct opengl_drawable *base ) TRACE( "drawable %s\n", debugstr_opengl_drawable( base ) ); if (gl->drawable) pglXDestroyWindow( gdi_display, gl->drawable ); - if (gl->colormap) XFreeColormap( gdi_display, gl->colormap ); } static BOOL set_swap_interval( struct gl_drawable *gl, int interval ) @@ -923,23 +936,16 @@ static BOOL x11drv_surface_create( HWND hwnd, int format, struct opengl_drawable struct opengl_drawable *previous; struct client_surface *client; struct gl_drawable *gl; - Colormap colormap; Window window; RECT rect; if ((previous = *drawable) && previous->format == format) return TRUE; NtUserGetClientRect( hwnd, &rect, NtUserGetDpiForWindow( hwnd ) ); - colormap = XCreateColormap( gdi_display, get_dummy_parent(), fmt->visual->visual, - (fmt->visual->class == PseudoColor || fmt->visual->class == GrayScale || - fmt->visual->class == DirectColor) ? AllocAll : AllocNone ); - if (!colormap) return FALSE; - - if (!(window = x11drv_client_surface_create( hwnd, fmt->visual, colormap, &client ))) goto failed; + if (!(window = x11drv_client_surface_create( hwnd, format, &client ))) return FALSE; gl = opengl_drawable_create( sizeof(*gl), &x11drv_surface_funcs, format, client ); client_surface_release( client ); - if (!gl) goto failed; - gl->colormap = colormap; + if (!gl) return FALSE; if (!(gl->drawable = pglXCreateWindow( gdi_display, fmt->fbconfig, window, NULL ))) { @@ -953,10 +959,6 @@ static BOOL x11drv_surface_create( HWND hwnd, int format, struct opengl_drawable if (previous) opengl_drawable_release( previous ); *drawable = &gl->base; return TRUE; - -failed: - XFreeColormap( gdi_display, colormap ); - return FALSE; } static BOOL x11drv_describe_pixel_format( int format, struct wgl_pixel_format *pf ) @@ -1547,4 +1549,9 @@ void destroy_gl_drawable( HWND hwnd ) { } +BOOL visual_from_pixel_format( int format, XVisualInfo *visual ) +{ + return FALSE; +} + #endif /* defined(SONAME_LIBGL) */ diff --git a/dlls/winex11.drv/vulkan.c b/dlls/winex11.drv/vulkan.c index 61e4327e379..54e01759e37 100644 --- a/dlls/winex11.drv/vulkan.c +++ b/dlls/winex11.drv/vulkan.c @@ -58,7 +58,7 @@ static VkResult X11DRV_vulkan_surface_create( HWND hwnd, const struct vulkan_ins TRACE( "%p %p %p %p\n", hwnd, instance, handle, client ); - if (!(info.window = x11drv_client_surface_create( hwnd, &default_visual, default_colormap, client ))) return VK_ERROR_OUT_OF_HOST_MEMORY; + if (!(info.window = x11drv_client_surface_create( hwnd, 0, client ))) return VK_ERROR_OUT_OF_HOST_MEMORY; if (instance->p_vkCreateXlibSurfaceKHR( instance->host.instance, &info, NULL /* allocator */, handle )) { ERR("Failed to create Xlib surface\n"); diff --git a/dlls/winex11.drv/x11drv.h b/dlls/winex11.drv/x11drv.h index c69c83e5dd9..6e909e289ee 100644 --- a/dlls/winex11.drv/x11drv.h +++ b/dlls/winex11.drv/x11drv.h @@ -282,6 +282,8 @@ extern BOOL client_side_with_render; extern BOOL shape_layered_windows; extern const struct gdi_dc_funcs *X11DRV_XRender_Init(void); +extern BOOL visual_from_pixel_format( int format, XVisualInfo *visual ); + extern UINT X11DRV_OpenGLInit( UINT, const struct opengl_funcs *, const struct opengl_driver_funcs ** ); extern UINT X11DRV_VulkanInit( UINT, void *, const struct vulkan_driver_funcs ** ); @@ -358,7 +360,7 @@ extern BOOL needs_offscreen_rendering( HWND hwnd ); extern void set_dc_drawable( HDC hdc, Drawable drawable, const RECT *rect, int mode ); extern Drawable get_dc_drawable( HDC hdc, RECT *rect ); extern HRGN get_dc_monitor_region( HWND hwnd, HDC hdc ); -extern Window x11drv_client_surface_create( HWND hwnd, const XVisualInfo *visual, Colormap colormap, struct client_surface **client ); +extern Window x11drv_client_surface_create( HWND hwnd, int format, struct client_surface **client ); /************************************************************************** * X11 USER driver -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9093
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winex11.drv/opengl.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index 64640fb798c..711a3147d5a 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -487,7 +487,15 @@ BOOL visual_from_pixel_format( int format, XVisualInfo *visual ) { if (use_egl) { - *visual = default_visual; + EGLConfig config = egl_config_for_format( format ); + XVisualInfo *visuals; + int count; + + memset( visual, 0, sizeof(*visual) ); + funcs->p_eglGetConfigAttrib( egl->display, config, EGL_NATIVE_VISUAL_ID, (EGLint *)&visual->visualid ); + if (!(visuals = XGetVisualInfo( gdi_display, VisualIDMask, visual, &count ))) return FALSE; + *visual = *visuals; + XFree( visuals ); return TRUE; } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/9093
TL;DR: it might be an issue with NVIDIA drivers, but there's also an unhandled edge case crash (see backtrace). I'm still running into the same issue as before with a very simple SDL+OpenGL [test program](https://raw.githubusercontent.com/whrvt/sdlgpu_gears/772a34296ff64c3b75f07f9...). The chain reaction seems to start from [`get_window_unused_drawable`](https://gitlab.winehq.org/rbernon/wine/-/blob/cf83ab2ab915605e6bbea32082fa46...). I think the call stack goes like: ``` context_sync_drawables() -> new_draw = get_updated_drawable() -> get_window_unused_drawable() -> driver_funcs->p_surface_create() -> x11drv_egl_surface_create() -> gl->base.surface = funcs->p_eglCreateWindowSurface() == NULL ``` The final `eglCreateWindowSurface` call seems to always return NULL, with an `EGL_BAD_CONFIG` error, for any format that has alpha (i.e. format ids 1 through 20 in my log). If I just hardcode a format without alpha (e.g. 21) in the failing `eglCreateWindowSurface` call, like so: ``` if (!(gl->base.surface = funcs->p_eglCreateWindowSurface( egl->display, egl_config_for_format( 21 ), (void *)window, NULL ))) ``` the surface creation actually succeeds. Obviously, this fails later along the line when the surface is expected to have an alpha channel, but isn't. I'll also attach a brief log that shows this happening with `WINEDEBUG=opengl,wgl,wined3d` here: [eglgears.log](/uploads/a2a6c0ea93cdf0fb046fc9dbe8a3986c/eglgears.log) [gdb-segfault.txt](/uploads/4a08f2d8be0e419b95c91f04692e27d1/gdb-segfault.txt) We segfault on `opengl_drawable_add_ref(NULL)` here: ``` else if (draw_hdc && draw_hdc == read_hdc) opengl_drawable_add_ref( (new_read = new_draw) ); ``` because the `new_draw` we got from `get_updated_drawable` was NULL. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9093#note_117531
This is perhaps because the default visual, which is used for normal windows, doesn't usually have alpha? Creating a child client window with an alpha visual, or the EGL surface for it, then maybe fails? The question may be why it decides to use the pixel format with an alpha channel in the first place. Even though you have ~~`SDL_GL_DEPTH_SIZE := 24`~~ `R/G/B = 8/8/8` in the sample, it calls `wglChoosePixelFormat` with 32bpp and never matches the 24bpp formats. Could you make the same log with glX? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9093#note_117536
On Fri Oct 3 13:05:45 2025 +0000, Rémi Bernon wrote:
This is perhaps because the default visual, which is used for normal windows, doesn't usually have alpha? Creating a child client window with an alpha visual, or the EGL surface for it, then maybe fails? The question may be why it decides to use the pixel format with an alpha channel in the first place. Even though you have ~~`SDL_GL_DEPTH_SIZE := 24`~~ `R/G/B = 8/8/8` in the sample, it calls `wglChoosePixelFormat` with 32bpp and never matches the 24bpp formats. Could you make the same log with glX? Sure, here's a glX log:
[glx-gears.log](/uploads/c2729f61678ae733212f9332404007de/glx-gears.log) I cut it off around where EGL failed, at the first `wglMakeCurrent`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9093#note_117538
On Fri Oct 3 13:06:03 2025 +0000, William Horvath wrote:
Sure, here's a glX log: [glx-gears.log](/uploads/c2729f61678ae733212f9332404007de/glx-gears.log) I cut it off around where EGL failed, at the first `wglMakeCurrent`. Yeah this is weird, seems to also be using a format with an alpha channel, but things work fine here...
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/9093#note_117539
participants (2)
-
Rémi Bernon -
William Horvath (＠whrvt)