Bug 93583

Can you specify what you mean by "three finger click"? it's a big ambiguous and can mean a number of different things.

Sorry, I meant that I could not trigger a middle click by tapping with three fingers on the trackpad. The tap to click method, not clickfingers (although I'd like to use it as well)

Now that I think about it, it may not be specific to libinput, but could come from the xf86-input-libinput driver. Is there a way to check that? I tried running xev on xwayland on weston with enable_tab=true in weston.ini, but tapping did not work at all (weston did not run from within Xorg).

libinput-debug-events --enable-tap does show me:

- when I tap with one finger: code 272
- when I tap with two fingers: code 273
- doesn't show anything for three fingers tapping

can you attach a sudo evemu-record of a 3 finger tap sequence? this should work, could be something device-specific.

Created attachment 120816 [details]
evemu-record trace

I ran this here and I can reproduce it, though it looks more to be a kernel bug. Run sudo libinput-debug-events --enable-tap --verbose and do a three-finger tap. Here, with the evemu-emulated device I get the second and third touch detected as palm touches ("palm: palm detected (edge)" in the output) but that is likely a result of the evemu emulation.

Do you see this output as well? If so, get the git repo, put this printf at the top of tp_palm_detect: 
       printf("touch x/y: %d/%d\n", t->point.x, t->point.y);
compile it and run again. Thanks.

Hit the button too early.

What happens is that the second touch doesn't have x/y values and is thus at 0/0. That gives the TRIPLETAP fake touch a position of 0/0 as well, triggering palm detection for either. But we sync the slot state at startup, so any real device should be correct here (that's what the printf can confirm).

The palm detection causes it to fake a motion event, which changes the state from a pending multifinger tap to a "fingers holding on the touchpad" which won't trigger the button presses.

Created attachment 120825 [details]
libinput-debug-events --enable-tap --verbosee (with printf)

You're right, this got detected as palm tap. I addached the trace as you requested, with the added printf.

Benjamin, have a look at the event trace please. I think this may be a kernel bug: the slot state for slot 1 is 0/0 but the device doesn't send coordinates for slot 1 with the initial tracking ID, so we end up with the touch point at 0/0.

Mildred, please attach another trace of you tapping a few times, that'll show if it's a common problem and whether slot 1 updates at all.

Yep, looks like there is something fishy in the driver.

Mildred, in addition to what Peter requested, would you mind attaching a ps2emu[1] recording of your touchpad while doing a triple tap?

Also, I was not able to find a reliable picture of this particular model of laptop. Could you tell us whether you have physical buttons or not and if the touchpad is a clickpad one (one big button under the touchpad)?

[1] https://github.com/Lyude/ps2emu

Created attachment 120873 [details]
libinput-debug-events (3 finger taps multiple times)

Created attachment 120874 [details]
ps2emu-record output

The pad is a normal pad (not a clickpad) with two physical buttons under the pad

 ,-------------.
 |             |
 |             | <- touch area, not clickable
 |             |
 `-------------'
 |      |      | <- physical buttons, no touch
 `-------------'

I do not have a trackpoint either.

Comment on attachment 120873 [details]
libinput-debug-events (3 finger taps multiple times)

During this test, most times, three finger tap resulted in a right click. Some times, it resulted in no click at all.

Thanks for the logs.

The ps2emu-record shows that you have an Elantech v3 device. The thing is that the code in elantech.c (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/input/mouse/elantech.c#n478) shows that the current code just ignores the second finger when we receive 3 fingers...

I'll try to look at this in more details, but this is a generic problem we will have with all the Elantech v3 touchpads.

Oh boy:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/input/elantech.txt#n547
"3 finger touch only reports the position of the center of all three fingers."

And I think HW v2 is the same given that they also report a common packet when there are 1 or 3 fingers on the touchpad.

Mildred, would you mind sending an other ps2emu record with:
one finger down, wait 1-2 sec, drop an other one, wait, drop a third one, wait and release.

I want to check if the given coordinate is actually the "center of all fingers".

Peter, I am not sure we will be able to fix something in the kernel for those devices. How about we just disable palm detection for semi-mt devices?

Created attachment 120894 [details]
ps2emu-record log with one finger, then two, then three, and release

Thanks. That is what I was afraid of. The first finger stays put during the sequence, when you touch with the 2 other fingers. So it is not "the center of all fingers" which is reported in slot 0 during the triple tap.

Peter, this is definitively something I don't think I can fix in the kernel. The protocol and hardware is too old to report valuable information.

IIRC, we already disabled palm detection for semi-mt devices, has it been changed recently? (or is it just the device missing the semi-mt bits?).

yeah, it doesn't have the property set. we don't disable palm detection per-se (only gestures) but some other bits take it into account and it may just be the fix here. Mildred: try to set semi_mt to true in tp_init_slots() and see if that improves things.

if the device isn't truly semi-mt and you don't want to set it in the kernel, we can set a quirk for it in the hwdb.

Created attachment 120947 [details]
libinput-debug-events --enable-tap --verbose with semi_mt = true (1, 2 and 3 fingers tap))

Setting semi_mt = true in the code doesn't help. Three fingers tap is still detected as palm. See attached trace.

In the trace I tap 3 times. With 1 finger, then 2, and then 3 (all separated by a blank line)

> (In reply to Peter Hutterer from comment #19)
> if the device isn't truly semi-mt and you don't want to set it in the
> kernel, we can set a quirk for it in the hwdb.

I'll still submit a patch today to set this flag for all v2 and v3 Elan hardware.
They use the elantech_report_semi_mt_data() internal function, which means they should have the SEMI_MT flag set. The hwdb solution can be a temporary workaround the time the fix comes in the various distributions.

patch submitted for the kernel side of things:
http://thread.gmane.org/gmane.linux.kernel.input/47628

Mildred: try this diff please. This is *in addition* to forcing semi_mt to true, and make sure that udevadm info /sys/class/input/eventX actually shows the tag on your device. (if not, test with this condition simply set to true and we worry about the details later)

diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
index 2de2539..f91f839 100644
--- a/src/evdev-mt-touchpad.c
+++ b/src/evdev-mt-touchpad.c
@@ -1496,7 +1496,8 @@ tp_init_slots(struct tp_dispatch *tp,
         * explanation.
         */
        if (tp->semi_mt &&
-           (device->model_flags & EVDEV_MODEL_JUMPING_SEMI_MT)) {
+           (device->model_flags &
+            (EVDEV_MODEL_JUMPING_SEMI_MT|EVDEV_MODEL_ELANTECH_TOUCHPAD))) {
                tp->num_slots = 1;
                tp->slot = 0;
                tp->has_mt = false;

Created attachment 120985 [details]
libinput-debug-events --enable-tap with semi_mt=true and patch applied

With your patch, I successfully get the expected triple tap behaviour. Extract from the logs:

tap state: TAP_STATE_IDLE → TAP_EVENT_TOUCH → TAP_STATE_TOUCH
tap state: TAP_STATE_TOUCH → TAP_EVENT_TOUCH → TAP_STATE_TOUCH_2
tap state: TAP_STATE_TOUCH_2 → TAP_EVENT_TOUCH → TAP_STATE_TOUCH_3
touch x/y: 959/1062
touch x/y: 959/1062
touch x/y: 959/1062

Is this patch ready to be pushed to libinput or would it break other touchpads?

udevadm info /sys/class/input/event13

P: /devices/platform/i8042/serio2/input/input74/event13
N: input/event13
S: input/by-path/platform-i8042-serio-2-event-mouse
E: DEVLINKS=/dev/input/by-path/platform-i8042-serio-2-event-mouse
E: DEVNAME=/dev/input/event13
E: DEVPATH=/devices/platform/i8042/serio2/input/input74/event13
E: ID_INPUT=1
E: ID_INPUT_HEIGHT_MM=45
E: ID_INPUT_TOUCHPAD=1
E: ID_INPUT_WIDTH_MM=94
E: ID_PATH=platform-i8042-serio-2
E: ID_PATH_TAG=platform-i8042-serio-2
E: ID_SERIAL=noserial
E: LIBINPUT_ATTR_RESOLUTION_HINT=31x31
E: LIBINPUT_DEVICE_GROUP=11/2/e/0:isa0060/serio2
E: LIBINPUT_MODEL_ELANTECH_TOUCHPAD=1
E: MAJOR=13
E: MINOR=77
E: SUBSYSTEM=input
E: USEC_INITIALIZED=93333553088

http://lists.freedesktop.org/archives/wayland-devel/2016-January/026526.html

Note that this is only the diff above, to get semi-mt enabled you need the kernel patch (which has been merged upstream)

commit 556aac04b5d56f015d5da8b96e24fd78ad231760
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue Jan 12 12:24:18 2016 +1000

    touchpad: disable MT for elantech semi-mt touchpads

Thank you :)

hmm, I think I'll have to revert this patch, it'll break 2-finger pinch gestures on elantech semi-mt touchpads. scrolling would still work, but pinching is broken now, the second touchpoint mirrors the first touch point's position now.

Mildred: please attach evemu recordings of a two-finger tap, a two-finger pinch, a two-finger scroll gestures, and a recording where you put two fingers on the touchpad, wait half a second or so, then put the third finger on the touchpad (and release them in this order). Thanks

Created attachment 121054 [details] [review]
0001-touchpad-fix-elantech-semi-mt-three-finger-bug.patch

Tentative patch to work around the issue. This copies the coordinates of the first touch into the 0/0 touch when three or more fingers are down. 3fg tap works this way, but it's not yet clear what happens when you have two fingers, then tap and release the third finger - it may cause a jump. I'll need the evemu recordings to analyse this first.

Created attachment 121059 [details]
ps2emu-record: two fingers tap

Created attachment 121060 [details]
ps2emu-record: two fingers pinch

Created attachment 121061 [details]
ps2emu-record: two fingers scroll

Created attachment 121062 [details]
ps2emu-record: 2 fingers then 3 fingers then 2 again

Created attachment 121101 [details]
evemu-record: 2 fingers then 3 fingers then 2 again

conversion of attachment 121062 [details] to an evemu recording which is a bit nicer to read. Shows that the two-finger tracking is accurate, as soon as the third finger comes down we get one event in 0/0 for slot 1, then the rest of the events are for slot 0. once the third finger is lifted, the finger data is accurate again.

Benjamin: wouldn't it be better to work around this in the kernel? what seems needed here is simply suppressing the 0/0 event and sending BTN_TOOL_TRIPLETAP only instead.

(In reply to Peter Hutterer from comment #35)
> Benjamin: wouldn't it be better to work around this in the kernel? what
> seems needed here is simply suppressing the 0/0 event and sending
> BTN_TOOL_TRIPLETAP only instead.

Honestly, I don't think we will ever have a satisfying solution:
- Before 3c0213d17a09 ("Input: elantech - fix semi-mt protocol for v3 HW"), we did not set the slot for slot 2 and only reported at most 1 slot if num_fingers != 2.
- 3c0213d17a09 fixed to not remove one slot when going from double tap to triple tap.
- However, we now report 0,0 which can be considered as a move of the second touch
- with latest commit in this bug report (too lazy to get the commit id), we set the SEMI_MT flag, which basically says "the touches are wrong, but you might be able to take some information from it".

In the slow case (one, then two, then three fingers), if we stop reporting the 0,0 coordinate, this will effectively fix this issue here. The second slot will stay in place and will leave happily ever after.

However, if you triple tap directly, you end up receiving only the triple tap message, which means we will set the second slot in whatever position it was previously. If the previous gesture was a true palm, then we will end up in a special corner case where we will have to instruct libinput to ignore this case.

I would be more inclined to just disable all of our smart things we do in libinput if we have a semi-mt device.

yeah, fair call. I just got https://bugzilla.redhat.com/show_bug.cgi?id=1295073 as well which shows that Alps touchpads have the same issue. I thought about this yesterday for a while and while it is theoretically possible to handle the touchpoints in a different manner, the effort is too high and the gain is too low.

http://lists.freedesktop.org/archives/wayland-devel/2016-January/026665.html

commit 983a8ec4c2ed946e48cc43dd8aeb48e7a54deea5
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue Jan 19 09:05:31 2016 +1000

    touchpad: disable MT for all semi-mt devices