GNOME Bugzilla – Bug 733633
Handle touch events across gnome-shell
Last modified: 2014-07-31 08:57:03 UTC
On X11, gnome-shell largely relies on pointer emulation to deal with touchscreens, and that's usually fine as for most parts of it we don't want real multitouch. Even though, on native mutter there is no such pointer emulation, so gnome-shell needs to eventually support touch events in order to work with touchscreens as a wayland compositor. I'm attaching a bunch of patches that make the most prominent parts of the UI deal with touch events, either locking interaction to the first touch to arrive, or to a "pointer emulating" sequence provided by MetaDisplay in a patch in bug #733631.
Created attachment 281521 [details] [review] st-button: Handle touch events On touch events, StButton becomes locked to a single sequence, so no multiple touches can trigger it simultaneously.
Created attachment 281522 [details] [review] popupMenu: dismiss the menu on touch events No sequence checks are done, just any touch outside will dismiss the popup.
Created attachment 281523 [details] [review] panelMenu: Interact to touch events No sequence checks are done, these UI elements promptly trigger a grab that will cancel ongoing touches and redirect later ones somewhere else, so that works as a barrier to multi-toggling.
Created attachment 281524 [details] [review] panel: Make the "Activities" button react to touch events
Created attachment 281525 [details] [review] backgroundMenu: Allow for long presses on touch devices These don't have a button set. Also, popup the menu relative to gesture reported coordinates, and not relative to the pointer position everytime.
Created attachment 281526 [details] [review] slider: React to touch events The slider locks to the first interacting sequence, and ignores events from any other.
Created attachment 281527 [details] [review] keyboard: Handle touch events The code to create regular and extension keys has been made common, and touch events are handled, so that each button locks to a sequence, but several sequences may interact with multiple key buttons.
Created attachment 281528 [details] [review] appDisplay: Allow clicks/long presses through touch events The long press code has been refactored so it can be used on both pointer and touch events, and the click gesture has been made to account for button=0.
Created attachment 281529 [details] [review] grabHelper: handle touch events during grab modality The "pointer emulating" touch sequence will be handled in order to dismiss the grab, while others are just propagated. Also, when a button press / touch begin trigger the ungrab, ignore motions additionally to the release, otherwise clients below may end up with inconsistent streams of events (eg, touch_motion without a prior touch_down)
Review of attachment 281521 [details] [review]: Looks OK to me.
Review of attachment 281522 [details] [review]: OK.
Review of attachment 281523 [details] [review]: OK.
Review of attachment 281524 [details] [review]: ::: js/ui/panel.js @@ +616,3 @@ + this.menu.close(); + + return Clutter.EVENT_STOP; You change the semantics here to be EVENT_STOP instead of EVENT_PROPAGATE. I'm not sure it makes any difference, but can you do this in a cleanup commit first?
Review of attachment 281525 [details] [review]: LGTM.
Review of attachment 281526 [details] [review]: OK.
Review of attachment 281527 [details] [review]: Could you do the first cleanup as a separate commit?
Review of attachment 281528 [details] [review]: OK.
Review of attachment 281529 [details] [review]: Can you implement the MOTION fix as a separate commit and explain it better? ::: js/ui/grabHelper.js @@ +291,3 @@ + let touch_update = type == Clutter.EventType.TOUCH_UPDATE; + let touch_begin = type == Clutter.EventType.TOUCH_BEGIN; + let touch_end = type == Clutter.EventType.TOUCH_END; touchUpdate, touchBegin, touchEnd
Attachment 281521 [details] pushed as a2f263d - st-button: Handle touch events Attachment 281522 [details] pushed as da26a9d - popupMenu: dismiss the menu on touch events Attachment 281523 [details] pushed as e545ec5 - panelMenu: Interact to touch events Attachment 281525 [details] pushed as acb1497 - backgroundMenu: Allow for long presses on touch devices Attachment 281526 [details] pushed as dbbf409 - slider: React to touch events Attachment 281528 [details] pushed as f8899cf - appDisplay: Allow clicks/long presses through touch events
Created attachment 281619 [details] [review] panel: Make the "Activities" button react to touch events
Created attachment 281620 [details] [review] keyboard: Use common code to create regular and extension key buttons The code is almost the same, so pull this out to a generic _makeKey(), and use it from both places.
Created attachment 281621 [details] [review] keyboard: Handle touch events Handle touch events, so that an interacted button locks to a single sequence, but multiple sequences are free to interact with multiple key buttons.
Created attachment 281622 [details] [review] grabHelper: consume the press/motion/release sequence if a press dismisses the grab The grab would previously just consume the button release, while propagating motion events, possibly down to clients in wayland. This would produce inconsistent streams there. On pointer events, the inconsistency would just be having clients receiving events with the button 1 set in the mask, with no implicit grab. When touch events are handled, this would be more hindering as the client would receive touch_motion events with no prior touch_down nor later touch_up, something never supposed to happen.
Created attachment 281623 [details] [review] grabHelper: handle touch events during grab modality The "pointer emulating" touch sequence will be handled in order to dismiss the grab, while the others are just propagated.
(In reply to comment #13) > Review of attachment 281524 [details] [review]: > > ::: js/ui/panel.js > @@ +616,3 @@ > + this.menu.close(); > + > + return Clutter.EVENT_STOP; > > You change the semantics here to be EVENT_STOP instead of EVENT_PROPAGATE. I'm > not sure it makes any difference, but can you do this in a cleanup commit > first? That wasn't truly intentional, should be harmless either way, there aren't many actors to bubble to there, I've changed that to just propagate always.
Review of attachment 281619 [details] [review]: OK.
Review of attachment 281620 [details] [review]: Looks good to me.
Review of attachment 281621 [details] [review]: OK.
Attachment 281619 [details] pushed as 38d05a8 - panel: Make the "Activities" button react to touch events Attachment 281620 [details] pushed as 69d5cef - keyboard: Use common code to create regular and extension key buttons Attachment 281621 [details] pushed as a84fb99 - keyboard: Handle touch events
Review of attachment 281623 [details] [review]: OK.
Review of attachment 281622 [details] [review]: OK.
Attachment 281622 [details] pushed as bbfa616 - grabHelper: consume the press/motion/release sequence if a press dismisses the grab Attachment 281623 [details] pushed as 42b54aa - grabHelper: handle touch events during grab modality