From 16008b950a343ece4b45089a25de076af1379e94 Mon Sep 17 00:00:00 2001 From: Kovid Goyal Date: Wed, 3 Dec 2025 22:26:09 +0530 Subject: [PATCH] Wayland: Fix spurious key repeat events when some user defined callback takes a long time to execute On compositors that support compositor key repeat events, use those, for complete robustness. Sadly no actual compositor implements these yet. Otherwise use a timer fd/pipe to queue the repeat events and only dispatch them after events from the compositor are handled. This means release events from the compositor will prevent spurious repeat events. One can, in the worst case lose some repeat events if there is a very large interval between the start of the timer and the next poll, but that is unavoidable and is why repeat events should come from the compositor in the first place. Fixes #9224 --- docs/changelog.rst | 3 +++ glfw/backend_utils.c | 29 +++++++++++++++++--- glfw/backend_utils.h | 11 +++++++- glfw/wl_init.c | 63 ++++++++++++++++++++++++++++++-------------- glfw/wl_window.c | 21 +++++++++++++++ 5 files changed, 103 insertions(+), 24 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 0277346ef..0d4bdb471 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -181,6 +181,9 @@ Detailed list of changes pager in search mode. If any text is currently selected it is automatically searched for. +- Wayland: Fix spurious key repeat events when some user defined callback takes + a long time to execute (:iss:`9224`) + 0.44.0 [2025-11-03] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/glfw/backend_utils.c b/glfw/backend_utils.c index 55ea026d6..489b7fca9 100644 --- a/glfw/backend_utils.c +++ b/glfw/backend_utils.c @@ -234,6 +234,12 @@ mark_wakep_fd_ready(int fd UNUSED, int events UNUSED, void *data) { ((EventLoopData*)(data))->wakeup_fd_ready = true; } +static void +mark_key_repeat_fd_ready(int fd UNUSED, int events UNUSED, void *data) { + ((EventLoopData*)(data))->key_repeat_fd_ready = true; +} + + bool initPollData(EventLoopData *eld, int display_fd) { if (!addWatch(eld, "display", display_fd, POLLIN, 1, NULL, NULL)) return false; @@ -246,6 +252,19 @@ initPollData(EventLoopData *eld, int display_fd) { const int wakeup_fd = eld->wakeupFds[0]; #endif if (!addWatch(eld, "wakeup", wakeup_fd, POLLIN, 1, mark_wakep_fd_ready, eld)) return false; + +#ifdef HAS_TIMER_FD + eld->key_repeat_fd = timerfd_create(CLOCK_MONOTONIC, TFD_NONBLOCK | TFD_CLOEXEC); + if (eld->key_repeat_fd < 0) return false; + const int key_repeat_fd = eld->key_repeat_fd; +#else + if (pipe2(eld->key_repeat_fds, O_CLOEXEC | O_NONBLOCK) != 0) return false; + const int key_repeat_fd = eld->key_repeat_fds[0]; +#endif + (void)key_repeat_fd; (void)mark_key_repeat_fd_ready; +#ifdef _GLFW_WAYLAND + if (!addWatch(eld, "key_repeat", key_repeat_fd, POLLIN, 1, mark_key_repeat_fd_ready, eld)) return false; +#endif return true; } @@ -269,7 +288,6 @@ wakeupEventLoop(EventLoopData *eld) { #endif } -#ifndef HAS_EVENT_FD static void closeFds(int *fds, size_t count) { while(count--) { @@ -280,15 +298,20 @@ closeFds(int *fds, size_t count) { fds++; } } -#endif void finalizePollData(EventLoopData *eld) { + (void)closeFds; #ifdef HAS_EVENT_FD close(eld->wakeupFd); eld->wakeupFd = -1; #else closeFds(eld->wakeupFds, arraysz(eld->wakeupFds)); #endif +#ifdef HAS_TIMER_FD + close(eld->key_repeat_fd); eld->key_repeat_fd = -1; +#else + closeFds(eld->key_repeat_fds, arraysz(eld->key_repeat_fds)); +#endif } int @@ -298,7 +321,7 @@ pollForEvents(EventLoopData *eld, monotonic_t timeout, watch_callback_func displ EVDBG("pollForEvents final timeout: %.3f", monotonic_t_to_s_double(timeout)); int result; monotonic_t end_time = monotonic() + timeout; - eld->wakeup_fd_ready = false; + eld->wakeup_fd_ready = false; eld->key_repeat_fd_ready = false; while(1) { if (timeout >= 0) { diff --git a/glfw/backend_utils.h b/glfw/backend_utils.h index 88ad5690c..9f09a2c24 100644 --- a/glfw/backend_utils.h +++ b/glfw/backend_utils.h @@ -36,6 +36,10 @@ #define HAS_EVENT_FD #include #endif +#if __has_include() +#define HAS_TIMER_FD +#include +#endif #else #define HAS_EVENT_FD #include @@ -73,7 +77,12 @@ typedef struct { #else int wakeupFds[2]; #endif - bool wakeup_data_read, wakeup_fd_ready; +#ifdef HAS_TIMER_FD + int key_repeat_fd; +#else + int key_repeat_fds[2]; +#endif + bool wakeup_data_read, wakeup_fd_ready, key_repeat_fd_ready; nfds_t watches_count, timers_count; Watch watches[32]; Timer timers[128]; diff --git a/glfw/wl_init.c b/glfw/wl_init.c index 7b73128c8..5e6cd0bd7 100644 --- a/glfw/wl_init.c +++ b/glfw/wl_init.c @@ -298,6 +298,41 @@ static void keyboardHandleKeymap(void* data UNUSED, } +static void +start_key_repeat_timer(bool initial) { + if (_glfw.wl.keyboardRepeatRate <= 0) return; +#ifdef HAS_TIMER_FD + (void)initial; + struct itimerspec new_value = {.it_value={.tv_nsec = _glfw.wl.keyboardRepeatDelay}, .it_interval={.tv_nsec = (s_to_monotonic_t(1ll) / (monotonic_t)_glfw.wl.keyboardRepeatRate)}}; + if (_glfw.wl.eventLoopData.key_repeat_fd > -1) timerfd_settime( + _glfw.wl.eventLoopData.key_repeat_fd, 0, &new_value, NULL); +#else + monotonic_t interval = _glfw.wl.keyboardRepeatDelay; + if (!initial) interval = (s_to_monotonic_t(1ll) / (monotonic_t)_glfw.wl.keyboardRepeatRate); + changeTimerInterval(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, interval); + toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 1); +#endif +} + +static void +stop_key_repeat_timer(void) { +#ifdef HAS_TIMER_FD + struct itimerspec new_value = {0}; + if (_glfw.wl.eventLoopData.key_repeat_fd > -1) timerfd_settime(_glfw.wl.eventLoopData.key_repeat_fd, 0, &new_value, NULL); +#else + toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 0); +#endif +} + +#ifndef HAS_TIMER_FD +static void +send_key_repeat_timer_event(id_type timer_id UNUSED, void *data UNUSED) { + char b = 1; + b += write(_glfw.wl.eventLoopData.key_repeat_fds[1], &b, 1); + if (_glfw.wl.keyboardRepeatRate > 0) start_key_repeat_timer(false); +} +#endif + static void keyboardHandleEnter(void* data UNUSED, struct wl_keyboard* keyboard UNUSED, uint32_t serial, @@ -314,7 +349,7 @@ static void keyboardHandleEnter(void* data UNUSED, if (keys && _glfw.wl.keyRepeatInfo.key) { wl_array_for_each(key, keys) { if (*key == _glfw.wl.keyRepeatInfo.key) { - if (_glfw.wl.keyboardRepeatRate > 0) toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 1); + if (_glfw.wl.keyboardRepeatRate > 0) start_key_repeat_timer(true); break; } } @@ -334,22 +369,9 @@ static void keyboardHandleLeave(void* data UNUSED, _glfw.wl.serial = serial; _glfw.wl.keyboardFocusId = 0; _glfwInputWindowFocus(window, false); - toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 0); + stop_key_repeat_timer(); } -static void -dispatchPendingKeyRepeats(id_type timer_id UNUSED, void *data UNUSED) { - if (_glfw.wl.keyRepeatInfo.keyboardFocusId != _glfw.wl.keyboardFocusId || _glfw.wl.keyboardRepeatRate == 0) return; - _GLFWwindow* window = _glfwWindowForId(_glfw.wl.keyboardFocusId); - if (!window) return; - glfw_xkb_handle_key_event(window, &_glfw.wl.xkb, _glfw.wl.keyRepeatInfo.key, GLFW_REPEAT); - if (_glfw.wl.keyboardRepeatRate > 0) { - changeTimerInterval(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, (s_to_monotonic_t(1ll) / (monotonic_t)_glfw.wl.keyboardRepeatRate)); - toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 1); - } -} - - static void keyboardHandleKey(void* data UNUSED, struct wl_keyboard* keyboard UNUSED, uint32_t serial, @@ -375,11 +397,10 @@ static void keyboardHandleKey(void* data UNUSED, { _glfw.wl.keyRepeatInfo.key = key; _glfw.wl.keyRepeatInfo.keyboardFocusId = window->id; - changeTimerInterval(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, _glfw.wl.keyboardRepeatDelay); - toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 1); + start_key_repeat_timer(true); } else if (action == GLFW_RELEASE && key == _glfw.wl.keyRepeatInfo.key) { _glfw.wl.keyRepeatInfo.key = 0; - toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 0); + stop_key_repeat_timer(); } } @@ -447,7 +468,7 @@ static void seatHandleCapabilities(void* data UNUSED, wl_keyboard_destroy(_glfw.wl.keyboard); _glfw.wl.keyboard = NULL; _glfw.wl.keyboardFocusId = 0; - if (_glfw.wl.keyRepeatInfo.keyRepeatTimer) toggleTimer(&_glfw.wl.eventLoopData, _glfw.wl.keyRepeatInfo.keyRepeatTimer, 0); + stop_key_repeat_timer(); } } @@ -854,7 +875,9 @@ int _glfwPlatformInit(bool *supports_window_occlusion) } glfw_dbus_init(&_glfw.wl.dbus, &_glfw.wl.eventLoopData); glfw_initialize_desktop_settings(); - _glfw.wl.keyRepeatInfo.keyRepeatTimer = addTimer(&_glfw.wl.eventLoopData, "wayland-key-repeat", ms_to_monotonic_t(500ll), 0, true, dispatchPendingKeyRepeats, NULL, NULL); +#ifndef HAS_TIMER_FD + _glfw.wl.keyRepeatInfo.keyRepeatTimer = addTimer(&_glfw.wl.eventLoopData, "wayland-key-repeat", ms_to_monotonic_t(500ll), 0, true, send_key_repeat_timer_event, NULL, NULL); +#endif _glfw.wl.cursorAnimationTimer = addTimer(&_glfw.wl.eventLoopData, "wayland-cursor-animation", ms_to_monotonic_t(500ll), 0, true, animateCursorImage, NULL, NULL); _glfw.wl.registry = wl_display_get_registry(_glfw.wl.display); diff --git a/glfw/wl_window.c b/glfw/wl_window.c index 402d379d6..7632cab74 100644 --- a/glfw/wl_window.c +++ b/glfw/wl_window.c @@ -41,6 +41,7 @@ #include #include #include +#include #define debug debug_rendering @@ -1306,6 +1307,25 @@ wayland_read_events(int poll_result, int events, void *data UNUSED) { else wl_display_cancel_read(_glfw.wl.display); } +static void +handle_key_repeat_events(void) { + uint64_t num_events = 0; +#ifdef HAS_TIMER_FD + if (read(_glfw.wl.eventLoopData.key_repeat_fd, &num_events, sizeof(num_events)) < (ssize_t)sizeof(num_events)) return; +#else + char buf[16]; + while(1) { + ssize_t num = read(_glfw.wl.eventLoopData.key_repeat_fds[0], buf, sizeof(buf)); + if (num > 0) num_events += num; + else if (num == 0 || errno != EINTR) break; + } +#endif + if (_glfw.wl.keyRepeatInfo.keyboardFocusId != _glfw.wl.keyboardFocusId || _glfw.wl.keyboardRepeatRate == 0) return; + _GLFWwindow* window = _glfwWindowForId(_glfw.wl.keyboardFocusId); + if (!window || !_glfw.wl.keyRepeatInfo.key) return; + while (num_events--) glfw_xkb_handle_key_event(window, &_glfw.wl.xkb, _glfw.wl.keyRepeatInfo.key, GLFW_REPEAT); +} + static void handleEvents(monotonic_t timeout) { struct wl_display* display = _glfw.wl.display; @@ -1345,6 +1365,7 @@ static void handleEvents(monotonic_t timeout) glfw_dbus_session_bus_dispatch(); EVDBG("other dispatch done"); if (_glfw.wl.eventLoopData.wakeup_fd_ready) check_for_wakeup_events(&_glfw.wl.eventLoopData); + if (_glfw.wl.eventLoopData.key_repeat_fd_ready) handle_key_repeat_events(); } static struct wl_cursor*