git.lucas.co / cce-compositor
Wayland compositor (wlroots)
git clone https://git.lucas.co/cce-compositor.git

commite20fd6094e1d15731e02a15898cd61f8f06747ea
parentf7d3f9b71e
authorIsaac Freund <[email protected]>
date2026-03-16 10:09
KeyboardGroup: release keys on keyboard removal

Currently we fail to send release events to window and window
manager xkb bindings if keys are pressed at the time a keyboard
is destroyed or has its keymap/repeat info changed.

This patch fixes that and causes the necessary release events to be
sent.

Reported-by: kewuaa

 river/Cursor.zig        | 10 +---------
 river/Keyboard.zig      | 23 ++++++++++++++++++++---
 river/KeyboardGroup.zig | 11 ++++++++++-
 river/Seat.zig          |  2 +-
 river/XkbBinding.zig    | 10 ++++++----
 river/util.zig          | 11 +++++++++++
 6 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/river/Cursor.zig b/river/Cursor.zig
index 57d74ac..3aad15b 100644
--- a/river/Cursor.zig
+++ b/river/Cursor.zig
@@ -786,15 +786,7 @@ pub fn updateState(cursor: *Cursor) void {
     switch (cursor.mode) {
         .passthrough, .drag => {
             cursor.updateHovered();
-            const now = posix.clock_gettime(.MONOTONIC) catch @panic("CLOCK_MONOTONIC not supported");
-            // 2^32-1 milliseconds is ~50 days, which is a realistic uptime.
-            // This means that we must wrap if the monotonic time is greater than
-            // 2^32-1 milliseconds and hope that clients don't get too confused.
-            const msec: u32 = @intCast(@rem(
-                now.sec *% std.time.ms_per_s +% @divTrunc(now.nsec, std.time.ns_per_ms),
-                math.maxInt(u32),
-            ));
-            cursor.passthrough(msec);
+            cursor.passthrough(util.msecTimestamp());
         },
         .ignore, .down, .op => {},
     }
diff --git a/river/Keyboard.zig b/river/Keyboard.zig
index e2bb194..2990e8e 100644
--- a/river/Keyboard.zig
+++ b/river/Keyboard.zig
@@ -31,6 +31,11 @@ device_destroyed: bool = false,
 queued_events: u32 = 0,
 
 config: Config,
+
+/// Set of pressed keys that have been processed by processKey().
+/// Not equivalent to wlr_keyboard.keycodes.
+/// This state is necessary to handle removing keyboards from groups properly.
+pressed: std.AutoArrayHashMapUnmanaged(u32, void) = .empty,
 /// Only null during initialization or due to allocation failure.
 group: ?*KeyboardGroup = null,
 
@@ -62,6 +67,9 @@ pub fn create(seat: *Seat, wlr_device: *wlr.InputDevice, virtual: bool) !*Keyboa
     };
     errdefer if (keyboard.config.keymap) |keymap| keymap.unref();
 
+    try keyboard.pressed.ensureTotalCapacity(util.gpa, KeyboardGroup.pressed_count_max);
+    errdefer keyboard.pressed.deinit(util.gpa);
+
     try keyboard.device.init(seat, wlr_device, virtual);
     errdefer keyboard.device.deinit();
 
