16.09.2014 07:08, Peter Hutterer wrote:
Touchpads are limited by a fixed sampling rate (usually 80Hz). Some finger
changes may happen too fast for this sampling rate, resulting in two distinct
event sequences:
* finger 1 up and finger 2 down in the same EV_SYN frame. Synaptics sees one
   finger down before and after and the changed coordinates
* finger 1 up and finger 2 down _between_ two EV_SYN frames. Synaptics sees one
   touchpoint move from f1 position to f2 position.

That move causes a large cursor jump. The former could be solved (with
difficulty) by adding fake EV_SYN handling after releasing touchpoints but
that won't fix the latter case.

Hello.

Thanks for recognizing the problem and opting for a simple criterion for detecting such jumps.

However, AFAICS, the patch just discards the motion without telling the FSM that it is in fact a new touch. So it may be bad for gestures.

Attached is a patch that I use to work around this issue since July that does attempt to tell the FSM about a different finger.

So as a solution for now limit the finger movement to 20mm per event.
Tests on a T440 and an x220 showed that this is just above what a reasonable
finger movement would trigger. If a movement is greater than that limit, reset
it to 0/0.

On devices without resolution, use 0.25 the touchpad's diagonal instead.

I have no objection to the resolution-aware threshold. My patch doesn't have it, but you can obviously replace the heuristic there with your version.


Signed-off-by: Peter Hutterer <[email protected]>
---
See the git repo below for a simple script to verify valid movement deltas
on your device if you feel like it:
https://github.com/whot/input-data-analysis/tree/master/touchpad-max-delta
Max I got was 9mm on the x220 and 17mm on the T440s but the latter was
really ripping the cursor around the screen beyond what I'd deem useful.

  src/synaptics.c    | 33 +++++++++++++++++++++++++++++++++
  src/synapticsstr.h |  2 ++
  2 files changed, 35 insertions(+)

diff --git a/src/synaptics.c b/src/synaptics.c
index 2e3ad0c..756751d 100644
--- a/src/synaptics.c
+++ b/src/synaptics.c
@@ -780,6 +780,23 @@ set_default_parameters(InputInfoPtr pInfo)
          pars->resolution_vert = 1;
      }

+    /* Touchpad sampling rate is too low to detect all movements.
+       A user may lift one finger and put another one down within the same
+       EV_SYN or even between samplings so the driver doesn't notice at all.
+
+       We limit the movement to 20 mm within one event, that is more than
+       recordings showed is needed (17mm on a T440).
+      */
+    if (pars->resolution_horiz > 1 &&
+        pars->resolution_vert > 1)
+        pars->maxDeltaMM = 20;
+    else {
+        /* on devices without resolution set the vector length to 0.25 of
+           the touchpad diagonal */
+        pars->maxDeltaMM = diag * 0.25;
+    }
+
+
      /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge 
