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