Re: wsmouse(4): tapping
Hi Ulf, * Ulf Brosziewski wrote: > On 5/3/22 10:03, Ulf Brosziewski wrote: > > 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? I tested your patch on a Thinkpad T450s and all functions (1/2/3 finger tapping) work as before. Addtionally, I can now use xterm's menu (Ctrl + left/right click) without using the physical mouse buttons. So IMO a great improvement for touchpad users not having physical buttons. Cheers and thanks Matthias
Re: wsmouse(4): tapping
On 5/3/22 10:03, Ulf Brosziewski wrote:
> 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
> [...]
This version of the patch extends the synchronization of button states
to the tapping timeouts. With this extension, even quite exotic
combinations of button and tapping inputs will produce proper pairs
of button-up and button-down events.
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 - 1.30
+++ dev/wscons/wstpad.c 6 May 2022 08:39:21 -
@@ -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 = >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(>orig.time,
>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);
}
+/* Determine the "tap button", keep track of whether a touch is masked. */
+u_int
+wstpad_tap_button(struct wstpad *tp)
+{
+ 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);
+}
+
/*
- * 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.
+ * In the hold/drag state, do not mask touches if no masking was involved
+ * in the preceding tap gesture.
*/
static inline int
-tap_finished(struct wstpad *tp, int nmasked)
+tap_unmask(struct wstpad *tp)
{
- return (tp->contacts == nmasked
- && (nmasked == 0 || !wstpad_is_tap(tp, tp->t)));
-}
-
-static inline u_int
-tap_btn(struct wstpad *tp, int nmasked)
-{
- 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
Re: wsmouse(4): tapping
... and this is the complete link:
https://marc.info/?l=openbsd-tech=165006255922269=2
On 5/3/22 10:03, Ulf Brosziewski wrote:
> 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 - 1.30
> +++ dev/wscons/wstpad.c 2 May 2022 22:17:57 -
> @@ -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 = >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(>orig.time,
> >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,
wsmouse(4): tapping
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 - 1.30
+++ dev/wscons/wstpad.c 2 May 2022 22:17:57 -
@@ -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 = >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(>orig.time,
>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;
