From e66cf8def182e1764bd9ce81e0854f2295e18ad6 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 26 Oct 2017 13:13:14 +1000 Subject: [PATCH 1/4] evdev: document the change-of-directions issue with the hysteresis Signed-off-by: Peter Hutterer --- src/evdev.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/evdev.h b/src/evdev.h index 4c42f6eb..2c410b24 100644 --- a/src/evdev.h +++ b/src/evdev.h @@ -610,6 +610,19 @@ evdev_to_left_handed(struct evdev_device *device, * calculation to do circular hysteresis are nontrivial, especially since * many touchpads have uneven x/y resolutions. * + * Given coordinates, 0, 1, 2, ... this is what we return for a margin of 3 + * and a center of 0: + * + * Input: 1 2 3 4 5 6 5 4 3 2 1 0 -1 + * Coord: 0 0 0 1 2 3 3 3 3 3 3 3 2 + * Center: 0 0 0 1 2 3 3 3 3 3 3 3 2 + * + * Problem: viewed from a stationary finger that starts moving, the + * hysteresis margin is M in both directions. Once we start moving + * continuously though, the margin is 0 in the movement direction and 2*M to + * change direction. That makes the finger less responsive to directional + * changes than to the original movement. + * * @param in The input coordinate * @param center Current center of the hysteresis * @param margin Hysteresis width (on each side) From c498c8c60bf9f0e706e6aa507563a88409157922 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 26 Oct 2017 13:20:01 +1000 Subject: [PATCH 2/4] touchpad: move hysteresis margin into its own struct No functional changes Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 8 ++++---- src/evdev-mt-touchpad.h | 4 +++- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 1ad66fb0..501744ac 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -143,10 +143,10 @@ tp_motion_hysteresis(struct tp_dispatch *tp, } else { x = evdev_hysteresis(x, t->hysteresis_center.x, - tp->hysteresis_margin.x); + tp->hysteresis.margin.x); y = evdev_hysteresis(y, t->hysteresis_center.y, - tp->hysteresis_margin.y); + tp->hysteresis.margin.y); t->hysteresis_center.x = x; t->hysteresis_center.y = y; t->point.x = x; @@ -2899,8 +2899,8 @@ tp_init_hysteresis(struct tp_dispatch *tp) res_x = tp->device->abs.absinfo_x->resolution; res_y = tp->device->abs.absinfo_y->resolution; - tp->hysteresis_margin.x = res_x/2; - tp->hysteresis_margin.y = res_y/2; + tp->hysteresis.margin.x = res_x/2; + tp->hysteresis.margin.y = res_y/2; } static void diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 9d9f0826..6110a2bc 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -269,7 +269,9 @@ struct tp_dispatch { double orientation_to_angle; } touch_size; - struct device_coords hysteresis_margin; + struct { + struct device_coords margin; + } hysteresis; struct { double x_scale_coeff; From 8b923d371e2b104d7a993fe7768110214d500e56 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 26 Oct 2017 13:27:07 +1000 Subject: [PATCH 3/4] touchpad: add an enabled toggle to the hysteresis Hardcoded to 'enabled' right now No functional changes Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 4 ++++ src/evdev-mt-touchpad.h | 1 + 2 files changed, 5 insertions(+) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index 501744ac..a3e243b3 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -138,6 +138,9 @@ tp_motion_hysteresis(struct tp_dispatch *tp, int x = t->point.x, y = t->point.y; + if (!tp->hysteresis.enabled) + return; + if (t->history.count == 0) { t->hysteresis_center = t->point; } else { @@ -2901,6 +2904,7 @@ tp_init_hysteresis(struct tp_dispatch *tp) res_y = tp->device->abs.absinfo_y->resolution; tp->hysteresis.margin.x = res_x/2; tp->hysteresis.margin.y = res_y/2; + tp->hysteresis.enabled = true; } static void diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index 6110a2bc..ab4bcde1 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -270,6 +270,7 @@ struct tp_dispatch { } touch_size; struct { + bool enabled; struct device_coords margin; } hysteresis; From 50daa7b30fd1e13545944540a0ad3794bbf2ef09 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Thu, 26 Oct 2017 13:56:44 +1000 Subject: [PATCH 4/4] touchpad: automatically disable the hysteresis where not required Touchpads that require the hysteresis do not have filtering in the firmware and holding a finger still causes continuous cursor movements. This implies that we get a continuous stream of events with motion data. If the finger is on the touchpad but we don't see any motion, the finger is stationary and the touchpad firmware does filtering. In that case, we don't need to add a hysteresis on top. https://bugs.freedesktop.org/show_bug.cgi?id=98839 Signed-off-by: Peter Hutterer --- src/evdev-mt-touchpad.c | 19 +++++++++++++++++++ src/evdev-mt-touchpad.h | 2 ++ test/litest.c | 6 ++++++ test/litest.h | 3 +++ 4 files changed, 30 insertions(+) diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c index a3e243b3..44bc9780 100644 --- a/src/evdev-mt-touchpad.c +++ b/src/evdev-mt-touchpad.c @@ -131,6 +131,21 @@ tp_motion_history_push(struct tp_touch *t) t->history.index = motion_index; } +static inline void +tp_maybe_disable_hysteresis(struct tp_dispatch *tp, uint64_t time) +{ + /* If the finger is down for 80ms without seeing motion events, + the firmware filters and we don't need a software hysteresis */ + if (time - tp->hysteresis.last_motion_time > ms2us(80)) { + tp->hysteresis.enabled = false; + evdev_log_debug(tp->device, "hysteresis disabled\n"); + return; + } + + if (tp->queued & TOUCHPAD_EVENT_MOTION) + tp->hysteresis.last_motion_time = time; +} + static inline void tp_motion_hysteresis(struct tp_dispatch *tp, struct tp_touch *t) @@ -276,6 +291,7 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time) t->thumb.first_touch_time = time; t->tap.is_thumb = false; assert(tp->nfingers_down >= 1); + tp->hysteresis.last_motion_time = time; } /** @@ -1529,6 +1545,9 @@ static void tp_handle_state(struct tp_dispatch *tp, uint64_t time) { + if (tp->hysteresis.enabled) + tp_maybe_disable_hysteresis(tp, time); + tp_process_state(tp, time); tp_post_events(tp, time); tp_post_process_state(tp, time); diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h index ab4bcde1..cb87b3ff 100644 --- a/src/evdev-mt-touchpad.h +++ b/src/evdev-mt-touchpad.h @@ -272,6 +272,8 @@ struct tp_dispatch { struct { bool enabled; struct device_coords margin; + unsigned int other_event_count; + uint64_t last_motion_time; } hysteresis; struct { diff --git a/test/litest.c b/test/litest.c index d34bd3c5..1fd5a6e9 100644 --- a/test/litest.c +++ b/test/litest.c @@ -3223,6 +3223,12 @@ litest_timeout_tablet_proxout(void) msleep(70); } +void +litest_timeout_hysteresis(void) +{ + msleep(90); +} + void litest_push_event_frame(struct litest_device *dev) { diff --git a/test/litest.h b/test/litest.h index 53d8153d..d6d5c82c 100644 --- a/test/litest.h +++ b/test/litest.h @@ -766,6 +766,9 @@ litest_timeout_trackpoint(void); void litest_timeout_tablet_proxout(void); +void +litest_timeout_hysteresis(void); + void litest_push_event_frame(struct litest_device *dev);