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 765009 - support virtual input devices
support virtual input devices
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on: 760439
Blocks:
 
 
Reported: 2016-04-13 18:12 UTC by Matthias Clasen
Modified: 2016-08-10 09:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
clutter: Add virtual input device API (27.58 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
ClutterDeviceManagerEvdev: Split out seat into a separate file (28.30 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDevice: Keep track of the device manager (7.02 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
clutter/evdev: Move keyboard and pointer notification into seat (32.17 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDevice: Store the device type (4.32 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDeviceEvdev: Construct with a specific seat (5.01 KB, patch)
2016-07-26 16:45 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDeviceEvdev: Create associated ClutterInputDevice (3.29 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDeviceEvdev: Forward motion events (2.08 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
ClutterSeatEvdev: Keep track of button count (3.22 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
ClutterVirtualInputDeviceEvdev: Forward button and key presses (6.60 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
clutter/evdev: Allow specifying the ClutterInputMode of virtual devices (4.87 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
clutter: Make ClutterVirtualInputDevice public (5.44 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
clutter: Add ClutterVirtualInputDevice vmethod to notify keysyms (3.17 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
clutter/evdev: Implement ClutterVirtualInputDevice::notify_keyval (6.73 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
backends: Prepare for virtual devices (3.99 KB, patch)
2016-07-26 16:46 UTC, Carlos Garnacho
committed Details | Review
keyboard: Implement more of the wayland caribou adapter (3.37 KB, patch)
2016-07-26 16:48 UTC, Carlos Garnacho
committed Details | Review

Description Matthias Clasen 2016-04-13 18:12:48 UTC
This is needed to make remoting work nicely.
Comment 1 Matthias Clasen 2016-07-21 20:45:07 UTC
Carlos has a branch for this.
Comment 2 Carlos Garnacho 2016-07-26 16:45:31 UTC
Created attachment 332149 [details] [review]
clutter: Add virtual input device API

Virtual input devices aim to enable injecting input events as if they
came from hardware events. This is useful for things such as remote
controlling, for example via a remote desktop session.

The API so far only consists of stumps.
Comment 3 Carlos Garnacho 2016-07-26 16:45:38 UTC
Created attachment 332150 [details] [review]
ClutterDeviceManagerEvdev: Split out seat into a separate file

Split out ClutterSeatEvdev functionality into a separate file.
Comment 4 Carlos Garnacho 2016-07-26 16:45:43 UTC
Created attachment 332151 [details] [review]
ClutterVirtualInputDevice: Keep track of the device manager
Comment 5 Carlos Garnacho 2016-07-26 16:45:48 UTC
Created attachment 332152 [details] [review]
clutter/evdev: Move keyboard and pointer notification into seat

We notify per seat; so lets move the logic there. Touch and tablets to
follow later.
Comment 6 Carlos Garnacho 2016-07-26 16:45:53 UTC
Created attachment 332153 [details] [review]
ClutterVirtualInputDevice: Store the device type
Comment 7 Carlos Garnacho 2016-07-26 16:45:59 UTC
Created attachment 332154 [details] [review]
ClutterVirtualInputDeviceEvdev: Construct with a specific seat

We are still single seated, so until we are properly multi seated its
always the main seat.
Comment 8 Carlos Garnacho 2016-07-26 16:46:04 UTC
Created attachment 332155 [details] [review]
ClutterVirtualInputDeviceEvdev: Create associated ClutterInputDevice
Comment 9 Carlos Garnacho 2016-07-26 16:46:10 UTC
Created attachment 332156 [details] [review]
ClutterVirtualInputDeviceEvdev: Forward motion events
Comment 10 Carlos Garnacho 2016-07-26 16:46:17 UTC
Created attachment 332157 [details] [review]
ClutterSeatEvdev: Keep track of button count

libinput does it for us, but only for physical devices. When we add
virtual devices to the same seat, we need to track button press count
ourself.
Comment 11 Carlos Garnacho 2016-07-26 16:46:23 UTC
Created attachment 332158 [details] [review]
ClutterVirtualInputDeviceEvdev: Forward button and key presses
Comment 12 Carlos Garnacho 2016-07-26 16:46:28 UTC
Created attachment 332159 [details] [review]
clutter/evdev: Allow specifying the ClutterInputMode of virtual devices

The seat core keyboard/pointer will be "master", the ones created through
ClutterVirtualInputDevice will be "slaves".
Comment 13 Carlos Garnacho 2016-07-26 16:46:34 UTC
Created attachment 332160 [details] [review]
clutter: Make ClutterVirtualInputDevice public

This includes adding documentation and introspection annotations,
and marking the functions as extern.
Comment 14 Carlos Garnacho 2016-07-26 16:46:40 UTC
Created attachment 332161 [details] [review]
clutter: Add ClutterVirtualInputDevice vmethod to notify keysyms

Evcodes don't cut it when we have something already specifying the
character to be printed, despite the current group/level. This API
allows some more control on the intended output.
Comment 15 Carlos Garnacho 2016-07-26 16:46:46 UTC
Created attachment 332162 [details] [review]
clutter/evdev: Implement ClutterVirtualInputDevice::notify_keyval

This is somewhat gross at the moment, because we're after all mimicking
real keyboard events, we can only lookup keycodes that are available
in the current map, and the control of levels is rather limited.

Eventually, we want to implement the text_input protocol, handle these
events separately to MetaWaylandKeyboard, so event->key.keyval is
is guaranteed to be the final result. Until then, this is the farthest
we can get.
Comment 16 Carlos Garnacho 2016-07-26 16:46:51 UTC
Created attachment 332163 [details] [review]
backends: Prepare for virtual devices

Those have no backing libinput_device, and configuration does not
apply to those.
Comment 17 Carlos Garnacho 2016-07-26 16:48:55 UTC
Created attachment 332165 [details] [review]
keyboard: Implement more of the wayland caribou adapter

Have it notify properly of changes to the current input source, as
well as exposing those in get_groups().

The support for virtual keyboard events has been replaced by
ClutterVirtualInputDevice, which can be thought of as the equivalent
to the XTEST devices in X11.
Comment 18 Florian Müllner 2016-08-09 15:41:17 UTC
Review of attachment 332149 [details] [review]:

LGTM
Comment 19 Florian Müllner 2016-08-09 15:41:22 UTC
Review of attachment 332150 [details] [review]:

::: clutter/clutter/evdev/clutter-device-manager-evdev.c
@@ +1169,3 @@
+        {
+          seat = priv->main_seat;
+        }

Style nit: no braces
Comment 20 Florian Müllner 2016-08-09 15:41:25 UTC
Review of attachment 332151 [details] [review]:

::: clutter/clutter/clutter-types.h
@@ +96,3 @@
 typedef struct _ClutterInputDeviceTool          ClutterInputDeviceTool;
 typedef struct _ClutterInputDevice              ClutterInputDevice;
+typedef struct _ClutterVirtualInputDevice       ClutterVirtualInputDevice;

Does this make sense in the first patch?
Comment 21 Florian Müllner 2016-08-09 15:41:28 UTC
Review of attachment 332152 [details] [review]:

OK
Comment 22 Florian Müllner 2016-08-09 15:41:32 UTC
Review of attachment 332153 [details] [review]:

OK
Comment 23 Florian Müllner 2016-08-09 15:41:34 UTC
Review of attachment 332154 [details] [review]:

LGTM
Comment 24 Florian Müllner 2016-08-09 15:41:37 UTC
Review of attachment 332155 [details] [review]:

OK
Comment 25 Florian Müllner 2016-08-09 15:41:40 UTC
Review of attachment 332156 [details] [review]:

OK
Comment 26 Florian Müllner 2016-08-09 15:41:42 UTC
Review of attachment 332157 [details] [review]:

OK
Comment 27 Florian Müllner 2016-08-09 15:41:45 UTC
Review of attachment 332158 [details] [review]:

::: clutter/clutter/evdev/clutter-virtual-input-device-evdev.c
@@ +148,3 @@
+          break;
+        case EVDEV_BUTTON_TYPE_NONE:
+          g_assert_not_reached ();

We are looping over all possible codes, so I'd expect this condition to be hit. Should probably filter out codes that aren't pressed above, in which case the assert would make sense ...
Comment 28 Florian Müllner 2016-08-09 15:41:47 UTC
Review of attachment 332159 [details] [review]:

LGTM
Comment 29 Florian Müllner 2016-08-09 15:41:49 UTC
Review of attachment 332160 [details] [review]:

Sure
Comment 30 Florian Müllner 2016-08-09 15:42:21 UTC
Review of attachment 332163 [details] [review]:

OK
Comment 31 Florian Müllner 2016-08-09 15:54:53 UTC
Review of attachment 332161 [details] [review]:

OK
Comment 32 Florian Müllner 2016-08-09 15:57:03 UTC
Review of attachment 332162 [details] [review]:

Eeeks :-)
Comment 33 Florian Müllner 2016-08-09 16:01:28 UTC
Review of attachment 332165 [details] [review]:

OK

::: js/ui/keyboard.js
@@ -761,3 @@
-        let focus = global.stage.get_key_focus();
-        if (focus instanceof Clutter.Text)
-            Shell.util_text_insert_keyval(focus, keyval);

This is the only place the function is used, so remove it?
Comment 34 Carlos Garnacho 2016-08-10 09:28:24 UTC
(In reply to Florian Müllner from comment #27)
> Review of attachment 332158 [details] [review] [review]:
> 
> ::: clutter/clutter/evdev/clutter-virtual-input-device-evdev.c
> @@ +148,3 @@
> +          break;
> +        case EVDEV_BUTTON_TYPE_NONE:
> +          g_assert_not_reached ();
> 
> We are looping over all possible codes, so I'd expect this condition to be
> hit. Should probably filter out codes that aren't pressed above, in which
> case the assert would make sense ...

Right, I added a check for the button being pressed before the switch. Also noticed we disallow multiple presses, but allow multiple releases. I fixed the update_button_count() checks to go both ways, and only emit the first time called.

(In reply to Florian Müllner from comment #32)
> Review of attachment 332162 [details] [review] [review]:
> 
> Eeeks :-)

Agreed :(. This is not too different from what Caribou does for X11 through Xtest (there's some more Xkb state mangling there, so it can do group switching in order to send weird keycodes).

I'm more optimistic wrt wayland, as there may be a protocol that sends something better than evcodes/keycodes (anything, really...). There's one in the works, but still needs fleshing out a bit more...

Anyhow, as per the commit log, I think this will still come out useful when we get there, if we ensure the keysym is preserved as-is in events, and flag those as synthetic, src/wayland/ could divert those to the input method protocol.

(In reply to Florian Müllner from comment #33)
> Review of attachment 332165 [details] [review] [review]:
> 
> OK
> 
> ::: js/ui/keyboard.js
> @@ -761,3 @@
> -        let focus = global.stage.get_key_focus();
> -        if (focus instanceof Clutter.Text)
> -            Shell.util_text_insert_keyval(focus, keyval);
> 
> This is the only place the function is used, so remove it?

Indeed :), handled as a separate commit.

Also fixed the other style issues, giving the last test drive and pushing.
Comment 35 Carlos Garnacho 2016-08-10 09:48:19 UTC
Attachment 332149 [details] pushed as 5db2be4 - clutter: Add virtual input device API
Attachment 332150 [details] pushed as 76eb27e - ClutterDeviceManagerEvdev: Split out seat into a separate file
Attachment 332151 [details] pushed as e38a836 - ClutterVirtualInputDevice: Keep track of the device manager
Attachment 332152 [details] pushed as 94016f7 - clutter/evdev: Move keyboard and pointer notification into seat
Attachment 332153 [details] pushed as 364b184 - ClutterVirtualInputDevice: Store the device type
Attachment 332154 [details] pushed as 8e335ce - ClutterVirtualInputDeviceEvdev: Construct with a specific seat
Attachment 332155 [details] pushed as bd326d3 - ClutterVirtualInputDeviceEvdev: Create associated ClutterInputDevice
Attachment 332156 [details] pushed as d2b05f0 - ClutterVirtualInputDeviceEvdev: Forward motion events
Attachment 332157 [details] pushed as e928370 - ClutterSeatEvdev: Keep track of button count
Attachment 332158 [details] pushed as 0992b5c - ClutterVirtualInputDeviceEvdev: Forward button and key presses
Attachment 332159 [details] pushed as 27a77fa - clutter/evdev: Allow specifying the ClutterInputMode of virtual devices
Attachment 332160 [details] pushed as 3c8b146 - clutter: Make ClutterVirtualInputDevice public
Attachment 332161 [details] pushed as 4abd31d - clutter: Add ClutterVirtualInputDevice vmethod to notify keysyms
Attachment 332162 [details] pushed as b53b94e - clutter/evdev: Implement ClutterVirtualInputDevice::notify_keyval
Attachment 332163 [details] pushed as 1ca57e0 - backends: Prepare for virtual devices
Comment 36 Carlos Garnacho 2016-08-10 09:52:22 UTC
Attachment 332165 [details] pushed as c3e6895 - keyboard: Implement more of the wayland caribou adapter