From 41cd0c6f13584d1dbb7fe0f775a4de3de5febd62 Mon Sep 17 00:00:00 2001 From: Benjamin Otte Date: Thu, 29 Aug 2024 01:20:25 +0200 Subject: [PATCH] gl: Fix initial EGL context creation on X11 After commit 447bc18c48281d7b718691c4e7f6c94ecb8b4dad EGL on X11 broke. But the handling of the GL context also was quite awkward because it was unclear who was responsible for ensuring it got reset. Change that by making gdk_gl_context_clear_current_if_surface() return the context (with a reference because it might be the last reference) it had unset, so that after changing EGL properties the code that caused the clearing can re-make it current. This moves the responsibility to the actual code that is dealing with updating properties and frees the outer layers of code from that task. And that means the X11 EGL code doesn't need to care and the code in the Wayland backend that did care can be removed. Related: !7662 Fixes: #6964 on X11 --- gdk/gdkglcontext.c | 18 ++++++++++++--- gdk/gdkglcontextprivate.h | 2 +- gdk/gdksurface.c | 35 ++++++++++++++++++++---------- gdk/wayland/gdkglcontext-wayland.c | 7 +----- gdk/wayland/gdksurface-wayland.c | 6 ++--- gdk/wayland/gdksurface-wayland.h | 2 +- gdk/x11/gdkglcontext-glx.c | 5 ++++- 7 files changed, 47 insertions(+), 28 deletions(-) diff --git a/gdk/gdkglcontext.c b/gdk/gdkglcontext.c index f7a486909d..fea9b73478 100644 --- a/gdk/gdkglcontext.c +++ b/gdk/gdkglcontext.c @@ -2037,8 +2037,11 @@ gdk_gl_context_clear_current (void) * * Does a gdk_gl_context_clear_current() if the current context is attached * to @surface, leaves the current context alone otherwise. + * + * Returns: (nullable) (transfer full): The context that was cleared, so that it can be + * re-made current later **/ -void +GdkGLContext * gdk_gl_context_clear_current_if_surface (GdkSurface *surface) { MaskedContext *current; @@ -2049,11 +2052,20 @@ gdk_gl_context_clear_current_if_surface (GdkSurface *surface) GdkGLContext *context = unmask_context (current); if (gdk_gl_context_get_surface (context) != surface) - return; + return NULL; + + g_object_ref (context); if (GDK_GL_CONTEXT_GET_CLASS (context)->clear_current (context)) - g_private_replace (&thread_current_context, NULL); + { + g_private_replace (&thread_current_context, NULL); + return context; + } + + g_object_unref (context); } + + return NULL; } /** diff --git a/gdk/gdkglcontextprivate.h b/gdk/gdkglcontextprivate.h index 99591fac51..3d3a22343c 100644 --- a/gdk/gdkglcontextprivate.h +++ b/gdk/gdkglcontextprivate.h @@ -120,7 +120,7 @@ gboolean gdk_gl_backend_can_be_used (GdkGLBackend GError **error); void gdk_gl_backend_use (GdkGLBackend backend_type); -void gdk_gl_context_clear_current_if_surface (GdkSurface *surface); +GdkGLContext * gdk_gl_context_clear_current_if_surface (GdkSurface *surface) G_GNUC_WARN_UNUSED_RESULT; GdkGLContext * gdk_gl_context_new (GdkDisplay *display, GdkSurface *surface, diff --git a/gdk/gdksurface.c b/gdk/gdksurface.c index 39dfbef89c..b82d4ac036 100644 --- a/gdk/gdksurface.c +++ b/gdk/gdksurface.c @@ -1136,6 +1136,7 @@ gdk_surface_set_egl_native_window (GdkSurface *self, { #ifdef HAVE_EGL GdkSurfacePrivate *priv = gdk_surface_get_instance_private (self); + GdkGLContext *current = NULL; /* This checks that all EGL platforms we support conform to the same struct sizes. * When this ever fails, there will be some fun times happening for whoever tries @@ -1146,13 +1147,19 @@ gdk_surface_set_egl_native_window (GdkSurface *self, { GdkDisplay *display = gdk_surface_get_display (self); + current = gdk_gl_context_clear_current_if_surface (self); + eglDestroySurface (gdk_display_get_egl_display (display), priv->egl_surface); priv->egl_surface = NULL; } - gdk_gl_context_clear_current_if_surface (self); - priv->egl_native_window = native_window; + + if (current) + { + gdk_gl_context_make_current (current); + g_object_unref (current); + } } gpointer /* EGLSurface */ @@ -1180,20 +1187,18 @@ gdk_surface_ensure_egl_surface (GdkSurface *self, depth = priv->egl_surface_depth; } - if (priv->egl_surface_depth != depth && - priv->egl_surface != NULL && - gdk_display_get_egl_config (display, priv->egl_surface_depth) != gdk_display_get_egl_config (display, depth)) - { - gdk_gl_context_clear_current_if_surface (self); - eglDestroySurface (gdk_display_get_egl_display (display), priv->egl_surface); - priv->egl_surface = NULL; - } - - if (priv->egl_surface == NULL) + if (priv->egl_surface == NULL || + (priv->egl_surface != NULL && + gdk_display_get_egl_config (display, priv->egl_surface_depth) != gdk_display_get_egl_config (display, depth))) { + GdkGLContext *cleared; EGLint attribs[4]; int i; + cleared = gdk_gl_context_clear_current_if_surface (self); + if (priv->egl_surface != NULL) + eglDestroySurface (gdk_display_get_egl_display (display), priv->egl_surface); + i = 0; if (depth == GDK_MEMORY_U8_SRGB && display->have_egl_gl_colorspace) { @@ -1218,6 +1223,12 @@ gdk_surface_ensure_egl_surface (GdkSurface *self, NULL); } priv->egl_surface_depth = depth; + + if (cleared) + { + gdk_gl_context_make_current (cleared); + g_object_unref (cleared); + } } return priv->egl_surface_depth; diff --git a/gdk/wayland/gdkglcontext-wayland.c b/gdk/wayland/gdkglcontext-wayland.c index 9acd6961d5..01ca529356 100644 --- a/gdk/wayland/gdkglcontext-wayland.c +++ b/gdk/wayland/gdkglcontext-wayland.c @@ -53,14 +53,9 @@ gdk_wayland_gl_context_begin_frame (GdkDrawContext *draw_context, GdkColorState **out_color_state, GdkMemoryDepth *out_depth) { - gboolean created_window; - - created_window = gdk_wayland_surface_ensure_wl_egl_window (gdk_draw_context_get_surface (draw_context)); + gdk_wayland_surface_ensure_wl_egl_window (gdk_draw_context_get_surface (draw_context)); GDK_DRAW_CONTEXT_CLASS (gdk_wayland_gl_context_parent_class)->begin_frame (draw_context, depth, region, out_color_state, out_depth); - - if (created_window) - gdk_gl_context_make_current (GDK_GL_CONTEXT (draw_context)); } static void diff --git a/gdk/wayland/gdksurface-wayland.c b/gdk/wayland/gdksurface-wayland.c index ded954e179..6343c816ec 100644 --- a/gdk/wayland/gdksurface-wayland.c +++ b/gdk/wayland/gdksurface-wayland.c @@ -1447,21 +1447,19 @@ _gdk_wayland_surface_offset_next_wl_buffer (GdkSurface *surface, impl->pending_buffer_offset_y = y; } -gboolean +void gdk_wayland_surface_ensure_wl_egl_window (GdkSurface *surface) { GdkWaylandSurface *impl = GDK_WAYLAND_SURFACE (surface); int width, height; if (impl->display_server.egl_window != NULL) - return FALSE; + return; get_egl_window_size (surface, &width, &height); impl->display_server.egl_window = wl_egl_window_create (impl->display_server.wl_surface, width, height); gdk_surface_set_egl_native_window (surface, impl->display_server.egl_window); - - return TRUE; } /* }}} */ diff --git a/gdk/wayland/gdksurface-wayland.h b/gdk/wayland/gdksurface-wayland.h index 93f77b84d7..d856000163 100644 --- a/gdk/wayland/gdksurface-wayland.h +++ b/gdk/wayland/gdksurface-wayland.h @@ -26,6 +26,6 @@ G_BEGIN_DECLS -gboolean gdk_wayland_surface_ensure_wl_egl_window (GdkSurface *surface); +void gdk_wayland_surface_ensure_wl_egl_window (GdkSurface *surface); G_END_DECLS diff --git a/gdk/x11/gdkglcontext-glx.c b/gdk/x11/gdkglcontext-glx.c index 90d87d8107..35bbd78680 100644 --- a/gdk/x11/gdkglcontext-glx.c +++ b/gdk/x11/gdkglcontext-glx.c @@ -84,10 +84,13 @@ gdk_x11_surface_get_glx_drawable (GdkSurface *surface) void gdk_x11_surface_destroy_glx_drawable (GdkX11Surface *self) { + GdkGLContext *context; + if (self->glx_drawable == None) return; - gdk_gl_context_clear_current_if_surface (GDK_SURFACE (self)); + context = gdk_gl_context_clear_current_if_surface (GDK_SURFACE (self)); + g_clear_object (&context); glXDestroyWindow (gdk_x11_display_get_xdisplay (gdk_surface_get_display (GDK_SURFACE (self))), self->glx_drawable);