SDL: wayland: Avoid spurious key repeats when not pumping events

From 9e6249fa5460364fce0133866a1dc7529a7d906c Mon Sep 17 00:00:00 2001
From: Joan Bruguera <[EMAIL REDACTED]>
Date: Sat, 8 Jan 2022 19:24:47 +0100
Subject: [PATCH] wayland: Avoid spurious key repeats when not pumping events

Previous to this commit, key repeats events were typically generated when
pumping events, based on the time of when the events are pumped. However,
if an application doesn't call `SDL_PumpEvents` for some seconds, this time
can be multiple seconds in the future compared to the actual key up event time,
and generates key repeats even if a key was pressed only for an instant.

In practice, this can happen when the user presses a key which causes the
application to do something without pumping events (e.g. load a level).
In Crispy Doom & PrBoom+, when the user presses the key bound to "Restart
level/demo", the game doesn't pump events during the "screen melt" effect,
and the level is restarted multiple times due to spurious repeats.

To fix this, if the key up event is among the events to be pumped, we generate
the key repeats there, since in the Wayland callback we receive the time when
the key up event happened. Otherwise, we know no key up event happened and we
can generate as many repeats as necessary after pumping.

Signed-off-by: Joan Bruguera <joanbrugueram@gmail.com>
---
 src/video/wayland/SDL_waylandevents.c   | 26 ++++++++++++++++---------
 src/video/wayland/SDL_waylandevents_c.h |  1 +
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/video/wayland/SDL_waylandevents.c b/src/video/wayland/SDL_waylandevents.c
index 1ee8da93a89..bb726f45c14 100644
--- a/src/video/wayland/SDL_waylandevents.c
+++ b/src/video/wayland/SDL_waylandevents.c
@@ -229,12 +229,13 @@ keyboard_repeat_clear(SDL_WaylandKeyboardRepeat* repeat_info) {
 }
 
 static void
-keyboard_repeat_set(SDL_WaylandKeyboardRepeat* repeat_info,
+keyboard_repeat_set(SDL_WaylandKeyboardRepeat* repeat_info, uint32_t wl_press_time,
                     uint32_t scancode, SDL_bool has_text, char text[8]) {
     if (!repeat_info->is_initialized || !repeat_info->repeat_rate) {
         return;
     }
     repeat_info->is_key_down = SDL_TRUE;
+    repeat_info->wl_press_time = wl_press_time;
     repeat_info->sdl_press_time = SDL_GetTicks();
     repeat_info->next_repeat_ms = repeat_info->repeat_delay;
     repeat_info->scancode = scancode;
@@ -348,11 +349,6 @@ Wayland_PumpEvents(_THIS)
     }
 #endif
 
-    if (input && keyboard_repeat_is_set(&input->keyboard_repeat)) {
-        uint32_t elapsed = SDL_GetTicks() - input->keyboard_repeat.sdl_press_time;
-        keyboard_repeat_handle(&input->keyboard_repeat, elapsed);
-    }
-
     /* 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) {
@@ -366,6 +362,11 @@ Wayland_PumpEvents(_THIS)
     /* Dispatch any pre-existing pending events or new events we may have read */
     err = WAYLAND_wl_display_dispatch_pending(d->display);
 
+    if (input && keyboard_repeat_is_set(&input->keyboard_repeat)) {
+        uint32_t elapsed = SDL_GetTicks() - input->keyboard_repeat.sdl_press_time;
+        keyboard_repeat_handle(&input->keyboard_repeat, elapsed);
+    }
+
     if (err == -1 && !d->display_disconnected) {
         /* Something has failed with the Wayland connection -- for example,
          * the compositor may have shut down and closed its end of the socket,
@@ -959,6 +960,15 @@ keyboard_handle_key(void *data, struct wl_keyboard *keyboard,
 
     if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
         has_text = keyboard_input_get_text(text, input, key, &handled_by_ime);
+    } else {
+        if (keyboard_repeat_is_set(&input->keyboard_repeat)) {
+            // Send any due key repeat events before stopping the repeat and generating the key up event
+            // Compute time based on the Wayland time, as it reports when the release event happened
+            // Using SDL_GetTicks would be wrong, as it would report when the release event is processed,
+            // which may be off if the application hasn't pumped events for a while
+            keyboard_repeat_handle(&input->keyboard_repeat, time - input->keyboard_repeat.wl_press_time);
+            keyboard_repeat_clear(&input->keyboard_repeat);
+        }
     }
 
     if (!handled_by_ime && key < SDL_arraysize(xfree86_scancode_table2)) {
@@ -977,9 +987,7 @@ keyboard_handle_key(void *data, struct wl_keyboard *keyboard,
                 SDL_SendKeyboardText(text);
             }
         }
-        keyboard_repeat_set(&input->keyboard_repeat, scancode, has_text, text);
-    } else {
-        keyboard_repeat_clear(&input->keyboard_repeat);
+        keyboard_repeat_set(&input->keyboard_repeat, time, scancode, has_text, text);
     }
 }
 
diff --git a/src/video/wayland/SDL_waylandevents_c.h b/src/video/wayland/SDL_waylandevents_c.h
index a106823c7f8..24256a7d024 100644
--- a/src/video/wayland/SDL_waylandevents_c.h
+++ b/src/video/wayland/SDL_waylandevents_c.h
@@ -36,6 +36,7 @@ typedef struct {
     SDL_bool is_initialized;
 
     SDL_bool is_key_down;
+    uint32_t wl_press_time; // Key press time as reported by the Wayland API
     uint32_t sdl_press_time; // Key press time expressed in SDL ticks
     uint32_t next_repeat_ms;
     uint32_t scancode;