[PATCH weston v4] input: don't send to clients key events eaten by bindings
Giulio Camuffo
giuliocamuffo at gmail.com
Tue Nov 11 06:10:22 PST 2014
2014-11-11 15:10 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> On Tue, 11 Nov 2014 11:23: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.
>> ---
>>
>> v4: - initialize and release the eaten_keys array
>> - check the eaten_keys array is empty before updating the keymap
>> (i'm am not completely sure this is correct to be honest, but i think so :) )
>>
>> src/bindings.c | 7 ++++--
>> src/compositor.h | 3 ++-
>> src/input.c | 67 ++++++++++++++++++++++++++++++++++++++++----------------
>> 3 files changed, 55 insertions(+), 22 deletions(-)
>>
>> diff --git a/src/bindings.c b/src/bindings.c
>> index 5e24725..369c81a 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 eaten;
>>
>> /* 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..5cb2703 100644
>> --- a/src/input.c
>> +++ b/src/input.c
>> @@ -526,6 +526,7 @@ weston_keyboard_create(void)
>> wl_list_init(&keyboard->focus_resource_listener.link);
>> keyboard->focus_resource_listener.notify = keyboard_focus_resource_destroyed;
>> wl_array_init(&keyboard->keys);
>> + wl_array_init(&keyboard->eaten_keys);
>> keyboard->default_grab.interface = &default_keyboard_grab_interface;
>> keyboard->default_grab.keyboard = keyboard;
>> keyboard->grab = &keyboard->default_grab;
>> @@ -552,6 +553,7 @@ weston_keyboard_destroy(struct weston_keyboard *keyboard)
>> #endif
>>
>> wl_array_release(&keyboard->keys);
>> + wl_array_release(&keyboard->eaten_keys);
>> wl_list_remove(&keyboard->focus_resource_listener.link);
>> free(keyboard);
>> }
>> @@ -1307,6 +1309,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 +1337,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,32 +1349,31 @@ 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. */
>> + 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,
>> - state);
>> + eaten = weston_compositor_run_key_binding(compositor, seat,
>> + time, key, state);
>> 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 &&
>> - keyboard->keys.size == 0)
>> + keyboard->keys.size == 0 &&
>> + keyboard->eaten_keys.size == 0)
>> update_keymap(seat);
>>
>> if (update_state == STATE_UPDATE_AUTOMATIC) {
>> @@ -1430,6 +1453,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;
>>
>> @@ -2083,7 +2111,8 @@ weston_seat_update_keymap(struct weston_seat *seat, struct xkb_keymap *keymap)
>> xkb_keymap_unref(seat->keyboard->pending_keymap);
>> seat->keyboard->pending_keymap = xkb_keymap_ref(keymap);
>>
>> - if (seat->keyboard->keys.size == 0)
>> + if (seat->keyboard->keys.size == 0 &&
>> + seat->keyboard->eaten_keys.size == 0)
>> update_keymap(seat);
>> #endif
>> }
>
> Looks good enough to me, pushed.
>
> I do wonder about notify_keyboard_focus_in(), though. That one is also
> running key bindings, so should it too be filtering eaten keys?
>
> Actually, should focus_in be running key bindings at all?
I've been wondering that too, I don't think it makes much sense. I can
make a follow up patch if we have an agreement.
Thanks,
Giulio
>
>
> Thanks,
> pq
More information about the wayland-devel
mailing list