Bug 96687 - Lazy four fingered swipe does not work
Summary: Lazy four fingered swipe does not work
Status: RESOLVED FIXED
Alias: None
Product: Wayland
Classification: Unclassified
Component: libinput (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-27 00:32 UTC by Darcy
Modified: 2016-07-04 07:16 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
output from evemu (58.63 KB, text/plain)
2016-06-27 01:26 UTC, Darcy
Details
0001-gestures-make-the-gesture-movement-threshold-dependi.patch (2.03 KB, patch)
2016-06-29 00:07 UTC, Peter Hutterer
Details | Splinter Review

Description Darcy 2016-06-27 00:32:07 UTC
The four-fingered gesture to switch workspaces in Gnome 3.20 is only recognized when the four figures are aligned parallel with the spacebar.  

A rotation of the four fingers by 45 degrees (such as when they are in the typing position) does not register a gesture, even though all of the fingers are moving in an upward direction.  I drew a pathetic ASCII diagram of the situation.

|---------------------|
|                 x   |
|           x         |
|      x              |
|  x                  |
|---------------------|

Let me know if you would like additional information or if this is filed in the wrong place.  My specs are:

Fedora 24 with libinput 1.3.3.  Late 2013 macbook pro.

Thanks.
Comment 1 Peter Hutterer 2016-06-27 01:08:39 UTC
record one of the non-working swipes with evemu-record please and attach the output here, thanks.
Comment 2 Darcy 2016-06-27 01:26:31 UTC
Created attachment 124731 [details]
output from evemu
Comment 3 Peter Hutterer 2016-06-27 04:10:57 UTC
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.
Comment 4 Darcy 2016-06-27 04:32:03 UTC
Thanks for sorting that out so quickly.  Let me know if you want me to test any changes made etc.
Comment 5 Peter Hutterer 2016-06-28 23:33:56 UTC
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
Comment 6 Peter Hutterer 2016-06-29 00:07:30 UTC
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.
Comment 7 Darcy 2016-06-29 02:17:34 UTC
(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?
Comment 8 Peter Hutterer 2016-06-29 02:23:05 UTC
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
Comment 9 Peter Hutterer 2016-06-29 02:31:05 UTC
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 :)
Comment 10 Darcy 2016-06-29 03:16:21 UTC
(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
Comment 11 Darcy 2016-06-29 03:19:12 UTC
(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.
Comment 12 Peter Hutterer 2016-06-29 03:40:32 UTC
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.
Comment 13 Darcy 2016-06-29 07:58:56 UTC
(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.
Comment 14 Peter Hutterer 2016-06-29 08:21:20 UTC
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
Comment 15 Darcy 2016-06-29 08:46:01 UTC
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.
Comment 16 Peter Hutterer 2016-06-29 09:23:07 UTC
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?
Comment 17 Darcy 2016-06-29 21:32:46 UTC
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.
Comment 18 Peter Hutterer 2016-06-29 22:11:41 UTC
(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 :)
Comment 19 Darcy 2016-06-30 00:29:09 UTC
Switched the symlinks and now Gnome picks it up.

The swipes work really well.  Great job!
Comment 21 Peter Hutterer 2016-07-04 07:16:32 UTC
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


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.