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

Peter Hutterer peter.hutterer at who-t.net
Thu Feb 4 00:33:36 UTC 2016


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>
---
 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);
-- 
2.5.0



More information about the wayland-devel mailing list