Improved support for Apple trackpads: tests needed

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Improved support for Apple trackpads: tests needed

Ulf Brosziewski
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.c 30 Mar 2016 23:34:12 -0000 1.12
+++ dev/usb/ubcmtp.c 9 Mar 2017 22:17:50 -0000
@@ -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_t sc_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)letoh16(pkt[i].abs_x)) != sc->pos[finger].x) {
- sc->pos[finger].x = t;
- diff = 1;
- }
-
- if ((t = (int16_t)letoh16(pkt[i].abs_y)) != sc->pos[finger].y) {
- sc->pos[finger].y = t;
- diff = 1;
- }
-
- if ((t = (int16_t)letoh16(pkt[i].rel_x)) != sc->pos[finger].dx) {
- sc->pos[finger].dx = t;
- diff = 1;
- }
-
- if ((t = (int16_t)letoh16(pkt[i].rel_y)) != sc->pos[finger].dy) {
- sc->pos[finger].dy = t;
- diff = 1;
- }
-
- finger++;
- }
-
- if (sc->dev_type->type == UBCMTP_TYPE2 ||
-    sc->dev_type->type == UBCMTP_TYPE3 ||
-    sc->dev_type->type == UBCMTP_TYPE4) {
- if (sc->dev_type->type == UBCMTP_TYPE2)
- t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE2_BTOFF]));
- else if (sc->dev_type->type == UBCMTP_TYPE3)
- t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE3_BTOFF]));
- else if (sc->dev_type->type == UBCMTP_TYPE4)
- t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE4_BTOFF]));
-
- if (sc->btn != t) {
- sc->btn = t;
- diff = 1;
-
- DPRINTF("%s: [button]\n", sc->sc_dev.dv_xname);
- }
- }
+ if ((int16_t)letoh16(pkt[i].touch_major) == 0)
+ continue; /* finger lifted */
+ sc->frame[contacts].x = (int16_t)letoh16(pkt[i].abs_x);
+ sc->frame[contacts].y = (int16_t)letoh16(pkt[i].abs_y);
+ sc->frame[contacts].pressure = DEFAULT_PRESSURE;
+ contacts++;
+ }
+
+ btn = sc->btn;
+ if (sc->dev_type->type == UBCMTP_TYPE2)
+ sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE2_BTOFF]));
+ else if (sc->dev_type->type == UBCMTP_TYPE3)
+ sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE3_BTOFF]));
+ else if (sc->dev_type->type == UBCMTP_TYPE4)
+ sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE4_BTOFF]));
 
- if (diff) {
+ if (contacts || sc->contacts || sc->btn != btn) {
+ sc->contacts = contacts;
  s = spltty();
-
- DPRINTF("%s: ", sc->sc_dev.dv_xname);
-
- if (sc->wsmode == WSMOUSE_NATIVE) {
- DPRINTF("absolute input %d, %d (finger %d, button %d)\n",
-    sc->pos[0].x, sc->pos[0].y, finger, sc->btn);
- WSMOUSE_TOUCH(sc->sc_wsmousedev, sc->btn, sc->pos[0].x,
-    sc->pos[0].y, (finger ? 50 : 0), finger);
- } else {
- DPRINTF("relative input %d, %d (button %d)\n",
-    sc->pos[0].dx, sc->pos[0].dy, sc->btn);
- WSMOUSE_INPUT(sc->sc_wsmousedev, sc->btn,
-    sc->pos[0].dx, sc->pos[0].dy, 0, 0);
- }
+ wsmouse_buttons(sc->sc_wsmousedev, sc->btn);
+ wsmouse_mtframe(sc->sc_wsmousedev, sc->frame, contacts);
+ wsmouse_input_sync(sc->sc_wsmousedev);
  splx(s);
  }
 }

Reply | Threaded
Open this post in threaded view
|

Re: Improved support for Apple trackpads: tests needed

Gleydson Soares-3
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

Reply | Threaded
Open this post in threaded view
|

