From 408a93a1ec9766ff8c527d9ed2d61bcee565e1cb Mon Sep 17 00:00:00 2001
From: Cameron Gutman <[EMAIL REDACTED]>
Date: Sat, 23 Oct 2021 15:43:04 -0500
Subject: [PATCH] wayland: Use multi-thread event reading APIs
Wayland provides the prepare_read()/read_events() family of APIs for
reading from the display fd in a deadlock-free manner across multiple
threads in a multi-threaded application. Let's use those instead of
trying to roll our own solution using a mutex.
This fixes an issue where a call to SDL_GL_SwapWindow() doesn't swap
buffers if it happens to collide with SDL_PumpEvents() in the main
thread. It also allows coexistence with other code or toolkits in
our process that may want read and dispatch events themselves.
---
src/video/wayland/SDL_waylandevents.c | 22 +++++++++----------
src/video/wayland/SDL_waylandopengles.c | 29 +++++++++++++------------
src/video/wayland/SDL_waylandsym.h | 4 ++++
src/video/wayland/SDL_waylandvideo.c | 3 ---
src/video/wayland/SDL_waylandvideo.h | 1 -
5 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index 11a4ebdbc7..7a03039f29 100644
--- a/src/video/wayland/SDL_waylandevents.c
+++ b/src/video/wayland/SDL_waylandevents.c
@@ -231,20 +231,18 @@ Wayland_PumpEvents(_THIS)
keyboard_repeat_handle(&input->keyboard_repeat, now);
}
- /* If we're trying to dispatch the display in another thread,
- * we could trigger a race condition and end up blocking
- * in wl_display_dispatch() */
- if (SDL_TryLockMutex(d->display_dispatch_lock) != 0) {
- return;
- }
-
- if (SDL_IOReady(WAYLAND_wl_display_get_fd(d->display), SDL_FALSE, 0)) {
- err = WAYLAND_wl_display_dispatch(d->display);
- } else {
- err = WAYLAND_wl_display_dispatch_pending(d->display);
+ /* wl_display_prepare_read() will return -1 if the default queue is not empty.
+ * If the default queue is empty, it will prepare us for our SDL_IOReady() call. */
+ if (WAYLAND_wl_display_prepare_read(d->display) == 0) {
+ if (SDL_IOReady(WAYLAND_wl_display_get_fd(d->display), SDL_FALSE, 0) > 0) {
+ WAYLAND_wl_display_read_events(d->display);
+ } else {
+ WAYLAND_wl_display_cancel_read(d->display);
+ }
}
- SDL_UnlockMutex(d->display_dispatch_lock);
+ /* Dispatch any pre-existing pending events or new events we may have read */
+ err = WAYLAND_wl_display_dispatch_pending(d->display);
if (err == -1 && !d->display_disconnected) {
/* Something has failed with the Wayland connection -- for example,
diff --git a/src/video/wayland/SDL_waylandopengles.c b/src/video/wayland/SDL_waylandopengles.c
index c6d5659105..934406f435 100644
--- a/src/video/wayland/SDL_waylandopengles.c
+++ b/src/video/wayland/SDL_waylandopengles.c
@@ -136,33 +136,34 @@ Wayland_GLES_SwapWindow(_THIS, SDL_Window *window)
while (SDL_AtomicGet(&data->swap_interval_ready) == 0) {
Uint32 now;
- /* !!! FIXME: this is just the crucial piece of Wayland_PumpEvents */
WAYLAND_wl_display_flush(display);
- if (WAYLAND_wl_display_dispatch_queue_pending(display, data->frame_event_queue) > 0) {
- /* We dispatched some pending events. Check if the frame callback happened. */
+
+ /* wl_display_prepare_read_queue() will return -1 if the event queue is not empty.
+ * If the event queue is empty, it will prepare us for our SDL_IOReady() call. */
+ if (WAYLAND_wl_display_prepare_read_queue(display, data->frame_event_queue) != 0) {
+ /* We have some pending events. Check if the frame callback happened. */
+ WAYLAND_wl_display_dispatch_queue_pending(display, data->frame_event_queue);
continue;
}
+ /* Beyond this point, we must either call wl_display_cancel_read() or wl_display_read_events() */
+
now = SDL_GetTicks();
if (SDL_TICKS_PASSED(now, max_wait)) {
- /* Timeout expired */
+ /* Timeout expired. Cancel the read. */
+ WAYLAND_wl_display_cancel_read(display);
break;
}
- /* Make sure we're not competing with SDL_PumpEvents() for any new
- * events, or one of us may end up blocking in wl_display_dispatch */
- if (SDL_TryLockMutex(videodata->display_dispatch_lock) != 0) {
- continue;
- }
-
if (SDL_IOReady(WAYLAND_wl_display_get_fd(display), SDL_FALSE, max_wait - now) <= 0) {
- /* Error or timeout expired without any events for us */
- SDL_UnlockMutex(videodata->display_dispatch_lock);
+ /* Error or timeout expired without any events for us. Cancel the read. */
+ WAYLAND_wl_display_cancel_read(display);
break;
}
- WAYLAND_wl_display_dispatch_queue(display, data->frame_event_queue);
- SDL_UnlockMutex(videodata->display_dispatch_lock);
+ /* We have events. Read and dispatch them. */
+ WAYLAND_wl_display_read_events(display);
+ WAYLAND_wl_display_dispatch_queue_pending(display, data->frame_event_queue);
}
SDL_AtomicSet(&data->swap_interval_ready, 0);
}
diff --git a/src/video/wayland/SDL_waylandsym.h b/src/video/wayland/SDL_waylandsym.h
index 37c4b42ddf..d6e6a761d3 100644
--- a/src/video/wayland/SDL_waylandsym.h
+++ b/src/video/wayland/SDL_waylandsym.h
@@ -56,6 +56,10 @@ SDL_WAYLAND_SYM(int, wl_display_dispatch, (struct wl_display *))
SDL_WAYLAND_SYM(int, wl_display_dispatch_queue, (struct wl_display *, struct wl_event_queue *))
SDL_WAYLAND_SYM(int, wl_display_dispatch_queue_pending, (struct wl_display *, struct wl_event_queue *))
SDL_WAYLAND_SYM(int, wl_display_dispatch_pending, (struct wl_display *))
+SDL_WAYLAND_SYM(int, wl_display_prepare_read, (struct wl_display *))
+SDL_WAYLAND_SYM(int, wl_display_prepare_read_queue, (struct wl_display *, struct wl_event_queue *))
+SDL_WAYLAND_SYM(int, wl_display_read_events, (struct wl_display *))
+SDL_WAYLAND_SYM(void, wl_display_cancel_read, (struct wl_display *))
SDL_WAYLAND_SYM(int, wl_display_get_error, (struct wl_display *))
SDL_WAYLAND_SYM(int, wl_display_flush, (struct wl_display *))
SDL_WAYLAND_SYM(int, wl_display_roundtrip, (struct wl_display *))
diff --git a/src/video/wayland/SDL_waylandvideo.c b/src/video/wayland/SDL_waylandvideo.c
index df915eea68..03464d1938 100644
--- a/src/video/wayland/SDL_waylandvideo.c
+++ b/src/video/wayland/SDL_waylandvideo.c
@@ -169,7 +169,6 @@ Wayland_DeleteDevice(SDL_VideoDevice *device)
WAYLAND_wl_display_flush(data->display);
WAYLAND_wl_display_disconnect(data->display);
}
- SDL_DestroyMutex(data->display_dispatch_lock);
SDL_free(data);
SDL_free(device);
SDL_WAYLAND_UnloadSymbols();
@@ -202,8 +201,6 @@ Wayland_CreateDevice(int devindex)
data->display = display;
- data->display_dispatch_lock = SDL_CreateMutex();
-
/* Initialize all variables that we clean on shutdown */
device = SDL_calloc(1, sizeof(SDL_VideoDevice));
if (!device) {
diff --git a/src/video/wayland/SDL_waylandvideo.h b/src/video/wayland/SDL_waylandvideo.h
index 42da52e0e4..2d8b6323a1 100644
--- a/src/video/wayland/SDL_waylandvideo.h
+++ b/src/video/wayland/SDL_waylandvideo.h
@@ -86,7 +86,6 @@ typedef struct {
char *classname;
int relative_mouse_mode;
- SDL_mutex *display_dispatch_lock;
} SDL_VideoData;
typedef struct {