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 751036 - Fix touch events on nested wayland
Fix touch events on nested wayland
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2015-06-16 11:54 UTC by Carlos Garnacho
Modified: 2015-06-16 18:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
x11: Fix touch events on nested wayland (1.84 KB, patch)
2015-06-16 11:55 UTC, Carlos Garnacho
reviewed Details | Review
x11: Fix touch events on nested wayland (1.87 KB, patch)
2015-06-16 18:13 UTC, Carlos Garnacho
accepted-commit_now Details | Review

Description Carlos Garnacho 2015-06-16 11:54:56 UTC
In order to fix touch events on mutter --wayland, we should:

- Stop attempting to take the passive touch grab on the root window, this can only be confusing to the nested and x11 compositors in case of success.
- Set the touch events in the stage window event mask, just like any other clients do.

I'm attaching a patch to fix both, with it touch events are forwarded just fine through mutter and the wayland clients in the nested compositor.
Comment 1 Carlos Garnacho 2015-06-16 11:55:45 UTC
Created attachment 305383 [details] [review]
x11: Fix touch events on nested wayland

If we're running as a nested compositor, we must not attempt to
passive grab on the root window, and we should be setting the
touch event mask on the stage window.
Comment 2 Jasper St. Pierre (not reading bugmail) 2015-06-16 15:10:15 UTC
Review of attachment 305383 [details] [review]:

::: src/backends/x11/meta-backend-x11.c
@@ +764,3 @@
+       * we must set the touch events on the stage mask then.
+       */
+      XISetMask (mask.mask, XI_TouchBegin);

Why can't we take touch events in our mask as an X11 compositor again?

The original commit was here: https://git.gnome.org/browse/mutter/commit/?id=3b51405

But we've fixed that since, right? All our actors are truly touch-capable now, and we take the grab.
Comment 3 Jasper St. Pierre (not reading bugmail) 2015-06-16 15:15:29 UTC
Also, the "meta_is_wayland_compositor" checks were getting a bit ridiculous, so I just pushed a quick cleanup.

https://git.gnome.org/browse/mutter/commit/?id=6aead0c67cafa4b1bd5982c8601c77d035fe1bd8
Comment 4 Carlos Garnacho 2015-06-16 16:56:37 UTC
(In reply to Jasper St. Pierre from comment #2)
> Review of attachment 305383 [details] [review] [review]:
> 
> ::: src/backends/x11/meta-backend-x11.c
> @@ +764,3 @@
> +       * we must set the touch events on the stage mask then.
> +       */
> +      XISetMask (mask.mask, XI_TouchBegin);
> 
> Why can't we take touch events in our mask as an X11 compositor again?

We otherwise receive events for the sequence twice on the right circumstances:

1) you press your touchscreen, the passive grab takes over
2) the compositor gives up on the touch, ends up replaying the touch sequence. TouchEnd is emitted
3) touch handling is punted to the next client in the list, and events sent again (TouchBegin included). If it happens to be mutter again through the root window evmask, clutter/mutter see the same stream of events twice.
Comment 5 Jasper St. Pierre (not reading bugmail) 2015-06-16 17:09:03 UTC
Oh, really? I figured the client would have been skipped because it has the same client ID, since that's how it works for e.g. MapRequest.
Comment 6 Carlos Garnacho 2015-06-16 17:33:38 UTC
(In reply to Jasper St. Pierre from comment #5)
> Oh, really? I figured the client would have been skipped because it has the
> same client ID, since that's how it works for e.g. MapRequest.

It doesn't work that way... see http://cgit.freedesktop.org/xorg/xserver/tree/dix/touch.c#n904 , the list of listeners is built early before TouchBegin, I don't see checks for ClientPtr though. After that all it does is just punting to next-to-current in case of receiving XIRejectTouch from the current listener.
Comment 7 Jasper St. Pierre (not reading bugmail) 2015-06-16 17:59:01 UTC
that's disappointing, but it is what it is i guess
Comment 8 Carlos Garnacho 2015-06-16 18:13:17 UTC
Created attachment 305417 [details] [review]
x11: Fix touch events on nested wayland

If we're running as a nested compositor, we must not attempt to
passive grab on the root window, and we should be setting the
touch event mask on the stage window.
Comment 9 Jasper St. Pierre (not reading bugmail) 2015-06-16 18:27:19 UTC
Review of attachment 305417 [details] [review]:

Suggesting a better comment here which better expresses the problem here. Otherwise, thanks for the patch! Looks good.

::: src/backends/x11/meta-backend-x11.c
@@ +780,3 @@
+    {
+      /* On nested we don't set up the passive touch
+       * we must set the touch events on the stage mask then.

/* When we're an X11 compositor, we can't take these events or else
 * replaying events from our passive root window grab will cause
 * them to come back to us.
 *
 * When we're a nested application, we want to behave like any other
 * application, so select these events like normal apps do.
 */
Comment 10 Carlos Garnacho 2015-06-16 18:41:10 UTC
Cheers! changed the comment as you suggested and pushed.

Attachment 305417 [details] pushed as 82a7060 - x11: Fix touch events on nested wayland