Re: [PATCH libinput] touchpad: while a key is held down, don't disable dwt
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 Patch looks good to me: Reviewed-by: Hans de Goede 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 #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 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); +
[PATCH libinput] 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 --- 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 #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 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