Re: Improved support for Apple trackpads: tests needed

Joerg Jung
In reply to this post by Ulf Brosziewski
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 -0000 1.12
> +++ dev/usb/ubcmtp.c 9 Mar 2017 22:17:50 -0000
> @@ -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_t sc_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)letoh16(pkt[i].abs_x)) != sc->pos[finger].x) {
> - sc->pos[finger].x = t;
> - diff = 1;
> - }
> -
> - if ((t = (int16_t)letoh16(pkt[i].abs_y)) != sc->pos[finger].y) {
> - sc->pos[finger].y = t;
> - diff = 1;
> - }
> -
> - if ((t = (int16_t)letoh16(pkt[i].rel_x)) != sc->pos[finger].dx) {
> - sc->pos[finger].dx = t;
> - diff = 1;
> - }
> -
> - if ((t = (int16_t)letoh16(pkt[i].rel_y)) != sc->pos[finger].dy) {
> - sc->pos[finger].dy = t;
> - diff = 1;
> - }
> -
> - finger++;
> - }
> -
> - if (sc->dev_type->type == UBCMTP_TYPE2 ||
> -    sc->dev_type->type == UBCMTP_TYPE3 ||
> -    sc->dev_type->type == UBCMTP_TYPE4) {
> - if (sc->dev_type->type == UBCMTP_TYPE2)
> - t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE2_BTOFF]));
> - else if (sc->dev_type->type == UBCMTP_TYPE3)
> - t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE3_BTOFF]));
> - else if (sc->dev_type->type == UBCMTP_TYPE4)
> - t = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE4_BTOFF]));
> -
> - if (sc->btn != t) {
> - sc->btn = t;
> - diff = 1;
> -
> - DPRINTF("%s: [button]\n", sc->sc_dev.dv_xname);
> - }
> - }
> + if ((int16_t)letoh16(pkt[i].touch_major) == 0)
> + continue; /* finger lifted */
> + sc->frame[contacts].x = (int16_t)letoh16(pkt[i].abs_x);
> + sc->frame[contacts].y = (int16_t)letoh16(pkt[i].abs_y);
> + sc->frame[contacts].pressure = DEFAULT_PRESSURE;
> + contacts++;
> + }
> +
> + btn = sc->btn;
> + if (sc->dev_type->type == UBCMTP_TYPE2)
> + sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE2_BTOFF]));
> + else if (sc->dev_type->type == UBCMTP_TYPE3)
> + sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE3_BTOFF]));
> + else if (sc->dev_type->type == UBCMTP_TYPE4)
> + sc->btn = !!((int16_t)letoh16(sc->tp_pkt[UBCMTP_TYPE4_BTOFF]));
>  
> - if (diff) {
> + if (contacts || sc->contacts || sc->btn != btn) {
> + sc->contacts = contacts;
>   s = spltty();
> -
> - DPRINTF("%s: ", sc->sc_dev.dv_xname);
> -
> - if (sc->wsmode == WSMOUSE_NATIVE) {
> - DPRINTF("absolute input %d, %d (finger %d, button %d)\n",
> -    sc->pos[0].x, sc->pos[0].y, finger, sc->btn);
> - WSMOUSE_TOUCH(sc->sc_wsmousedev, sc->btn, sc->pos[0].x,
> -    sc->pos[0].y, (finger ? 50 : 0), finger);
> - } else {
> - DPRINTF("relative input %d, %d (button %d)\n",
> -    sc->pos[0].dx, sc->pos[0].dy, sc->btn);
> - WSMOUSE_INPUT(sc->sc_wsmousedev, sc->btn,
> -    sc->pos[0].dx, sc->pos[0].dy, 0, 0);
> - }
> + wsmouse_buttons(sc->sc_wsmousedev, sc->btn);
> + wsmouse_mtframe(sc->sc_wsmousedev, sc->frame, contacts);
> + wsmouse_input_sync(sc->sc_wsmousedev);
>   splx(s);
>   }
>  }
>