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 792005 - Single touch not supported in login screen using Wayland backend
Single touch not supported in login screen using Wayland backend
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: st
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2017-12-28 07:36 UTC by John
Modified: 2018-01-17 15:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
clutter/evdev: Ensure a valid ClutterEventSequence on single-touch devices (1.30 KB, patch)
2018-01-16 23:22 UTC, Carlos Garnacho
committed Details | Review

Description John 2017-12-28 07:36:23 UTC
[Description]

Only multi-touch devices can work in login-screen. Single touch devices will not work at all, and will require using mouse and keyboard to login.

[Explanation]

(1) In newer versions of gdm3, Wayland is enabled as the default.
(2) Wayland uses libinput to read touch events.
(3) In libinput, the "slot" value is set to "-1" for single touch events.
(4) In clutter, the "sequence" value is set to "0" for single touch events.
(5) In gnome-shell toolkit (st), touch events are converted to button events.
(6) In st_button_release(), it will call g_signal_emit(button_signals[CLICKED]) if "sequence" value is not "0", therefor ignoring event from single touch devices.
Comment 1 Carlos Garnacho 2018-01-16 23:22:41 UTC
Created attachment 366907 [details] [review]
clutter/evdev: Ensure a valid ClutterEventSequence on single-touch devices

Libinput shall report those as having slot=-1, which gets mistakenly
translated into the special "NULL" ClutterEventSequence. Making those
events get a non-NULL sequence will make single touch devices work.
Comment 2 Florian Müllner 2018-01-17 00:51:13 UTC
Review of attachment 366907 [details] [review]:

::: clutter/clutter/evdev/clutter-device-manager-evdev.c
@@ +434,2 @@
   /* "NULL" sequences are special cased in clutter */
+  event->touch.sequence = GINT_TO_POINTER (MAX (1, slot + 1));

Is 0 a valid slot? If yes, can we accidentally mix up events with slots -1 and 0?
Comment 3 Carlos Garnacho 2018-01-17 10:53:00 UTC
It is valid, hence the +1 dance. But it can't get mixed usually (it would not from the same source ClutterInputDevice at least). Gestures and event handlers should ideally check for both.

Another option is making ClutterEventSequence an opaque struct that we allocate, that'd ensure each is unique. But code depending on this could need some double checking, IIRC wl_touch needs translating that back to slot IDs for protocol purposes.
Comment 4 Florian Müllner 2018-01-17 11:29:49 UTC
(In reply to Carlos Garnacho from comment #3)
> IIRC wl_touch needs translating that back to slot
> IDs for protocol purposes.

Mmmh, so how does it know whether "1" translates to a slot ID of 0 or -1?
Comment 5 Carlos Garnacho 2018-01-17 11:46:20 UTC
Right, that's kind of broken if mixed input from 2 touchscreens is received regardless of them being single/multitouch or a combination, because MetaWaylandTouch doesn't check for overlapping slots.

The protocol just goes as far as telling the slot IDs should be unique for the time being, so for single touchscreen usecases it shouldn't matter we use 0 or -1 there.
Comment 6 Florian Müllner 2018-01-17 11:49:09 UTC
OK. If you say it works, I'll take your word for it :-)
Comment 7 Carlos Garnacho 2018-01-17 15:58:43 UTC
Cheers :), pushed to master. There's certainly some room for
improvement, but it takes some determination and HW to make it
break, it's not as important.

Attachment 366907 [details] pushed as 7346419 - clutter/evdev: Ensure a valid ClutterEventSequence on single-touch devices