parameters */
      if (pars->top_edge > pars->bottom_edge) {
          int tmp = pars->top_edge;
@@ -2229,6 +2246,13 @@ get_delta(SynapticsPrivate *priv, const struct 
SynapticsHwState *hw,
      *dy = integral;
  }

+/* Vector length, but not sqrt'ed, we only need it for comparison */
+static inline double
+vlenpow2(double x, double y)
+{
+    return x * x + y * y;
+}
+
  /**
   * Compute relative motion ('deltas') including edge motion.
   */
@@ -2238,6 +2262,7 @@ ComputeDeltas(SynapticsPrivate * priv, const struct 
SynapticsHwState *hw,
  {
      enum MovingState moving_state;
      double dx, dy;
+    double vlen;
      int delay = 1000000000;

      dx = dy = 0;
@@ -2283,6 +2308,14 @@ ComputeDeltas(SynapticsPrivate * priv, const struct 
SynapticsHwState *hw,
   out:
      priv->prevFingers = hw->numFingers;

+    vlen = vlenpow2(dx/priv->synpara.resolution_horiz,
+                    dy/priv->synpara.resolution_vert);
+
+    if (vlen > priv->synpara.maxDeltaMM * priv->synpara.maxDeltaMM) {
+        dx = 0;
+        dy = 0;
+    }
+
      *dxP = dx;
      *dyP = dy;

diff --git a/src/synapticsstr.h b/src/synapticsstr.h
index 4bd32ac..75f52d5 100644
--- a/src/synapticsstr.h
+++ b/src/synapticsstr.h
@@ -226,6 +226,8 @@ typedef struct _SynapticsParameters {
      int area_left_edge, area_right_edge, area_top_edge, area_bottom_edge;     
  /* area coordinates absolute */
      int softbutton_areas[4][4]; /* soft button area coordinates, 0 => right, 1 => 
middle , 2 => secondary right, 3 => secondary middle button */
      int hyst_x, hyst_y;         /* x and y width of hysteresis box */
+
+    int maxDeltaMM;               /* maximum delta movement (vector length) in 
mm */
  } SynapticsParameters;

  struct _SynapticsPrivateRec {

>From 0ffca2abdd53a278e93fc2ef38e223d1a200f2d0 Mon Sep 17 00:00:00 2001
From: "Alexander E. Patrakov" <[email protected]>
Date: Fri, 28 Mar 2014 23:57:56 +0600
Subject: [PATCH] Detect and discard huge coordinate jumps on touchpads.

Such jumps are usually consequences of the touchpad firmware
failing to notice that a different finger is in fact touching the
touchpad. If not discarded, such events lead to sudden pointer
jumps into screen corners.

BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=76722
Signed-off-by: Alexander E. Patrakov <[email protected]>
---
 src/evdev-mt-touchpad.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 src/evdev-mt-touchpad.h |  3 +++
 2 files changed, 50 insertions(+)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index d831b83..1ef40b8 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -31,6 +31,7 @@
 
 #define DEFAULT_ACCEL_NUMERATOR 1200.0
 #define DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR 700.0
+#define DEFAULT_JUMP_DETECTION_DENOMINATOR 8.0
 
 static inline int
 tp_hysteresis(int in, int center, int margin)
@@ -160,6 +161,47 @@ tp_end_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
 	tp->queued |= TOUCHPAD_EVENT_MOTION;
 }
 
+static inline void
+tp_disrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+	if (t->state == TOUCH_NONE)
+		return;
+
+	tp_end_touch(tp, t, time);
+	t->state = TOUCH_DISRUPT;
+}
+
+static inline void
+tp_undisrupt_touch(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+	if (t->state != TOUCH_DISRUPT)
+		return;
+
+	tp_begin_touch(tp, t, time);
+}
+
+static inline void
+tp_detect_jumps(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
+{
+	/* Motivation: https://bugs.freedesktop.org/show_bug.cgi?id=76722 */
+
+	struct tp_motion *oldpos;
+	int dx, dy;
+
+	if (t->history.count == 0)
+		return;
+
+	/* This is called before tp_motion_history_push(),
+	   so the latest historical datum is at offset 0.
+	 */
+	oldpos = tp_motion_history_offset(t, 0);
+	dx = abs(t->x - oldpos->x);
+	dy = abs(t->y - oldpos->y);
+
+	if (dx > tp->jump_threshold || dy > tp->jump_threshold)
+		tp_disrupt_touch(tp, t, time);
+}
+
 static double
 tp_estimate_delta(int x0, int x1, int x2, int x3)
 {
@@ -423,6 +465,7 @@ tp_process_state(struct tp_dispatch *tp, uint64_t time)
 
 		tp_palm_detect(tp, t, time);
 
+		tp_detect_jumps(tp, t, time);
 		tp_motion_hysteresis(tp, t);
 		tp_motion_history_push(t);
 
@@ -451,6 +494,8 @@ tp_post_process_state(struct tp_dispatch *tp, uint64_t time)
 		if (!t->dirty)
 			continue;
 
+		tp_undisrupt_touch(tp, t, time);
+
 		if (t->state == TOUCH_END)
 			t->state = TOUCH_NONE;
 		else if (t->state == TOUCH_BEGIN)
@@ -809,6 +854,8 @@ tp_init(struct tp_dispatch *tp,
 	tp->hysteresis.margin_y =
 		diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
 
+	tp->jump_threshold = diagonal / DEFAULT_JUMP_DETECTION_DENOMINATOR;
+
 	if (tp_init_scroll(tp) != 0)
 		return -1;
 
diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
index 83edf4f..0ee5fda 100644
--- a/src/evdev-mt-touchpad.h
+++ b/src/evdev-mt-touchpad.h
@@ -46,6 +46,7 @@ enum touch_state {
 	TOUCH_NONE = 0,
 	TOUCH_BEGIN,
 	TOUCH_UPDATE,
+	TOUCH_DISRUPT,
 	TOUCH_END
 };
 
@@ -167,6 +168,8 @@ struct tp_dispatch {
 		int32_t margin_y;
 	} hysteresis;
 
+	int32_t jump_threshold;
+
 	struct motion_filter *filter;
 
 	struct {
-- 
2.0.4

_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Reply via email to