Re: [spice-gtk v3 7/7] Sync only on focus change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> Hi
> 
> ----- Original Message -----
> > Limit the virtual keystrokes sent to the remote machine.
> > The modifiers are synced only when the application receive or lose
> > the focus. This reduce a lot the possible virtual keystrokes sent
> > to the guest to synchronize the modifiers.
> > This affect the situations where modifiers are configured
> > differently in client and guest.
> > When the application receive the focus the synchronization is
> > attempted from client to guest while when the application lose
> > focus is attempted guest to client (basically is moved following
> > user moving).
> 
> Does this mean that modifiers changes with the application in focus? That
> would be quite irritating imho, and I will likely want an option to disable
> that (actually the default).
> 

Actually the default is trying to copy the modifiers to the VM.
The change affects only VirtViewer, not the entire system.
Let's make an example.
1- you are working on the client with an editor;
2- caps is off;
3- you move to VirtViewer;
4- caps is send to VM as VirtViewer get the focus;
5- you work on the VM;
6- you get back to your editor;
7- caps is synchronized back to client if does not match.

7 can happen only is guest is not behaving as expected as when you
press the caps client should switch the lock, send the caps to VM
which should switch too.

If on 4 VM is not changing the lock and you keep sending if trying
to sync on 5 you'll have problems typing.

With normal (like English) cases 4 is what is done now, on 7 you
have nothing to do and leds are keep in sync unless the VM change
them not in response to a client key (yes, this will lead to LEDs
not synced).

> What I don't understand is what you improving here, saving a few key events
> sent to the guest?
> 

Yes, once per second which can make the VM unusable.

> I don't understand the issue with the current behavior, and the change of
> behavior you propose here. Could you give an example?
> 

You press Caps, VM send back modifiers with no Caps set, you try to fix
sending another Caps, VM send back modifiers with no Caps set ... and so on.

