On Tue, Feb 21, 2023 at 08:10:36PM +0100, Ulf Brosziewski wrote:
> This diff is an extension of Tobias Heider's proposal, which aims at
> providing "Apple-like" button inputs on clickpads.  I have added some
> things in order to approximate the behaviour of other input drivers.
> 
> It's a quick shot, and I have no idea whether it is sufficient in
> practice, it certainly needs thorough testing.
> 
> The wsconsctl part doesn't provide a named field yet.  With a
> recompiled wsconsctl and kernel, the command
> 
>     # wsconsctl mouse.param=72:1
> 
> activates the feature, if it is available (see below).
> 
> The patch contains a simple filter for distinguishing the two-finger
> inputs that should trigger right-button events from the ones that
> shouldn't:  If the distance between two contacts is small, the driver
> generates a right-button event; if it is greater than some threshold
> value, the second contact will be ignored.
> 
> When a touch is resting in the bottom area, it will be ignored, and no
> further filtering applies to the other touches.
> 
> You can inspect the threshold value with
> 
>     # wsconsctl mouse.param=143
> 
> and change it with
> 
>     # wsconsctl mouse.param=143:<new value>
> 
> The value is given in device units.  If the driver for your touchpad is
> imt(4), the default should correspond, roughly, to a distance of 35mm.
> The threshold is reduced by one third if a two-finger click involves a
> touch in the bottom area.  (On medium-sized touchpads, this may be
> necessary to leave enough room for left-button clicks performed by the
> thumb while the pointer-controlling touch remains on the touchpad.)
> 
> The feature won't work decently on small touchpads, and it cannot work
> on touchpads without MT-support in our kernel.  wsmouse checks whether
> a touchpad
>     1) has MT support,
>     2) is a clickpad,
>     3) its resolution is reported to wsmouse,
>     4) it reports a horizontal size greater than 100mm, and
>     5) a vertical size greater than 60mm.
> 
> If these conditions aren't met, wsmouse sets the distance limit to -1,
> which blocks the MTBUTTONS feature.  I think only imt(4) touchpads can
> meet these criteria; however, the value can be overridden manually or
> programmatically, and ubcmtp and aplms do this on initialization.
> These drivers don't report resolution values; the distance limit will
> be set to a fourth of the length of the touchpad diagonal.  That's a
> workaround based on a wild guess, and I couldn't test it with Apple
> hardware.  If you want to apply it to an Elantech-v4 touchpad run by
> pms(4), try
> 
>     # wsconsctl mouse.param=143:0,72:1
> 
> (A change from -1 to 0 will trigger the workaround.)

Wow, thank you for looking into this! I've used your version for a few days
now and it works really well for me (on a m2 macbook air).  I actually think
the default works so well that we can default to 0 for param 143.

IMO we can add it to the tree at this point to give others the change to
test it before the diff gets even bigger.

