[PATCH libinput 1/4] evdev: Ignore key/button release events if key was never pressed
Ran Benita
ran234 at gmail.com
Sun Jul 27 23:48:05 PDT 2014
Hi Jonas,
On Sun, Jul 27, 2014 at 11:28:28PM +0200, Jonas Ådahl wrote:
> The kernel may send a 'release' event without ever having sent a key
> 'pressed' event in case the key was pressed before libinput was
> initiated. Ignore these events so that we always guarantee a release
> event always comes after a pressed event for any given key or button.
Would it be possible to describe in the docs the invariants that
libinput is keeping w.r.t. key press/release ordering and count?
If the user can rely on such invariants, it can simplify how he
interfaces with e.g. libxkbcommon, which expects a coherent key event
stream (things like modifier press with a missed release can cause some
fun).
Ran
> Signed-off-by: Jonas Ådahl <jadahl at gmail.com>
> ---
> src/evdev.c | 33 ++++++++++++++++++++++++++++++++-
> src/evdev.h | 2 ++
> src/libinput-util.h | 29 +++++++++++++++++++++++++++++
> test/keyboard.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index 5872856..0f4874c 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -47,6 +47,18 @@ enum evdev_key_type {
> EVDEV_KEY_TYPE_BUTTON,
> };
>
> +static void
> +set_key_pressed(struct evdev_device *device, int code, int pressed)
> +{
> + long_set_bit_state(device->key_mask, code, pressed);
> +}
> +
> +static int
> +is_key_pressed(struct evdev_device *device, int code)
> +{
> + return long_bit_is_set(device->key_mask, code);
> +}
> +
> void
> evdev_device_led_update(struct evdev_device *device, enum libinput_led leds)
> {
> @@ -294,6 +306,8 @@ static inline void
> evdev_process_key(struct evdev_device *device,
> struct input_event *e, uint64_t time)
> {
> + enum evdev_key_type type;
> +
> /* ignore kernel key repeat */
> if (e->value == 2)
> return;
> @@ -306,7 +320,24 @@ evdev_process_key(struct evdev_device *device,
>
> evdev_flush_pending_event(device, time);
>
> - switch (get_key_type(e->code)) {
> + type = get_key_type(e->code);
> +
> + /* Ignore key release events from the kernel for keys that libinput
> + * never got a pressed event for. */
> + if (e->value == 0) {
> + switch (type) {
> + case EVDEV_KEY_TYPE_NONE:
> + break;
> + case EVDEV_KEY_TYPE_KEY:
> + case EVDEV_KEY_TYPE_BUTTON:
> + if (!is_key_pressed(device, e->code))
> + return;
> + }
> + }
> +
> + set_key_pressed(device, e->code, e->value);
> +
> + switch (type) {
> case EVDEV_KEY_TYPE_NONE:
> break;
> case EVDEV_KEY_TYPE_KEY:
> diff --git a/src/evdev.h b/src/evdev.h
> index fad1f84..f71d387 100644
> --- a/src/evdev.h
> +++ b/src/evdev.h
> @@ -94,6 +94,8 @@ struct evdev_device {
> struct {
> struct motion_filter *filter;
> } pointer;
> +
> + unsigned long key_mask[NLONGS(KEY_CNT)];
> };
>
> #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
> diff --git a/src/libinput-util.h b/src/libinput-util.h
> index c0235ef..2f1a1db 100644
> --- a/src/libinput-util.h
> +++ b/src/libinput-util.h
> @@ -72,6 +72,8 @@ int list_empty(const struct list *list);
> pos = tmp, \
> tmp = container_of(pos->member.next, tmp, member))
>
> +#define LONG_BITS (sizeof(long) * 8)
> +#define NLONGS(x) (((x) + LONG_BITS - 1) / LONG_BITS)
> #define ARRAY_LENGTH(a) (sizeof (a) / sizeof (a)[0])
> #define ARRAY_FOR_EACH(_arr, _elem) \
> for (size_t _i = 0; (_elem = &_arr[_i]) && _i < ARRAY_LENGTH(_arr); _i++)
> @@ -150,4 +152,31 @@ vector_get_direction(int dx, int dy)
> return dir;
> }
>
> +static inline int
> +long_bit_is_set(const unsigned long *array, int bit)
> +{
> + return !!(array[bit / LONG_BITS] & (1LL << (bit % LONG_BITS)));
> +}
> +
> +static inline void
> +long_set_bit(unsigned long *array, int bit)
> +{
> + array[bit / LONG_BITS] |= (1LL << (bit % LONG_BITS));
> +}
> +
> +static inline void
> +long_clear_bit(unsigned long *array, int bit)
> +{
> + array[bit / LONG_BITS] &= ~(1LL << (bit % LONG_BITS));
> +}
> +
> +static inline void
> +long_set_bit_state(unsigned long *array, int bit, int state)
> +{
> + if (state)
> + long_set_bit(array, bit);
> + else
> + long_clear_bit(array, bit);
> +}
> +
> #endif /* LIBINPUT_UTIL_H */
> diff --git a/test/keyboard.c b/test/keyboard.c
> index a55405c..a7f500c 100644
> --- a/test/keyboard.c
> +++ b/test/keyboard.c
> @@ -112,10 +112,62 @@ START_TEST(keyboard_seat_key_count)
> }
> END_TEST
>
> +START_TEST(keyboard_ignore_no_pressed_release)
> +{
> + struct litest_device *dev;
> + struct libinput *libinput;
> + struct libinput_event *event;
> + struct libinput_event_keyboard *kevent;
> + int events[] = {
> + EV_KEY, KEY_A,
> + -1, -1,
> + };
> + enum libinput_key_state *state;
> + enum libinput_key_state expected_states[] = {
> + LIBINPUT_KEY_STATE_PRESSED,
> + LIBINPUT_KEY_STATE_RELEASED,
> + };
> +
> +
> + libinput = litest_create_context();
> + dev = litest_add_device_with_overrides(libinput,
> + LITEST_KEYBOARD,
> + "Generic keyboard",
> + NULL, NULL, events);
> +
> + litest_drain_events(libinput);
> +
> + litest_keyboard_key(dev, KEY_A, false);
> + litest_keyboard_key(dev, KEY_A, true);
> + litest_keyboard_key(dev, KEY_A, false);
> +
> + libinput_dispatch(libinput);
> +
> + ARRAY_FOR_EACH(expected_states, state) {
> + event = libinput_get_event(libinput);
> + ck_assert_notnull(event);
> + ck_assert_int_eq(libinput_event_get_type(event),
> + LIBINPUT_EVENT_KEYBOARD_KEY);
> + kevent = libinput_event_get_keyboard_event(event);
> + ck_assert_int_eq(libinput_event_keyboard_get_key(kevent),
> + KEY_A);
> + ck_assert_int_eq(libinput_event_keyboard_get_key_state(kevent),
> + *state);
> + libinput_event_destroy(event);
> + libinput_dispatch(libinput);
> + }
> +
> + litest_assert_empty_queue(libinput);
> + litest_delete_device(dev);
> + libinput_unref(libinput);
> +}
> +END_TEST
> +
> int
> main(int argc, char **argv)
> {
> litest_add_no_device("keyboard:seat key count", keyboard_seat_key_count);
> + litest_add_no_device("keyboard:ignore no pressed release", keyboard_ignore_no_pressed_release);
>
> return litest_run(argc, argv);
> }
> --
> 1.8.5.1
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list