[PATCH libinput] touchpad: while a key is held down, don't disable dwt

Hans de Goede hdegoede at redhat.com
Thu Feb 4 13:16:15 UTC 2016


Hi,

On 04-02-16 01:33, Peter Hutterer wrote:
> If a key enables dwt and is held down when the timeout expires, re-issue the
> timeout.
>
> There is a corner case where dwt may not work as expected:
> 1. key down and held down
> 2. dwt timer expires, dwt is re-issued
> 3. touch starts
> 4. key is released
> 5. dwt timer expires
> 6. touch now starts moving the pointer
>
> This is an effect of the smart touch detection. A touch starting after the
> last key press is released for pointer motion once dwt turns off again. This
> is what happens in the above case, the dwt timer expiring is the last virtual
> key press. This is a corner case and likely hard to trigger by a real user.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=93984
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Patch looks good to me: Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards.

Hans


> ---
>   src/evdev-mt-touchpad.c |  18 +++++-
>   src/evdev-mt-touchpad.h |   1 +
>   src/libinput-util.h     |  14 +++++
>   test/touchpad.c         | 142 ++++++++++++++++++++++++++++++++++++++++++++++++
>   4 files changed, 173 insertions(+), 2 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index f249116..d0d1ddd 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -1196,6 +1196,15 @@ tp_keyboard_timeout(uint64_t now, void *data)
>   {
>   	struct tp_dispatch *tp = data;
>
> +	if (long_any_bit_set(tp->dwt.key_mask,
> +			     ARRAY_LENGTH(tp->dwt.key_mask))) {
> +		libinput_timer_set(&tp->dwt.keyboard_timer,
> +				   now + DEFAULT_KEYBOARD_ACTIVITY_TIMEOUT_2);
> +		tp->dwt.keyboard_last_press_time = now;
> +		log_debug(tp_libinput_context(tp), "palm: keyboard timeout refresh\n");
> +		return;
> +	}
> +
>   	tp_tap_resume(tp, now);
>
>   	tp->dwt.keyboard_active = false;
> @@ -1240,6 +1249,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>   	struct tp_dispatch *tp = data;
>   	struct libinput_event_keyboard *kbdev;
>   	unsigned int timeout;
> +	unsigned int key;
>
>   	if (!tp->dwt.dwt_enabled)
>   		return;
> @@ -1248,15 +1258,18 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>   		return;
>
>   	kbdev = libinput_event_get_keyboard_event(event);
> +	key = libinput_event_keyboard_get_key(kbdev);
>
>   	/* Only trigger the timer on key down. */
>   	if (libinput_event_keyboard_get_key_state(kbdev) !=
> -	    LIBINPUT_KEY_STATE_PRESSED)
> +	    LIBINPUT_KEY_STATE_PRESSED) {
> +		long_clear_bit(tp->dwt.key_mask, key);
>   		return;
> +	}
>
>   	/* modifier keys don't trigger disable-while-typing so things like
>   	 * ctrl+zoom or ctrl+click are possible */
> -	if (tp_key_ignore_for_dwt(libinput_event_keyboard_get_key(kbdev)))
> +	if (tp_key_ignore_for_dwt(key))
>   		return;
>
>   	if (!tp->dwt.keyboard_active) {
> @@ -1270,6 +1283,7 @@ tp_keyboard_event(uint64_t time, struct libinput_event *event, void *data)
>   	}
>
>   	tp->dwt.keyboard_last_press_time = time;
> +	long_set_bit(tp->dwt.key_mask, key);
>   	libinput_timer_set(&tp->dwt.keyboard_timer,
>   			   time + timeout);
>   }
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 0053122..87d34b2 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -343,6 +343,7 @@ struct tp_dispatch {
>   		struct libinput_event_listener keyboard_listener;
>   		struct libinput_timer keyboard_timer;
>   		struct evdev_device *keyboard;
> +		unsigned long key_mask[NLONGS(KEY_CNT)];
>
>   		uint64_t keyboard_last_press_time;
>   	} dwt;
> diff --git a/src/libinput-util.h b/src/libinput-util.h
> index 6adbbc9..522c19c 100644
> --- a/src/libinput-util.h
> +++ b/src/libinput-util.h
> @@ -25,6 +25,7 @@
>   #ifndef LIBINPUT_UTIL_H
>   #define LIBINPUT_UTIL_H
>
> +#include <assert.h>
>   #include <unistd.h>
>   #include <math.h>
>   #include <stdarg.h>
> @@ -171,6 +172,19 @@ long_set_bit_state(unsigned long *array, int bit, int state)
>   		long_clear_bit(array, bit);
>   }
>
> +static inline int
> +long_any_bit_set(unsigned long *array, size_t size)
> +{
> +	unsigned long i;
> +
> +	assert(size > 0);
> +
> +	for (i = 0; i < size; i++)
> +		if (array[i] != 0)
> +			return 1;
> +	return 0;
> +}
> +
>   struct matrix {
>   	float val[3][3]; /* [row][col] */
>   };
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 9376cd5..0d3aa03 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -2438,6 +2438,145 @@ START_TEST(touchpad_dwt_key_hold)
>   }
>   END_TEST
>
> +START_TEST(touchpad_dwt_key_hold_timeout)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, true);
> +	libinput_dispatch(li);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	litest_timeout_dwt_long();
> +	libinput_dispatch(li);
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	litest_touch_up(touchpad, 0);
> +
> +	litest_assert_empty_queue(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, false);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	/* key is up, but still within timeout */
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	litest_touch_up(touchpad, 0);
> +	litest_assert_empty_queue(li);
> +
> +	/* expire timeout */
> +	litest_timeout_dwt_long();
> +	libinput_dispatch(li);
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	litest_touch_up(touchpad, 0);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_dwt_key_hold_timeout_existing_touch_cornercase)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	/* Note: this tests for the current behavior of a cornercase, and
> +	 * the behaviour is essentially a bug. If this test fails it may be
> +	 * because the buggy behavior was fixed.
> +	 */
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, true);
> +	libinput_dispatch(li);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	litest_timeout_dwt_long();
> +	libinput_dispatch(li);
> +
> +	/* Touch starting after re-issuing the dwt timeout */
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +
> +	litest_assert_empty_queue(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, false);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	/* key is up, but still within timeout */
> +	litest_touch_move_to(touchpad, 0, 70, 50, 50, 50, 5, 1);
> +	litest_assert_empty_queue(li);
> +
> +	/* Expire dwt timeout. Because the touch started after re-issuing
> +	 * the last timeout, it looks like the touch started after the last
> +	 * key press. Such touches are enabled for pointer motion by
> +	 * libinput when dwt expires.
> +	 * This is buggy behavior and not what a user would typically
> +	 * expect. But it's hard to trigger in real life too.
> +	 */
> +	litest_timeout_dwt_long();
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	litest_touch_up(touchpad, 0);
> +	/* If the below check for motion event fails because no events are
> +	 * in the pipe, the buggy behavior was fixed and this test case
> +	 * can be removed */
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_MOTION);
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
> +START_TEST(touchpad_dwt_key_hold_timeout_existing_touch)
> +{
> +	struct litest_device *touchpad = litest_current_device();
> +	struct litest_device *keyboard;
> +	struct libinput *li = touchpad->libinput;
> +
> +	if (!has_disable_while_typing(touchpad))
> +		return;
> +
> +	keyboard = dwt_init_paired_keyboard(li, touchpad);
> +	litest_disable_tap(touchpad->libinput_device);
> +	litest_drain_events(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, true);
> +	libinput_dispatch(li);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	litest_touch_down(touchpad, 0, 50, 50);
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	libinput_dispatch(li);
> +	litest_timeout_dwt_long();
> +	libinput_dispatch(li);
> +
> +	litest_assert_empty_queue(li);
> +
> +	litest_keyboard_key(keyboard, KEY_A, false);
> +	litest_assert_only_typed_events(li, LIBINPUT_EVENT_KEYBOARD_KEY);
> +	/* key is up, but still within timeout */
> +	litest_touch_move_to(touchpad, 0, 70, 50, 50, 50, 5, 1);
> +	litest_assert_empty_queue(li);
> +
> +	/* expire timeout, but touch started before release */
> +	litest_timeout_dwt_long();
> +	litest_touch_move_to(touchpad, 0, 50, 50, 70, 50, 5, 1);
> +	litest_touch_up(touchpad, 0);
> +	litest_assert_empty_queue(li);
> +
> +	litest_delete_device(keyboard);
> +}
> +END_TEST
> +
>   START_TEST(touchpad_dwt_type)
>   {
>   	struct litest_device *touchpad = litest_current_device();
> @@ -3669,6 +3808,9 @@ litest_setup_tests(void)
>   	litest_add("touchpad:dwt", touchpad_dwt_enable_touch, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_touch_hold, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_key_hold, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_key_hold_timeout, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_key_hold_timeout_existing_touch, LITEST_TOUCHPAD, LITEST_ANY);
> +	litest_add("touchpad:dwt", touchpad_dwt_key_hold_timeout_existing_touch_cornercase, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_type, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_type_short_timeout, LITEST_TOUCHPAD, LITEST_ANY);
>   	litest_add("touchpad:dwt", touchpad_dwt_tap, LITEST_TOUCHPAD, LITEST_ANY);
>


More information about the wayland-devel mailing list