keyboard: simplify key state handling

We used to store both `pressed` keys and `bound` keys and derive
`pressed_sent` keys from them, which are notified to the client when
switching focus. But I think this was overengineered and we can remove
`pressed` keys and `bound` keys. Instead we now directly modify
`pressed_sent` keys just before calling `wlr_seat_keyboard_notify_key()`.

Technically we could even remove `pressed_sent` keys as they are
duplicated in wlr_seat->keyboard_state.keyboard->keycodes, but we keep
`pressed_sent` keys as it is easier to access.

The only place which required `bound` keys was `handle_modifiers()`.
This function checked `bound` keys to keep widnow switcher alive when
Alt-Tab is pressed and only the Alt modifier is released. The window
switcher is then finished when Tab key is released. I replaced the check
with `(seat->keyboard_group->keyboard.num_keycodes > 0)`. This slightly
changes the behavior; when Alt-Tab and any other key e.g. X are pressed
and Tab key and Alt modifier are released, the window switcher is now
kept alive. But I don't think this breaks any workflows for users.

include/input/key-state.h

@@ -2,9 +2,6 @@
 #ifndef LABWC_KEY_STATE_H
 #define LABWC_KEY_STATE_H
 
-#include <stdbool.h>
-#include <stdint.h>
-
 struct seat;
 
 /*
@@ -16,18 +13,4 @@ struct seat;
 void key_state_indicator_update(struct seat *seat);
 void key_state_indicator_toggle(void);
 
-/**
- * key_state_pressed_sent_keycodes - generate array of pressed+sent keys
- * Note: The array is generated by subtracting any bound keys from _all_ pressed
- * keys (because bound keys were not forwarded to clients).
- */
-uint32_t *key_state_pressed_sent_keycodes(void);
-int key_state_nr_pressed_sent_keycodes(void);
-
-void key_state_set_pressed(uint32_t keycode, bool is_pressed);
-void key_state_store_pressed_key_as_bound(uint32_t keycode);
-bool key_state_corresponding_press_event_was_bound(uint32_t keycode);
-void key_state_bound_key_remove(uint32_t keycode);
-int key_state_nr_bound_keys(void);
-
 #endif /* LABWC_KEY_STATE_H */

include/labwc.h

@@ -83,6 +83,9 @@ struct seat {
 
 	struct lab_set bound_buttons;
 
+	/* Keys that have been pressed and not consumed by compositor actions */
+	struct lab_set pressed_sent_keys;
+
 	struct {
 		bool active;
 		struct {

src/input/key-state.c

@@ -2,8 +2,6 @@
 #include "input/key-state.h"
 #include <stdbool.h>
 #include <stdint.h>
-#include <stdio.h>
-#include <stdlib.h>
 #include <wlr/types/wlr_keyboard.h>
 #include <wlr/types/wlr_scene.h>
 #include <xkbcommon/xkbcommon.h>
@@ -14,8 +12,6 @@
 #include "labwc.h"
 #include "scaled-buffer/scaled-font-buffer.h"
 
-static struct lab_set pressed, bound, pressed_sent;
-
 static bool show_debug_indicator;
 static struct indicator_state {
 	struct wlr_scene_tree *tree;
@@ -109,31 +105,12 @@ update_key_indicator_callback(void *data)
 
 	struct buf buf = BUF_INIT;
 
-	buf_add(&buf, "pressed=");
-	for (int i = 0; i < pressed.size; i++) {
-		const char *keyname = keycode_to_keyname(indicator_state.keymap,
-					pressed.values[i]);
-		buf_add_fmt(&buf, "%s (%d), ", keyname, pressed.values[i]);
-	}
-	scaled_font_buffer_update(indicator_state.sfb_pressed, buf.data,
-		-1, &rc.font_osd, black, white);
-
-	buf_clear(&buf);
-	buf_add(&buf, "bound=");
-	for (int i = 0; i < bound.size; i++) {
-		const char *keyname = keycode_to_keyname(indicator_state.keymap,
-					bound.values[i]);
-		buf_add_fmt(&buf, "%s (%d), ", keyname, bound.values[i]);
-	}
-	scaled_font_buffer_update(indicator_state.sfb_bound, buf.data,
-		-1, &rc.font_osd, black, white);
-
-	buf_clear(&buf);
+	struct lab_set *pressed_sent = &server.seat.pressed_sent_keys;
 	buf_add(&buf, "pressed_sent=");
-	for (int i = 0; i < pressed_sent.size; i++) {
+	for (int i = 0; i < pressed_sent->size; i++) {
 		const char *keyname = keycode_to_keyname(indicator_state.keymap,
-					pressed_sent.values[i]);
-		buf_add_fmt(&buf, "%s (%d), ", keyname, pressed_sent.values[i]);
+					pressed_sent->values[i]);
+		buf_add_fmt(&buf, "%s (%d), ", keyname, pressed_sent->values[i]);
 	}
 	scaled_font_buffer_update(indicator_state.sfb_pressed_sent, buf.data, -1,
 		&rc.font_osd, black, white);
@@ -168,55 +145,3 @@ key_state_indicator_toggle(void)
 {
 	show_debug_indicator = !show_debug_indicator;
 }
-
-uint32_t *
-key_state_pressed_sent_keycodes(void)
-{
-	/* pressed_sent = pressed - bound */
-	pressed_sent = pressed;
-	for (int i = 0; i < bound.size; ++i) {
-		lab_set_remove(&pressed_sent, bound.values[i]);
-	}
-
-	return pressed_sent.values;
-}
-
-int
-key_state_nr_pressed_sent_keycodes(void)
-{
-	return pressed_sent.size;
-}
-
-void
-key_state_set_pressed(uint32_t keycode, bool is_pressed)
-{
-	if (is_pressed) {
-		lab_set_add(&pressed, keycode);
-	} else {
-		lab_set_remove(&pressed, keycode);
-	}
-}
-
-void
-key_state_store_pressed_key_as_bound(uint32_t keycode)
-{
-	lab_set_add(&bound, keycode);
-}
-
-bool
-key_state_corresponding_press_event_was_bound(uint32_t keycode)
-{
-	return lab_set_contains(&bound, keycode);
-}
-
-void
-key_state_bound_key_remove(uint32_t keycode)
-{
-	lab_set_remove(&bound, keycode);
-}
-
-int
-key_state_nr_bound_keys(void)
-{
-	return bound.size;
-}

src/input/keyboard.c

@@ -153,7 +153,7 @@ handle_modifiers(struct wl_listener *listener, void *data)
 	if ((cycling || seat->workspace_osd_shown_by_modifier)
 			&& !keyboard_get_all_modifiers(seat)) {
 		if (cycling) {
-			if (key_state_nr_bound_keys()) {
+			if (seat->keyboard_group->keyboard.num_keycodes > 0) {
 				should_cancel_cycling_on_next_key_release = true;
 			} else {
 				should_cancel_cycling_on_next_key_release = false;
@@ -381,7 +381,7 @@ handle_key_release(uint32_t evdev_keycode)
 	 * Release events for keys that were not bound should always be
 	 * forwarded to clients to avoid stuck keys.
 	 */
-	if (!key_state_corresponding_press_event_was_bound(evdev_keycode)) {
+	if (lab_set_contains(&server.seat.pressed_sent_keys, evdev_keycode)) {
 		return LAB_KEY_HANDLED_FALSE;
 	}
 
@@ -403,7 +403,6 @@ handle_key_release(uint32_t evdev_keycode)
 	 * If a press event was handled by a compositor binding, then do
 	 * not forward the corresponding release event to clients.
 	 */
-	key_state_bound_key_remove(evdev_keycode);
 	return LAB_KEY_HANDLED_TRUE;
 }
 
@@ -496,12 +495,8 @@ handle_compositor_keybindings(struct keyboard *keyboard,
 	struct keyinfo keyinfo = get_keyinfo(wlr_keyboard, event->keycode);
 	bool locked = server.session_lock_manager->locked;
 
-	key_state_set_pressed(event->keycode,
-		event->state == WL_KEYBOARD_KEY_STATE_PRESSED);
-
 	if (event->state == WL_KEYBOARD_KEY_STATE_RELEASED) {
 		if (cur_keybind && cur_keybind->on_release) {
-			key_state_bound_key_remove(event->keycode);
 			if (locked && !cur_keybind->allow_when_locked) {
 				cur_keybind = NULL;
 				return LAB_KEY_HANDLED_TRUE;
@@ -515,7 +510,6 @@ handle_compositor_keybindings(struct keyboard *keyboard,
 
 	/* Catch C-A-F1 to C-A-F12 to change tty */
 	if (handle_change_vt_key(keyboard, &keyinfo.translated)) {
-		key_state_store_pressed_key_as_bound(event->keycode);
 		return LAB_KEY_HANDLED_TRUE_AND_VT_CHANGED;
 	}
 
@@ -526,12 +520,10 @@ handle_compositor_keybindings(struct keyboard *keyboard,
 	 */
 	if (!locked) {
 		if (server.input_mode == LAB_INPUT_STATE_MENU) {
-			key_state_store_pressed_key_as_bound(event->keycode);
 			handle_menu_keys(&keyinfo.translated);
 			return LAB_KEY_HANDLED_TRUE;
 		} else if (server.input_mode == LAB_INPUT_STATE_CYCLE) {
 			if (handle_cycle_view_key(&keyinfo)) {
-				key_state_store_pressed_key_as_bound(event->keycode);
 				return LAB_KEY_HANDLED_TRUE;
 			}
 		}
@@ -542,12 +534,6 @@ handle_compositor_keybindings(struct keyboard *keyboard,
 	 */
 	cur_keybind = match_keybinding(&keyinfo, keyboard->is_virtual);
 	if (cur_keybind && (!locked || cur_keybind->allow_when_locked)) {
-		/*
-		 * Update key-state before action_run() because the action
-		 * might lead to seat_focus() in which case we pass the
-		 * 'pressed-sent' keys to the new surface.
-		 */
-		key_state_store_pressed_key_as_bound(event->keycode);
 		if (!cur_keybind->on_release) {
 			actions_run(NULL, &cur_keybind->actions, NULL);
 		}
@@ -637,11 +623,10 @@ handle_key(struct wl_listener *listener, void *data)
 	enum lab_key_handled handled =
 		handle_compositor_keybindings(keyboard, event);
 
-	if (handled == LAB_KEY_HANDLED_TRUE_AND_VT_CHANGED) {
-		return;
-	}
-
-	if (handled) {
+	switch (handled) {
+	case LAB_KEY_HANDLED_TRUE_AND_VT_CHANGED:
+		break;
+	case LAB_KEY_HANDLED_TRUE:
 		/*
 		 * We do not start the repeat-timer on pressed modifiers (like
 		 * Super_L) because it is only for our own internal use with
@@ -651,10 +636,25 @@ handle_key(struct wl_listener *listener, void *data)
 				&& event->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
 			start_keybind_repeat(keyboard, event);
 		}
-	} else if (!input_method_keyboard_grab_forward_key(keyboard, event)) {
+		break;
+	case LAB_KEY_HANDLED_FALSE:
+		/*
+		 * Note: Technically we could just use
+		 * wlr_seat->keyboard_state.keyboard->keycodes instead, but we
+		 * have seat->pressed_sent_keys for convenience.
+		 */
+		if (event->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
+			lab_set_add(&seat->pressed_sent_keys, event->keycode);
+		} else {
+			lab_set_remove(&seat->pressed_sent_keys, event->keycode);
+		}
+		if (input_method_keyboard_grab_forward_key(keyboard, event)) {
+			break;
+		}
 		wlr_seat_set_keyboard(wlr_seat, keyboard->wlr_keyboard);
 		wlr_seat_keyboard_notify_key(wlr_seat, event->time_msec,
 			event->keycode, event->state);
+		break;
 	}
 }
 

src/seat.c

@@ -782,12 +782,11 @@ seat_force_focus_surface(struct seat *seat, struct wlr_surface *surface)
 	if (server.session_lock_manager->locked) {
 		return;
 	}
-	uint32_t *pressed_sent_keycodes = key_state_pressed_sent_keycodes();
-	int nr_pressed_sent_keycodes = key_state_nr_pressed_sent_keycodes();
 	struct wlr_keyboard *kb = &seat->keyboard_group->keyboard;
 
 	wlr_seat_keyboard_enter(seat->wlr_seat, surface,
-		pressed_sent_keycodes, nr_pressed_sent_keycodes, &kb->modifiers);
+		seat->pressed_sent_keys.values, seat->pressed_sent_keys.size,
+		&kb->modifiers);
 }
 
 static void
@@ -832,12 +831,10 @@ seat_focus(struct seat *seat, struct wlr_surface *surface,
 	 * those that were actually _sent_ to clients (that is, those that were
 	 * not bound).
 	 */
-	uint32_t *pressed_sent_keycodes = key_state_pressed_sent_keycodes();
-	int nr_pressed_sent_keycodes = key_state_nr_pressed_sent_keycodes();
-
 	struct wlr_keyboard *kb = &seat->keyboard_group->keyboard;
 	wlr_seat_keyboard_notify_enter(seat->wlr_seat, surface,
-		pressed_sent_keycodes, nr_pressed_sent_keycodes, &kb->modifiers);
+		seat->pressed_sent_keys.values, seat->pressed_sent_keys.size,
+		&kb->modifiers);
 
 	input_method_relay_set_focus(seat->input_method_relay, surface);