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;