When drawing on a tablet, the hand usually rests on the device, causing touch events. The kernel arbitrates for us in most cases, so we get a touch up and no events while the stylus is in proximity. When lifting the hand off in a natural position, the hand still touches the device when the pen goes out of proximity. This is 'immediately' followed by the hand lifting off the device.
When kernel pen/touch arbitration is active, the pen proximity out causes a touch begin for the hand still on the pad. This is followed by a touch up when the hand lifts which happens to look exactly like a tap-to-click. Fix this by delaying the 'arbitration is now off' toggle, causing any touch that starts immediately after proximity out to be detected as palm and ignored for its lifetime. https://bugs.freedesktop.org/show_bug.cgi?id=104985 Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net> --- src/evdev-mt-touchpad.c | 51 +++++++++++++++++++++++++--- src/evdev-mt-touchpad.h | 6 +++- src/evdev.c | 2 +- test/litest.c | 6 ++++ test/litest.h | 3 ++ test/test-tablet.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 150 insertions(+), 7 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 3b11343e..62ba678e 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -912,7 +912,7 @@ tp_palm_detect_arbitration_triggered(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) { - if (!tp->in_arbitration) + if (!tp->arbitration.in_arbitration) return false; t->palm.state = PALM_ARBITRATION; @@ -1682,6 +1682,8 @@ tp_interface_destroy(struct evdev_dispatch *dispatch) { struct tp_dispatch *tp = tp_dispatch(dispatch); + libinput_timer_cancel(&tp->arbitration.arbitration_timer); + libinput_timer_destroy(&tp->arbitration.arbitration_timer); libinput_timer_destroy(&tp->palm.trackpoint_timer); libinput_timer_destroy(&tp->dwt.keyboard_timer); libinput_timer_destroy(&tp->tap.timer); @@ -2301,6 +2303,15 @@ evdev_tag_touchpad(struct evdev_device *device, } } +static void +tp_arbitration_timeout(uint64_t now, void *data) +{ + struct tp_dispatch *tp = data; + + if (tp->arbitration.in_arbitration) + tp->arbitration.in_arbitration = false; +} + static void tp_interface_toggle_touch(struct evdev_dispatch *dispatch, struct evdev_device *device, @@ -2310,13 +2321,24 @@ tp_interface_toggle_touch(struct evdev_dispatch *dispatch, struct tp_dispatch *tp = tp_dispatch(dispatch); bool arbitrate = !enable; - if (arbitrate == tp->in_arbitration) + if (arbitrate == tp->arbitration.in_arbitration) return; - if (arbitrate) + if (arbitrate) { + libinput_timer_cancel(&tp->arbitration.arbitration_timer); tp_clear_state(tp); - - tp->in_arbitration = arbitrate; + tp->arbitration.in_arbitration = true; + } else { + /* if in-kernel arbitration is in use and there is a touch + * and a pen in proximity, lifting the pen out of proximity + * causes a touch being for the touch. On a hand-lift the + * proximity out precedes the touch up by a few ms, so we + * get what looks like a tap. Fix this by delaying + * arbitration by just a little bit so that any touch in + * event is caught as palm touch. */ + libinput_timer_set(&tp->arbitration.arbitration_timer, + time + ms2us(90)); + } } static struct evdev_dispatch_interface tp_interface = { @@ -2796,6 +2818,23 @@ tp_init_palmdetect_size(struct tp_dispatch *tp, tp->palm.size_threshold = threshold; } +static inline void +tp_init_palmdetect_arbitration(struct tp_dispatch *tp, + struct evdev_device *device) +{ + char timer_name[64]; + + snprintf(timer_name, + sizeof(timer_name), + "%s arbitration", + evdev_device_get_sysname(device)); + libinput_timer_init(&tp->arbitration.arbitration_timer, + tp_libinput_context(tp), + timer_name, + tp_arbitration_timeout, tp); + tp->arbitration.in_arbitration = false; +} + static void tp_init_palmdetect(struct tp_dispatch *tp, struct evdev_device *device) @@ -2805,6 +2844,8 @@ tp_init_palmdetect(struct tp_dispatch *tp, tp->palm.left_edge = INT_MIN; tp->palm.upper_edge = INT_MIN; + tp_init_palmdetect_arbitration(tp, device); + if (device->tags & EVDEV_TAG_EXTERNAL_TOUCHPAD && !tp_is_tpkb_combo_below(device)) return; diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 55244064..afd0983d 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -240,7 +240,11 @@ struct tp_dispatch { bool has_mt; bool semi_mt; - bool in_arbitration; /* pen/touch arbitration */ + /* pen/touch arbitration */ + struct { + bool in_arbitration; + struct libinput_timer arbitration_timer; + } arbitration; unsigned int num_slots; /* number of slots */ unsigned int ntouches; /* no slots inc. fakes */ diff --git a/src/evdev.c b/src/evdev.c index 257824aa..5b23201c 100644 --- a/src/evdev.c +++ b/src/evdev.c @@ -848,7 +848,7 @@ evdev_process_event(struct evdev_device *device, struct input_event *e) struct evdev_dispatch *dispatch = device->dispatch; uint64_t time = tv2us(&e->time); -#if 0 +#if 1 evdev_print_event(device, e); #endif diff --git a/test/litest.c b/test/litest.c index c102afa8..f5bc8660 100644 --- a/test/litest.c +++ b/test/litest.c @@ -3354,6 +3354,12 @@ litest_timeout_tablet_proxout(void) msleep(70); } +void +litest_timeout_touch_arbitration(void) +{ + msleep(100); +} + void litest_timeout_hysteresis(void) { diff --git a/test/litest.h b/test/litest.h index 50347fea..df725f87 100644 --- a/test/litest.h +++ b/test/litest.h @@ -778,6 +778,9 @@ litest_timeout_trackpoint(void); void litest_timeout_tablet_proxout(void); +void +litest_timeout_touch_arbitration(void); + void litest_timeout_hysteresis(void); diff --git a/test/test-tablet.c b/test/test-tablet.c index 77230ed0..a5a58bc9 100644 --- a/test/test-tablet.c +++ b/test/test-tablet.c @@ -4113,6 +4113,9 @@ touch_arbitration(struct litest_device *dev, litest_assert_only_typed_events(li, LIBINPUT_EVENT_TABLET_TOOL_PROXIMITY); + litest_timeout_touch_arbitration(); + libinput_dispatch(li); + /* finger still down */ litest_touch_move_to(finger, 0, 80, 80, 30, 30, 10, 1); litest_touch_up(finger, 0); @@ -4192,6 +4195,11 @@ touch_arbitration_stop_touch(struct litest_device *dev, litest_touch_move_to(finger, 1, 30, 30, 80, 80, 10, 1); litest_assert_empty_queue(li); litest_touch_up(finger, 1); + libinput_dispatch(li); + + litest_timeout_touch_arbitration(); + libinput_dispatch(li); + litest_touch_down(finger, 0, 30, 30); litest_touch_move_to(finger, 0, 30, 30, 80, 80, 10, 1); litest_touch_up(finger, 0); @@ -4264,6 +4272,9 @@ touch_arbitration_suspend_touch(struct litest_device *dev, LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT); litest_assert_only_typed_events(li, LIBINPUT_EVENT_DEVICE_REMOVED); + litest_timeout_touch_arbitration(); + libinput_dispatch(li); + litest_touch_down(dev, 0, 30, 30); litest_touch_move_to(dev, 0, 30, 30, 80, 80, 10, 1); litest_touch_up(dev, 0); @@ -4384,6 +4395,9 @@ touch_arbitration_remove_tablet(struct litest_device *dev, LIBINPUT_TABLET_TOOL_PROXIMITY_STATE_OUT); litest_assert_only_typed_events(li, LIBINPUT_EVENT_DEVICE_REMOVED); + litest_timeout_touch_arbitration(); + libinput_dispatch(li); + /* Touch is still down, don't enable */ litest_touch_move_to(dev, 0, 80, 80, 30, 30, 10, 1); litest_touch_up(dev, 0); @@ -4416,6 +4430,79 @@ START_TEST(cintiq_touch_arbitration_remove_tablet) } END_TEST +START_TEST(intuos_touch_arbitration_keep_ignoring) +{ + struct litest_device *tablet = litest_current_device(); + struct litest_device *finger; + struct libinput *li = tablet->libinput; + struct axis_replacement axes[] = { + { ABS_DISTANCE, 10 }, + { ABS_PRESSURE, 0 }, + { -1, -1 } + }; + + finger = litest_add_device(li, LITEST_WACOM_FINGER); + litest_enable_tap(finger->libinput_device); + litest_tablet_proximity_in(tablet, 10, 10, axes); + litest_tablet_motion(tablet, 10, 10, axes); + litest_tablet_motion(tablet, 20, 40, axes); + + litest_touch_down(finger, 0, 30, 30); + litest_drain_events(li); + + litest_tablet_proximity_out(tablet); + litest_drain_events(li); + + /* a touch during pen interaction stays a palm after the pen lifts. + */ + litest_touch_move_to(finger, 0, 30, 30, 80, 80, 10, 1); + litest_touch_up(finger, 0); + libinput_dispatch(li); + + litest_assert_empty_queue(li); + + litest_delete_device(finger); +} +END_TEST + +START_TEST(intuos_touch_arbitration_late_touch_lift) +{ + struct litest_device *tablet = litest_current_device(); + struct litest_device *finger; + struct libinput *li = tablet->libinput; + struct axis_replacement axes[] = { + { ABS_DISTANCE, 10 }, + { ABS_PRESSURE, 0 }, + { -1, -1 } + }; + + finger = litest_add_device(li, LITEST_WACOM_FINGER); + litest_enable_tap(finger->libinput_device); + litest_tablet_proximity_in(tablet, 10, 10, axes); + litest_tablet_motion(tablet, 10, 10, axes); + litest_tablet_motion(tablet, 20, 40, axes); + litest_drain_events(li); + + litest_tablet_proximity_out(tablet); + litest_drain_events(li); + + /* with kernel arbitration, a finger + stylus in prox only generates + * stylus events. When lifting the hand off, the stylus goes out of + * prox when the hand is still touching. A few ms later, the hand + * goes out of prox, this can generate a fake tap event. + */ + litest_touch_down(finger, 0, 30, 30); + litest_touch_up(finger, 0); + libinput_dispatch(li); + litest_timeout_tap(); + libinput_dispatch(li); + + litest_assert_empty_queue(li); + + litest_delete_device(finger); +} +END_TEST + START_TEST(huion_static_btn_tool_pen) { struct litest_device *dev = litest_current_device(); @@ -4689,6 +4776,8 @@ litest_setup_tests_tablet(void) litest_add_for_device("tablet:touch-arbitration", intuos_touch_arbitration_suspend_touch_device, LITEST_WACOM_FINGER); litest_add_for_device("tablet:touch-arbitration", intuos_touch_arbitration_remove_touch, LITEST_WACOM_INTUOS); litest_add_for_device("tablet:touch-arbitration", intuos_touch_arbitration_remove_tablet, LITEST_WACOM_FINGER); + litest_add_for_device("tablet:touch-arbitration", intuos_touch_arbitration_keep_ignoring, LITEST_WACOM_INTUOS); + litest_add_for_device("tablet:touch-arbitration", intuos_touch_arbitration_late_touch_lift, LITEST_WACOM_INTUOS); litest_add_for_device("tablet:touch-arbitration", cintiq_touch_arbitration, LITEST_WACOM_CINTIQ_13HDT_PEN); litest_add_for_device("tablet:touch-arbitration", cintiq_touch_arbitration_stop_touch, LITEST_WACOM_CINTIQ_13HDT_PEN); -- 2.14.3 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
