wstpad: counting vs timing

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

wstpad: counting vs timing

Ulf Brosziewski
This patch removes a loose end in the filter mechanisms of wstpad; it's
related to the one that was fixed a few weeks ago.

In order to determine whether a touch is moving stably, the driver counts
how often its position updates match, roughly, the same direction.  The
method works as intended, but it's an improvisation that looks a bit
hackish.  Moreover, while it identifies stable movements, it does not
identify touches that are resting stably (yes, that can be a problem).
The "thumb detection" method can interfere with two-finger scrolling for
this reason.

The new version identifies stable states by means of timestamps; instead
of counting matches, it simply checks for how long the current state has
been valid.  For recognizing resting touches properly, the direction
updates now require hysteresis-filtered input, and the callers are adapted
accordingly.

If you are masochistic and want to test this, take a laptop with an MT
touchpad and start upward scrolling with one finger in the bottom area,
and the other one in the main area of the touchpad; if you do that
repeatedly and somewhat sloppily, it may happen that you move the pointer.
With the patch, the detection should be much more reliable.

OK?


Index: dev/wscons/wstpad.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
retrieving revision 1.19
diff -u -p -r1.19 wstpad.c
--- dev/wscons/wstpad.c 10 Nov 2018 14:27:51 -0000 1.19
+++ dev/wscons/wstpad.c 5 Dec 2018 00:09:26 -0000
@@ -60,7 +60,8 @@

 #define CLICKDELAY_MS 20
 #define FREEZE_MS 100
