From 238dd2a8ea144db52f383f700ebaf710adfcc55e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Joan=20Bruguera=20Mic=C3=B3?= Date: Sun, 15 Oct 2023 23:00:33 +0000 Subject: [PATCH] Ignore spurious map-events on Wayland's wxGLCanvasEGL In the recent changes for handling map/unmap events on Wayland's wxGLCanvasEGL, we use the following GTK widget signals: * The "map-event" signal to create the canvas's subsurface. The earlier "map" signal can not be used, as the associated toplevel window's Wayland surface may not yet exist by the time we receive it. * The "unmap" signal to destroy the canvas's subsurface. Using the later "unmap-event" signal is problematic, due to other resources we build upon being already destroyed by then. Usually, the "map-event" signal comes before "unmap" and resources are created and destroyed appropriately. However, there's an edge case: If a canvas is shown and then immediately hidden (before wxWidgets can pump from the event loop), "unmap" will come before "map-event". This happens because signals like "map" and "unmap" are delivered immediately (when calling e.g. `gtk_widget_hide`), while signals like "map-event" and "unmap-event" are delivered later on the event loop. For the same reason, showing a canvas, then immediately hiding it, then immediately showing it again, will cause two "map-event"s to get delivered enqueued without a "unmap" in between. This condition can be hit quite easily when setting up a complex UIs, and in particular it is triggered by Aegisub during startup, leading to a crash (Wayland protocol error) when opening a video later, or when specifying a video directly on the startup command line. To avoid this breaking our resource management, add some checks to detect those "map-event"s we shouldn't handle - either the ones that happen after "unmap", or the duplicate ones without an "unmap" in between. See #23961, #23968. (cherry picked from commit 133f7731b25df46bd99519e7c52cf4a90ca2cc1a) --- src/unix/glegl.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/unix/glegl.cpp b/src/unix/glegl.cpp index 25cf9a0655..4b4e316eb9 100644 --- a/src/unix/glegl.cpp +++ b/src/unix/glegl.cpp @@ -641,6 +641,21 @@ wxGLCanvasEGL::~wxGLCanvasEGL() void wxGLCanvasEGL::CreateWaylandSubsurface() { #ifdef GDK_WINDOWING_WAYLAND + // It's possible that we get in here unnecessarily in two ways: + // (1) If the canvas widget is shown, and then immediately hidden, we will + // still receive a map-event signal, but by that point, the subsurface + // does not need to be created anymore as the canvas is hidden + // (2) If the canvas widget is shown, and then immediately hidden, and then + // immediately shown again, we will receive two map-event signals. + // By the second time we get it, the subsurface will already be created + // Not ignoring either of the two scenarios will likely cause the subsurface + // to be created twice, leading to a crash due to a Wayland protocol error + // See https://github.com/wxWidgets/wxWidgets/issues/23961 + if ( !gtk_widget_get_mapped(m_widget) || m_wlSubsurface ) + { + return; + } + GdkWindow *window = GTKGetDrawingWindow(); struct wl_surface *surface = gdk_wayland_window_get_wl_surface(window);