The implementation of the tapping mechanism in wsmouse(4) has a bug
concerning the transitions into the hold/drag state, see
https://marc.info/...
for details. The patch proposed in that message is obsolete. I've
been made aware that there is another problem, the transition only
works with left-button taps.
This patch merges tap detection and the handling of hold/drag states,
which is a cleaner and more generic approach. It fixes the problems
mentioned above, and it permits a better synchronization of button
states when tap inputs and the use of hardware buttons are mixed.
OK?
Index: dev/wscons/wstpad.c
===================================================================
RCS file: /cvs/src/sys/dev/wscons/wstpad.c,v
retrieving revision 1.30
diff -u -p -r1.30 wstpad.c
--- dev/wscons/wstpad.c 24 Mar 2021 18:28:24 -0000 1.30
+++ dev/wscons/wstpad.c 2 May 2022 22:17:57 -0000
@@ -82,18 +82,17 @@ enum tap_state {
TAP_DETECT,
TAP_IGNORE,
TAP_LIFTED,
- TAP_2ND_TOUCH,
TAP_LOCKED,
- TAP_NTH_TOUCH,
+ TAP_LOCKED_DRAG,
};
enum tpad_cmd {
CLEAR_MOTION_DELTAS,
SOFTBUTTON_DOWN,
SOFTBUTTON_UP,
+ TAPBUTTON_SYNC,
TAPBUTTON_DOWN,
TAPBUTTON_UP,
- TAPBUTTON_DOUBLECLK,
VSCROLL,
HSCROLL,
};
@@ -212,8 +211,10 @@ struct wstpad {
struct {
enum tap_state state;
int contacts;
- int centered;
+ int valid;
+ u_int pending;
u_int button;
+ int masked;
int maxdist;
struct timeout to;
/* parameters: */
@@ -651,6 +652,7 @@ wstpad_softbuttons(struct wsmouseinput *
}
}
+/* Check whether the duration of t is within the tap limit. */
int
wstpad_is_tap(struct wstpad *tp, struct tpad_touch *t)
{
@@ -675,7 +677,7 @@ wstpad_tap_filter(struct wstpad *tp, str
dy = abs(t->y - t->orig.y) * tp->ratio;
dist = (dx >= dy ? dx + 3 * dy / 8 : dy + 3 * dx / 8);
}
- tp->tap.centered = (CENTERED(t) && dist <= (tp->tap.maxdist << 12));
+ tp->tap.valid = (CENTERED(t) && dist <= (tp->tap.maxdist << 12));
}
@@ -694,7 +696,7 @@ wstpad_tap_touch(struct wsmouseinput *in
lifted = (input->mt.sync[MTS_TOUCH] & ~input->mt.touches);
FOREACHBIT(lifted, slot) {
s = &tp->tpad_touches[slot];
- if (tp->tap.state == TAP_DETECT && !tp->tap.centered)
+ if (tp->tap.state == TAP_DETECT && !tp->tap.valid)
wstpad_tap_filter(tp, s);
if (t == NULL || timespeccmp(&t->orig.time,
&s->orig.time, >))
@@ -703,7 +705,7 @@ wstpad_tap_touch(struct wsmouseinput *in
} else {
if (tp->t->state == TOUCH_END) {
t = tp->t;
- if (tp->tap.state == TAP_DETECT && !tp->tap.centered)
+ if (tp->tap.state == TAP_DETECT && !tp->tap.valid)
wstpad_tap_filter(tp, t);
}
}
@@ -711,30 +713,31 @@ wstpad_tap_touch(struct wsmouseinput *in
return (t);
}
-/*
- * If each contact in a sequence of contacts that overlap in time
- * is a tap, a button event may be generated when the number of
- * contacts drops to zero, or to one if there is a masked touch.
- */
-static inline int
-tap_finished(struct wstpad *tp, int nmasked)
+/* Determine the "tap button", keep track of whether a touch is masked. */
+static inline u_int
+tap_button(struct wstpad *tp)
{
- return (tp->contacts == nmasked
- && (nmasked == 0 || !wstpad_is_tap(tp, tp->t)));
+ int n = tp->tap.contacts - tp->contacts - 1;
+
+ tp->tap.masked = tp->contacts;
+
+ return (n >= 0 && n < TAP_BTNMAP_SIZE ? tp->tap.btnmap[n] : 0);
}
-static inline u_int
-tap_btn(struct wstpad *tp, int nmasked)
+/*
+ * In the hold/drag state, do not mask touches if no masking was involved
+ * in the preceding tap gesture.
+ */
+static inline int
+tap_unmask(struct wstpad *tp)
{
- int n = tp->tap.contacts - nmasked;
-
- return (n <= TAP_BTNMAP_SIZE ? tp->tap.btnmap[n - 1] : 0);
+ return ((tp->tap.button || tp->tap.pending) && tp->tap.masked == 0);
}
/*
- * This handler supports one-, two-, and three-finger-taps, which
- * are mapped to left-button, right-button and middle-button events,
- * respectively; moreover, it supports tap-and-drag operations with
+ * In the default configuration, this handler maps one-, two-, and
+ * three-finger taps to left-button, right-button, and middle-button
+ * events, respectively. Setting the LOCKTIME parameter enables
* "locked drags", which are finished by a timeout or a tap-to-end
* gesture.
*/
@@ -743,140 +746,111 @@ wstpad_tap(struct wsmouseinput *input, u
{
struct wstpad *tp = input->tp;
struct tpad_touch *t;
- int nmasked, err = 0;
+ int contacts, is_tap, slot, err = 0;
- if (tp->btns) {
- /*
- * Don't process tapping while hardware buttons are being
- * pressed. If the handler is not in its initial state,
- * release the "tap button".
- */
- if (tp->tap.state > TAP_IGNORE) {
- timeout_del(&tp->tap.to);
- *cmds |= 1 << TAPBUTTON_UP;
- }
- /*
- * It might be possible to produce a click within the tap
- * timeout; ignore the current touch.
- */
- tp->tap.state = TAP_IGNORE;
- tp->tap.contacts = 0;
- tp->tap.centered = 0;
- }
+ /* Synchronize the button states, if necessary. */
+ if (input->btn.sync)
+ *cmds |= 1 << TAPBUTTON_SYNC;
/*
- * If a touch from the bottom area is masked, reduce the
- * contact counts and ignore it.
+ * It is possible to produce a click within the tap timeout.
+ * Wait for a new touch before generating new button events.
*/
- nmasked = (input->mt.ptr_mask ? 1 : 0);
+ if (PRIMARYBTN_RELEASED(tp))
+ tp->tap.contacts = 0;
+
+ /* Reset the detection state whenever a new touch starts. */
+ if (tp->contacts > tp->prev_contacts || (IS_MT(tp) &&
+ (input->mt.touches & input->mt.sync[MTS_TOUCH]))) {
+ tp->tap.contacts = tp->contacts;
+ tp->tap.valid = 0;
+ }
/*
- * Only touches in the TOUCH_END state are relevant here.
- * t is NULL if no touch has been lifted.
+ * The filtered number of active touches excludes a masked
+ * touch if its duration exceeds the tap limit.
*/
- t = wstpad_tap_touch(input);
+ contacts = tp->contacts;
+ if ((slot = ffs(input->mt.ptr_mask) - 1) >= 0
+ && !wstpad_is_tap(tp, &tp->tpad_touches[slot])
+ && !tap_unmask(tp)) {
+ contacts--;
+ }
switch (tp->tap.state) {
case TAP_DETECT:
- if (tp->contacts > tp->tap.contacts)
- tp->tap.contacts = tp->contacts;
-
+ /* Find the oldest touch in the TOUCH_END state. */
+ t = wstpad_tap_touch(input);
if (t) {
- if (wstpad_is_tap(tp, t)) {
- if (tap_finished(tp, nmasked)) {
- if (tp->tap.centered) {
- tp->tap.state = TAP_LIFTED;
- tp->tap.button =
- tap_btn(tp, nmasked);
- }
- tp->tap.contacts = 0;
- tp->tap.centered = 0;
+ is_tap = wstpad_is_tap(tp, t);
+ if (is_tap && contacts == 0) {
+ if (tp->tap.button)
+ *cmds |= 1 << TAPBUTTON_UP;
+ tp->tap.pending = (tp->tap.valid
+ ? tap_button(tp) : 0);
+ if (tp->tap.pending) {
+ tp->tap.state = TAP_LIFTED;
+ err = !timeout_add_msec(&tp->tap.to,
+ CLICKDELAY_MS);
}
- } else {
- if (tp->contacts > nmasked)
+ } else if (!is_tap && tp->tap.locktime == 0) {
+ if (contacts == 0 && tp->tap.button)
+ *cmds |= 1 << TAPBUTTON_UP;
+ else if (contacts)
tp->tap.state = TAP_IGNORE;
- tp->tap.contacts = 0;
- tp->tap.centered = 0;
- }
- if (tp->tap.state == TAP_LIFTED) {
- if (tp->tap.button != 0) {
- *cmds |= 1 << TAPBUTTON_DOWN;
+ } else if (!is_tap && tp->tap.button) {
+ if (contacts == 0) {
+ tp->tap.state = TAP_LOCKED;
err = !timeout_add_msec(&tp->tap.to,
- tp->tap.clicktime);
+ tp->tap.locktime);
} else {
- tp->tap.state = TAP_DETECT;
+ tp->tap.state = TAP_LOCKED_DRAG;
}
}
}
break;
-
case TAP_IGNORE:
- if (tp->contacts == nmasked)
+ if (contacts == 0) {
tp->tap.state = TAP_DETECT;
- break;
- case TAP_LIFTED:
- if (tp->contacts > nmasked) {
- timeout_del(&tp->tap.to);
- if (tp->tap.button == LEFTBTN) {
- tp->tap.state = TAP_2ND_TOUCH;
- } else {
+ if (tp->tap.button)
*cmds |= 1 << TAPBUTTON_UP;
- tp->tap.state = TAP_DETECT;
- }
}
break;
- case TAP_2ND_TOUCH:
- if (t) {
- if (wstpad_is_tap(tp, t)) {
- *cmds |= 1 << TAPBUTTON_DOUBLECLK;
- tp->tap.state = TAP_LIFTED;
- err = !timeout_add_msec(&tp->tap.to,
- CLICKDELAY_MS);
- } else if (tp->contacts == nmasked) {
- if (tp->tap.locktime == 0) {
- *cmds |= 1 << TAPBUTTON_UP;
- tp->tap.state = TAP_DETECT;
- } else {
- tp->tap.state = TAP_LOCKED;
- err = !timeout_add_msec(&tp->tap.to,
- tp->tap.locktime);
- }
- }
- } else if (tp->contacts != nmasked + 1) {
- *cmds |= 1 << TAPBUTTON_UP;
+ case TAP_LIFTED:
+ if (contacts) {
+ timeout_del(&tp->tap.to);
tp->tap.state = TAP_DETECT;
+ if (tp->tap.pending)
+ *cmds |= 1 << TAPBUTTON_DOWN;
}
break;
case TAP_LOCKED:
- if (tp->contacts > nmasked) {
+ if (contacts) {
timeout_del(&tp->tap.to);
- tp->tap.state = TAP_NTH_TOUCH;
+ tp->tap.state = TAP_LOCKED_DRAG;
}
break;
- case TAP_NTH_TOUCH:
- if (t) {
- if (wstpad_is_tap(tp, t)) {
+ case TAP_LOCKED_DRAG:
+ if (contacts == 0) {
+ t = wstpad_tap_touch(input);
+ if (t && wstpad_is_tap(tp, t)) {
/* "tap-to-end" */
*cmds |= 1 << TAPBUTTON_UP;
tp->tap.state = TAP_DETECT;
- } else if (tp->contacts == nmasked) {
+ } else {
tp->tap.state = TAP_LOCKED;
err = !timeout_add_msec(&tp->tap.to,
tp->tap.locktime);
}
- } else if (tp->contacts != nmasked + 1) {
- *cmds |= 1 << TAPBUTTON_UP;
- tp->tap.state = TAP_DETECT;
}
break;
}
if (err) { /* Did timeout_add fail? */
- if (tp->tap.state == TAP_LIFTED)
- *cmds &= ~(1 << TAPBUTTON_DOWN);
- else
- *cmds |= 1 << TAPBUTTON_UP;
-
+ input->sbtn.buttons &= ~tp->tap.button;
+ input->sbtn.sync |= tp->tap.button;
+ tp->tap.pending = 0;
+ tp->tap.button = 0;
tp->tap.state = TAP_DETECT;
}
}
@@ -888,19 +862,31 @@ wstpad_tap_timeout(void *p)
struct wstpad *tp = input->tp;
struct evq_access evq;
u_int btn;
- int s;
+ int s, ev;
s = spltty();
evq.evar = *input->evar;
if (evq.evar != NULL && tp != NULL &&
(tp->tap.state == TAP_LIFTED || tp->tap.state == TAP_LOCKED)) {
- tp->tap.state = TAP_DETECT;
- input->sbtn.buttons &= ~tp->tap.button;
- btn = ffs(tp->tap.button) - 1;
+
+ if (tp->tap.pending) {
+ tp->tap.button = tp->tap.pending;
+ tp->tap.pending = 0;
+ ev = BTN_DOWN_EV;
+ btn = ffs(tp->tap.button) - 1;
+ input->sbtn.buttons |= tp->tap.button;
+ timeout_add_msec(&tp->tap.to, tp->tap.clicktime);
+ } else {
+ ev = BTN_UP_EV;
+ btn = ffs(tp->tap.button) - 1;
+ input->sbtn.buttons &= ~tp->tap.button;
+ tp->tap.button = 0;
+ tp->tap.state = TAP_DETECT;
+ }
evq.put = evq.evar->put;
evq.result = EVQ_RESULT_NONE;
getnanotime(&evq.ts);
- wsmouse_evq_put(&evq, BTN_UP_EV, btn);
+ wsmouse_evq_put(&evq, ev, btn);
wsmouse_evq_put(&evq, SYNC_EV, 0);
if (evq.result == EVQ_RESULT_SUCCESS) {
if (input->flags & LOG_EVENTS) {
@@ -928,15 +914,12 @@ wstpad_click(struct wsmouseinput *input)
set_freeze_ts(tp, 0, FREEZE_MS);
}
-/*
- * Translate the "command" bits into the sync-state of wsmouse, or into
- * wscons events.
- */
+/* Translate the "command" bits into the sync-state of wsmouse. */
void
-wstpad_cmds(struct wsmouseinput *input, struct evq_access *evq, u_int cmds)
+wstpad_cmds(struct wsmouseinput *input, u_int cmds)
{
struct wstpad *tp = input->tp;
- u_int btn, sbtns_dn = 0, sbtns_up = 0;
+ u_int tapbtn, sbtns_dn = 0, sbtns_up = 0;
int n;
FOREACHBIT(cmds, n) {
@@ -955,24 +938,24 @@ wstpad_cmds(struct wsmouseinput *input,
sbtns_up |= tp->softbutton;
tp->softbutton = 0;
continue;
+ case TAPBUTTON_SYNC:
+ if ((tapbtn = (tp->tap.button | tp->tap.pending))) {
+ input->btn.sync &= ~tapbtn;
+ sbtns_up &= ~tapbtn;
+ sbtns_dn &= ~tapbtn;
+ }
+ continue;
case TAPBUTTON_DOWN:
+ tp->tap.button = tp->tap.pending;
+ tp->tap.pending = 0;
sbtns_dn |= tp->tap.button;
continue;
case TAPBUTTON_UP:
- sbtns_up |= tp->tap.button;
- continue;
- case TAPBUTTON_DOUBLECLK:
- /*
- * We cannot add the final BTN_UP event here, a
- * delay is required. This is the reason why the
- * tap handler returns from the 2ND_TOUCH state
- * into the LIFTED state with a short timeout
- * (CLICKDELAY_MS).
- */
- btn = ffs(PRIMARYBTN) - 1;
- wsmouse_evq_put(evq, BTN_UP_EV, btn);
- wsmouse_evq_put(evq, SYNC_EV, 0);
- wsmouse_evq_put(evq, BTN_DOWN_EV, btn);
+ if ((tp->tap.button & input->btn.buttons) == 0
+ && tp->tap.button != tp->softbutton) {
+ sbtns_up |= tp->tap.button;
+ }
+ tp->tap.button = 0;
continue;
case HSCROLL:
input->motion.dw = tp->scroll.dw;
@@ -1276,7 +1259,7 @@ wstpad_process_input(struct wsmouseinput
}
}
- wstpad_cmds(input, evq, cmds);
+ wstpad_cmds(input, cmds);
if (IS_MT(tp))
clear_touchstates(input, TOUCH_NONE);
