Skip to content

Commit a87c857

Browse files
committed
Merge branch '3.2-egl-fixes' into 3.2
Backport wxGLCanvas fixes from master fixing regressions in 3.2.3. See wxWidgets#24018.
2 parents cc99018 + 264c499 commit a87c857

File tree

2 files changed

+55
-37
lines changed

2 files changed

+55
-37
lines changed

docs/changes.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,7 @@ wxGTK:
243243

244244
- Fix some regressions introduced in 3.2.3:
245245
* Don't crash in console apps linked with GUI libraries (#23981).
246+
* Fix possible crash and too high CPU use when using EGL (#24018).
246247
* Fix losing clipboard contents when clearing a different selection (#23988).
247248

248249
wxMSW:

src/unix/glegl.cpp

Lines changed: 54 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -453,24 +453,6 @@ void wxGLCanvasEGL::OnWLFrameCallback()
453453
#ifdef GDK_WINDOWING_WAYLAND
454454
wxLogTrace(TRACE_EGL, "In frame callback handler for %p", this);
455455

456-
if ( !gs_alreadySetSwapInterval.count(this) )
457-
{
458-
// Ensure that eglSwapBuffers() doesn't block, as we use the surface
459-
// callback to know when we should draw ourselves already.
460-
if ( eglSwapInterval(m_display, 0) )
461-
{
462-
wxLogTrace(TRACE_EGL, "Set EGL swap interval to 0 for %p", this);
463-
464-
// It shouldn't be necessary to set it again.
465-
gs_alreadySetSwapInterval.insert(this);
466-
}
467-
else
468-
{
469-
wxLogTrace(TRACE_EGL, "eglSwapInterval(0) failed for %p: %#x",
470-
this, eglGetError());
471-
}
472-
}
473-
474456
m_readyToDraw = true;
475457
g_clear_pointer(&m_wlFrameCallbackHandler, wl_callback_destroy);
476458
SendSizeEvent();
@@ -648,6 +630,7 @@ wxGLCanvasEGL::~wxGLCanvasEGL()
648630
if ( m_surface )
649631
eglDestroySurface(m_display, m_surface);
650632
#ifdef GDK_WINDOWING_WAYLAND
633+
DestroyWaylandSubsurface();
651634
g_clear_pointer(&m_wlEGLWindow, wl_egl_window_destroy);
652635
g_clear_pointer(&m_wlSurface, wl_surface_destroy);
653636

@@ -658,6 +641,21 @@ wxGLCanvasEGL::~wxGLCanvasEGL()
658641
void wxGLCanvasEGL::CreateWaylandSubsurface()
659642
{
660643
#ifdef GDK_WINDOWING_WAYLAND
644+
// It's possible that we get in here unnecessarily in two ways:
645+
// (1) If the canvas widget is shown, and then immediately hidden, we will
646+
// still receive a map-event signal, but by that point, the subsurface
647+
// does not need to be created anymore as the canvas is hidden
648+
// (2) If the canvas widget is shown, and then immediately hidden, and then
649+
// immediately shown again, we will receive two map-event signals.
650+
// By the second time we get it, the subsurface will already be created
651+
// Not ignoring either of the two scenarios will likely cause the subsurface
652+
// to be created twice, leading to a crash due to a Wayland protocol error
653+
// See https://github.com/wxWidgets/wxWidgets/issues/23961
654+
if ( !gtk_widget_get_mapped(m_widget) || m_wlSubsurface )
655+
{
656+
return;
657+
}
658+
661659
GdkWindow *window = GTKGetDrawingWindow();
662660
struct wl_surface *surface = gdk_wayland_window_get_wl_surface(window);
663661

@@ -803,42 +801,59 @@ void wxGLCanvasEGL::FreeDefaultConfig()
803801

804802
bool wxGLCanvasEGL::SwapBuffers()
805803
{
804+
// Before doing anything else, ensure that eglSwapBuffers() doesn't block:
805+
// under Wayland we don't want it to because we use the surface callback to
806+
// know when we should draw anyhow and with X11 it blocks for up to a
807+
// second when the window is entirely occluded and because we can't detect
808+
// this currently (our IsShownOnScreen() doesn't account for all cases in
809+
// which this happens) we must prevent it from blocking to avoid making the
810+
// entire application completely unusable just because one of its windows
811+
// using wxGLCanvas got occluded or unmapped (e.g. due to a move to another
812+
// workspace).
813+
if ( !gs_alreadySetSwapInterval.count(this) )
814+
{
815+
// Ensure that eglSwapBuffers() doesn't block, as we use the surface
816+
// callback to know when we should draw ourselves already.
817+
if ( eglSwapInterval(m_display, 0) )
818+
{
819+
wxLogTrace(TRACE_EGL, "Set EGL swap interval to 0 for %p", this);
820+
821+
// It shouldn't be necessary to set it again.
822+
gs_alreadySetSwapInterval.insert(this);
823+
}
824+
else
825+
{
826+
wxLogTrace(TRACE_EGL, "eglSwapInterval(0) failed for %p: %#x",
827+
this, eglGetError());
828+
}
829+
}
830+
806831
GdkWindow* const window = GTKGetDrawingWindow();
807832
#ifdef GDK_WINDOWING_X11
808833
if (wxGTKImpl::IsX11(window))
809834
{
810835
if ( !IsShownOnScreen() )
811836
{
812-
// Trying to draw on a hidden window is useless and can actually be
813-
// harmful if the compositor blocks in eglSwapBuffers() in this
814-
// case, so avoid it.
815-
wxLogTrace(TRACE_EGL, "Not drawing hidden window");
837+
// Trying to draw on a hidden window is useless.
838+
wxLogTrace(TRACE_EGL, "Window %p is hidden, not drawing", this);
816839
return false;
817840
}
818841
}
819842
#endif // GDK_WINDOWING_X11
820843
#ifdef GDK_WINDOWING_WAYLAND
821844
if (wxGTKImpl::IsWayland(window))
822845
{
823-
// Under Wayland, we must not draw before we're actually ready to, as
824-
// this would be inefficient at best and could result in a deadlock at
825-
// worst if we're called before the window is realized.
846+
// Under Wayland, we must not draw before the window has been realized,
847+
// as this could result in a deadlock inside eglSwapBuffers()
826848
if ( !m_readyToDraw )
827849
{
828-
wxLogTrace(TRACE_EGL, "Not ready to draw yet");
850+
wxLogTrace(TRACE_EGL, "Window %p is not not ready to draw yet", this);
829851
return false;
830852
}
831-
832-
// Ensure that we redraw again when the frame becomes ready.
833-
m_readyToDraw = false;
834-
wl_surface* surface = gdk_wayland_window_get_wl_surface(window);
835-
m_wlFrameCallbackHandler = wl_surface_frame(surface);
836-
wl_callback_add_listener(m_wlFrameCallbackHandler,
837-
&wl_frame_listener, this);
838853
}
839854
#endif // GDK_WINDOWING_WAYLAND
840855

841-
wxLogTrace(TRACE_EGL, "Swapping buffers");
856+
wxLogTrace(TRACE_EGL, "Swapping buffers for window %p", this);
842857

843858
return eglSwapBuffers(m_display, m_surface);
844859
}
@@ -851,10 +866,12 @@ bool wxGLCanvasEGL::IsShownOnScreen() const
851866
case wxDisplayX11:
852867
return GetXWindow() && wxGLCanvasBase::IsShownOnScreen();
853868
case wxDisplayWayland:
854-
return m_readyToDraw && wxGLCanvasBase::IsShownOnScreen();
855-
default:
856-
return false;
869+
return m_wlSubsurface && wxGLCanvasBase::IsShownOnScreen();
870+
case wxDisplayNone:
871+
break;
857872
}
873+
874+
return false;
858875
}
859876

860877
#endif // wxUSE_GLCANVAS && wxUSE_GLCANVAS_EGL

0 commit comments

Comments
 (0)