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 731380 - gtk3-demo button box works badly with a touch screen under wayland
gtk3-demo button box works badly with a touch screen under wayland
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: Wayland
3.13.x
Other Mac OS
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2014-06-08 21:29 UTC by Sjoerd Simons
Modified: 2014-08-26 21:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Test hack to verify the cause of some of the oddness (1.95 KB, patch)
2014-06-08 21:33 UTC, Sjoerd Simons
none Details | Review
button: Track gesture point to maintain priv->in_button (3.12 KB, patch)
2014-08-22 13:22 UTC, Carlos Garnacho
committed Details | Review
wayland: Add internal API to unset a touch implicit grab (2.05 KB, patch)
2014-08-22 13:22 UTC, Carlos Garnacho
committed Details | Review
wayland: unset sequence if the xdg surface is moved/resized through touch (3.16 KB, patch)
2014-08-22 13:22 UTC, Carlos Garnacho
committed Details | Review

Description Sjoerd Simons 2014-06-08 21:29:50 UTC
To reproduce, simply open up the gtk button demo under weston (using gtk+ master + my patch from #731371). Two issues occur:
  0) If pointer focus is outside the window,  no buttons can be pressed using touch.
  1) If the pointer focus is in the window, pressing buttons behaves irretatic, typically working on second press

For the first issue, the problem is likely due to no _ENTER_NOTIFY events being triggered under wayland when touching a window. Unlike X where the pointer & touch devices are more tightly coupled, causing the pointer to end a window upon first touch. I've not investigated this further thusfar.

The second issue is somewhat ellusive. It seems that on X when hitting the screen one first gets an (emulated) Motion event followed by the Touch begin, while on Wayland one only gets the Touch begin..  

This event sequence means that on X, the emulated pointer event causes pointer_info->window_under_pointer in gdkwindow.c to get set to the window under the touch. Which in turn means that when the TouchBegin is received in X the synthisized crossing event will succeed (gdkwindow.c line 8805). While under wayland, when the window_under_pointer entry is still set to the old window causing the same event synthesizing not to happen.
Comment 1 Sjoerd Simons 2014-06-08 21:33:30 UTC
Created attachment 278112 [details] [review]
Test hack to verify the cause of some of the oddness

I've used this hack to verify the difference in behaviour between X and Wayland touch handling. Simply by having the wayland input backend generated an emulated motion event when the user hits the touchscreen (as X seems to do) the touch behaviour between both backend matches again. (assuming the pointer focus was already in the relevant top level window)
Comment 2 Carlos Garnacho 2014-06-09 14:52:45 UTC
This deserves some investigation indeed, I will look on that. The way the backend sets the emulated pointer event flag on touch events from the first touching slot (in gdk/wayland/gdkdevice-wayland.c:1294) should be enough for GDK/GTK to convert those to emulated pointer events depending on masks/grabs.
Comment 3 Carlos Garnacho 2014-08-22 13:17:40 UTC
I indeed were able to discover a few lurking bugs here, it is worth noting that regardless of visual status, the ::clicked signal was still being emitted. First there's styling issues in GtkButton on the complete lack of crossing events (which you get implicitly in x11 for the pointer emulating touch sequence)

Second, after dragging the window through touch, information about that touch implicit grab would be left stuck when the sequence is grabbed by the compositor, which would lead to strange behavior in the grab window tracking, although this is not strictly related to buttons, it was made really noticeable in this demo.

I'm attaching patches for these.
Comment 4 Carlos Garnacho 2014-08-22 13:20:59 UTC
Oh, forgot to mention, the gdk patches depend on the ones in bug #734374, so we can fetch the GdkEventSequence together with the serial.
Comment 5 Carlos Garnacho 2014-08-22 13:22:28 UTC
Created attachment 284197 [details] [review]
button: Track gesture point to maintain priv->in_button

This makes the active state work invariably with both mouse/touch, and
regardless of X11 pointer emulation being friendly and sending crossing
events for the emulated pointer events in the latter.

This makes GtkButtons' active state look correct when pressing on
touchscreens on wayland.
Comment 6 Carlos Garnacho 2014-08-22 13:22:32 UTC
Created attachment 284198 [details] [review]
wayland: Add internal API to unset a touch implicit grab

This removes both the wayland specific accounting, and the Gdk implicit
grab tracking.
Comment 7 Carlos Garnacho 2014-08-22 13:22:36 UTC
Created attachment 284199 [details] [review]
wayland: unset sequence if the xdg surface is moved/resized through touch

The latest implicit grab serial is used in order to start the compositor
grab, If it belongs to a touch event, remove that touch sequence, as the
rest of the sequence will be gone for good.

This avoids stale sequences (and implicit grab info) after a window is
moved/resized.
Comment 8 Matthias Clasen 2014-08-26 00:49:36 UTC
Review of attachment 284199 [details] [review]:

makes sense to me
Comment 9 Matthias Clasen 2014-08-26 00:50:28 UTC
Review of attachment 284198 [details] [review]:

ok
Comment 10 Matthias Clasen 2014-08-26 00:55:29 UTC
Review of attachment 284197 [details] [review]:

::: gtk/gtkbutton.c
@@ +635,3 @@
+  gtk_gesture_get_point (gesture, sequence, &x, &y);
+
+  in_button = (x >= 0 && y >= 0 && x < allocation.width && y < allocation.height);

Shouldn't allocation.x/y be used here ?
Comment 11 Carlos Garnacho 2014-08-26 11:45:58 UTC
Attachment 284198 [details] pushed as 2c36dc7 - wayland: Add internal API to unset a touch implicit grab
Attachment 284199 [details] pushed as 29d9b2f - wayland: unset sequence if the xdg surface is moved/resized through touch
Comment 12 Carlos Garnacho 2014-08-26 11:52:33 UTC
(In reply to comment #10)
> Review of attachment 284197 [details] [review]:
> 
> ::: gtk/gtkbutton.c
> @@ +635,3 @@
> +  gtk_gesture_get_point (gesture, sequence, &x, &y);
> +
> +  in_button = (x >= 0 && y >= 0 && x < allocation.width && y <
> allocation.height);
> 
> Shouldn't allocation.x/y be used here ?

no :), gtk_gesture_get_point() always returns widget-based coordinates, regardless of event/widget windows.
Comment 13 Matthias Clasen 2014-08-26 20:01:28 UTC
Review of attachment 284197 [details] [review]:

ok then
Comment 14 Carlos Garnacho 2014-08-26 21:23:24 UTC
Attachment 284197 [details] pushed as ea830ae - button: Track gesture point to maintain priv->in_button