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 <jad...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel