[PATCH weston v3] input: don't send to clients key events eaten by bindings
Pekka Paalanen
ppaalanen at gmail.com
Mon Nov 10 23:21:48 PST 2014
On Mon, 10 Nov 2014 22:08:40 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:
> weston key bindings are supposed to eat the key events, and not pass it
> on to clients, and indeed the wl_keyboard.key event is not sent. But
> we must also not put the key in the keys array to pass to client with
> the wl_keyboard.enter event, or else we may send the 'eaten' one too.
> In the case of a key binding hiding a surface having the keyboard focus,
> the shell may decide to give the focus to another surface, but that will
> happen before the key is released, so the new focus surface will receive
> the code of the bound key in the wl_keyboard.enter array.
> ---
>
> v3: call weston_compositor_idle_inhibit() for all the keys pressed
>
> src/bindings.c | 7 +++++--
> src/compositor.h | 3 ++-
> src/input.c | 57 ++++++++++++++++++++++++++++++++++++++++----------------
> 3 files changed, 48 insertions(+), 19 deletions(-)
Hi Giulio,
I would have committed this as is, but you forgot to add array init and
release for the new eaten_keys array.
Since we need a v4 anyway, I'll comment on the unimportant issues below,
too. ;-)
>
> diff --git a/src/bindings.c b/src/bindings.c
> index 5e24725..452d224 100644
> --- a/src/bindings.c
> +++ b/src/bindings.c
> @@ -255,16 +255,17 @@ install_binding_grab(struct weston_seat *seat, uint32_t time, uint32_t key)
> weston_keyboard_start_grab(seat->keyboard, &grab->grab);
> }
>
> -WL_EXPORT void
> +WL_EXPORT int
> weston_compositor_run_key_binding(struct weston_compositor *compositor,
> struct weston_seat *seat,
> uint32_t time, uint32_t key,
> enum wl_keyboard_key_state state)
> {
> struct weston_binding *b, *tmp;
> + int eaten = 0;
>
> if (state == WL_KEYBOARD_KEY_STATE_RELEASED)
> - return;
> + return 0;
Using 'return eaten' would have more documentary value, no?
>
> /* Invalidate all active modifier bindings. */
> wl_list_for_each(b, &compositor->modifier_binding_list, link)
> @@ -281,8 +282,10 @@ weston_compositor_run_key_binding(struct weston_compositor *compositor,
> if (seat->keyboard->grab ==
> &seat->keyboard->default_grab)
> install_binding_grab(seat, time, key);
> + ++eaten;
> }
> }
> + return eaten;
> }
>
> WL_EXPORT void
> diff --git a/src/compositor.h b/src/compositor.h
> index 27ea693..55f73db 100644
> --- a/src/compositor.h
> +++ b/src/compositor.h
> @@ -473,6 +473,7 @@ struct weston_keyboard {
> uint32_t grab_time;
>
> struct wl_array keys;
> + struct wl_array eaten_keys;
>
> struct {
> uint32_t mods_depressed;
> @@ -1144,7 +1145,7 @@ weston_binding_destroy(struct weston_binding *binding);
> void
> weston_binding_list_destroy_all(struct wl_list *list);
>
> -void
> +int
> weston_compositor_run_key_binding(struct weston_compositor *compositor,
> struct weston_seat *seat, uint32_t time,
> uint32_t key,
> diff --git a/src/input.c b/src/input.c
> index b504d06..7819a41 100644
> --- a/src/input.c
> +++ b/src/input.c
> @@ -1307,6 +1307,26 @@ update_keymap(struct weston_seat *seat)
> }
> #endif
>
> +/* remove the key from the array if it is being released,
> + * else return -1 and do nothing */
> +static int
> +remove_key(uint32_t key, enum wl_keyboard_key_state state,
> + struct wl_array *array)
> +{
> + uint32_t *k, *end;
> + end = array->data + array->size;
> + for (k = array->data; k < end; k++) {
> + if (*k == key) {
> + /* Ignore server-generated repeats. */
> + if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
> + return -1;
> + *k = *--end;
> + }
> + }
> + array->size = (void *) end - array->data;
> + return 0;
> +}
> +
> WL_EXPORT void
> notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
> enum wl_keyboard_key_state state,
> @@ -1315,7 +1335,9 @@ notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
> struct weston_compositor *compositor = seat->compositor;
> struct weston_keyboard *keyboard = seat->keyboard;
> struct weston_keyboard_grab *grab = keyboard->grab;
> - uint32_t *k, *end;
> + uint32_t *k;
> + int eaten = 0;
> + struct wl_array *array;
>
> if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> weston_compositor_idle_inhibit(compositor);
> @@ -1325,28 +1347,26 @@ notify_key(struct weston_seat *seat, uint32_t time, uint32_t key,
> weston_compositor_idle_release(compositor);
> }
>
> - end = keyboard->keys.data + keyboard->keys.size;
> - for (k = keyboard->keys.data; k < end; k++) {
> - if (*k == key) {
> - /* Ignore server-generated repeats. */
> - if (state == WL_KEYBOARD_KEY_STATE_PRESSED)
> - return;
> - *k = *--end;
> - }
> - }
> - keyboard->keys.size = (void *) end - keyboard->keys.data;
> - if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> - k = wl_array_add(&keyboard->keys, sizeof *k);
> - *k = key;
> - }
> + /* Ignore server-generated repeats, so return if remove_key()
> + * returns -1, signaling the key is in the array already. */
And also pressed rather than released.
> + if (remove_key(key, state, &keyboard->keys) == -1)
> + return;
> + if (remove_key(key, state, &keyboard->eaten_keys) == -1)
> + return;
>
> if (grab == &keyboard->default_grab ||
> grab == &keyboard->input_method_grab) {
> - weston_compositor_run_key_binding(compositor, seat, time, key,
> + eaten = weston_compositor_run_key_binding(compositor, seat, time, key,
> state);
Could use realignment.
> grab = keyboard->grab;
> }
>
> + array = eaten == 0 ? &keyboard->keys : &keyboard->eaten_keys;
> + if (state == WL_KEYBOARD_KEY_STATE_PRESSED) {
> + k = wl_array_add(array, sizeof *k);
> + *k = key;
> + }
> +
> grab->interface->key(grab, time, key, state);
>
> if (keyboard->pending_keymap &&
> @@ -1430,6 +1450,11 @@ notify_keyboard_focus_out(struct weston_seat *seat)
> update_modifier_state(seat, serial, *k,
> WL_KEYBOARD_KEY_STATE_RELEASED);
> }
> + wl_array_for_each(k, &keyboard->eaten_keys) {
> + weston_compositor_idle_release(compositor);
> + /* No need to update the modifier state here, modifiers
> + * can't be in the eaten_keys array */
> + }
>
> seat->modifier_state = 0;
>
Thanks,
pq
More information about the wayland-devel
mailing list