-#define MATCHINTERVAL_MS 55
+#define MATCHINTERVAL_MS 45
+#define STOPINTERVAL_MS 55

 enum tpad_handlers {
  SOFTBUTTON_HDLR,
@@ -121,8 +122,8 @@ struct tpad_touch {
  int x;
  int y;
  int dir;
- int matches;
- struct timespec stop;
+ struct timespec start;
+ struct timespec match;
  struct {
  int x;
  int y;
@@ -226,6 +227,12 @@ struct wstpad {
  } scroll;
 };

+static const struct timespec match_interval =
+    { .tv_sec = 0, .tv_nsec = MATCHINTERVAL_MS * 1000000 };
+
+static const struct timespec stop_interval =
+    { .tv_sec = 0, .tv_nsec = STOPINTERVAL_MS * 1000000 };
+
 /*
  * Coordinates in the wstpad struct are "normalized" device coordinates,
  * the orientation is left-to-right and upward.
@@ -254,16 +261,11 @@ normalize_rel(struct axis_filter *filter
  *            7    |    4
  *               6 | 5
  *
- * Two direction values "match" each other if they are equal or adjacent in
- * this ring. Some handlers require that a movement is "stable" and check
- * the number of matches.
  */
 /* Tangent constants in [*.12] fixed-point format: */
 #define TAN_DEG_60 7094
 #define TAN_DEG_30 2365

-#define STABLE 3
-
 #define NORTH(d) ((d) == 0 || (d) == 11)
 #define SOUTH(d) ((d) == 5 || (d) == 6)
 #define EAST(d) ((d) == 2 || (d) == 3)
@@ -297,41 +299,59 @@ dircmp(int dir1, int dir2)
  return (diff <= 6 ? diff : 12 - diff);
 }

+/*
+ * Update direction and timespec attributes for a touch.  They are used to
+ * determine whether it is moving - or resting - stably.
+ *
+ * The callers pass touches from the current frame and the touches that are
+ * no longer present in the update cycle to this function.  Even though this
+ * ensures that pairs of zero deltas do not result from stale coordinates,
+ * zero deltas do not reset the state immediately.  A short time span - the
+ * "stop interval" - must pass before the state is cleared, which is
+ * necessary because some touchpads report intermediate stops when a touch
+ * is moving very slowly.
+ */
 void
 wstpad_set_direction(struct wstpad *tp, struct tpad_touch *t, int dx, int dy)
 {
  int dir;
- static const struct timespec interval =
-    { .tv_sec = 0, .tv_nsec = MATCHINTERVAL_MS * 1000000 };
+ struct timespec ts;

  if (t->state != TOUCH_UPDATE) {
  t->dir = -1;
- t->matches = 0;
- } else {
- dir = direction(dx, dy, tp->ratio);
- if (dir >= 0) {
- if (t->dir >= 0 && dircmp(t->dir, dir) <= 1)
- t->matches++;
- else
- t->matches = 1;
- t->dir = dir;
-
- timespecadd(&tp->time, &interval, &t->stop);
+ memcpy(&t->start, &tp->time, sizeof(struct timespec));
+ return;
+ }

- } else if (t->dir >= 0) {
- /*
- * Some touchpads report intermediate zero deltas
- * when a touch is moving very slowly.  Keep the
- * the state long enough to avoid errors.
- */
- if (timespeccmp(&tp->time, &t->stop, >=)) {
- t->dir = -1;
- t->matches = 0;
- }
+ dir = direction(dx, dy, tp->ratio);
+ if (dir >= 0) {
+ if (t->dir < 0 || dircmp(dir, t->dir) > 1) {
+ memcpy(&t->start, &tp->time, sizeof(struct timespec));
+ }
+ t->dir = dir;
+ memcpy(&t->match, &tp->time, sizeof(struct timespec));
+ } else if (t->dir >= 0) {
+ timespecsub(&tp->time, &t->match, &ts);
+ if (timespeccmp(&ts, &stop_interval, >=)) {
+ t->dir = -1;
+ memcpy(&t->start, &t->match, sizeof(struct timespec));
  }
  }
 }

+int
+wstpad_is_stable(struct wstpad *tp, struct tpad_touch *t)
+{
+ struct timespec ts;
+
+ if (t->dir >= 0)
+ timespecsub(&t->match, &t->start, &ts);
+ else
+ timespecsub(&tp->time, &t->start, &ts);
+
+ return (timespeccmp(&ts, &match_interval, >=));
+}
+
 /*
  * If a touch starts in an edge area, pointer movement will be
  * suppressed as long as it stays in that area.
@@ -387,11 +407,12 @@ chk_scroll_state(struct wstpad *tp)
  return (0);
  }
  /*
- * Try to exclude accidental scroll events by checking the matches.
- * The check, which causes a short delay, is only applied initially,
- * a touch that stops and resumes scrolling is not affected.
+ * Try to exclude accidental scroll events by checking whether the
+ * pointer-controlling touch is stable.  The check, which may cause
+ * a short delay, is only applied initially, a touch that stops and
+ * resumes scrolling is not affected.
  */
- if (tp->t->matches < STABLE && !(tp->scroll.dz || tp->scroll.dw))
+ if (!wstpad_is_stable(tp, tp->t) && !(tp->scroll.dz || tp->scroll.dw))
  return (0);

  return (tp->dx || tp->dy);
@@ -460,15 +481,14 @@ wstpad_f2scroll(struct wsmouseinput *inp
  return;
  if ((dx > 0 && !EAST(dir)) || (dx < 0 && !WEST(dir)))
  return;
- if (t2->matches < STABLE &&
+ if (!wstpad_is_stable(tp, t2) &&
     !(tp->scroll.dz || tp->scroll.dw))
  return;
  centered |= CENTERED(t2);
  }
  if (centered) {
  wstpad_scroll(tp, dx, dy, cmds);
- if (tp->t->matches > STABLE)
- set_freeze_ts(tp, 0, FREEZE_MS);
+ set_freeze_ts(tp, 0, FREEZE_MS);
  }
  }
 }
@@ -956,6 +976,8 @@ wstpad_mt_inputs(struct wsmouseinput *in
  dy = normalize_abs(&input->filter.v, mts->pos.y) - t->y;
  t->y += dy;
  t->flags &= (~EDGES | edge_flags(tp, t->x, t->y));
+ if (wsmouse_hysteresis(input, &mts->pos))
+ dx = dy = 0;
  wstpad_set_direction(tp, t, dx, dy);
  } else if ((1 << slot) & inactive) {
  wstpad_set_direction(tp, t, 0, 0);
@@ -971,7 +993,7 @@ wstpad_mt_masks(struct wsmouseinput *inp
  struct wstpad *tp = input->tp;
  struct tpad_touch *t;
  u_int mask;
- int d, slot;
+ int slot;

  tp->ignore &= input->mt.touches;

@@ -999,15 +1021,16 @@ wstpad_mt_masks(struct wsmouseinput *inp
  * touch is not, treat the latter as "thumb".  It will not block
  * pointer movement, and wstpad_f2scroll will ignore it.
  */
- if ((tp->dx || tp->dy) && (input->mt.ptr_mask & ~input->mt.ptr)) {
+ if (tp->t->dir >= 0
+    && wstpad_is_stable(tp, tp->t)
+    && (input->mt.ptr_mask & ~input->mt.ptr)) {
+
  slot = ffs(input->mt.ptr_mask) - 1;
  t = &tp->tpad_touches[slot];
- if (t->flags & B_EDGE) {
- d = tp->t->matches - t->matches;
- /* Do not hamper upward scrolling. */
- if (d > STABLE && (!NORTH(t->dir) || d > 2 * STABLE))
- tp->ignore = input->mt.ptr_mask;
- }
+
+ if ((t->flags & B_EDGE)
+    && t->dir < 0 && wstpad_is_stable(tp, t))
+ tp->ignore = input->mt.ptr_mask;
  }
 }

@@ -1018,9 +1041,17 @@ wstpad_touch_inputs(struct wsmouseinput
  struct tpad_touch *t;
  int slot;

- /* Use the unfiltered deltas. */
- tp->dx = normalize_rel(&input->filter.h, input->motion.pos.dx);
- tp->dy = normalize_rel(&input->filter.v, input->motion.pos.dy);
+ /*
+ * Use the normalized, hysteresis-filtered, but otherwise untransformed
+ * relative coordinates of the pointer-controlling touch for filtering
+ * and scrolling.
+ */
+ if (wsmouse_hysteresis(input, &input->motion.pos)) {
+ tp->dx = tp->dy = 0;
+ } else {
+ tp->dx = normalize_rel(&input->filter.h, input->motion.pos.dx);
+ tp->dy = normalize_rel(&input->filter.v, input->motion.pos.dy);
+ }

  tp->btns = input->btn.buttons;
  tp->btns_sync = input->btn.sync;