[PATCH weston v3] input: don't send to clients key events eaten by bindings

Giulio Camuffo giuliocamuffo at gmail.com
Tue Nov 11 00:00:15 PST 2014


2014-11-11 9:21 GMT+02:00 Pekka Paalanen <ppaalanen at gmail.com>:
> 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.

Gah, sorry.

>
> 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.

Uh? there are no released keys in the array. Or am i misunderstanding
what you mean?

>
>> +     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