Bug 96687

record one of the non-working swipes with evemu-record please and attach the output here, thanks.

Created attachment 124731 [details]
output from evemu

The problem is two-fold here. The gesture is recognised as a pinch gesture instead, caused by 83f3dbd. Thats arguably wrong for a 4 finger gesture but the immediate fix (checking for == 2 fingers in that condition) doesn't fix your issue either because only two fingers are detected in the first event. So it still triggers, with extra fingers added later.

Looks like this is the case where we need to start using the cancel event more aggressively.

Thanks for sorting that out so quickly.  Let me know if you want me to test any changes made etc.

first part of the fix


commit 11917061fe320c08aab66134d10038052fb5ade0
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Tue Jun 28 15:25:42 2016 +1000

    touchpad: only check for vertical finger distribution on 2fg gestures

Created attachment 124773 [details] [review]
0001-gestures-make-the-gesture-movement-threshold-dependi.patch

Give this one a try please and let me know how you go. Not a final patch but I wonder if that's a good enough approach.

(In reply to Peter Hutterer from comment #6)
> Created attachment 124773 [details] [review] [review]
> 0001-gestures-make-the-gesture-movement-threshold-dependi.patch
> 
> Give this one a try please and let me know how you go. Not a final patch but
> I wonder if that's a good enough approach.

I applied the patch and compiled with ./autogen.sh, make all and make install.  A which gives me:

[dbeurle@xx ~]$ which libinput-list-devices 
/usr/local/bin/libinput-list-devices

It doesn't seem to work.  Would you like a renewed evemu output, or perhaps some way of making sure the newly compiled library is being linked correctly?

run ./autogen.sh --prefix=/usr --libdir=/usr/lib64, that should put everything in the right directory. If you install the gtk+-devel package you can also run the ./tools/event-gui tool directly, that maps libinput events to some graphical output for testing and debugging without the need to restart X

fwiw, I did some analysis on your recording and the problem (at least with that recording) is that you have some initial pinch-like movement where two fingers move in the opposite direction. That's why the pinch triggers and the current implementation works that once you're in pinch mode you can't switch out of it anymore (to allow for pinch/rotate/move in one gesture). The threshold patch fixes that for the recording, some more sophisticated approach is possible but if the simple solution works I'd be happy to go with that first :)

(In reply to Peter Hutterer from comment #8)
> run ./autogen.sh --prefix=/usr --libdir=/usr/lib64, that should put
> everything in the right directory. If you install the gtk+-devel package you
> can also run the ./tools/event-gui tool directly, that maps libinput events
> to some graphical output for testing and debugging without the need to
> restart X

Thanks, I did that.  I see that when I try using this with the debug program I get the following output.  The first is for the swipe that triggers a workspace change and the next is one that doesn't get picked up by Gnome.

event14	GESTURE_SWIPE_BEGIN +33.89s	4
event14	GESTURE_SWIPE_UPDATE +33.89s	4  0.00/27.32 ( 0.00/38.49 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.89s	4 -0.07/32.07 (-0.10/45.17 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.90s	4 -0.61/44.27 (-0.76/55.34 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.91s	4 -1.42/52.34 (-1.78/65.42 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.92s	4 -1.93/59.27 (-2.41/74.09 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.92s	4 -2.12/66.80 (-2.64/83.50 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.94s	4 -2.18/74.72 (-2.72/93.40 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.94s	4 -2.12/85.80 (-2.64/107.25 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.95s	4 -3.08/96.24 (-3.85/120.30 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +33.96s	4 -3.31/102.83 (-4.14/128.54 unaccelerated)
event14	GESTURE_SWIPE_END +33.99s	4
event14	GESTURE_SWIPE_BEGIN +36.85s	4
event14	GESTURE_SWIPE_UPDATE +36.85s	4 -1.48/24.32 (-2.02/33.17 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.86s	4 -1.60/25.85 (-2.46/39.66 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.87s	4 -2.18/36.16 (-2.72/45.20 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.88s	4 -2.39/41.34 (-2.98/51.67 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.88s	4 -2.60/47.07 (-3.25/58.84 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.90s	4 -2.85/50.03 (-3.56/62.53 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.90s	4 -3.08/52.36 (-3.85/65.45 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.91s	4 -4.13/58.84 (-5.16/73.55 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.92s	4 -4.71/67.21 (-5.89/84.01 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.93s	4 -3.29/70.59 (-4.11/88.23 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.93s	4 -3.25/71.91 (-4.06/89.89 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.94s	4 -3.25/74.59 (-4.06/93.24 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.95s	4 -0.77/73.39 (-0.97/91.74 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.96s	4  0.38/71.91 ( 0.47/89.89 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.97s	4  0.13/68.45 ( 0.16/85.56 unaccelerated)
event14	GESTURE_SWIPE_UPDATE +36.98s	4  0.27/59.57 ( 0.34/74.46 unaccelerated)
event14	GESTURE_SWIPE_END +37.00s	4

(In reply to Peter Hutterer from comment #9)
> fwiw, I did some analysis on your recording and the problem (at least with
> that recording) is that you have some initial pinch-like movement where two
> fingers move in the opposite direction. That's why the pinch triggers and
> the current implementation works that once you're in pinch mode you can't
> switch out of it anymore (to allow for pinch/rotate/move in one gesture).
> The threshold patch fixes that for the recording, some more sophisticated
> approach is possible but if the simple solution works I'd be happy to go
> with that first :)

When I saw it was registering a pinch, I tried it out on evince which supports pinch and my swipe up caused it to zoom.

ok, thanks. I'll need a few more recordings of swipe gestures that are interpreted as pinch, clearly this single patch wasn't enough to fix it. Make them separate please so I can address them one-by-one, thanks.

(In reply to Peter Hutterer from comment #12)
> ok, thanks. I'll need a few more recordings of swipe gestures that are
> interpreted as pinch, clearly this single patch wasn't enough to fix it.
> Make them separate please so I can address them one-by-one, thanks.

I'm not sure that they were registered as a pinch.  Isn't the output from what I posted indicate that both of them were a swipe?  When I pinched I got a pitch event reported.

weird, I'm not sure how I managed to misread this but I did read the second one as pinch... you're right, of course it's a swipe but I'm not sure why gnome wouldn't pick it up. did you restart X after install or did you just run the debug tool from the current tree? If the latter, that'd be a reason then

I'm running the tool from the current debug tree.  How come Gnome isn't picking up the changes to libinput, even though the changes were made to install to /usr/lib64 ?

It seems pretty hard to make it do a pinch motion, definitely far better than before!  That's a win.

so you're saying it doesn't work even after a restart? that's odd, I can't think of a good reason why that'd happen. it overwrites the libinput.so.10.8.5 file correctly and the symlinks all point to that?

if pinch gestures are hard to trigger that's something else we'll have to fix but that's less urgent I guess. Are you happy with the patch as-is otherwise then?

The output is the following:

[dbeurle@tesla lib64]$ ll | grep -i libinput
-rwxr-xr-x. 1 root root      953 Jun 29 18:03 libinput.la
lrwxrwxrwx. 1 root root       18 Jun 29 18:03 libinput.so -> libinput.so.10.8.3
lrwxrwxrwx. 1 root root       18 Jun 29 18:03 libinput.so.10 -> libinput.so.10.8.6
-rwxr-xr-x. 1 root root   895992 Jun 29 18:03 libinput.so.10.8.3
-rwxr-xr-x. 1 root root   183864 Jun 24 15:40 libinput.so.10.8.6

Before what I meant by the pinch, is that now it is difficult to get register a pinch when doing the four-finger swipe for any conceivable orientation.  The pinch gesture is still registered all the same.

(In reply to Darcy from comment #17)
> The output is the following:
> 
> [dbeurle@tesla lib64]$ ll | grep -i libinput
> -rwxr-xr-x. 1 root root      953 Jun 29 18:03 libinput.la
> lrwxrwxrwx. 1 root root       18 Jun 29 18:03 libinput.so ->
> libinput.so.10.8.3
> lrwxrwxrwx. 1 root root       18 Jun 29 18:03 libinput.so.10 ->
> libinput.so.10.8.6

this one is the problem. it points to the system-installed version

> -rwxr-xr-x. 1 root root   895992 Jun 29 18:03 libinput.so.10.8.3
> -rwxr-xr-x. 1 root root   183864 Jun 24 15:40 libinput.so.10.8.6

long story short, the 1.3 branch saw a couple of libtool version bumps and it's at 10.8.6, the master branch is still unchanged from the 1.3.0 release. if you fix that symlink it should pick up the right one now


> Before what I meant by the pinch, is that now it is difficult to get
> register a pinch when doing the four-finger swipe for any conceivable
> orientation.  The pinch gesture is still registered all the same.

ah, cool. I'm happy with that then :)

Switched the symlinks and now Gnome picks it up.

The swipes work really well.  Great job!

https://lists.freedesktop.org/archives/wayland-devel/2016-June/029745.html

commit 4923c404e8199c49106700a5c1f96f0ac3428f64
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Wed Jun 29 10:03:23 2016 +1000

    gestures: make the gesture movement threshold depending on finger count