From b27f04689e60a9148523124aa610b23dbe173377 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 4 Feb 2016 08:31:25 +1000 Subject: [PATCH] touchpad: while a key is held down, don't disable dwt 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 Reviewed-by: Hans de Goede --- 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 0f728076..9b6c689a 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 0053122b..87d34b20 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 6adbbc91..522c19cf 100644 --- a/src/libinput-util.h +++ b/src/libinput-util.h @@ -25,6 +25,7 @@ #ifndef LIBINPUT_UTIL_H #define LIBINPUT_UTIL_H +#include #include #include #include @@ -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 19a9e98e..8b7cce7d 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);