Is there enough interest in this feature among OpenBSD users? I haven't
seen many requests for it, if any. Moreover, is it a good idea to configure
different input methods on this or that hardware just because another OS
has different defaults?
Just in case the answer to these questions turns out to be "yes", here are
some remarks on the diff.
First, I think the initialization bug should be fixed at its origin.
Currently, passing parameters to wsmouse_configure() only works with the
general wsmouse parameters (WSMOUSECFG_DX_SCALE .. WSMOUSECFG_REVERSE_
SCROLLING), and a subset of the touchpad-specific ones. Changing
wsmouse.c as follows will make it work with all of them:
diff --git a/sys/dev/wscons/wsmouse.c b/sys/dev/wscons/wsmouse.c
index c786af18208..0feae6824bb 100644
--- a/sys/dev/wscons/wsmouse.c
+++ b/sys/dev/wscons/wsmouse.c
@@ -1662,11 +1662,11 @@ wsmouse_configure(struct device *sc,
"Initialization failed.\n");
return (-1);
}
+ input->flags |= CONFIGURED;
if (params != NULL) {
if ((error = wsmouse_set_params(sc, params, nparams)))
return (error);
}
- input->flags |= CONFIGURED;
}
if (IS_TOUCHPAD(input))
wsmouse_set_mode(sc, WSMOUSE_COMPAT);
(We might as well remove the 'params' arguments from wsmouse_configure, and
leave the call to wsmouse_set_params() to the hardware drivers; up to now,
only pms changes default configurations.)
Two more remarks are inline.
On 9/18/22 16:42, Tobias Heider wrote:
> On Sun, Sep 18, 2022 at 02:21:06PM +0200, Tobias Heider wrote:
>> Hi,
>>
>> the diff below adds a new mouse type WSMOUSE_TYPE_APPLE which emulates Apples
>> touchpad behaviour. Instead of mapping soft-buttons to an area on the pad,
>> the different mouse buttons are mapped to single-finger, two-finger and
>> three-finger clicks as is the default in macos.
>>
>> The diff enables the new mode on apldcms(4) and aplms(4) which are the
>> drivers
>> used by Apple silicon laptops.
>>
>> Tested on an m2 air by me and an m1 by robert@.
>>
>> ok?
>
> Here's an updated version that does not add a new WSMOUSE_TYPE and as such
> does
> not require any changes in X. Instead I just pass the button configuration
> via
> params in wsmouse_configure().
>
> To make this work I had to fix a bug in wstpad where the CONFIGURE flag is not
> set after initial configuration, which causes all values to be overwritten
> with
> the defaults on each reconfigure triggered from wsmouse_set_params().
>
> diff --git sys/arch/arm64/dev/apldc.c sys/arch/arm64/dev/apldc.c
> index 09a03c734da..7962a3c645a 100644
> --- sys/arch/arm64/dev/apldc.c
> +++ sys/arch/arm64/dev/apldc.c
> @@ -1317,6 +1317,11 @@ const struct wsmouse_accessops apldcms_accessops = {
> .ioctl = apldcms_ioctl,
> };
>
> +static struct wsmouse_param apldcms_params[] = {
> + { WSMOUSECFG_SOFTBUTTONS, 0 },
> + { WSMOUSECFG_MTBUTTONS, 1 },
> +};
> +
> int apldcms_match(struct device *, void *, void *);
> void apldcms_attach(struct device *, struct device *, void *);
>
> @@ -1372,7 +1377,7 @@ apldcms_configure(struct apldcms_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, apldcms_params, 2);
> }
>
> void
> diff --git sys/arch/arm64/dev/aplhidev.c sys/arch/arm64/dev/aplhidev.c
> index 2b00f7e217d..ecfb5b8f4eb 100644
> --- sys/arch/arm64/dev/aplhidev.c
> +++ sys/arch/arm64/dev/aplhidev.c
> @@ -608,6 +608,11 @@ const struct wsmouse_accessops aplms_accessops = {
> .ioctl = aplms_ioctl,
> };
>
> +static struct wsmouse_param aplms_params[] = {
> + { WSMOUSECFG_SOFTBUTTONS, 0 },
> + { WSMOUSECFG_MTBUTTONS, 1 },
> +};
> +
> int aplms_match(struct device *, void *, void *);
> void aplms_attach(struct device *, struct device *, void *);
>
> @@ -663,7 +668,7 @@ 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_params, 2);
> }
>
> void
> diff --git sys/dev/wscons/wsconsio.h sys/dev/wscons/wsconsio.h
> index de483493360..497e9a32db7 100644
> --- sys/dev/wscons/wsconsio.h
> +++ sys/dev/wscons/wsconsio.h
> @@ -313,6 +313,7 @@ enum wsmousecfg {
> WSMOUSECFG_SOFTBUTTONS = 64, /* 2 soft-buttons at the bottom edge */
> WSMOUSECFG_SOFTMBTN, /* add a middle-button area */
> WSMOUSECFG_TOPBUTTONS, /* 3 soft-buttons at the top edge */
> + WSMOUSECFG_MTBUTTONS, /* multi-finger buttons */
Even though it requires updating a line of wsconsctl code, I think the
MTBUTTONS entry should be placed at the end of the "Touchpad features"
block, there is no strong reason for changing the enumeration in an
incompatible way.
> WSMOUSECFG_TWOFINGERSCROLL, /* enable two-finger scrolling */
> WSMOUSECFG_EDGESCROLL, /* enable edge scrolling */
> WSMOUSECFG_HORIZSCROLL, /* enable horizontal edge scrolling */
> diff --git sys/dev/wscons/wstpad.c sys/dev/wscons/wstpad.c
> index be074b89fb8..4384370545e 100644
> --- sys/dev/wscons/wstpad.c
> +++ sys/dev/wscons/wstpad.c
> @@ -72,6 +72,7 @@
> enum tpad_handlers {
> SOFTBUTTON_HDLR,
> TOPBUTTON_HDLR,
> + MTBUTTON_HDLR,
> TAP_HDLR,
> F2SCROLL_HDLR,
> EDGESCROLL_HDLR,
> @@ -149,6 +150,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)
>
> @@ -646,7 +648,23 @@ wstpad_softbuttons(struct wsmouseinput *input, u_int
> *cmds, int hdlr)
> }
>
> if (tp->softbutton == 0 && PRIMARYBTN_CLICKED(tp)) {
> - tp->softbutton = wstpad_get_sbtn(input, top);
> + if (hdlr == MTBUTTON_HDLR) {
> + switch (tp->contacts) {
> + case 2:
> + tp->softbutton = RIGHTBTN;
> + break;
> + case 3:
> + tp->softbutton = MIDDLEBTN;
> + break;
> + case 1:
> + tp->softbutton = LEFTBTN;
> + break;
wstpad_get_sbtn() doesn't produce a 'softbutton' mapping for left-button events,
it isn't necessary. I would prefer to keep that consistent (and also eliminate
the redundant 0-assignment below).
> + default:
> + tp->softbutton = 0;
> + break;
> + }
> + } else
> + tp->softbutton = wstpad_get_sbtn(input, top);
> if (tp->softbutton)
> *cmds |= 1 << SOFTBUTTON_DOWN;
> }
> @@ -1237,12 +1255,14 @@ wstpad_process_input(struct wsmouseinput *input,
> struct evq_access *evq)
> cmds = 0;
> handlers = tp->handlers;
> if (DISABLE(tp))
> - handlers &= ((1 << TOPBUTTON_HDLR) | (1 << SOFTBUTTON_HDLR));
> + handlers &= ((1 << TOPBUTTON_HDLR) | (1 << SOFTBUTTON_HDLR) |
> + (1 << MTBUTTON_HDLR));
>
> FOREACHBIT(handlers, hdlr) {
> switch (hdlr) {
> case SOFTBUTTON_HDLR:
> case TOPBUTTON_HDLR:
> + case MTBUTTON_HDLR:
> wstpad_softbuttons(input, &cmds, hdlr);
> continue;
> case TAP_HDLR:
> @@ -1599,6 +1619,7 @@ wstpad_configure(struct wsmouseinput *input)
> tp->scroll.hdist = 4 * h_unit;
> tp->scroll.vdist = 4 * v_unit;
> tp->tap.maxdist = 4 * h_unit;
> + input->flags |= CONFIGURED;
> }
>
> /* A touch with a flag set in this mask does not move the pointer. */
> @@ -1621,6 +1642,8 @@ wstpad_configure(struct wsmouseinput *input)
>
> tp->handlers = 0;
>
> + if (tp->features & WSTPAD_MTBUTTONS)
> + tp->handlers |= 1 << MTBUTTON_HDLR;
> if (tp->features & WSTPAD_SOFTBUTTONS)
> tp->handlers |= 1 << SOFTBUTTON_HDLR;
> if (tp->features & WSTPAD_TOPBUTTONS)
> @@ -1702,6 +1725,9 @@ wstpad_set_param(struct wsmouseinput *input, int key,
> int val)
> case WSMOUSECFG_TOPBUTTONS:
> flag = WSTPAD_TOPBUTTONS;
> break;
> + case WSMOUSECFG_MTBUTTONS:
> + flag = WSTPAD_MTBUTTONS;
> + break;
> case WSMOUSECFG_TWOFINGERSCROLL:
> flag = WSTPAD_TWOFINGERSCROLL;
> break;
> @@ -1796,6 +1822,9 @@ wstpad_get_param(struct wsmouseinput *input, int key,
> int *pval)
> case WSMOUSECFG_TOPBUTTONS:
> flag = WSTPAD_TOPBUTTONS;
> break;
> + case WSMOUSECFG_MTBUTTONS:
> + flag = WSTPAD_MTBUTTONS;
> + break;
> case WSMOUSECFG_TWOFINGERSCROLL:
> flag = WSTPAD_TWOFINGERSCROLL;
> break;
>
