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

[Eric Engestrom]

On Friday, 2017-12-08 10:17:17 +1000, Peter Hutterer wrote:
> 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;

This will break out of the `for()` now :)

>  			}
> 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
>