From a363f73e0290c7c5ca8707e48e302100b9ed52ca Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Mon, 11 Jul 2022 17:05:08 +0100 Subject: [PATCH 1/6] Revert "keyboard: cancel repeat when handling key-bind" This reverts commit e62bb51bfb33ee520e800cf98553d766824fe9cf. Fixes #510 --- include/key-state.h | 4 +--- src/key-state.c | 3 +-- src/keyboard.c | 8 +------- 3 files changed, 3 insertions(+), 12 deletions(-) diff --git a/include/key-state.h b/include/key-state.h index e5f72919..1e7e5410 100644 --- a/include/key-state.h +++ b/include/key-state.h @@ -5,8 +5,6 @@ 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); - -/* returns numbers of keys still pressed in a consumed key combination */ -int key_state_bound_key_remove(uint32_t keycode); +void key_state_bound_key_remove(uint32_t keycode); #endif /* __LABWC_KEY_STATE_H */ diff --git a/src/key-state.c b/src/key-state.c index d2be5467..bc4844f6 100644 --- a/src/key-state.c +++ b/src/key-state.c @@ -63,9 +63,8 @@ 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); - return bound.nr_keys; } diff --git a/src/keyboard.c b/src/keyboard.c index 2f0dd66d..1b033a41 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -67,14 +67,12 @@ 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); actions_run(NULL, server, &keybind->actions, 0); return true; } @@ -120,11 +118,7 @@ handle_compositor_keybindings(struct wl_listener *listener, */ if (key_state_corresponding_press_event_was_bound(keycode) && event->state == WL_KEYBOARD_KEY_STATE_RELEASED) { - int nr_bound_keys = key_state_bound_key_remove(keycode); - if (!nr_bound_keys) { - wlr_keyboard_set_repeat_info(keyboard, - rc.repeat_rate, rc.repeat_delay); - } + key_state_bound_key_remove(keycode); return true; } From e1467b9aac4d140e636ab6ba1da574a4ad52e199 Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Tue, 30 Aug 2022 15:41:50 +0100 Subject: [PATCH 2/6] src/keyboard.c: stored handled keys as bound when window-cycling ...and changing TTY --- src/keyboard.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index 1b033a41..6b306b7b 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -134,7 +134,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; } } } @@ -147,7 +148,9 @@ handle_compositor_keybindings(struct wl_listener *listener, osd_preview_restore(server); /* osd_finish() additionally resets cycle_view to NULL */ osd_finish(server); - return true; + + handled = true; + goto out; } } @@ -169,7 +172,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 */ @@ -179,6 +183,7 @@ handle_compositor_keybindings(struct wl_listener *listener, } } +out: if (handled) { key_state_store_pressed_keys_as_bound(); } From ffb2efe7333a7aa4100cf247ce6e153886de2892 Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Tue, 30 Aug 2022 15:43:57 +0100 Subject: [PATCH 3/6] src/keyboard.c: reflow comment to shorten line --- src/keyboard.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index 6b306b7b..bdcb6c31 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -144,9 +144,13 @@ 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); handled = true; From 20c4ffa539d3065bf96db30089aa3b0e9955753a Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Tue, 30 Aug 2022 15:47:00 +0100 Subject: [PATCH 4/6] src/keyboard.c: do not end window-cycling on modifier release only 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. Just to clarify the position here: Issue #176 describes a behaviour whereby dmenu gets stuck on repeat after being launched with a keybind. This patch does not resolve that issue but reflects that in Wayland, the client is responsible for implementing "key repeat". Changing the key repeat rate/delay in (labwc/labwc@e62bb51) was dirty fix that need should never have been made. --- include/key-state.h | 1 + src/key-state.c | 6 ++++++ src/keyboard.c | 39 +++++++++++++++++++++++++++++++++------ 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/include/key-state.h b/include/key-state.h index 1e7e5410..cfe9df9c 100644 --- a/include/key-state.h +++ b/include/key-state.h @@ -6,5 +6,6 @@ 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); +int key_state_nr_keys(void); #endif /* __LABWC_KEY_STATE_H */ diff --git a/src/key-state.c b/src/key-state.c index bc4844f6..0c53cf9f 100644 --- a/src/key-state.c +++ b/src/key-state.c @@ -68,3 +68,9 @@ key_state_bound_key_remove(uint32_t keycode) { remove_key(&bound, keycode); } + +int +key_state_nr_keys(void) +{ + return bound.nr_keys; +} diff --git a/src/keyboard.c b/src/keyboard.c index bdcb6c31..ae68b8f5 100644 --- a/src/keyboard.c +++ b/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 */ - 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); + if (key_state_nr_keys()) { + should_cancel_cycling_on_next_key_release = true; + } else { + end_cycling(server); + } } if (seat->workspace_osd_shown_by_modifier) { workspaces_osd_hide(seat); @@ -112,6 +124,21 @@ handle_compositor_keybindings(struct wl_listener *listener, key_state_set_pressed(keycode, event->state == WL_KEYBOARD_KEY_STATE_PRESSED); + /* + * 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 From 4108313f96f9d9bfd10b009d938999347f1bfd60 Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Tue, 30 Aug 2022 21:07:27 +0100 Subject: [PATCH 5/6] src/keyboard.c: register keys before inhibit check Call key_state_set_pressed() before checking seat->active_client_while_inhibited to avoid missing release events for clients using the inhibit protocol (for example swaylock). --- src/keyboard.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/keyboard.c b/src/keyboard.c index ae68b8f5..4849a2f4 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -124,6 +124,15 @@ handle_compositor_keybindings(struct wl_listener *listener, key_state_set_pressed(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 @@ -233,12 +242,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; - - /* ignore labwc keybindings if input is inhibited */ - if (!seat->active_client_while_inhibited) { - handled = handle_compositor_keybindings(listener, event); - } + bool handled = handle_compositor_keybindings(listener, event); if (!handled) { wlr_seat_set_keyboard(wlr_seat, keyboard); From de99a8ba33c36139d7fa62deb0925ccbfab2bd2e Mon Sep 17 00:00:00 2001 From: Johan Malm Date: Tue, 20 Sep 2022 20:46:39 +0100 Subject: [PATCH 6/6] seat: only pass on sent keys on surface-focus Key events associated with keybindings (both pressed and released) are not sent to clients. When using wlr_seat_keyboard_notify_enter() it it 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). This approach is consistent with sway's implementation in input/seat.c https://github.com/swaywm/sway/blob/cffb006feba52c318e66f73c3463032fa76782dc/sway/input/seat.c#L173-L175 Fixes issue #510 --- include/key-state.h | 14 ++++++++++++++ src/key-state.c | 21 ++++++++++++++++++++- src/keyboard.c | 9 +++++---- src/seat.c | 16 ++++++++++++++-- 4 files changed, 53 insertions(+), 7 deletions(-) diff --git a/include/key-state.h b/include/key-state.h index cfe9df9c..00767258 100644 --- a/include/key-state.h +++ b/include/key-state.h @@ -2,6 +2,20 @@ #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); diff --git a/src/key-state.c b/src/key-state.c index 0c53cf9f..775b2e9b 100644 --- a/src/key-state.c +++ b/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) { diff --git a/src/keyboard.c b/src/keyboard.c index 4849a2f4..63e46a68 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -85,6 +85,7 @@ handle_keybinding(struct server *server, uint32_t modifiers, xkb_keysym_t sym) } for (size_t i = 0; i < keybind->keysyms_len; i++) { if (xkb_keysym_to_lower(sym) == keybind->keysyms[i]) { + key_state_store_pressed_keys_as_bound(); actions_run(NULL, server, &keybind->actions, 0); return true; } @@ -121,7 +122,7 @@ 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); /* @@ -152,9 +153,9 @@ handle_compositor_keybindings(struct wl_listener *listener, * 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) { - key_state_bound_key_remove(keycode); + key_state_bound_key_remove(event->keycode); return true; } @@ -247,7 +248,7 @@ keyboard_key_notify(struct wl_listener *listener, void *data) if (!handled) { wlr_seat_set_keyboard(wlr_seat, keyboard); wlr_seat_keyboard_notify_key(wlr_seat, event->time_msec, - event->keycode, event->state); + event->keycode, event->state); } } diff --git a/src/seat.c b/src/seat.c index ddc723ed..b775cf4c 100644 --- a/src/seat.c +++ b/src/seat.c @@ -9,6 +9,7 @@ #include #include #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 =