Commit 408a93a1ec9766ff8c527d9ed2d61bcee565e1cb
2021-10-23T15:43:04
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.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136
diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index 11a4ebd..7a03039 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 c6d5659..934406f 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 37c4b42..d6e6a76 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 df915ee..03464d1 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 42da52e..2d8b632 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 {