[PATCH libinput 4/4] touchpad: delay arbitration by 90ms after touch toggle
Peter Hutterer
peter.hutterer at who-t.net
Wed Feb 21 06:22:32 UTC 2018
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.hutterer at 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
More information about the wayland-devel
mailing list