> > 
> > Only work on X11 and Windows, other platforms are unaffected.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> >  src/spice-gtk-session-priv.h |  2 +-
> >  src/spice-gtk-session.c      | 76
> >  ++++++++++++++++++++++++++++++++++++++++++--
> >  src/spice-widget.c           |  5 ++-
> >  3 files changed, 78 insertions(+), 5 deletions(-)
> > 
> > diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-session-priv.h
> > index d7fe313..abf90d6 100644
> > --- a/src/spice-gtk-session-priv.h
> > +++ b/src/spice-gtk-session-priv.h
> > @@ -38,13 +38,13 @@ struct _SpiceGtkSessionClass
> >  void spice_gtk_session_request_auto_usbredir(SpiceGtkSession *self,
> >                                               gboolean state);
> >  gboolean spice_gtk_session_get_read_only(SpiceGtkSession *self);
> > -void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self);
> >  void spice_gtk_session_set_pointer_grabbed(SpiceGtkSession *self, gboolean
> >  grabbed);
> >  gboolean spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self);
> >  void spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession *self,
> >  gboolean keyboard_has_focus);
> >  void spice_gtk_session_set_mouse_has_pointer(SpiceGtkSession *self,
> >  gboolean
> >  mouse_has_pointer);
> >  gboolean spice_gtk_session_get_keyboard_has_focus(SpiceGtkSession *self);
> >  gboolean spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession *self);
> > +void spice_gtk_session_set_focus(SpiceGtkSession *self, gboolean focus);
> >  
> >  G_END_DECLS
> >  
> > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > index d88fed4..61a23fd 100644
> > --- a/src/spice-gtk-session.c
> > +++ b/src/spice-gtk-session.c
> > @@ -67,6 +67,8 @@ struct _SpiceGtkSessionPrivate {
> >      gboolean                keyboard_has_focus;
> >      gboolean                mouse_has_pointer;
> >      gboolean                sync_modifiers;
> > +    gboolean                has_focus;
> > +    guint32                 guest_modifiers;
> >  };
> >  
> >  /**
> > @@ -104,6 +106,7 @@ static void channel_new(SpiceSession *session,
> > SpiceChannel *channel,
> >  static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
> >                              gpointer user_data);
> >  static gboolean read_only(SpiceGtkSession *self);
> > +static void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession
> > *self);
> >  
> >  /* ------------------------------------------------------------------ */
> >  /* gobject glue                                                       */
> > @@ -127,7 +130,7 @@ static void
> > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
> >                                                                    SpiceInputsChannel*
> >                                                                    inputs,
> >                                                                    gboolean
> >                                                                    force)
> >  {
> > -    guint32 guest_modifiers = 0, client_modifiers = 0;
> > +    guint32 guest_modifiers, client_modifiers;
> >  
> >      g_return_if_fail(SPICE_IS_INPUTS_CHANNEL(inputs));
> >  
> > @@ -136,7 +139,20 @@ static void
> > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
> >          return;
> >      }
> >  
> > +    // get and cache client modifiers
> > +    guest_modifiers = G_MAXUINT32;
> >      g_object_get(inputs, "key-modifiers", &guest_modifiers, NULL);
> > +    if (guest_modifiers == G_MAXUINT32) {
> > +        return;
> > +    }
> > +    self->priv->guest_modifiers = guest_modifiers;
> > +
> > +    // if client synchronization is enabled the
> > +    // synchronization is done in spice_gtk_session_set_focus
> > +    if (!force && can_set_keyboard_lock_modifiers()) {
> > +        return;
> > +    }
> > +
> >      client_modifiers = get_keyboard_lock_modifiers();
> >  
> >      if (force || client_modifiers != guest_modifiers) {
> > @@ -146,10 +162,39 @@ static void
> > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
> >      }
> >  }
> >  
> > +static void
> > spice_gtk_session_set_client_keyboard_modifiers(SpiceGtkSession
> > *self)
> > +{
> > +    guint32 guest_modifiers = 0, client_modifiers = 0;
> > +
> > +    if (SPICE_IS_GTK_SESSION(self) && !self->priv->sync_modifiers) {
> > +        SPICE_DEBUG("Syncing modifiers is disabled");
> > +        return;
> > +    }
> > +
> > +    if (!can_set_keyboard_lock_modifiers()) {
> > +        return;
> > +    }
> > +
> > +    guest_modifiers = self->priv->guest_modifiers;
> > +    client_modifiers = get_keyboard_lock_modifiers();
> > +    if (client_modifiers == guest_modifiers) {
> > +        return;
> > +    }
> > +
> > +    set_keyboard_lock_modifiers(guest_modifiers);
> > +}
> > +
> > +/* Keyboard state changed in the client, try to sync
> > + */
> >  static void keymap_modifiers_changed(GdkKeymap *keymap, gpointer data)
> >  {
> >      SpiceGtkSession *self = data;
> >  
> > +    /* client sync, sync done only when focus change */
> > +    if (can_set_keyboard_lock_modifiers()) {
> > +        return;
> > +    }
> > +
> >      spice_gtk_session_sync_keyboard_modifiers(self);
> >  }
> >  
> > @@ -1207,8 +1252,7 @@ void
> > spice_gtk_session_paste_from_guest(SpiceGtkSession
> > *self)
> >      s->clip_hasdata[selection] = FALSE;
> >  }
> >  
> > -G_GNUC_INTERNAL
> > -void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession *self)
> > +static void spice_gtk_session_sync_keyboard_modifiers(SpiceGtkSession
> > *self)
> >  {
> >      GList *l = NULL, *channels =
> >      spice_session_get_channels(self->priv->session);
> >  
> > @@ -1231,6 +1275,32 @@ void
> > spice_gtk_session_set_pointer_grabbed(SpiceGtkSession *self, gboolean grabb
> >  }
> >  
> >  G_GNUC_INTERNAL
> > +void spice_gtk_session_set_focus(SpiceGtkSession *self, gboolean focus)
> > +{
> > +    g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> > +
> > +    /* TODO detect switch between monitors */
> > +
> > +    /* not changed ? */
> > +    if (self->priv->has_focus == focus)
> > +        return;
> > +
> > +    if (focus) {
> > +        /* User switched to the viewer, try to syncronize the keyboard
> > +         * modifiers.
> > +         * This should be called before setting has_focus
> > +         */
> > +        spice_gtk_session_sync_keyboard_modifiers(self);
> > +    } else {
> > +        /* User swicthed to another application, syncronize the client
> > +         * keyboard */
> > +        spice_gtk_session_set_client_keyboard_modifiers(self);
> > +    }
> > +
> > +    self->priv->has_focus = focus;
> > +}
> > +
> > +G_GNUC_INTERNAL
> >  gboolean spice_gtk_session_get_pointer_grabbed(SpiceGtkSession *self)
> >  {
> >      g_return_val_if_fail(SPICE_IS_GTK_SESSION(self), FALSE);
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index 9c8f7d1..8f601e7 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -1745,7 +1745,7 @@ static gboolean focus_in_event(GtkWidget *widget,
> > GdkEventFocus *focus G_GNUC_UN
> >      }
> >  #endif
> >      if (!d->disable_inputs)
> > -        spice_gtk_session_sync_keyboard_modifiers(d->gtk_session);
> > +        spice_gtk_session_set_focus(d->gtk_session, TRUE);
> >      if (d->keyboard_grab_released)
> >          memset(d->activeseq, 0, sizeof(gboolean) * d->grabseq->nkeysyms);
> >      update_keyboard_focus(display, true);
> > @@ -1772,6 +1772,9 @@ static gboolean focus_out_event(GtkWidget *widget,
> > GdkEventFocus *focus G_GNUC_U
> >      if (d->keyboard_grab_active)
> >          return true;
> >  
> > +    if (!d->disable_inputs)
> > +        spice_gtk_session_set_focus(d->gtk_session, FALSE);
> > +
> >      release_keys(display);
> >      update_keyboard_focus(display, false);
> >  
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]