[PATCH libinput] fallback: send key events out immediately upon receiving them

Peter Hutterer peter.hutterer at who-t.net
Fri Dec 8 00:17:17 UTC 2017


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 at who-t.net>
---
 src/evdev-fallback.c | 35 ++++++++++++++++----------------
 test/test-keyboard.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 18 deletions(-)

diff --git a/src/evdev-fallback.c b/src/evdev-fallback.c
index 52a3bde1..0cd2497e 100644
--- a/src/evdev-fallback.c
+++ b/src/evdev-fallback.c
@@ -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
@@ -834,27 +850,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;
 			}
diff --git a/test/test-keyboard.c b/test/test-keyboard.c
index dc2e630e..db836381 100644
--- a/test/test-keyboard.c
+++ b/test/test-keyboard.c
@@ -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);
 
-- 
2.13.6



More information about the wayland-devel mailing list