fallback: send key events out immediately upon receiving them
authorPeter Hutterer <peter.hutterer@who-t.net>
Thu, 7 Dec 2017 23:41:07 +0000 (09:41 +1000)
committerPeter Hutterer <peter.hutterer@who-t.net>
Fri, 8 Dec 2017 00:28:56 +0000 (10:28 +1000)
Commit db3b6fe5f7f8 "fallback: change to handle the state at EV_SYN time"
introduced regressions for two types of event sequences.

One is a kernel bug - some devices/drivers like the asus-wireless send a key
press + release within the same event frame which now cancels out and
disappears into the ether. This should be fixed in the kernel drivers but
there appear to be enough of them that we can't just pretend it's an outlier.

The second issue is a libinput bug. If we get two key events in the same frame
(e.g. shift + A) we update the state correctly but the events are sent in the
order of the event codes. KEY_A sorts before KEY_LEFTSHIFT and our shift + A
becomes A + shift.

Fix this by treating key events as before db3b6fe5f7f8 - by sending them out
as we get them.

https://bugs.freedesktop.org/show_bug.cgi?id=104030

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 1c8636923b7d8245a5f55dfbfa191c0129ad4414)

src/evdev-fallback.c
test/test-keyboard.c

index 7bfcaf90e16f03277b2e43536f4040fba28ce892..d45f604e65e7d7293a2304dee553034d779e37fa 100644 (file)
@@ -501,6 +501,22 @@ fallback_process_key(struct fallback_dispatch *dispatch,
        }
 
        hw_set_key_down(dispatch, e->code, e->value);
+
+       switch (type) {
+       case KEY_TYPE_NONE:
+               break;
+       case KEY_TYPE_KEY:
+               fallback_keyboard_notify_key(
+                            dispatch,
+                            device,
+                            time,
+                            e->code,
+                            e->value ? LIBINPUT_KEY_STATE_PRESSED :
+                                       LIBINPUT_KEY_STATE_RELEASED);
+               break;
+       case KEY_TYPE_BUTTON:
+               break;
+       }
 }
 
 static void
@@ -827,27 +843,10 @@ fallback_handle_state(struct fallback_dispatch *dispatch,
        if (dispatch->pending_event & EVDEV_KEY) {
                bool want_debounce = false;
                for (unsigned int code = 0; code <= KEY_MAX; code++) {
-                       bool new_state;
-
                        if (!hw_key_has_changed(dispatch, code))
                                continue;
 
-                       new_state = hw_is_key_down(dispatch, code);
-
-                       switch (get_key_type(code)) {
-                       case KEY_TYPE_NONE:
-                               break;
-                       case KEY_TYPE_KEY:
-                               fallback_keyboard_notify_key(
-                                                    dispatch,
-                                                    device,
-                                                    time,
-                                                    code,
-                                                    new_state ?
-                                                            LIBINPUT_KEY_STATE_PRESSED :
-                                                            LIBINPUT_KEY_STATE_RELEASED);
-                               break;
-                       case KEY_TYPE_BUTTON:
+                       if (get_key_type(code) == KEY_TYPE_BUTTON) {
                                want_debounce = true;
                                break;
                        }
index dc2e630e0e0bdbcea2c9e205b80a2a1667fadbb0..db8363814bb398ae6c631f8222a46472d1314def 100644 (file)
@@ -365,6 +365,61 @@ START_TEST(keyboard_no_buttons)
 }
 END_TEST
 
+START_TEST(keyboard_frame_order)
+{
+       struct litest_device *dev = litest_current_device();
+       struct libinput *li = dev->libinput;
+
+       if (!libevdev_has_event_code(dev->evdev, EV_KEY, KEY_A) ||
+           !libevdev_has_event_code(dev->evdev, EV_KEY, KEY_LEFTSHIFT))
+               return;
+
+       litest_drain_events(li);
+
+       litest_event(dev, EV_KEY, KEY_LEFTSHIFT, 1);
+       litest_event(dev, EV_KEY, KEY_A, 1);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       libinput_dispatch(li);
+
+       litest_assert_key_event(li,
+                               KEY_LEFTSHIFT,
+                               LIBINPUT_KEY_STATE_PRESSED);
+       litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_PRESSED);
+
+       litest_event(dev, EV_KEY, KEY_LEFTSHIFT, 0);
+       litest_event(dev, EV_KEY, KEY_A, 0);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       libinput_dispatch(li);
+
+       litest_assert_key_event(li,
+                               KEY_LEFTSHIFT,
+                               LIBINPUT_KEY_STATE_RELEASED);
+       litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_RELEASED);
+
+       litest_event(dev, EV_KEY, KEY_A, 1);
+       litest_event(dev, EV_KEY, KEY_LEFTSHIFT, 1);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       libinput_dispatch(li);
+
+       litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_PRESSED);
+       litest_assert_key_event(li,
+                               KEY_LEFTSHIFT,
+                               LIBINPUT_KEY_STATE_PRESSED);
+
+       litest_event(dev, EV_KEY, KEY_A, 0);
+       litest_event(dev, EV_KEY, KEY_LEFTSHIFT, 0);
+       litest_event(dev, EV_SYN, SYN_REPORT, 0);
+       libinput_dispatch(li);
+
+       litest_assert_key_event(li, KEY_A, LIBINPUT_KEY_STATE_RELEASED);
+       litest_assert_key_event(li,
+                               KEY_LEFTSHIFT,
+                               LIBINPUT_KEY_STATE_RELEASED);
+
+       libinput_dispatch(li);
+}
+END_TEST
+
 START_TEST(keyboard_leds)
 {
        struct litest_device *dev = litest_current_device();
@@ -432,6 +487,7 @@ litest_setup_tests_keyboard(void)
        litest_add("keyboard:time", keyboard_time_usec, LITEST_KEYS, LITEST_ANY);
 
        litest_add("keyboard:events", keyboard_no_buttons, LITEST_KEYS, LITEST_ANY);
+       litest_add("keyboard:events", keyboard_frame_order, LITEST_KEYS, LITEST_ANY);
 
        litest_add("keyboard:leds", keyboard_leds, LITEST_ANY, LITEST_ANY);