On Wed, Jan 14, 2015 at 12:23:13AM +0100, Ulf Brosziewski wrote:
> Currently pms and the wsconscomm module of the synaptics driver offer a
> somewhat limited support for Elantech clickpads with hardware version 4.
> Above all, I missed the options of performing click-and-drag operations
> with two fingers and of using a "soft button area" for the emulation of
> right button clicks (tap-and-drag and two-finger tapping are no
> alternatives for me, usually I don't enable tapping).
> 
> I have written two patches that provide these options (I'm using them
> on an Acer V5-131 netbook with OpenBSD 5.6/amd64, the clickpad hardware
> and firmware is identified as "Elantech Clickpad, version 4, firmware
> 0x461f02"). There is, however, an open question concerning wsconscomm.
> If someone is interested in testing and using the patches, or in
> assessing whether they can be useful for the OpenBSD project, some
> explanations might help.
> 
> Two-finger scrolling and click-and-drag actions on clickpads require
> that the multitouch input is filtered or "translated" into coherent
> sequences of coordinate pairs. The pms patch - which I have added
> below - implements a simple filter that checks the motion deltas and
> ensures that if there is at least one finger moving on the clickpad, a
> moving one will be tracked. Some care is needed when the input "slot"
> changes, but this works well even within the infrastructure of wscons,
> which doesn't define and handle "MT" events.
> 
> The problem with wsconscomm is related to the X/Linux way of handling
> multitouches. If I understand it correctly, it produces MT events with
> slot data as well as standard events with coordinates for "pointer
> control", and the latter are determined by the "oldest" touch. That
> mechanism is sufficient if two fingers are moving in parallel, but it
> cannot cover the click-and-drag case because usually only one finger
> is moving, and it is not necessarily the one that has touched the
> clickpad first. This might be the reason why the synaptics driver
> implements an additional mapping. It accumulates the motion deltas
> of the multitouch slots in a special coordinate pair. As long as no
> button is pressed, the pair isn't used, and its values will be
> synchronized with the standard coordinates. When clickpad support is
> enabled ("$ synclient ClickPad=1") and a button is down, the
> "cumulative" values will be used. This makes the cursor movement
> independent of the order of touches (and you can create, e.g., a
> diagonal cursor movement by combining a horizontal and a vertical
> finger movement).
> 
> It seems that the current development version of the synaptics driver
> applies the cumulative coordinates to two-finger-scrolling as well
> (independently of clickpad support).
> 
> In OpenBSD wsconscomm updates the cumulative coordinates when no
> button is being pressed, but it does nothing with them otherwise. If
> clickpad support is enabled, this has the effect that the X cursor
> "freezes" as long as the clickpad button is down, and no click-and-
> drag operation - not even the one-finger variant - is possible (for
> this reason I couldn't use soft button areas, which require the
> clickpad support).
> 
> As wsconscomm doesn't deal with MT events, couldn't it simply always
> keep the coordinates in sync? To make click-and-drag work in my
> installation I have applied the following patch to wsconscomm.c:
> 
> diff --git a/wsconscomm.c b/wsconscomm.c
> index 0e0c81d..a6540db 100644
> --- a/wsconscomm.c
> +++ b/wsconscomm.c
> @@ -131,12 +131,6 @@ WSConsReadHwState(InputInfoPtr pInfo,
>      struct wscons_event event;
>      Bool v;
> 
> -    /* Reset cumulative values if buttons were not previously pressed */
> -    if (!hw->left && !hw->right && !hw->middle) {
> -        hw->cumulative_dx = hw->x;
> -        hw->cumulative_dy = hw->y;
> -    }
> -
>      while (WSConsReadEvent(pInfo, &event)) {
>          switch (event.type) {
>          case WSCONS_EVENT_MOUSE_UP:
> @@ -186,9 +180,11 @@ WSConsReadHwState(InputInfoPtr pInfo,
>              break;
>          case WSCONS_EVENT_MOUSE_ABSOLUTE_X:
>              hw->x = event.value;
> +            hw->cumulative_dx = hw->x;
>              break;
>          case WSCONS_EVENT_MOUSE_ABSOLUTE_Y:
>              hw->y = priv->maxy - event.value + priv->miny;
> +            hw->cumulative_dy = hw->y;
>              break;
>          case WSCONS_EVENT_MOUSE_ABSOLUTE_Z:
>              hw->z = event.value;
> 
> Please note that this patch might "unmask" an inconsistent treatment
> of multitouches. For example, if I use it without applying the pms
> patch, an attempt to click-and-drag can lead to jumps of the cursor
> and inverted vertical movements. Could something similar happen with
> other hardware where clickpad support would make sense? And if this
> is the case, could the change in wsconscomm be restricted in a
> reasonable way?
> 

Sorry for the long answer.

I tested xenocara part on x201(synaptics touchpad) and x220(synaptics clickpad),
this works, no regress, even works click-and-drag :)

I have no other touchpads (ALPS, Elantech) and I can not test patch on them. 

Has anyone else tried this diff?

> If the pms patch is applied without changing wsconscomm, it would only
> fix minor flaws in the clickpad behaviour (currently it is possible to
> produce a "tap" by putting two fingers on the clickpad and lifting them
> successively within a tap interval; two-finger scrolling may start with
> a jump).
> 
> The diffs are against the (5.6) release versions, I hope it doesn't
> matter.
> 
> 
> diff --git a/pms.c b/pms.c
> index 77904ae..697b3ef 100644
> --- a/pms.c
> +++ b/pms.c
> @@ -125,8 +125,15 @@ struct elantech_softc {
>       struct {
>               unsigned int x;
>               unsigned int y;
> +             unsigned int z;
>       } mt[ELANTECH_MAX_FINGERS];
> -     int fingers[ELANTECH_MAX_FINGERS];
> +     int mt_slots;
> +     int mt_count;
> +     int mt_filter;
> +     int mt_lastid;
> +     int mt_lastcount;
> +     int mt_buttons;
> +
>       int width;
> 
>       u_char parity[256];
> @@ -295,6 +302,7 @@ int       elantech_set_absolute_mode_v2(struct pms_softc 
> *);
>  int  elantech_set_absolute_mode_v3(struct pms_softc *);
>  int  elantech_set_absolute_mode_v4(struct pms_softc *);
> 
> +void elantech_send_mt_input(struct pms_softc *, int);
> 
>  struct cfattach pms_ca = {
>       sizeof(struct pms_softc), pmsprobe, pmsattach, NULL,
> @@ -2215,77 +2223,67 @@ void
>  pms_proc_elantech_v4(struct pms_softc *sc)
>  {
>       struct elantech_softc *elantech = sc->elantech;
> -     int z, delta_x1 = 0, delta_x2 = 0, delta_y1 = 0, delta_y2 = 0;
> -     int i, weight, finger, fingers = 0, id, sid;
> +     int n, id, slots, weight, dx, dy;
> 
>       switch (sc->packet[3] & 0x1f) {
>       case ELANTECH_V4_PKT_STATUS:
> -             fingers = sc->packet[1] & 0x1f;
> -             for (i = 0; i < ELANTECH_MAX_FINGERS; i++) {
> -                     finger = ((fingers & (1 << i)) != 0);
> -                     if (elantech->fingers[i] && !finger)
> -                             /* notify that we lifted */
> -                             elantech_send_input(sc, elantech->mt[i].x,
> -                                 elantech->mt[i].y, 0, 0);
> -
> -                     elantech->fingers[i] = finger;
> -             }
> +             if (elantech->mt_slots == 0)
> +                     elantech->mt_lastid = -1;
> +             slots = sc->packet[1] & 0x1f;
> +             if (slots == 0 && elantech->mt_lastid > -1)
> +                     /* Notify that we lifted. */
> +                     elantech_send_input(sc,
> +                         elantech->mt[elantech->mt_lastid].x,
> +                         elantech->mt[elantech->mt_lastid].y, 0, 0);
> +
> +             elantech->mt_filter = elantech->mt_slots = slots;
> +
> +             for (elantech->mt_count = 0; slots != 0; slots >>= 1)
> +                     elantech->mt_count += (1 & slots);
> 
>               break;
> 
>       case ELANTECH_V4_PKT_HEAD:
>               id = ((sc->packet[3] & 0xe0) >> 5) - 1;
> -             if (id < 0)
> -                     return;
> -
> -             for (i = 0; i < ELANTECH_MAX_FINGERS; i++)
> -                     if (elantech->fingers[i])
> -                             fingers++;
> -
> -             elantech->mt[id].x = ((sc->packet[1] & 0x0f) << 8) |
> -                 sc->packet[2];
> -             elantech->mt[id].y = (((sc->packet[4] & 0x0f) << 8) |
> -                 sc->packet[5]);
> -             z = (sc->packet[1] & 0xf0) | ((sc->packet[4] & 0xf0) >> 4);
> -
> -             elantech_send_input(sc, elantech->mt[id].x, elantech->mt[id].y,
> -                 z, fingers);
> -
> +             if (id > -1 && id < ELANTECH_MAX_FINGERS) {
> +                     elantech->mt[id].x =
> +                         ((sc->packet[1] & 0x0f) << 8) | sc->packet[2];
> +                     elantech->mt[id].y =
> +                         ((sc->packet[4] & 0x0f) << 8) | sc->packet[5];
> +                     elantech->mt[id].z =
> +                         (sc->packet[1] & 0xf0)
> +                         | ((sc->packet[4] & 0xf0) >> 4);
> +
> +                     if (elantech->mt_filter & (1 << id)) {
> +                             elantech_send_mt_input(sc, id);
> +                             elantech->mt_filter = (1 << id);
> +                     }
> +             }
>               break;
> 
>       case ELANTECH_V4_PKT_MOTION:
> -             id = ((sc->packet[0] & 0xe0) >> 5) - 1;
> -             if (id < 0)
> -                     return;
> -
> -             sid = ((sc->packet[3] & 0xe0) >> 5) - 1;
>               weight = (sc->packet[0] & 0x10) ? ELANTECH_V4_WEIGHT_VALUE : 1;
> -
> -             delta_x1 = (signed char)sc->packet[1];
> -             delta_y1 = (signed char)sc->packet[2];
> -             delta_x2 = (signed char)sc->packet[4];
> -             delta_y2 = (signed char)sc->packet[5];
> -
> -             elantech->mt[id].x += delta_x1 * weight;
> -             elantech->mt[id].y -= delta_y1 * weight;
> -
> -             for (i = 0; i < ELANTECH_MAX_FINGERS; i++)
> -                     if (elantech->fingers[i])
> -                             fingers++;
> -
> -             elantech_send_input(sc, elantech->mt[id].x, elantech->mt[id].y,
> -                 1, fingers);
> -
> -             if (sid >= 0) {
> -                     elantech->mt[sid].x += delta_x2 * weight;
> -                     elantech->mt[sid].y -= delta_y2 * weight;
> -                     /* XXX: can only send one finger of input */
> -                     /*
> -                     elantech_send_input(sc, elantech->mt[sid].x,
> -                         elantech->mt[sid].y, 1, fingers);
> -                     */
> +             for (n = 0; n < 6; n += 3) {
> +                     id = ((sc->packet[n] & 0xe0) >> 5) - 1;
> +                     if (id < 0 || id >= ELANTECH_MAX_FINGERS)
> +                             continue;
> +                     dx = weight * (signed char)sc->packet[n + 1];
> +                     dy = weight * (signed char)sc->packet[n + 2];
> +                     elantech->mt[id].x += dx;
> +                     elantech->mt[id].y += dy;
> +                     elantech->mt[id].z = 1;
> +                     if (elantech->mt_filter & (1 << id)) {
> +                             if ((dx | dy)
> +                                 || elantech->mt_count !=
> +                                 elantech->mt_lastcount
> +                                 || (sc->packet[0] & 3) !=
> +                                 elantech->mt_buttons)
> +                                     elantech_send_mt_input(sc, id);
> +
> +                             elantech->mt_filter = (dx | dy) ?
> +                                 (1 << id) : elantech->mt_slots;
> +                     }
>               }
> -
>               break;
> 
>       default:
> @@ -2296,6 +2294,37 @@ pms_proc_elantech_v4(struct pms_softc *sc)
>  }
> 
>  void
> +elantech_send_mt_input(struct pms_softc *sc, int id)
> +{
> +     struct elantech_softc *elantech = sc->elantech;
> +
> +     if (id != elantech->mt_lastid) {
> +             /* Correct for compatibility mode, but not useful yet: */
> +             elantech->old_x = elantech->mt[id].x;
> +             elantech->old_y = elantech->mt[id].y;
> +             /*
> +              * To avoid a jump of the cursor, simulate a change of the
> +              * number of touches (without producing tapping gestures
> +              * accidentally). It should suffice to do that only if
> +              * mt_count hasn't changed, but we cannot rely on the
> +              * synaptics driver, which alters its finger counts when
> +              * handling click-and-drag actions (see HandleTapProcessing
> +              * and ComputeDeltas in synaptics.c).
> +              */
> +             if (elantech->mt_lastid > -1)
> +                     elantech_send_input(sc,
> +                         elantech->mt[id].x, elantech->mt[id].y,
> +                         elantech->mt[id].z, ELANTECH_MAX_FINGERS);
> +             elantech->mt_lastid = id;
> +     }
> +     elantech->mt_lastcount = elantech->mt_count;
> +     elantech->mt_buttons = sc->packet[0] & 3;
> +     elantech_send_input(sc,
> +         elantech->mt[id].x, elantech->mt[id].y,
> +         elantech->mt[id].z, elantech->mt_count);
> +}
> +
> +void
>  elantech_send_input(struct pms_softc *sc, int x, int y, int z, int w)
>  {
>       struct elantech_softc *elantech = sc->elantech;
> 

-- 
Alexandr Shadchin

Reply via email to