After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765937 - Handle touchpad pinch gestures with n_fingers>2
Handle touchpad pinch gestures with n_fingers>2
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-05-03 11:39 UTC by Carlos Garnacho
Modified: 2017-07-20 15:20 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
clutte: Add n_fingers field to ClutterTouchpadPinchEvent (684 bytes, patch)
2016-05-03 11:40 UTC, Carlos Garnacho
committed Details | Review
clutter/evdev: Fill n_fingers information in touchpad pinch events (3.70 KB, patch)
2016-05-03 11:40 UTC, Carlos Garnacho
committed Details | Review
clutter: Rename touchpad gesture n_fingers getter to be more generic (3.31 KB, patch)
2016-05-03 11:40 UTC, Carlos Garnacho
committed Details | Review
wayland: Forward touchpad pinch n_fingers information to clients (1.55 KB, patch)
2016-05-03 11:40 UTC, Carlos Garnacho
committed Details | Review
windowManager: Update API call (1023 bytes, patch)
2016-05-03 11:41 UTC, Carlos Garnacho
committed Details | Review
viewSelection: Handle touchpad 3-finger pinches (2.53 KB, patch)
2016-05-03 11:41 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2016-05-03 11:39:23 UTC
Not long ago, libinput gained support for 3 finger pinches. We could be using that in gnome-shell in order to show the overview, just like the gesture found in touchscreens.

I'm attaching 4 patches to mutter and 2 to gnome-shell to allow that.
Comment 1 Carlos Garnacho 2016-05-03 11:40:15 UTC
Created attachment 327212 [details] [review]
clutte: Add n_fingers field to ClutterTouchpadPinchEvent
Comment 2 Carlos Garnacho 2016-05-03 11:40:20 UTC
Created attachment 327213 [details] [review]
clutter/evdev: Fill n_fingers information in touchpad pinch events

Libinput tells us this much (at least for 3 fingers), so make sure
we propagate this info.
Comment 3 Carlos Garnacho 2016-05-03 11:40:24 UTC
Created attachment 327214 [details] [review]
clutter: Rename touchpad gesture n_fingers getter to be more generic

This can also be used now on pinch events, so make a more generic name
for this function.
Comment 4 Carlos Garnacho 2016-05-03 11:40:29 UTC
Created attachment 327215 [details] [review]
wayland: Forward touchpad pinch n_fingers information to clients

We used to hardcode 2 because those were the only events we used to
handle. Now the ClutterTouchpadPinchEvent will tell us.
Comment 5 Carlos Garnacho 2016-05-03 11:41:16 UTC
Created attachment 327216 [details] [review]
windowManager: Update API call

This function has now a more generic name, so it caters for both
swipe and pinch touchpad gestures.
Comment 6 Carlos Garnacho 2016-05-03 11:41:21 UTC
Created attachment 327217 [details] [review]
viewSelection: Handle touchpad 3-finger pinches

In order to show the overview, just like touchscreens do.
Comment 7 Carlos Garnacho 2016-05-03 11:45:09 UTC
Note that this introduces what's possibly the first deviation from "upstream" clutter. There used to be a clutter_event_get_swipe_gesture_finger_count() that's been renamed bo be more generic, another approach might be adding a separate clutter_event_get_pinch_gesture_finger_count(), I'm not sure how useful is to maintain compatibility on this scarcely used function though.
Comment 8 Florian Müllner 2016-11-04 14:32:16 UTC
Review of attachment 327212 [details] [review]:

Nit: clutte => clutter in the subject
Comment 9 Florian Müllner 2016-11-04 14:32:19 UTC
Review of attachment 327213 [details] [review]:

LGTM
Comment 10 Florian Müllner 2016-11-04 14:32:22 UTC
Review of attachment 327214 [details] [review]:

::: clutter/clutter/clutter-event.h
@@ +675,3 @@
+
+CLUTTER_AVAILABLE_IN_1_24
+guint                   clutter_event_get_gesture_pinch_finger_count (const ClutterEvent     *event);

Left-over from an older patch version?
Comment 11 Florian Müllner 2016-11-04 14:32:25 UTC
Review of attachment 327215 [details] [review]:

OK
Comment 12 Florian Müllner 2016-11-04 14:32:28 UTC
Review of attachment 327216 [details] [review]:

OK
Comment 13 Florian Müllner 2016-11-04 14:32:31 UTC
Review of attachment 327217 [details] [review]:

LGTM (though playing with it, it kinda makes me miss a gesture to leave the overview again ...)
Comment 14 Carlos Garnacho 2016-11-04 15:31:37 UTC
(In reply to Florian Müllner from comment #10)
> Review of attachment 327214 [details] [review] [review]:
> 
> ::: clutter/clutter/clutter-event.h
> @@ +675,3 @@
> +
> +CLUTTER_AVAILABLE_IN_1_24
> +guint                   clutter_event_get_gesture_pinch_finger_count (const
> ClutterEvent     *event);
> 
> Left-over from an older patch version?

It indeed is :).

(In reply to Florian Müllner from comment #13)
> Review of attachment 327217 [details] [review] [review]:
> 
> LGTM (though playing with it, it kinda makes me miss a gesture to leave the
> overview again ...)

Right... natural choice would be 3fg zoom, although I guess better done for both touchpad/touchscreen.

That said, assuming there was a reason to visit the overview, next action would be tapping somewhere :).
Comment 15 Carlos Garnacho 2016-11-04 15:33:31 UTC
Attachment 327212 [details] pushed as 4186833 - clutter: Add n_fingers field to ClutterTouchpadPinchEvent
Attachment 327213 [details] pushed as cf0a453 - clutter/evdev: Fill n_fingers information in touchpad pinch events
Attachment 327214 [details] pushed as 4cfc21d - clutter: Rename touchpad gesture n_fingers getter to be more generic
Attachment 327215 [details] pushed as 15db189 - wayland: Forward touchpad pinch n_fingers information to clients
Comment 16 Carlos Garnacho 2016-11-04 15:37:09 UTC
Attachment 327216 [details] pushed as 7e52746 - windowManager: Update API call
Attachment 327217 [details] pushed as 0b05b7a - viewSelection: Handle touchpad 3-finger pinches
Comment 17 Adrien Bustany 2017-07-20 15:20:00 UTC
I got to "play" with this new feature since I upgraded to Fedora 26 (mutter 3.24.3), and so far I get way more "accidental" pinches than real ones. There is appears to be a threshold already (0.7 scale) before triggering the overview, so I wonder if the problem resides in mutter, in libinput (1.7.3) or in my touchpad driver (bcm5974). Assuming the issue resides in mutter, I could suggest either increasing the threshold before triggering the overview, or maybe storing its value in GSettings so that it can be adjusted/disabled (I don't use the feature myself)?