Keyboard: ignore >32 simultaneous key presses

wlroots implements this behavior with its key press tracking but
continues to forward the events to the compositor. Matching the wlroots
behavior here seems like the best way to avoid strange edge cases and
this is unlikely to ever be an annoying limit in practice.

Also take this oppurtunity to finally refactor away the hasMapping()
function in a way that doesn't sacrifice correctness even when hitting
this 32 key press limit.
This commit is contained in:
Isaac Freund 2024-02-19 11:33:38 +01:00
parent e1970e4d52
commit ec9a1b4303
No known key found for this signature in database
GPG Key ID: 86DED400DDFD7A11
2 changed files with 27 additions and 37 deletions

@ -47,19 +47,25 @@ pub const Pressed = struct {
pub const capacity = 32; pub const capacity = 32;
comptime { comptime {
// wlroots uses a buffer of length 32 for pressed keys and asserts that it does not overflow. // wlroots uses a buffer of length 32 to track pressed keys and does not track pressed
// keys beyond that limit. It seems likely that this can cause some inconsistency within
// wlroots in the case that someone has 32 fingers and the hardware supports N-key rollover.
//
// Furthermore, wlroots will continue to forward key press/release events to river if more
// than 32 keys are pressed. Therefore river chooses to ignore keypresses that would take
// the keyboard beyond 32 simultaneously pressed keys.
assert(capacity == @typeInfo(std.meta.fieldInfo(wlr.Keyboard, .keycodes).type).Array.len); assert(capacity == @typeInfo(std.meta.fieldInfo(wlr.Keyboard, .keycodes).type).Array.len);
} }
keys: std.BoundedArray(Key, capacity) = .{}, keys: std.BoundedArray(Key, capacity) = .{},
pub fn add(pressed: *Pressed, new: Key) void { fn addAssumeCapacity(pressed: *Pressed, new: Key) void {
for (pressed.keys.constSlice()) |item| assert(new.code != item.code); for (pressed.keys.constSlice()) |item| assert(new.code != item.code);
pressed.keys.append(new) catch unreachable; // see comptime assert above pressed.keys.appendAssumeCapacity(new);
} }
pub fn remove(pressed: *Pressed, code: u32) ?KeyConsumer { fn remove(pressed: *Pressed, code: u32) ?KeyConsumer {
for (pressed.keys.constSlice(), 0..) |item, idx| { for (pressed.keys.constSlice(), 0..) |item, idx| {
if (item.code == code) return pressed.keys.swapRemove(idx).consumer; if (item.code == code) return pressed.keys.swapRemove(idx).consumer;
} }
@ -177,15 +183,18 @@ fn handleKey(listener: *wl.Listener(*wlr.Keyboard.event.Key), event: *wlr.Keyboa
const consumer: KeyConsumer = blk: { const consumer: KeyConsumer = blk: {
// Decision is made on press; release only follows it // Decision is made on press; release only follows it
if (released) break :blk self.pressed.remove(event.keycode) orelse { if (released) {
// This can happen for example when switching from a different tty // The released key might not be in the pressed set when switching from a different tty
return; // or if the press was ignored due to >32 keys being pressed simultaneously.
}; break :blk self.pressed.remove(event.keycode) orelse return;
}
if (self.device.seat.hasMapping(keycode, modifiers, released, xkb_state)) { // Ignore key presses beyond 32 simultaneously pressed keys (see comments in Pressed).
// The key must be added to the set of pressed keys with the correct consumer before // We must ensure capacity before calling handleMapping() to ensure that we either run
// the mapping is run. Otherwise, the mapping may, for example, trigger a focus change // both the press and release mapping for certain key or neither mapping.
// which sends an incorrect wl_keyboard.enter event. self.pressed.keys.ensureUnusedCapacity(1) catch return;
if (self.device.seat.handleMapping(keycode, modifiers, released, xkb_state)) {
break :blk .mapping; break :blk .mapping;
} else if (self.getInputMethodGrab() != null) { } else if (self.getInputMethodGrab() != null) {
break :blk .im_grab; break :blk .im_grab;
@ -194,14 +203,14 @@ fn handleKey(listener: *wl.Listener(*wlr.Keyboard.event.Key), event: *wlr.Keyboa
break :blk .focus; break :blk .focus;
}; };
if (!released) self.pressed.add(.{ .code = event.keycode, .consumer = consumer }); if (!released) {
self.pressed.addAssumeCapacity(.{ .code = event.keycode, .consumer = consumer });
}
switch (consumer) { switch (consumer) {
.mapping => if (!released) { // Press mappings are handled above when determining the consumer of the press
// We handle release mappings separately, regardless of whether press mapping exists // Release mappings are handled separately as they are executed independent of the consumer.
const handled = self.device.seat.handleMapping(keycode, modifiers, released, xkb_state); .mapping => {},
assert(handled);
},
.im_grab => if (self.getInputMethodGrab()) |keyboard_grab| { .im_grab => if (self.getInputMethodGrab()) |keyboard_grab| {
keyboard_grab.setKeyboard(keyboard_grab.keyboard); keyboard_grab.setKeyboard(keyboard_grab.keyboard);
keyboard_grab.sendKey(event.time_msec, event.keycode, event.state); keyboard_grab.sendKey(event.time_msec, event.keycode, event.state);

@ -355,25 +355,6 @@ pub fn enterMode(self: *Self, mode_id: u32) void {
} }
} }
/// Is there a user-defined mapping for passed keycode, modifiers and keyboard state?
pub fn hasMapping(
self: *Self,
keycode: xkb.Keycode,
modifiers: wlr.Keyboard.ModifierMask,
released: bool,
xkb_state: *xkb.State,
) bool {
const modes = &server.config.modes;
for (modes.items[self.mode_id].mappings.items) |*mapping| {
if (mapping.match(keycode, modifiers, released, xkb_state, .no_translate) or
mapping.match(keycode, modifiers, released, xkb_state, .translate))
{
return true;
}
}
return false;
}
/// Handle any user-defined mapping for passed keycode, modifiers and keyboard state /// Handle any user-defined mapping for passed keycode, modifiers and keyboard state
/// Returns true if a mapping was run /// Returns true if a mapping was run
pub fn handleMapping( pub fn handleMapping(