> 
> 
> diff --git a/sbin/wsconsctl/mousecfg.c b/sbin/wsconsctl/mousecfg.c
> index 76a9984bd86..d6609218372 100644
> --- a/sbin/wsconsctl/mousecfg.c
> +++ b/sbin/wsconsctl/mousecfg.c
> @@ -40,9 +40,9 @@
>  #define TP_FILTER_FIRST              WSMOUSECFG_DX_MAX
>  #define TP_FILTER_LAST               WSMOUSECFG_SMOOTHING
>  #define TP_FEATURES_FIRST    WSMOUSECFG_SOFTBUTTONS
> -#define TP_FEATURES_LAST     WSMOUSECFG_DISABLE
> +#define TP_FEATURES_LAST     WSMOUSECFG_MTBUTTONS
>  #define TP_SETUP_FIRST               WSMOUSECFG_LEFT_EDGE
> -#define TP_SETUP_LAST                WSMOUSECFG_TAP_THREE_BTNMAP
> +#define TP_SETUP_LAST                WSMOUSECFG_MTBTN_MAXDIST
>  #define LOG_FIRST            WSMOUSECFG_LOG_INPUT
>  #define LOG_LAST             WSMOUSECFG_LOG_EVENTS
> 
> diff --git a/sys/arch/arm64/dev/aplhidev.c b/sys/arch/arm64/dev/aplhidev.c
> index 265c5196168..b3bf4838fe8 100644
> --- a/sys/arch/arm64/dev/aplhidev.c
> +++ b/sys/arch/arm64/dev/aplhidev.c
> @@ -680,6 +680,10 @@ struct ubcmtp_finger {
>  /* Use a constant, synaptics-compatible pressure value for now. */
>  #define DEFAULT_PRESSURE     40
> 
> +static struct wsmouse_param aplms_wsmousecfg[] = {
> +     { WSMOUSECFG_MTBTN_MAXDIST, 0 }, /* 0: Compute a default value. */
> +};
> +
>  struct aplms_softc {
>       struct device   sc_dev;
>       struct device   *sc_wsmousedev;
> @@ -759,7 +763,8 @@ aplms_configure(struct aplms_softc *sc)
>       hw->mt_slots = UBCMTP_MAX_FINGERS;
>       hw->flags = WSMOUSEHW_MT_TRACKING;
> 
> -     return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
> +     return wsmouse_configure(sc->sc_wsmousedev,
> +         aplms_wsmousecfg, nitems(aplms_wsmousecfg));
>  }
> 
>  void
> diff --git a/sys/dev/hid/hidmt.c b/sys/dev/hid/hidmt.c
> index 62b500a4f44..9e01fe597bf 100644
> --- a/sys/dev/hid/hidmt.c
> +++ b/sys/dev/hid/hidmt.c
> @@ -103,7 +103,7 @@ hidmt_get_resolution(struct hid_item *h)
>               phy_extent *= 10;
>       }
> 
> -     return (log_extent / phy_extent);
> +     return ((log_extent + phy_extent / 2) / phy_extent);
>  }
> 
>  int
> diff --git a/sys/dev/usb/ubcmtp.c b/sys/dev/usb/ubcmtp.c
> index d86883bd6c2..b5acdadef46 100644
> --- a/sys/dev/usb/ubcmtp.c
> +++ b/sys/dev/usb/ubcmtp.c
> @@ -309,6 +309,10 @@ static const struct ubcmtp_dev ubcmtp_devices[] = {
>       },
>  };
> 
> +static struct wsmouse_param ubcmtp_wsmousecfg[] = {
> +     { WSMOUSECFG_MTBTN_MAXDIST, 0 }, /* 0: Compute a default value. */
> +};
> +
>  struct ubcmtp_softc {
>       struct device           sc_dev;         /* base device */
> 
> @@ -529,7 +533,8 @@ ubcmtp_configure(struct ubcmtp_softc *sc)
>       hw->mt_slots = UBCMTP_MAX_FINGERS;
>       hw->flags = WSMOUSEHW_MT_TRACKING;
> 
> -     return wsmouse_configure(sc->sc_wsmousedev, NULL, 0);
> +     return wsmouse_configure(sc->sc_wsmousedev,
> +         ubcmtp_wsmousecfg, nitems(ubcmtp_wsmousecfg));
>  }
> 
>  int
> diff --git a/sys/dev/wscons/wsconsio.h b/sys/dev/wscons/wsconsio.h
> index de483493360..7de7c356e9a 100644
> --- a/sys/dev/wscons/wsconsio.h
> +++ b/sys/dev/wscons/wsconsio.h
> @@ -319,6 +319,7 @@ enum wsmousecfg {
>       WSMOUSECFG_SWAPSIDES,           /* invert soft-button/scroll areas */
>       WSMOUSECFG_DISABLE,             /* disable all output except for
>                                          clicks in the top-button area */
> +     WSMOUSECFG_MTBUTTONS,           /* multi-touch buttons */
> 
>       /*
>        * Touchpad options
> @@ -340,6 +341,8 @@ enum wsmousecfg {
>       WSMOUSECFG_TAP_ONE_BTNMAP,      /* one-finger tap button mapping */
>       WSMOUSECFG_TAP_TWO_BTNMAP,      /* two-finger tap button mapping */
>       WSMOUSECFG_TAP_THREE_BTNMAP,    /* three-finger tap button mapping */
> +     WSMOUSECFG_MTBTN_MAXDIST,       /* MTBUTTONS: distance limit for
> +                                        two-finger clicks */
> 
>       /*
>        * Enable/Disable debug output.
> @@ -347,7 +350,7 @@ enum wsmousecfg {
>       WSMOUSECFG_LOG_INPUT = 256,
>       WSMOUSECFG_LOG_EVENTS,
>  };
> -#define WSMOUSECFG_MAX       41      /* max size of param array per ioctl */
> +#define WSMOUSECFG_MAX       43      /* max size of param array per ioctl */
> 
>  struct wsmouse_param {
>       enum wsmousecfg key;
> diff --git a/sys/dev/wscons/wstpad.c b/sys/dev/wscons/wstpad.c
> index be074b89fb8..ea32242a9a9 100644
> --- a/sys/dev/wscons/wstpad.c
> +++ b/sys/dev/wscons/wstpad.c
> @@ -149,6 +149,7 @@ struct tpad_touch {
>  #define WSTPAD_HORIZSCROLL   (1 << 5)
>  #define WSTPAD_SWAPSIDES     (1 << 6)
>  #define WSTPAD_DISABLE               (1 << 7)
> +#define WSTPAD_MTBUTTONS     (1 << 8)
> 
>  #define WSTPAD_MT            (1 << 31)
> 
> @@ -201,6 +202,8 @@ struct wstpad {
>               /* two-finger contacts */
>               int f2pressure;
>               int f2width;
> +             /* MTBUTTONS: distance limit for two-finger clicks */
> +             int mtbtn_maxdist;
>       } params;
> 
>       /* handler state and configuration: */
> @@ -634,6 +637,37 @@ wstpad_get_sbtn(struct wsmouseinput *input, int top)
>       return (btn != PRIMARYBTN ? btn : 0);
>  }
> 
> +int
> +wstpad_mtbtn_contacts(struct wsmouseinput *input)
> +{
> +     struct wstpad *tp = input->tp;
> +     struct tpad_touch *t;
> +     int dx, dy, dist, limit;
> +
> +     if (tp->ignore != 0)
> +             return (tp->contacts - 1);
> +
> +     if (tp->contacts == 2 && (t = get_2nd_touch(input)) != NULL) {
> +             dx = abs(t->x - tp->t->x) << 12;
> +             dy = abs(t->y - tp->t->y) * tp->ratio;
> +             dist = (dx >= dy ? dx + 3 * dy / 8 : dy + 3 * dx / 8);
> +             limit = tp->params.mtbtn_maxdist << 12;
> +             if (input->mt.ptr_mask != 0)
> +                     limit = limit * 2 / 3;
> +             if (dist > limit)
> +                     return (1);
> +     }
> +     return (tp->contacts);
> +}
> +
> +u_int
> +wstpad_get_mtbtn(struct wsmouseinput *input)
> +{
> +     int contacts = wstpad_mtbtn_contacts(input);
> +     return (contacts == 2 ? RIGHTBTN : (contacts == 3 ? MIDDLEBTN : 0));
> +}
> +
> +
>  void
>  wstpad_softbuttons(struct wsmouseinput *input, u_int *cmds, int hdlr)
>  {
> @@ -646,7 +680,8 @@ wstpad_softbuttons(struct wsmouseinput *input, u_int 
> *cmds, int hdlr)
>       }
> 
>       if (tp->softbutton == 0 && PRIMARYBTN_CLICKED(tp)) {
> -             tp->softbutton = wstpad_get_sbtn(input, top);
> +             tp->softbutton = ((tp->features & WSTPAD_MTBUTTONS)
> +                 ? wstpad_get_mtbtn(input) : wstpad_get_sbtn(input, top));
>               if (tp->softbutton)
>                       *cmds |= 1 << SOFTBUTTON_DOWN;
>       }
> @@ -1599,6 +1634,15 @@ wstpad_configure(struct wsmouseinput *input)
>               tp->scroll.hdist = 4 * h_unit;
>               tp->scroll.vdist = 4 * v_unit;
>               tp->tap.maxdist = 4 * h_unit;
> +
> +             if (IS_MT(tp) && h_res > 1 && v_res > 1 &&
> +                 input->hw.hw_type == WSMOUSEHW_CLICKPAD &&
> +                 (width + h_res / 2) / h_res > 100 &&
> +                 (height + v_res / 2) / v_res > 60) {
> +                     tp->params.mtbtn_maxdist = h_res * 35;
> +             } else {
> +                     tp->params.mtbtn_maxdist = -1; /* not available */
> +             }
>       }
> 
>       /* A touch with a flag set in this mask does not move the pointer. */
> @@ -1621,7 +1665,7 @@ wstpad_configure(struct wsmouseinput *input)
> 
>       tp->handlers = 0;
> 
> -     if (tp->features & WSTPAD_SOFTBUTTONS)
> +     if (tp->features & (WSTPAD_SOFTBUTTONS | WSTPAD_MTBUTTONS))
>               tp->handlers |= 1 << SOFTBUTTON_HDLR;
>       if (tp->features & WSTPAD_TOPBUTTONS)
>               tp->handlers |= 1 << TOPBUTTON_HDLR;
> @@ -1685,13 +1729,14 @@ int
>  wstpad_set_param(struct wsmouseinput *input, int key, int val)
>  {
>       struct wstpad *tp = input->tp;
> +     int w, h;
>       u_int flag;
> 
>       if (tp == NULL)
>               return (EINVAL);
> 
>       switch (key) {
> -     case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_DISABLE:
> +     case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_MTBUTTONS:
>               switch (key) {
>               case WSMOUSECFG_SOFTBUTTONS:
>                       flag = WSTPAD_SOFTBUTTONS;
> @@ -1717,6 +1762,9 @@ wstpad_set_param(struct wsmouseinput *input, int key, 
> int val)
>               case WSMOUSECFG_DISABLE:
>                       flag = WSTPAD_DISABLE;
>                       break;
> +             case WSMOUSECFG_MTBUTTONS:
> +                     flag = (tp->params.mtbtn_maxdist >= 0 ? 
> WSTPAD_MTBUTTONS : 0);
> +                     break;
>               }
>               if (val)
>                       tp->features |= flag;
> @@ -1768,6 +1816,16 @@ wstpad_set_param(struct wsmouseinput *input, int key, 
> int val)
>       case WSMOUSECFG_TAP_THREE_BTNMAP:
>               tp->tap.btnmap[2] = BTNMASK(val);
>               break;
> +     case WSMOUSECFG_MTBTN_MAXDIST:
> +             if (IS_MT(tp)) {
> +                     if (tp->params.mtbtn_maxdist == -1 && val == 0) {
> +                             w = abs(input->hw.x_max - input->hw.x_min);
> +                             h = abs(input->hw.y_max - input->hw.y_min);
> +                             val = isqrt(w * w + h * h) / 4;
> +                     }
> +                     tp->params.mtbtn_maxdist = val;
> +             }
> +             break;
>       default:
>               return (ENOTSUP);
>       }
> @@ -1785,7 +1843,7 @@ wstpad_get_param(struct wsmouseinput *input, int key, 
> int *pval)
>               return (EINVAL);
> 
>       switch (key) {
> -     case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_DISABLE:
> +     case WSMOUSECFG_SOFTBUTTONS ... WSMOUSECFG_MTBUTTONS:
>               switch (key) {
>               case WSMOUSECFG_SOFTBUTTONS:
>                       flag = WSTPAD_SOFTBUTTONS;
> @@ -1811,6 +1869,9 @@ wstpad_get_param(struct wsmouseinput *input, int key, 
> int *pval)
>               case WSMOUSECFG_DISABLE:
>                       flag = WSTPAD_DISABLE;
>                       break;
> +             case WSMOUSECFG_MTBUTTONS:
> +                     flag = WSTPAD_MTBUTTONS;
> +                     break;
>               }
>               *pval = !!(tp->features & flag);
>               break;
> @@ -1859,6 +1920,9 @@ wstpad_get_param(struct wsmouseinput *input, int key, 
> int *pval)
>       case WSMOUSECFG_TAP_THREE_BTNMAP:
>               *pval = ffs(tp->tap.btnmap[2]);
>               break;
> +     case WSMOUSECFG_MTBTN_MAXDIST:
> +             *pval = tp->params.mtbtn_maxdist;
> +             break;
>       default:
>               return (ENOTSUP);
>       }
> 

Reply via email to