Hi, On Tue, Apr 09, 2019 at 08:12:25AM +0000, Victor Toso wrote: > From: Olivier Fourdan <ofourdan@xxxxxxxxxx> > > Spice gtk-session would try to synchronize the modifiers state whenever > the keymap changes, but doing so is inherently racy. > > While the there is a keyboard grab in effect, all key events are > forwarded to the guest, hence all modifiers key press get processed by > the kernel on the guest. > > Trying to synchronize the modifiers will generate additional key press/ > release events which will result in the opposite effect and effectively > desynchronize the modifier states in the guest. > > Synchronizing modifiers from the host should therefore be limited to > focus change, as actual press/release events might have occurred without > the guest knowing. Otherwise, no need to synchronize the modifiers. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > Signed-off-by: Olivier Fourdan <ofourdan@xxxxxxxxxx> > --- > > Merge request spice-gtk!6 or > https://gitlab.freedesktop.org/spice/spice-gtk/merge_requests/6 > > src/spice-gtk-session.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c > index b48f92a..65442b2 100644 > --- a/src/spice-gtk-session.c > +++ b/src/spice-gtk-session.c > @@ -163,6 +163,13 @@ static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data) > { > SpiceGtkSession *self = data; > > + /* set_key_locks() is inherently racy, but no need to sync modifiers > + * if we have focus as the regular keypress/keyrelease will have set > + * the expected modifiers state in the guest. > + */ > + if (self->priv->keyboard_has_focus) > + return; > + Doing a quick test, one can see that on keyboard_focus, before this patch we had (NumLock on Wayland) | spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x2 | spice-widget.c:1646 0:0 key_event press: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: scancode 69 | spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x0, guest_modifiers:0x2 | spice-widget.c:1646 0:0 key_event release: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: release scancode 69 and | spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x0 | spice-widget.c:1646 0:0 key_event press: keycode: 77 state: 0 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: scancode 69 | spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x2 | spice-widget.c:1646 0:0 key_event release: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: release scancode 69 | spice-gtk-session.c:157 inputs-3:0: client_modifiers:0x2, guest_modifiers:0x0 With this patch, becomes: | spice-widget.c:1644 0:0 key_event press: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: scancode 69 | spice-widget.c:1644 0:0 key_event release: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: release scancode 69 and | spice-widget.c:1644 0:0 key_event press: keycode: 77 state: 0 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: scancode 69 | spice-widget.c:1644 0:0 key_event release: keycode: 77 state: 16 group 0 modifier 1 | spice-util.c:270 spice_make_scancode: release scancode 69 So, keypress alone is enough indeed when we have focus. Without focus, this is still needed to keep the modifiers synced. Many thanks for the investigation, explanation and fixing it! Acked-by: Victor Toso <victortoso@xxxxxxxxxx>
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel