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

[Pekka Paalanen]

On Tue, 11 Nov 2014 10:00:15 +0200
Giulio Camuffo <giuliocamuffo at gmail.com> wrote:

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

...

> >> +/* 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?

It returns -1 only if a) the key is in the array, and b) state=PRESSED.

If state is not pressed, then it merely guarantees the key will no
longer be in the array, and always returns 0 even if the key was in
the array.

Hair-splitting, I know, nevermind. :-)

> >
> >> +     if (remove_key(key, state, &keyboard->keys) == -1)
> >> +             return;
> >> +     if (remove_key(key, state, &keyboard->eaten_keys) == -1)
> >> +             return;

Thanks,
pq