Re: Improved support for Apple trackpads: tests needed
On Fri, Mar 10, 2017 at 12:47:37AM +0100, Ulf Brosziewski wrote: > This patch for ubcmtp makes it use the multitouch-input functions of > wsmouse. It's the first driver that would apply the "tracking" variant > (wsmouse_mtframe). > > No wonders will result from the change, but the two-finger gestures that > involve movement - scrolling and click-and-drag with two fingers on a > clickpad - should work without flaws. > > A quick way to check whether all touch positions are available and the > selection of the active touch works properly is two-finger-scrolling, > performed with only one finger moving, then switching to the other one. > > Please note that click-and-drag will still require that the "ClickPad" > attribute is set in the synaptics(4) configuration. > > The patch has been tested on a MacBookPro 8,2 (from 2011). It would be > nice to learn that it doesn't introduce regressions on older or newer > models. > > If you don't use a brand-new version of the synaptics driver, you may > encounter the following bug: If the X cursor is in a window with > scrollable content and you put two finger on the touchpad without moving > them, the window content may quickly scroll up and down by a small > distance. This bug is fixed in the latest version. Tested and works fine MacBookAir6,2 with some slightly tweaked values: synclient ClickFinger2=3 synclient ClickFinger3=2 synclient ClickPad=1 synclient VertScrollDelta=-237 synclient HorizScrollDelta=-237 synclient HorizTwoFingerScroll=1 ClickPad/click-and-drag works, awesome! :) Diff reads fine. Thanks! OK jung@ > Index: dev/usb/ubcmtp.c > === > RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v > retrieving revision 1.12 > diff -u -p -r1.12 ubcmtp.c > --- dev/usb/ubcmtp.c 30 Mar 2016 23:34:12 - 1.12 > +++ dev/usb/ubcmtp.c 9 Mar 2017 22:17:50 - > @@ -125,6 +125,12 @@ struct ubcmtp_finger { > #define UBCMTP_SN_COORD 250 > #define UBCMTP_SN_ORIENT 10 > > +/* Identify clickpads in ubcmtp_configure. */ > +#define IS_CLICKPAD(ubcmtp_type) (ubcmtp_type != UBCMTP_TYPE1) > + > +/* Use a constant, synaptics-compatible pressure value for now. */ > +#define DEFAULT_PRESSURE 40 > + > struct ubcmtp_limit { > int limit; > int min; > @@ -316,17 +322,6 @@ static struct ubcmtp_dev ubcmtp_devices[ > }, > }; > > -/* coordinates for each tracked finger */ > -struct ubcmtp_pos { > - int down; > - int x; > - int y; > - int z; > - int w; > - int dx; > - int dy; > -}; > - > struct ubcmtp_softc { > struct device sc_dev; /* base device */ > > @@ -355,7 +350,8 @@ struct ubcmtp_softc { > uint32_tsc_status; > #define UBCMTP_ENABLED 1 > > - struct ubcmtp_pos pos[UBCMTP_MAX_FINGERS]; > + struct mtpoint frame[UBCMTP_MAX_FINGERS]; > + int contacts; > int btn; > }; > > @@ -519,6 +515,24 @@ ubcmtp_activate(struct device *self, int > } > > int > +ubcmtp_configure(struct ubcmtp_softc *sc) > +{ > + struct wsmousehw *hw = wsmouse_get_hw(sc->sc_wsmousedev); > + > + hw->type = WSMOUSE_TYPE_ELANTECH; /* see ubcmtp_ioctl */ > + hw->hw_type = (IS_CLICKPAD(sc->dev_type->type) > + ? WSMOUSEHW_CLICKPAD : WSMOUSEHW_TOUCHPAD); > + hw->x_min = sc->dev_type->l_x.min; > + hw->x_max = sc->dev_type->l_x.max; > + hw->y_min = sc->dev_type->l_y.min; > + hw->y_max = sc->dev_type->l_y.max; > + hw->mt_slots = UBCMTP_MAX_FINGERS; > + hw->flags = WSMOUSEHW_MT_TRACKING; > + > + return wsmouse_configure(sc->sc_wsmousedev, NULL, 0); > +} > + > +int > ubcmtp_enable(void *v) > { > struct ubcmtp_softc *sc = v; > @@ -534,6 +548,9 @@ ubcmtp_enable(void *v) > return (1); > } > > + if (ubcmtp_configure(sc)) > + return (1); > + > if (ubcmtp_setup_pipes(sc) == 0) { > sc->sc_status |= UBCMTP_ENABLED; > return (0); > @@ -608,6 +625,7 @@ ubcmtp_ioctl(void *v, unsigned long cmd, > wsmode); > return (EINVAL); > } > + wsmouse_set_mode(sc->sc_wsmousedev, wsmode); > > DPRINTF("%s: changing mode to %s\n", > sc->sc_dev.dv_xname, (wsmode == WSMOUSE_COMPAT ? "compat" : > @@ -779,7 +797,7 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v > struct ubcmtp_softc *sc = priv; > struct ubcmtp_finger *pkt; > u_int32_t pktlen; > - int i, diff = 0, finger = 0, fingers = 0, s, t; > + int i, s, btn, contacts, fingers; > > if (usbd_is_dying(sc->sc_udev) || !(sc->sc_status & UBCMTP_ENABLED)) > return; > @@ -803,76 +821,30 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v > pkt = (struct ubcmtp_finger *)(sc->tp_pkt + sc->tp_offset); > fingers = (pktlen -
Re: Improved support for Apple trackpads: tests needed
Hi Ulf, > This patch for ubcmtp makes it use the multitouch-input functions of > wsmouse. It's the first driver that would apply the "tracking" variant > (wsmouse_mtframe). > > No wonders will result from the change, but the two-finger gestures that > involve movement - scrolling and click-and-drag with two fingers on a > clickpad - should work without flaws. > > A quick way to check whether all touch positions are available and the > selection of the active touch works properly is two-finger-scrolling, > performed with only one finger moving, then switching to the other one. > > Please note that click-and-drag will still require that the "ClickPad" > attribute is set in the synaptics(4) configuration. > > The patch has been tested on a MacBookPro 8,2 (from 2011). It would be > nice to learn that it doesn't introduce regressions on older or newer > models. > > If you don't use a brand-new version of the synaptics driver, you may > encounter the following bug: If the X cursor is in a window with > scrollable content and you put two finger on the touchpad without moving > them, the window content may quickly scroll up and down by a small > distance. This bug is fixed in the latest version. i've been running a kernel with your patch in, works here, MacBookAir 6,1 notable improvement, gestures is more responsive, i've tested text selection and scrolling with two-fingers and also by putting one-finger and scroling with the second finger... works fine... thanks for the patch, it's just makes my life more confortable. // gsoares
Improved support for Apple trackpads: tests needed
This patch for ubcmtp makes it use the multitouch-input functions of wsmouse. It's the first driver that would apply the "tracking" variant (wsmouse_mtframe). No wonders will result from the change, but the two-finger gestures that involve movement - scrolling and click-and-drag with two fingers on a clickpad - should work without flaws. A quick way to check whether all touch positions are available and the selection of the active touch works properly is two-finger-scrolling, performed with only one finger moving, then switching to the other one. Please note that click-and-drag will still require that the "ClickPad" attribute is set in the synaptics(4) configuration. The patch has been tested on a MacBookPro 8,2 (from 2011). It would be nice to learn that it doesn't introduce regressions on older or newer models. If you don't use a brand-new version of the synaptics driver, you may encounter the following bug: If the X cursor is in a window with scrollable content and you put two finger on the touchpad without moving them, the window content may quickly scroll up and down by a small distance. This bug is fixed in the latest version. Index: dev/usb/ubcmtp.c === RCS file: /cvs/src/sys/dev/usb/ubcmtp.c,v retrieving revision 1.12 diff -u -p -r1.12 ubcmtp.c --- dev/usb/ubcmtp.c30 Mar 2016 23:34:12 - 1.12 +++ dev/usb/ubcmtp.c9 Mar 2017 22:17:50 - @@ -125,6 +125,12 @@ struct ubcmtp_finger { #define UBCMTP_SN_COORD250 #define UBCMTP_SN_ORIENT 10 +/* Identify clickpads in ubcmtp_configure. */ +#define IS_CLICKPAD(ubcmtp_type) (ubcmtp_type != UBCMTP_TYPE1) + +/* Use a constant, synaptics-compatible pressure value for now. */ +#define DEFAULT_PRESSURE 40 + struct ubcmtp_limit { int limit; int min; @@ -316,17 +322,6 @@ static struct ubcmtp_dev ubcmtp_devices[ }, }; -/* coordinates for each tracked finger */ -struct ubcmtp_pos { - int down; - int x; - int y; - int z; - int w; - int dx; - int dy; -}; - struct ubcmtp_softc { struct device sc_dev; /* base device */ @@ -355,7 +350,8 @@ struct ubcmtp_softc { uint32_tsc_status; #define UBCMTP_ENABLED 1 - struct ubcmtp_pos pos[UBCMTP_MAX_FINGERS]; + struct mtpoint frame[UBCMTP_MAX_FINGERS]; + int contacts; int btn; }; @@ -519,6 +515,24 @@ ubcmtp_activate(struct device *self, int } int +ubcmtp_configure(struct ubcmtp_softc *sc) +{ + struct wsmousehw *hw = wsmouse_get_hw(sc->sc_wsmousedev); + + hw->type = WSMOUSE_TYPE_ELANTECH; /* see ubcmtp_ioctl */ + hw->hw_type = (IS_CLICKPAD(sc->dev_type->type) + ? WSMOUSEHW_CLICKPAD : WSMOUSEHW_TOUCHPAD); + hw->x_min = sc->dev_type->l_x.min; + hw->x_max = sc->dev_type->l_x.max; + hw->y_min = sc->dev_type->l_y.min; + hw->y_max = sc->dev_type->l_y.max; + hw->mt_slots = UBCMTP_MAX_FINGERS; + hw->flags = WSMOUSEHW_MT_TRACKING; + + return wsmouse_configure(sc->sc_wsmousedev, NULL, 0); +} + +int ubcmtp_enable(void *v) { struct ubcmtp_softc *sc = v; @@ -534,6 +548,9 @@ ubcmtp_enable(void *v) return (1); } + if (ubcmtp_configure(sc)) + return (1); + if (ubcmtp_setup_pipes(sc) == 0) { sc->sc_status |= UBCMTP_ENABLED; return (0); @@ -608,6 +625,7 @@ ubcmtp_ioctl(void *v, unsigned long cmd, wsmode); return (EINVAL); } + wsmouse_set_mode(sc->sc_wsmousedev, wsmode); DPRINTF("%s: changing mode to %s\n", sc->sc_dev.dv_xname, (wsmode == WSMOUSE_COMPAT ? "compat" : @@ -779,7 +797,7 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v struct ubcmtp_softc *sc = priv; struct ubcmtp_finger *pkt; u_int32_t pktlen; - int i, diff = 0, finger = 0, fingers = 0, s, t; + int i, s, btn, contacts, fingers; if (usbd_is_dying(sc->sc_udev) || !(sc->sc_status & UBCMTP_ENABLED)) return; @@ -803,76 +821,30 @@ ubcmtp_tp_intr(struct usbd_xfer *xfer, v pkt = (struct ubcmtp_finger *)(sc->tp_pkt + sc->tp_offset); fingers = (pktlen - sc->tp_offset) / sizeof(struct ubcmtp_finger); + contacts = 0; for (i = 0; i < fingers; i++) { - if ((int16_t)letoh16(pkt[i].touch_major) == 0) { - /* finger lifted */ - sc->pos[i].down = 0; - diff = 1; - continue; - } - - if (!sc->pos[finger].down) { - sc->pos[finger].down = 1; - diff = 1; - } - - if ((t = (int16_t)