[Spice-devel] [PATCH spice-gtk 3/3] widget: set keypress-delay to 0 on unix socket

[Marc-André Lureau]

Hi

----- Original Message -----
> Hi
> 
> ----- Original Message -----
> > > 
> > > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > 
> > > There is no strong need for keypress-delay on local connection (not
> > > verified: unless the system is heavily loaded, in which case the VM will
> > > probably be stuck too and may or not repeat the key when running).
> > > 
> > > The benefit of removing keypress-delay is that games or interfaces that
> > > require "realtime" responses, such as FPS, are slightly better without
> > > the 100ms input delay.
> > > 
> > 
> > The same apply to tcp connection so I would just set this to 0,
> > not only for unix sockets.
> > Or perhaps set to a value (like 10?) that causes no problem.
> > Considering that if network is quite busy the kernel should collapse
> > tcp packets I would prefer to set this delay to 0.
> 
> The keypress-delay and SPICE_INPUTS_CAP_SCANCODE were introduced to fix key
> repeat that happen quite easily over the network.
> 
> We use TCP_NODELAY, not sure it's always a good idea then.
> 
> So we can't blindly remove the keypress-delay, and I don't have a better
> solution to avoid spurious key repeats over the network when typing atm.

Just to clarify, keypress-delay is not used to delay all key events, but rather to wait for the next "key-release" event. As soon as it comes, the input event "press-release" is sent.  (see keypress-delay documentation) 

> 
> > 
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > > ---
> > >  src/spice-widget.c | 25 ++++++++++++++++++++++++-
> > >  1 file changed, 24 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > > index 46e0e49..67e66b7 100644
> > > --- a/src/spice-widget.c
> > > +++ b/src/spice-widget.c
> > > @@ -97,6 +97,8 @@ enum {
> > >      SPICE_DISPLAY_LAST_SIGNAL,
> > >  };
> > >  
> > > +#define DEFAULT_KEYPRESS_DELAY 100
> > > +
> > >  static guint signals[SPICE_DISPLAY_LAST_SIGNAL];
> > >  
> > >  #ifdef G_OS_WIN32
> > > @@ -2181,7 +2183,7 @@ static void
> > > spice_display_class_init(SpiceDisplayClass
> > > *klass)
> > >          (gobject_class, PROP_KEYPRESS_DELAY,
> > >           g_param_spec_uint("keypress-delay", "Keypress delay",
> > >                             "Keypress delay",
> > > -                           0, G_MAXUINT, 100,
> > > +                           0, G_MAXUINT, DEFAULT_KEYPRESS_DELAY,
> > >                             G_PARAM_READWRITE |
> > >                             G_PARAM_CONSTRUCT |
> > >                             G_PARAM_STATIC_STRINGS));
> > > @@ -2658,6 +2660,25 @@ static void cursor_reset(SpiceCursorChannel
> > > *channel,
> > > gpointer data)
> > >      gdk_window_set_cursor(window, NULL);
> > >  }
> > >  
> > > +static void inputs_channel_event(SpiceChannel *channel,
> > > SpiceChannelEvent
> > > event,
> > > +                                 gpointer data)
> > > +{
> > > +    SpiceDisplay *display = data;
> > > +    guint delay = DEFAULT_KEYPRESS_DELAY;
> > > +    GSocket *sock;
> > > +
> > > +    if (event != SPICE_CHANNEL_OPENED)
> > > +        return;
> > > +
> > > +    g_object_get(channel, "socket", &sock, NULL);
> > > +    if (g_socket_get_family(sock) == G_SOCKET_FAMILY_UNIX) {
> > > +        delay = 0;
> > > +    }
> > > +    g_object_unref(sock);
> > > +
> > > +    spice_display_set_keypress_delay(display, delay);
> > > +}
> > > +
> > >  #ifndef G_OS_WIN32
> > >  G_GNUC_INTERNAL
> > >  void spice_display_widget_gl_scanout(SpiceDisplay *display)
> > > @@ -2795,6 +2816,8 @@ static void channel_new(SpiceSession *s,
> > > SpiceChannel
> > > *channel, gpointer data)
> > >      if (SPICE_IS_INPUTS_CHANNEL(channel)) {
> > >          d->inputs = SPICE_INPUTS_CHANNEL(channel);
> > >          spice_channel_connect(channel);
> > > +        spice_g_signal_connect_object(channel, "channel-event",
> > > +                                      G_CALLBACK(inputs_channel_event),
> > > display, 0);
> > >          return;
> > >      }
> > >  
> > 
> > Frediano
> > 
>