From 3f646ec42bee39056549098b77aceed7a00a26a4 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Sun, 1 Nov 2026 00:11:46 +0100 Subject: [PATCH] fix(wayland): the first key repeat lands at the delay, not as a burst of the periods since the press keyboard_key set lastFireTime to the press and TickKeyRepeats caught up on every period since it, so the first tick past the compositor's delay fired floor(delay / period) repeats at once: with rate 25 and delay 600 a held Backspace deleted sixteen characters at the delay mark and a held letter put fifteen copies in the field, from a hardware keyboard and from an on-screen keyboard that sends Backspace as key events. A rate above 1000 made the period zero and the loop endless. The first repeat now lands at the delay, as wl_keyboard.repeat_info promises (one after `delay`, then `rate` a second): the first tick past the delay sets lastFireTime to pressTime + delay - period. The period is at least a millisecond, and the catch-up after a stalled frame is capped (kKeyRepeatCatchUp, three) with the rest of the stall dropped, so a held key is never a burst of edits. TickKeyRepeats takes the clock as a parameter for the tests; the frame path's call is unchanged. tests/KeyRepeat drives TickKeyRepeats with an injected clock on a default-constructed Window: nothing before the delay, one at it, one per period after, a capped catch-up, a key without text, a rate above 1000, a released key, rate 0. --- implementations/Crafter.Graphics-Device.cpp | 35 +++++-- interfaces/Crafter.Graphics-Device.cppm | 5 + project.cpp | 7 ++ tests/KeyRepeat/main.cpp | 105 ++++++++++++++++++++ 4 files changed, 143 insertions(+), 9 deletions(-) create mode 100644 tests/KeyRepeat/main.cpp diff --git a/implementations/Crafter.Graphics-Device.cpp b/implementations/Crafter.Graphics-Device.cpp index a5876d3..87e738d 100644 --- a/implementations/Crafter.Graphics-Device.cpp +++ b/implementations/Crafter.Graphics-Device.cpp @@ -634,6 +634,7 @@ void Device::keyboard_key(void *data, wl_keyboard *keyboard, uint32_t serial, ui keyRepeat.utf8 = std::move(utf8); keyRepeat.pressTime = std::chrono::steady_clock::now(); keyRepeat.lastFireTime = keyRepeat.pressTime; + keyRepeat.fired = false; } else { keyboardWindow->heldKeys.erase(code); keyboardWindow->onRawKeyUp.Invoke(code); @@ -659,28 +660,44 @@ void Device::keyboard_repeat_info(void *data, wl_keyboard *keyboard, int32_t rat } void Device::TickKeyRepeats() { + TickKeyRepeats(std::chrono::steady_clock::now()); +} + +void Device::TickKeyRepeats(std::chrono::steady_clock::time_point now) { if (!keyRepeat.active || !keyboardWindow) return; if (keyRepeat.rate <= 0) return; - auto now = std::chrono::steady_clock::now(); using ms = std::chrono::milliseconds; - auto sincePress = std::chrono::duration_cast(now - keyRepeat.pressTime).count(); - if (sincePress < keyRepeat.delay) return; + const auto delay = ms(keyRepeat.delay); + if (now - keyRepeat.pressTime < delay) return; - auto period = std::chrono::milliseconds(1000 / keyRepeat.rate); - auto sinceLastFire = std::chrono::duration_cast(now - keyRepeat.lastFireTime).count(); - if (sinceLastFire < period.count()) return; + // At least a millisecond: a compositor's rate above 1000 would make the + // period zero and the loop below endless. + const auto period = ms(std::max(1, 1000 / keyRepeat.rate)); - // Catch up — emit one event per missed period so a paused frame doesn't - // make the repeat permanently lag behind. - while (now - keyRepeat.lastFireTime >= period) { + // The first repeat lands at the delay — wl_keyboard.repeat_info's + // contract: one after `delay`, then `rate` a second — so the delay is not + // counted as periods missed. + if (!keyRepeat.fired) { + keyRepeat.lastFireTime = keyRepeat.pressTime + delay - period; + keyRepeat.fired = true; + } + if (now - keyRepeat.lastFireTime < period) return; + + // Catch up — one event per missed period, so a paused frame doesn't make + // the repeat permanently lag behind — but a few at most: a long stall is + // not a burst of edits, the rest of it is dropped. + int fires = 0; + while (now - keyRepeat.lastFireTime >= period && fires < kKeyRepeatCatchUp) { keyboardWindow->onRawKeyDown.Invoke(keyRepeat.key); keyboardWindow->onRawKeyHold.Invoke(keyRepeat.key); if (!keyRepeat.utf8.empty()) { keyboardWindow->onTextInput.Invoke(keyRepeat.utf8); } keyRepeat.lastFireTime += period; + ++fires; } + if (now - keyRepeat.lastFireTime >= period) keyRepeat.lastFireTime = now; } void Device::ForgetTouches() { diff --git a/interfaces/Crafter.Graphics-Device.cppm b/interfaces/Crafter.Graphics-Device.cppm index a277188..c0b7408 100644 --- a/interfaces/Crafter.Graphics-Device.cppm +++ b/interfaces/Crafter.Graphics-Device.cppm @@ -34,7 +34,11 @@ export namespace Crafter { std::string utf8; // UTF-8 to re-emit as onTextInput, if any std::chrono::time_point pressTime; std::chrono::time_point lastFireTime; + bool fired = false; // a repeat has fired since the press: before it, lastFireTime is the press itself }; + // Repeats one tick may emit at once when frames stalled; the rest of the + // stall is dropped, so a held key is never a burst of edits. + inline constexpr int kKeyRepeatCatchUp = 3; #endif #ifndef CRAFTER_GRAPHICS_WINDOW_DOM @@ -564,6 +568,7 @@ export namespace Crafter { #ifdef CRAFTER_GRAPHICS_WINDOW_WAYLAND inline static KeyRepeatState keyRepeat; static void TickKeyRepeats(); + static void TickKeyRepeats(std::chrono::steady_clock::time_point now); // the clock injected, for the tests #else static void TickKeyRepeats() {} #endif diff --git a/project.cpp b/project.cpp index 9eac5c6..624c1a7 100644 --- a/project.cpp +++ b/project.cpp @@ -439,6 +439,13 @@ extern "C" Configuration CrafterBuildProject(std::span a if (!windows) { cfg.tests.push_back(LibraryTest("MouseScroll")); + // Key repeat as wl_keyboard.repeat_info promises it: the first + // repeat at the delay (not a burst that counts the delay as missed + // periods), one per period after, a capped catch-up after a stall, + // a period of at least a millisecond. Drives TickKeyRepeats with + // an injected clock; no compositor. + cfg.tests.push_back(LibraryTest("KeyRepeat")); + // The pointer's enter, move and leave through the helpers both // native backends report with (Window::PointerEnter and co.). // Drives the wl_pointer callbacks directly, as MouseScroll does. diff --git a/tests/KeyRepeat/main.cpp b/tests/KeyRepeat/main.cpp new file mode 100644 index 0000000..0e53511 --- /dev/null +++ b/tests/KeyRepeat/main.cpp @@ -0,0 +1,105 @@ +//SPDX-License-Identifier: LGPL-3.0-only +//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Key repeat as wl_keyboard.repeat_info promises it: one repeat after `delay` +// milliseconds, then `rate` a second. The first implementation set the last +// fire time to the press and let the catch-up loop count the delay as missed +// periods, so the first repeat was a burst of floor(delay / period) events — +// a held Backspace deleted a dozen characters at once — and a rate above 1000 +// made the period zero and the loop endless. Drives Device::TickKeyRepeats +// with an injected clock on a default-constructed Window (inert without a +// compositor): no compositor, no keyboard needed. + +#include + +import Crafter.Graphics; +import Crafter.Event; +import std; +using namespace Crafter; +using namespace std::chrono_literals; + +namespace { + +int failures = 0; + +void Check(bool ok, std::string_view what) { + std::println("{} {}", ok ? "PASS" : "FAIL", what); + if (!ok) ++failures; +} + +using Clock = std::chrono::steady_clock; + +// A press of `key` at t0 with the compositor's rate and delay, as keyboard_key records it. +void Press(KeyCode key, std::string utf8, int rate, int delay, Clock::time_point t0) { + Device::keyRepeat.rate = rate; + Device::keyRepeat.delay = delay; + Device::keyRepeat.active = rate > 0; + Device::keyRepeat.key = key; + Device::keyRepeat.utf8 = std::move(utf8); + Device::keyRepeat.pressTime = t0; + Device::keyRepeat.lastFireTime = t0; + Device::keyRepeat.fired = false; +} + +} // namespace + +int main() { + Window window; // default ctor creates no surface — safe without a compositor + int downs = 0; + int texts = 0; + EventListener down(&window.onRawKeyDown, [&](KeyCode) { ++downs; }); + EventListener text(&window.onTextInput, [&](const std::string_view) { ++texts; }); + Device::keyboardWindow = &window; + + const Clock::time_point t0 = Clock::now(); + + // rate 25 (a 40 ms period), delay 600: wlroots' and KWin's defaults. + Press(14, "a", 25, 600, t0); + Device::TickKeyRepeats(t0 + 30ms); + Device::TickKeyRepeats(t0 + 599ms); + Check(downs == 0 && texts == 0, "nothing repeats before the delay"); + Device::TickKeyRepeats(t0 + 600ms); + Check(downs == 1 && texts == 1, "one repeat at the delay, not a burst of fifteen"); + Device::TickKeyRepeats(t0 + 620ms); + Check(downs == 1, "none before the next period"); + Device::TickKeyRepeats(t0 + 640ms); + Check(downs == 2, "one per period from there"); + Device::TickKeyRepeats(t0 + 680ms); + Device::TickKeyRepeats(t0 + 720ms); + Check(downs == 4, "a tick per period fires once each"); + + // Frames stalled for a second: a few catch up, the rest of the stall is dropped. + Device::TickKeyRepeats(t0 + 1720ms); + Check(downs == 4 + kKeyRepeatCatchUp, "a stall catches up by a few repeats, not one per missed period"); + Device::TickKeyRepeats(t0 + 1730ms); + Check(downs == 4 + kKeyRepeatCatchUp, "and the stall's remainder is dropped"); + Device::TickKeyRepeats(t0 + 1760ms); + Check(downs == 5 + kKeyRepeatCatchUp, "the next period after the stall fires once"); + + // A key with no text (Backspace): the key event alone. + downs = texts = 0; + Press(14, "", 25, 500, t0); + Device::TickKeyRepeats(t0 + 500ms); + Check(downs == 1 && texts == 0, "a key without text repeats the key alone"); + + // A rate above 1000 a second: a period of a millisecond, no endless loop. + downs = 0; + Press(14, "", 2000, 100, t0); + Device::TickKeyRepeats(t0 + 100ms); + Check(downs == 1, "a rate above 1000 repeats once at the delay"); + Device::TickKeyRepeats(t0 + 200ms); + Check(downs == 1 + kKeyRepeatCatchUp, "and a tick late by many periods fires the cap, not forever"); + + // Released: nothing more. + Device::keyRepeat.active = false; + Device::TickKeyRepeats(t0 + 300ms); + Check(downs == 1 + kKeyRepeatCatchUp, "a released key repeats no more"); + + // A compositor that disables repeat (rate 0). + Press(14, "", 0, 100, t0); + Device::TickKeyRepeats(t0 + 1000ms); + Check(downs == 1 + kKeyRepeatCatchUp, "rate 0 is no repeat"); + + Device::keyboardWindow = nullptr; + return failures == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +}