Merge pull request #438 from johanmalm/fix/repeat

Revert "keyboard: cancel repeat when handling key-bind" and
only pass on pressed+sent keycodes to surfaces on keyboard-focus.

include/key-state.h

@@ -2,11 +2,24 @@
 #ifndef __LABWC_KEY_STATE_H
 #define __LABWC_KEY_STATE_H
 
+/*
+ * All keycodes in these functions are (Linux) libinput evdev scancodes which is
+ * what 'wlr_keyboard' uses (e.g. 'seat->keyboard_group->keyboard->keycodes').
+ * Note: These keycodes are different to XKB scancodes by a value of 8.
+ */
+
+/**
+ * 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 ispressed);
 void key_state_store_pressed_keys_as_bound(void);
 bool key_state_corresponding_press_event_was_bound(uint32_t keycode);
-
+void key_state_bound_key_remove(uint32_t keycode);
-/* returns numbers of keys still pressed in a consumed key combination */
+int key_state_nr_keys(void);
-int key_state_bound_key_remove(uint32_t keycode);
 
 #endif /* __LABWC_KEY_STATE_H */

src/key-state.c

@@ -10,7 +10,7 @@ struct key_array {
 	int nr_keys;
 };
 
-static struct key_array pressed, bound;
+static struct key_array pressed, bound, pressed_sent;
 
 static void
 remove_key(struct key_array *array, uint32_t keycode)
@@ -35,6 +35,25 @@ add_key(struct key_array *array, uint32_t keycode)
 	array->keys[array->nr_keys++] = keycode;
 }
 
+uint32_t *
+key_state_pressed_sent_keycodes(void)
+{
+	/* pressed_sent = pressed - bound */
+	memcpy(pressed_sent.keys, pressed.keys,
+		MAX_PRESSED_KEYS * sizeof(uint32_t));
+	pressed_sent.nr_keys = pressed.nr_keys;
+	for (int i = 0; i < bound.nr_keys; ++i) {
+		remove_key(&pressed_sent, bound.keys[i]);
+	}
+	return pressed_sent.keys;
+}
+
+int
+key_state_nr_pressed_sent_keycodes(void)
+{
+	return pressed_sent.nr_keys;
+}
+
 void
 key_state_set_pressed(uint32_t keycode, bool ispressed)
 {
@@ -63,9 +82,14 @@ key_state_corresponding_press_event_was_bound(uint32_t keycode)
 	return false;
 }
 
-int
+void
 key_state_bound_key_remove(uint32_t keycode)
 {
 	remove_key(&bound, keycode);
+}
+
+int
+key_state_nr_keys(void)
+{
 	return bound.nr_keys;
 }

src/keyboard.c

@@ -7,6 +7,8 @@
 #include "labwc.h"
 #include "workspaces.h"
 
+static bool should_cancel_cycling_on_next_key_release;
+
 static void
 change_vt(struct server *server, unsigned int vt)
 {
@@ -33,6 +35,17 @@ keyboard_any_modifiers_pressed(struct wlr_keyboard *keyboard)
 	return false;
 }
 
+static void
+end_cycling(struct server *server)
+{
+	desktop_focus_and_activate_view(&server->seat, server->osd_state.cycle_view);
+	desktop_move_to_front(server->osd_state.cycle_view);
+
+	/* osd_finish() additionally resets cycle_view to NULL */
+	osd_finish(server);
+	should_cancel_cycling_on_next_key_release = false;
+}
+
 static void
 keyboard_modifiers_notify(struct wl_listener *listener, void *data)
 {
@@ -46,12 +59,11 @@ keyboard_modifiers_notify(struct wl_listener *listener, void *data)
 		if (event->state == WL_KEYBOARD_KEY_STATE_RELEASED
 				&& !keyboard_any_modifiers_pressed(keyboard))  {
 			if (server->osd_state.cycle_view) {
-				/* end cycle */
+				if (key_state_nr_keys()) {
-				desktop_focus_and_activate_view(&server->seat,
+					should_cancel_cycling_on_next_key_release = true;
-					server->osd_state.cycle_view);
+				} else {
-				desktop_move_to_front(server->osd_state.cycle_view);
+					end_cycling(server);
-				/* osd_finish() additionally resets cycle_view to NULL */
+				}
-				osd_finish(server);
 			}
 			if (seat->workspace_osd_shown_by_modifier) {
 				workspaces_osd_hide(seat);
@@ -67,14 +79,13 @@ static bool
 handle_keybinding(struct server *server, uint32_t modifiers, xkb_keysym_t sym)
 {
 	struct keybind *keybind;
-	struct wlr_keyboard *kb = &server->seat.keyboard_group->keyboard;
 	wl_list_for_each_reverse (keybind, &rc.keybinds, link) {
 		if (modifiers ^ keybind->modifiers) {
 			continue;
 		}
 		for (size_t i = 0; i < keybind->keysyms_len; i++) {
 			if (xkb_keysym_to_lower(sym) == keybind->keysyms[i]) {
-				wlr_keyboard_set_repeat_info(kb, 0, 0);
+				key_state_store_pressed_keys_as_bound();
 				actions_run(NULL, server, &keybind->actions, 0);
 				return true;
 			}
@@ -111,20 +122,40 @@ handle_compositor_keybindings(struct wl_listener *listener,
 
 	bool handled = false;
 
-	key_state_set_pressed(keycode,
+	key_state_set_pressed(event->keycode,
 		event->state == WL_KEYBOARD_KEY_STATE_PRESSED);
 
+	/*
+	 * Ignore labwc keybindings if input is inhibited
+	 * It's important to do this after key_state_set_pressed() to ensure
+	 * _all_ key press/releases are registered
+	 */
+	if (seat->active_client_while_inhibited) {
+		return false;
+	}
+
+	/*
+	 * If a user lets go of the modifier (e.g. alt) before the 'normal' key
+	 * (e.g. tab) when window-cycling, we do not end the cycling until both
+	 * keys have been released.  If we end the window-cycling on release of
+	 * the modifier only, some XWayland clients such as hexchat realise that
+	 * tab is pressed (even though we did not forward the event) and because
+	 * we absorb the equivalent release event it gets stuck on repeat.
+	 */
+	if (should_cancel_cycling_on_next_key_release
+			&& event->state == WL_KEYBOARD_KEY_STATE_RELEASED) {
+		end_cycling(server);
+		handled = true;
+		goto out;
+	}
+
 	/*
 	 * If a press event was handled by a compositor binding, then do not
 	 * forward the corresponding release event to clients
 	 */
-	if (key_state_corresponding_press_event_was_bound(keycode)
+	if (key_state_corresponding_press_event_was_bound(event->keycode)
 			&& event->state == WL_KEYBOARD_KEY_STATE_RELEASED) {
-		int nr_bound_keys = key_state_bound_key_remove(keycode);
+		key_state_bound_key_remove(event->keycode);
-		if (!nr_bound_keys) {
-			wlr_keyboard_set_repeat_info(keyboard,
-				rc.repeat_rate, rc.repeat_delay);
-		}
 		return true;
 	}
 
@@ -140,7 +171,8 @@ handle_compositor_keybindings(struct wl_listener *listener,
 				 * Don't send any key events to clients when
 				 * changing tty
 				 */
-				return true;
+				handled = true;
+				goto out;
 			}
 		}
 	}
@@ -149,11 +181,17 @@ handle_compositor_keybindings(struct wl_listener *listener,
 		if (event->state == WL_KEYBOARD_KEY_STATE_PRESSED) {
 			for (int i = 0; i < nsyms; i++) {
 				if (syms[i] == XKB_KEY_Escape) {
-					/* cancel */
+					/*
+					 * Cancel view-cycle
+					 *
+					 * osd_finish() additionally resets
+					 * cycle_view to NULL
+					 */
 					osd_preview_restore(server);
-					/* osd_finish() additionally resets cycle_view to NULL */
 					osd_finish(server);
-					return true;
+
+					handled = true;
+					goto out;
 				}
 			}
 
@@ -175,7 +213,8 @@ handle_compositor_keybindings(struct wl_listener *listener,
 			}
 		}
 		/* don't send any key events to clients when osd onscreen */
-		return true;
+		handled = true;
+		goto out;
 	}
 
 	/* Handle compositor key bindings */
@@ -185,6 +224,7 @@ handle_compositor_keybindings(struct wl_listener *listener,
 		}
 	}
 
+out:
 	if (handled) {
 		key_state_store_pressed_keys_as_bound();
 	}
@@ -203,12 +243,7 @@ keyboard_key_notify(struct wl_listener *listener, void *data)
 	struct wlr_keyboard *keyboard = &seat->keyboard_group->keyboard;
 	wlr_idle_notify_activity(seat->wlr_idle, seat->seat);
 
-	bool handled = false;
+	bool handled = handle_compositor_keybindings(listener, event);
-
-	/* ignore labwc keybindings if input is inhibited */
-	if (!seat->active_client_while_inhibited) {
-		handled = handle_compositor_keybindings(listener, event);
-	}
 
 	if (!handled) {
 		wlr_seat_set_keyboard(wlr_seat, keyboard);

src/seat.c

@@ -9,6 +9,7 @@
 #include <wlr/types/wlr_touch.h>
 #include <wlr/util/log.h>
 #include "common/mem.h"
+#include "key-state.h"
 #include "labwc.h"
 
 static void
@@ -347,8 +348,19 @@ seat_focus_surface(struct seat *seat, struct wlr_surface *surface)
 		return;
 	}
 	struct wlr_keyboard *kb = &seat->keyboard_group->keyboard;
-	wlr_seat_keyboard_notify_enter(seat->seat, surface, kb->keycodes,
+
-		kb->num_keycodes, &kb->modifiers);
+	/*
+	 * Key events associated with keybindings (both pressed and released)
+	 * are not sent to clients. When changing surface-focus it is therefore
+	 * important not to send the keycodes of _all_ pressed keys, but only
+	 * 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();
+
+	wlr_seat_keyboard_notify_enter(seat->seat, surface,
+		pressed_sent_keycodes, nr_pressed_sent_keycodes, &kb->modifiers);
 
 	struct server *server = seat->server;
 	struct wlr_pointer_constraint_v1 *constraint =