@@ -110,7 +118,7 @@ pub fn setRepeatInfo(keyboard: *Keyboard, rate: u31, delay: u31) void {
     keyboard.config.repeat_rate = rate;
     keyboard.config.repeat_delay = delay;
     if (keyboard.group) |group| {
-        group.unref();
+        group.unref(keyboard.pressed.keys());
         keyboard.group = null;
     }
     keyboard.setGroup();
@@ -122,7 +130,7 @@ pub fn setKeymap(keyboard: *Keyboard, keymap: *xkb.Keymap) void {
     if (keyboard.config.keymap) |old| old.unref();
     keyboard.config.keymap = keymap.ref();
     if (keyboard.group) |group| {
-        group.unref();
+        group.unref(keyboard.pressed.keys());
         keyboard.group = null;
     }
     keyboard.setGroup();
@@ -147,8 +155,9 @@ fn maybeDestroy(keyboard: *Keyboard) void {
     }
 
     if (keyboard.config.keymap) |keymap| keymap.unref();
-    if (keyboard.group) |group| group.unref();
+    if (keyboard.group) |group| group.unref(keyboard.pressed.keys());
 
+    keyboard.pressed.deinit(util.gpa);
     util.gpa.destroy(keyboard);
 }
 
@@ -158,6 +167,14 @@ pub fn dropEvent(keyboard: *Keyboard) void {
 }
 
 pub fn processKey(keyboard: *Keyboard, key: *const wlr.Keyboard.event.Key) void {
+    if (key.state == .released) {
+        _ = keyboard.pressed.swapRemove(key.keycode);
+    } else {
+        assert(key.state == .pressed);
+        if (keyboard.pressed.count() < KeyboardGroup.pressed_count_max) {
+            keyboard.pressed.putAssumeCapacity(key.keycode, {});
+        }
+    }
     if (keyboard.group) |group| group.processKey(key);
     keyboard.dropEvent();
 }
diff --git a/river/KeyboardGroup.zig b/river/KeyboardGroup.zig
index 82fd372..f63850b 100644
--- a/river/KeyboardGroup.zig
+++ b/river/KeyboardGroup.zig
@@ -108,7 +108,16 @@ pub fn ref(group: *KeyboardGroup) *KeyboardGroup {
     return group;
 }
 
-pub fn unref(group: *KeyboardGroup) void {
+pub fn unref(group: *KeyboardGroup, to_release: []u32) void {
+    for (to_release) |keycode| {
+        group.processKey(&.{
+            .time_msec = util.msecTimestamp(),
+            .keycode = keycode,
+            .update_state = true,
+            .state = .released,
+        });
+    }
+
     group.ref_count -= 1;
     if (group.ref_count > 0) {
         return;
diff --git a/river/Seat.zig b/river/Seat.zig
index 8b09c9d..967ecb7 100644
--- a/river/Seat.zig
+++ b/river/Seat.zig
@@ -912,7 +912,7 @@ pub fn detachDevice(seat: *Seat, device: *InputDevice) void {
     if (device.wlr_device.type == .keyboard) {
         const keyboard: *Keyboard = @fieldParentPtr("device", device);
         if (keyboard.group) |group| {
-            group.unref();
+            group.unref(keyboard.pressed.keys());
             keyboard.group = null;
         }
     }
diff --git a/river/XkbBinding.zig b/river/XkbBinding.zig
index 0ba1ebb..2a278ff 100644
--- a/river/XkbBinding.zig
+++ b/river/XkbBinding.zig
@@ -137,9 +137,6 @@ fn handleRequest(
 
 pub fn pressed(binding: *XkbBinding) void {
     assert(!binding.sent_pressed);
-    // Input event processing should not continue after a state_change
-    // until that event is sent to the window manager in an update and acked.
-    assert(binding.wm_scheduled.state_change == .none);
     binding.wm_scheduled.state_change = .pressed;
     server.wm.dirtyWindowing();
 }
@@ -148,7 +145,12 @@ pub fn stopRepeat(binding: *XkbBinding) void {
     assert(binding.sent_pressed);
     // Input event processing should not continue after a state change
     // until that event is sent to the window manager in an update and acked.
-    assert(binding.wm_scheduled.state_change == .none);
+    // However, stop_repeat is special since it is triggered on any key event.
+    // This means that when a keyboard is removed from a group and all keys
+    // pressed on that keyboard are released at the same time stopRepeat()
+    // may be called more than once.
+    assert(binding.wm_scheduled.state_change == .none or
+        binding.wm_scheduled.state_change == .stop_repeat);
     binding.wm_scheduled.state_change = .stop_repeat;
     server.wm.dirtyWindowing();
 }
diff --git a/river/util.zig b/river/util.zig
index 5f62808..a713e46 100644
--- a/river/util.zig
+++ b/river/util.zig
@@ -5,3 +5,14 @@ const std = @import("std");
 
 /// The global general-purpose allocator used throughout river's code
 pub const gpa = std.heap.c_allocator;
+
+pub fn msecTimestamp() u32 {
+    const now = std.posix.clock_gettime(.MONOTONIC) catch @panic("CLOCK_MONOTONIC not supported");
+    // 2^32-1 milliseconds is ~50 days, which is a realistic uptime.
+    // This means that we must wrap if the monotonic time is greater than
+    // 2^32-1 milliseconds and hope that clients don't get too confused.
+    return @intCast(@rem(
+        now.sec *% std.time.ms_per_s +% @divTrunc(now.nsec, std.time.ns_per_ms),
+        std.math.maxInt(u32),
+    ));
+}