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

[Peter Hutterer]

On Sun, Apr 03, 2016 at 09:54:32AM +0200, Hans de Goede wrote:
> 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 ?

yeah, I think so. We've had this issue on a couple of touchpads and it stems
from the same reason, the finger flattens out and the narrows again on
release. The actual pressure value is very device-specific so we can't use
absolute values, but i'd like to see if the delta changes things.

I'll add this to the commit message though to make it obvious that this is
just hopening for things on other touchpads :)

> 
> Otherwise this series looks good and is:
> 
> Reviewed-by: Hans de Goede <hdegoede at redhat.com>

thanks, much appreciated

Cheers,
   Peter

> 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);
> >  }
> >