[Spice-devel] [spice-gtk EXP] Handle mismatching CapsLock

Frediano Ziglio fziglio at redhat.com
Wed May 4 09:12:57 UTC 2016


> 
> Hi,
> 
> as I said - switch to vt (ctrl+alt+fx), start typing, press capslock, it will
> bump after a while
> 

Pavel... this is based on a known bug in the console handling keyboard leds
and current code reading leds.

> another example - open a text editor - press a key and keep it pressed, press
> capslock (keep pressing it) - sometimes the keys don't change, sometimes it
> changes and in a "second" it switches back...
> 

Not really a common user case.

> Pavel
> 

Agreed, the patch cannot handle all possible cases.
I was thinking about another solution which could work on more cases.
Mostly trying to sync only on the transitions so getting or losing the focus.
However this requires in some cases client settings.

Frediano


> On Tue, 2016-05-03 at 06:16 -0400, Frediano Ziglio wrote:
> > Hi,
> >   I don't follow. Which issues did you find? The patch does not introduce
> >   any
> > delays. Can you do some examples of bumping?
> > 
> > Frediano
> > 
> > > 
> > > Hi Frediano,
> > > 
> > > unfortunately this patch does not work, it was hard for me to write when
> > > pressing and releasing capslock due to delays. Also the "bump" still
> > > exists
> > > (it
> > > is enough to switch to vt to see it).
> > > 
> > > imho the problem is on the server side (server sending capslock to the
> > > guest)
> > > 
> > > Pavel
> > > 
> > > On Fri, 2016-04-22 at 15:43 +0100, Frediano Ziglio wrote:
> > > > Do not sync if we don't have the focus.
> > > > If user press the CapsLock but got a mismatch (guest did not reply as
> > > > expected) do not try to fix the caps lock.
> > > > 
> > > > This patch is really experimental. For instance contains printf
> > > > as debugging. Also I think the keyboard logic would be best moved
> > > > entirely to SpiceGtkSession.
> > > > 
> > > > Despite however style looks to work very well with different keyboard
> > > > layouts and combinations. I would like to have it tested from the
> > > > user experience more that code.
> > > > 
> > > > Tested:
> > > > - reboot turn again CapsLock to on if client is on;
> > > > - English client and English guest;
> > > > - English client and Japanese guest;
> > > > - Japanese client with English guest;
> > > > - Japanese client with Japanese guest.
> > > > 
> > > > The only issue that I found is that when the client is focused
> > > > again the CapsLock is emulated and if guest layout is Japanese
> > > > this could cause input mode to be changed.
> > > > 
> > > > ---
> > > >  src/spice-gtk-session-priv.h |  3 ++-
> > > >  src/spice-gtk-session.c      | 54
> > > >  ++++++++++++++++++++++++++++++++++++++++++-
> > > > -
> > > >  src/spice-widget.c           |  9 +++++++-
> > > >  3 files changed, 62 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/spice-gtk-session-priv.h
> > > > b/src/spice-gtk-session-priv.h
> > > > index b2b6206..d5493ab 100644
> > > > --- a/src/spice-gtk-session-priv.h
> > > > +++ b/src/spice-gtk-session-priv.h
> > > > @@ -25,13 +25,14 @@ G_BEGIN_DECLS
> > > >  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);
> > > > +void spice_gtk_session_capslock_changed(SpiceGtkSession *self);
> > > >  
> > > >  G_END_DECLS
> > > >  
> > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > > index 4201ee0..74c84f7 100644
> > > > --- a/src/spice-gtk-session.c
> > > > +++ b/src/spice-gtk-session.c
> > > > @@ -66,6 +66,10 @@ struct _SpiceGtkSessionPrivate {
> > > >      gboolean                pointer_grabbed;
> > > >      gboolean                keyboard_has_focus;
> > > >      gboolean                mouse_has_pointer;
> > > > +    /** the session has a window with the focus */
> > > > +    gboolean                has_focus;
> > > > +    /** last time a caps lock was pressed */
> > > > +    gint64                  last_capslock_change;
> > > >  };
> > > >  
> > > >  /**
> > > > @@ -103,6 +107,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
> > > >                                                         */
> > > > @@ -178,6 +183,12 @@ static guint32 get_keyboard_lock_modifiers(void)
> > > >      return modifiers;
> > > >  }
> > > >  
> > > > +static gboolean spice_gtk_capslock_is_expected(SpiceGtkSession *self)
> > > > +{
> > > > +    /* expect a change in 3 seconds from press/release */
> > > > +    return g_get_monotonic_time() - self->priv->last_capslock_change <
> > > > 3000000;
> > > > +}
> > > > +
> > > >  static void
> > > > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSession
> > > > *self,
> > > >                                                                    SpiceIn
> > > > puts
> > > > Channel* inputs,
> > > >                                                                    gboolea
> > > > n
> > > > force)
> > > > @@ -189,6 +200,20 @@ static void
> > > > spice_gtk_session_sync_keyboard_modifiers_for_channel(SpiceGtkSessio
> > > >      g_object_get(inputs, "key-modifiers", &guest_modifiers, NULL);
> > > >      client_modifiers = get_keyboard_lock_modifiers();
> > > >  
> > > > +printf("sync force %d client %d guest %d has_focus %d\n",
> > > > +        force, client_modifiers&SPICE_INPUTS_CAPS_LOCK,
> > > > guest_modifiers&SPICE_INPUTS_CAPS_LOCK,
> > > > +        self->priv->has_focus);
> > > > +    /* detect if we should swicth the caps lock */
> > > > +    if (!force) {
> > > > +        if (!self->priv->has_focus)
> > > > +            return;
> > > > +        if (spice_gtk_capslock_is_expected(self)) {
> > > > +printf("not syncing as caps expected but mismatch\n");
> > > > +            client_modifiers = (client_modifiers &
> > > > ~SPICE_INPUTS_CAPS_LOCK)
> > > > +                               | (guest_modifiers &
> > > > SPICE_INPUTS_CAPS_LOCK);
> > > > +        }
> > > > +    }
> > > > +
> > > >      if (force || client_modifiers != guest_modifiers) {
> > > >          CHANNEL_DEBUG(inputs, "client_modifiers:0x%x,
> > > >          guest_modifiers:0x%x",
> > > >                        client_modifiers, guest_modifiers);
> > > > @@ -1199,8 +1224,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);
> > > >  
> > > > @@ -1223,6 +1247,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 */
> > > > +
> > > > +    /* user switched to the viewer, try to syncronize the keyboard
> > > > +     * modifiers */
> > > > +    if (focus)
> > > > +        spice_gtk_session_sync_keyboard_modifiers(self);
> > > > +
> > > > +
> > > > +printf("setting focus %d\n", focus);
> > > > +    self->priv->has_focus = focus;
> > > > +}
> > > > +
> > > > +G_GNUC_INTERNAL
> > > > +void spice_gtk_session_capslock_changed(SpiceGtkSession *self)
> > > > +{
> > > > +    g_return_if_fail(SPICE_IS_GTK_SESSION(self));
> > > > +
> > > > +printf("caps lock\n");
> > > > +    self->priv->last_capslock_change = g_get_monotonic_time();
> > > > +}
> > > > +
> > > > +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 ade3ae4..05aa97d 100644
> > > > --- a/src/spice-widget.c
> > > > +++ b/src/spice-widget.c
> > > > @@ -1465,6 +1465,10 @@ static gboolean key_event(GtkWidget *widget,
> > > > GdkEventKey *key)
> > > >          break;
> > > >      }
> > > >  
> > > > +    /* tell session a capslock was pressed/released */
> > > > +    if (scancode == 58)
> > > > +        spice_gtk_session_capslock_changed(d->gtk_session);
> > > > +
> > > >      return true;
> > > >  }
> > > >  
> > > > @@ -1568,7 +1572,7 @@ static gboolean focus_in_event(GtkWidget *widget,
> > > > GdkEventFocus *focus G_GNUC_UN
> > > >  
> > > >      release_keys(display);
> > > >      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);
> > > > @@ -1595,6 +1599,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);
> > > >  
> > > 
> 


More information about the Spice-devel mailing list