[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