[PATCH libinput 4/4] touchpad: reset the motion history on significant negative pressure changes

[Hans de Goede]

Hi,

On 04/01/2016 03:10 AM, Peter Hutterer wrote:
> Resetting the motion history has the side-effect of swallowing movements, we
> don't calculate deltas until we have 4 motion events. During a finger release,
> we're likely to get a large pressure change between two events, resetting the
> motion history prevents the cursor from jumping on release.
>
> The value of 7 found by trial-and-error, tested on the T440 and T450 hardware.
> The absolute value is highly variable but recordings show that the pressure
> changes only by 1 or 2 units during normal interaction. Higher pressure
> changes are during finger position changes but since those should not cause a
> jump anyway, we tend to win there too.
>
> Currently only enabled for negative pressure changes, let's see how we go with
> that.
>
> https://bugs.freedesktop.org/show_bug.cgi?id=94379
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
>   src/evdev-mt-touchpad.c |  4 ++++
>   src/evdev-mt-touchpad.h |  1 +
>   test/litest.c           | 38 +++++++++++++++++++++++++++-----------
>   test/litest.h           |  7 +++++++
>   test/touchpad.c         |  2 +-
>   5 files changed, 40 insertions(+), 12 deletions(-)
>
> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
> index e16aecb..0640974 100644
> --- a/src/evdev-mt-touchpad.c
> +++ b/src/evdev-mt-touchpad.c
> @@ -335,6 +335,7 @@ tp_process_absolute(struct tp_dispatch *tp,
>   			tp_end_sequence(tp, t, time);
>   		break;
>   	case ABS_MT_PRESSURE:
> +		t->pressure_delta = e->value - t->pressure;
>   		t->pressure = e->value;
>   		t->dirty = true;
>   		tp->queued |= TOUCHPAD_EVENT_OTHERAXIS;
> @@ -946,6 +947,9 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
>   		if (!t->dirty)
>   			continue;
>
> +		if (t->pressure_delta < -7)
> +			tp_motion_history_reset(t);
> +

Do you really want to do this on all touchpads and not just on t440 / t450
and friends ?

Otherwise this series looks good and is:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


>   		tp_thumb_detect(tp, t, time);
>   		tp_palm_detect(tp, t, time);
>
> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
> index 1f05a03..d1dae83 100644
> --- a/src/evdev-mt-touchpad.h
> +++ b/src/evdev-mt-touchpad.h
> @@ -154,6 +154,7 @@ struct tp_touch {
>   	uint64_t millis;
>   	int distance;				/* distance == 0 means touch */
>   	int pressure;
> +	int pressure_delta;
>
>   	struct {
>   		/* A quirk mostly used on Synaptics touchpads. In a
> diff --git a/test/litest.c b/test/litest.c
> index f679652..1d729ef 100644
> --- a/test/litest.c
> +++ b/test/litest.c
> @@ -1494,23 +1494,39 @@ litest_touch_move_extended(struct litest_device *d,
>   }
>
>   void
> +litest_touch_move_to_extended(struct litest_device *d,
> +			      unsigned int slot,
> +			      double x_from, double y_from,
> +			      double x_to, double y_to,
> +			      struct axis_replacement *axes,
> +			      int steps, int sleep_ms)
> +{
> +	for (int i = 0; i < steps - 1; i++) {
> +		litest_touch_move_extended(d, slot,
> +					   x_from + (x_to - x_from)/steps * i,
> +					   y_from + (y_to - y_from)/steps * i,
> +					   axes);
> +		if (sleep_ms) {
> +			libinput_dispatch(d->libinput);
> +			msleep(sleep_ms);
> +			libinput_dispatch(d->libinput);
> +		}
> +	}
> +	litest_touch_move_extended(d, slot, x_to, y_to, axes);
> +}
> +
> +void
>   litest_touch_move_to(struct litest_device *d,
>   		     unsigned int slot,
>   		     double x_from, double y_from,
>   		     double x_to, double y_to,
>   		     int steps, int sleep_ms)
>   {
> -	for (int i = 0; i < steps - 1; i++) {
> -		litest_touch_move(d, slot,
> -				  x_from + (x_to - x_from)/steps * i,
> -				  y_from + (y_to - y_from)/steps * i);
> -		if (sleep_ms) {
> -			libinput_dispatch(d->libinput);
> -			msleep(sleep_ms);
> -			libinput_dispatch(d->libinput);
> -		}
> -	}
> -	litest_touch_move(d, slot, x_to, y_to);
> +	litest_touch_move_to_extended(d, slot,
> +				      x_from, y_from,
> +				      x_to, y_to,
> +				      NULL,
> +				      steps, sleep_ms);
>   }
>
>   static int
> diff --git a/test/litest.h b/test/litest.h
> index e854210..c218361 100644
> --- a/test/litest.h
> +++ b/test/litest.h
> @@ -403,6 +403,13 @@ litest_touch_move_to(struct litest_device *d,
>   		     double x_from, double y_from,
>   		     double x_to, double y_to,
>   		     int steps, int sleep_ms);
> +void
> +litest_touch_move_to_extended(struct litest_device *d,
> +			      unsigned int slot,
> +			      double x_from, double y_from,
> +			      double x_to, double y_to,
> +			      struct axis_replacement *axes,
> +			      int steps, int sleep_ms);
>
>   void
>   litest_touch_move_two_touches(struct litest_device *d,
> diff --git a/test/touchpad.c b/test/touchpad.c
> index 4651b7a..a58b337 100644
> --- a/test/touchpad.c
> +++ b/test/touchpad.c
> @@ -3594,7 +3594,7 @@ START_TEST(touchpad_thumb_edgescroll)
>   	libinput_dispatch(li);
>   	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_AXIS);
>
> -	litest_touch_move_to(dev, 0, 99, 55, 99, 70, 10, 0);
> +	litest_touch_move_to_extended(dev, 0, 99, 55, 99, 70, axes, 10, 0);
>
>   	litest_assert_only_typed_events(li, LIBINPUT_EVENT_POINTER_AXIS);
>   }
>