[PATCH v4 libinput 3/3] touchpad: add wobbling detection

[Konstantin Kharlamov]

Looks fine to me. The series is

Reviewed-by: Konstantin Kharlamov <Hi-Angel at yandex.ru>

On 01.03.2018 09:39, Peter Hutterer wrote:
> From: Konstantin Kharlamov <Hi-Angel at yandex.ru>
> 
> The details are explained in comment in the code. That aside, I shall
> mention the check is so light, that it shouldn't influence CPU
> performance even a bit, and can blindly be kept always enabled.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104828
> 
> v2: rename the function, change comment style, and add calculation of
> x_diff (the one that was used is an absolute coordinate).
> 
> FWIW, the algorithm don't care that now added "prev_x" is unintialized,
> because that happening means the time threshold won't get satisfied
> either. It's not like I can't default-init it — it's just that asking
> oneself a question "what default value should it have" results in "none".
> 
> v3:
> * style fixes (empty line after a block, declaration at top)
> * s/x_diff/dx
> * s/x_motion_in_threashold/x_motion_history
> * compare with r_l_r instead of &
> * store whole point instead of just x
> * move x_motion_history and prev_coords into tp_touch
> * move everything around tp_detect_wobbling() call into the function,
> use goto for that purpose.
> * per request: add a comparison for when previous coordinates don't
> actually have the prev. coordinates yet.
> * increased timeout from 20ms to 40ms just in case — it's still out of
> human ability anyway, and in fact can be increased even further
> * ignore Y-only changes, again just in case — it could happen that Y and
> X events would be sent separately, and break the heuristic.
> 
> v4:
> * some more style fixes
> 
> Signed-off-by: Konstantin Kharlamov <Hi-Angel at yandex.ru>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Changes to v3:
> - more style fixes
> - ajudsted for patch 1/3
> 
>   src/evdev-mt-touchpad.c | 58 ++++++++++++++++++++++++++++++++++++++++++++++++-
>   src/evdev-mt-touchpad.h |  2 ++
>   2 files changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index 0c3e6a2b..cec4ba34 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -135,6 +135,61 @@ tp_motion_history_push(struct tp_touch *t)
>   	t->history.index = motion_index;
>   }
>   
> +/* Idea: if we got a tuple of *very* quick moves like {Left, Right,
> + * Left}, or {Right, Left, Right}, it means touchpad jitters since no
> + * human can move like that within thresholds.
> + *
> + * We encode left moves as zeroes, and right as ones. We also drop
> + * the array to all zeroes when contraints are not satisfied. Then we
> + * search for the pattern {1,0,1}. It can't match {Left, Right, Left},
> + * but it does match {Left, Right, Left, Right}, so it's okay.
> + *
> + * This only looks at x changes, y changes are ignored.
> + */
> +static inline void
> +tp_detect_wobbling(struct tp_dispatch *tp,
> +		   struct tp_touch *t,
> +		   uint64_t time)
> +{
> +	int dx, dy;
> +	uint64_t dtime;
> +
> +	if (!(tp->queued & TOUCHPAD_EVENT_MOTION) || tp->hysteresis.enabled)
> +		return;
> +
> +	if (t->last_point.x == 0) { /* first invocation */
> +		dx = 0;
> +		dy = 0;
> +	} else {
> +		dx = t->last_point.x - t->point.x;
> +		dy = t->last_point.y - t->point.y;
> +	}
> +
> +	dtime = time - tp->hysteresis.last_motion_time;
> +
> +	tp->hysteresis.last_motion_time = time;
> +	t->last_point = t->point;
> +
> +	if (dx == 0 && dy != 0) /* ignore y-only changes */
> +		return;
> +
> +	if (dtime > ms2us(40)) {
> +		t->hysteresis.x_motion_history = 0;
> +		return;
> +	}
> +
> +	t->hysteresis.x_motion_history <<= 1;
> +	if (dx > 0) { /* right move */
> +		static const char r_l_r = 0x5; /* {Right, Left, Right} */
> +
> +		t->hysteresis.x_motion_history |= 0x1;
> +		if (t->hysteresis.x_motion_history == r_l_r) {
> +			tp->hysteresis.enabled = true;
> +			evdev_log_debug(tp->device, "hysteresis enabled\n");
> +		}
> +	}
> +}
> +
>   static inline void
>   tp_motion_hysteresis(struct tp_dispatch *tp,
>   		     struct tp_touch *t)
> @@ -265,6 +320,7 @@ tp_new_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
>   	t->time = time;
>   	t->speed.last_speed = 0;
>   	t->speed.exceeded_count = 0;
> +	t->hysteresis.x_motion_history = 0;
>   	tp->queued |= TOUCHPAD_EVENT_MOTION;
>   }
>   
> @@ -1443,7 +1499,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
>   
>   		tp_thumb_detect(tp, t, time);
>   		tp_palm_detect(tp, t, time);
> -
> +		tp_detect_wobbling(tp, t, time);
>   		tp_motion_hysteresis(tp, t);
>   		tp_motion_history_push(t);
>   
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 3ce893d2..1fd8f1e3 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -149,6 +149,7 @@ struct tp_touch {
>   	bool has_ended;				/* TRACKING_ID == -1 */
>   	bool dirty;
>   	struct device_coords point;
> +	struct device_coords last_point;
>   	uint64_t time;
>   	int pressure;
>   	bool is_tool_palm; /* MT_TOOL_PALM */
> @@ -177,6 +178,7 @@ struct tp_touch {
>   
>   	struct {
>   		struct device_coords center;
> +		uint8_t x_motion_history;
>   	} hysteresis;
>   
>   	/* A pinned touchpoint is the one that pressed the physical button
>