evdev: Ignore key/button release events if key was never pressed
authorJonas Ådahl <jadahl@gmail.com>
Sun, 27 Jul 2014 13:43:59 +0000 (15:43 +0200)
committerJonas Ådahl <jadahl@gmail.com>
Mon, 18 Aug 2014 20:35:19 +0000 (22:35 +0200)
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.

Signed-off-by: Jonas Ådahl <jadahl@gmail.com>
src/evdev.c
src/evdev.h
src/libinput-util.h
src/libinput.h
test/keyboard.c

index 31ca620..bc0237f 100644 (file)
@@ -47,6 +47,18 @@ enum evdev_key_type {
        EVDEV_KEY_TYPE_BUTTON,
 };
 
+static void
+set_key_down(struct evdev_device *device, int code, int pressed)
+{
+       long_set_bit_state(device->key_mask, code, pressed);
+}
+
+static int
+is_key_down(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_down(device, e->code))
+                               return;
+               }
+       }
+
+       set_key_down(device, e->code, e->value);
+
+       switch (type) {
        case EVDEV_KEY_TYPE_NONE:
                break;
        case EVDEV_KEY_TYPE_KEY:
@@ -853,12 +884,8 @@ err:
 int
 evdev_device_get_keys(struct evdev_device *device, char *keys, size_t size)
 {
-       int len;
-
        memset(keys, 0, size);
-       len = ioctl(device->fd, EVIOCGKEY(size), keys);
-
-       return (len == -1) ? -errno : len;
+       return 0;
 }
 
 const char *
index a21b03e..68c1234 100644 (file)
@@ -95,6 +95,10 @@ struct evdev_device {
        struct {
                struct motion_filter *filter;
        } pointer;
+
+       /* Bitmask of pressed keys used to ignore initial release events from
+        * the kernel. */
+       unsigned long key_mask[NLONGS(KEY_CNT)];
 };
 
 #define EVDEV_UNHANDLED_DEVICE ((struct evdev_device *) 1)
index c0235ef..2f1a1db 100644 (file)
@@ -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 */
index 1e8ae67..9296a35 100644 (file)
@@ -1359,7 +1359,8 @@ libinput_device_led_update(struct libinput_device *device,
  */
 int
 libinput_device_get_keys(struct libinput_device *device,
-                        char *keys, size_t size);
+                        char *keys, size_t size)
+       LIBINPUT_ATTRIBUTE_DEPRECATED;
 
 /**
  * @ingroup device
index a55405c..c2040cc 100644 (file)
@@ -112,10 +112,73 @@ START_TEST(keyboard_seat_key_count)
 }
 END_TEST
 
+START_TEST(keyboard_ignore_no_pressed_release)
+{
+       struct litest_device *dev;
+       struct libinput *unused_libinput;
+       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,
+       };
+
+       /* We can't send pressed -> released -> pressed events using uinput
+        * as such non-symmetric events are dropped. Work-around this by first
+        * adding the test device to the tested context after having sent an
+        * initial pressed event. */
+       unused_libinput = litest_create_context();
+       dev = litest_add_device_with_overrides(unused_libinput,
+                                              LITEST_KEYBOARD,
+                                              "Generic keyboard",
+                                              NULL, NULL, events);
+
+       litest_keyboard_key(dev, KEY_A, true);
+       litest_drain_events(unused_libinput);
+
+       libinput = litest_create_context();
+       libinput_path_add_device(libinput,
+                                libevdev_uinput_get_devnode(dev->uinput));
+       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);
+       libinput_unref(unused_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:key counting", keyboard_ignore_no_pressed_release);
 
        return litest_run(argc, argv);
 }