[Spice-devel] [PATCH spice-gtk] widget: Set cursor in constructor

Pavel Grunt pgrunt at redhat.com
Fri Jan 13 14:30:53 UTC 2017


On Fri, 2017-01-13 at 09:17 -0500, Marc-André Lureau wrote:
> Hi
> 
> ----- Original Message -----
> > Hi,
> > 
> > On Fri, 2017-01-13 at 09:01 -0500, Marc-André Lureau wrote:
> > > Hi
> > > 
> > > ----- Original Message -----
> > > > In a multimonitor environment can easily happen that a cursor
> > > > is
> > > > set
> > > > before some of the SpiceDisplays are created. IOW the first
> > > > created
> > > > SpiceDisplay has the cursor but others don't.
> > > > 
> > > > To avoid the issue cache the information about the last set
> > > > cursor
> > > > in
> > > > the SpiceGtkSession and set the cursor shape in the
> > > > SpiceDisplay
> > > > constructor.
> > > > 
> > > > Resolves:
> > > > https://bugzilla.redhat.com/show_bug.cgi?id=1411380
> > > 
> > > Isn't that a server bug? In this situation, each display channel
> > > has
> > > its own independant cursor channel. I don't think we should mix
> > > all
> > > cursor channels in the same session, else what the point in
> > > having
> > > different display/cursor channels? Better be solved at the
> > > server
> > > side imho
> > 
> > Sorry, I forgot to mention that it is for multimonitor linux
> > guests
> > where is 1 display channel.
> 
> Ok, that wasn't clear. Same idea though, better not have a shared
> cursor in session when we actually have display/cursor channel pairs
> by protocol design.
> 
> We may want to have a "cursor" boxed/struct property with associated
> notify on the cursor channel, instead of "cursor-set" signal (to be
> kept for compatibility). Not sure why it wasn't done that way,
> probably for historical reasons (one of the first commits).

ok, originally I wanted to avoid a new api, but it sounds reasonable.
tbh I didn't think about the situation where displays have different
cursors.

Pavel

> 
> 
> > Pavel
> > 
> > > 
> > > > ---
> > > >  src/spice-gtk-session-priv.h |  3 +++
> > > >  src/spice-gtk-session.c      | 43
> > > >  +++++++++++++++++++++++++++++++++++++++++++
> > > >  src/spice-widget.c           | 14 +++++++++++++-
> > > >  3 files changed, 59 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/spice-gtk-session-priv.h b/src/spice-gtk-
> > > > session-
> > > > priv.h
> > > > index d7fe313..454013c 100644
> > > > --- a/src/spice-gtk-session-priv.h
> > > > +++ b/src/spice-gtk-session-priv.h
> > > > @@ -45,6 +45,9 @@ void
> > > > spice_gtk_session_set_keyboard_has_focus(SpiceGtkSession
> > > > *self,
> > > > gboolean ke
> > > >  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);
> > > > +gboolean spice_gtk_session_get_last_cursor(SpiceGtkSession
> > > > *self,
> > > > +                                           gint *width, gint
> > > > *height,
> > > > +                                           gpointer *rgba);
> > > >  
> > > >  G_END_DECLS
> > > >  
> > > > diff --git a/src/spice-gtk-session.c b/src/spice-gtk-session.c
> > > > index a3a2e90..75ee300 100644
> > > > --- a/src/spice-gtk-session.c
> > > > +++ b/src/spice-gtk-session.c
> > > > @@ -47,6 +47,12 @@
> > > >  
> > > >  #define CLIPBOARD_LAST
> > > > (VD_AGENT_CLIPBOARD_SELECTION_SECONDARY +
> > > > 1)
> > > >  
> > > > +typedef struct session_cursor {
> > > > +    gint width;
> > > > +    gint height;
> > > > +    gpointer rgba;
> > > > +} session_cursor;
> > > > +
> > > >  struct _SpiceGtkSessionPrivate {
> > > >      SpiceSession            *session;
> > > >      /* Clipboard related */
> > > > @@ -66,6 +72,7 @@ struct _SpiceGtkSessionPrivate {
> > > >      gboolean                keyboard_has_focus;
> > > >      gboolean                mouse_has_pointer;
> > > >      gboolean                sync_modifiers;
> > > > +    session_cursor          last_cursor;
> > > >  };
> > > >  
> > > >  /**
> > > > @@ -316,6 +323,8 @@ static void
> > > > spice_gtk_session_finalize(GObject
> > > > *gobject)
> > > >          g_clear_pointer(&s->clip_targets[i], g_free);
> > > >      }
> > > >  
> > > > +    g_clear_pointer(&s->last_cursor.rgba, g_free);
> > > > +
> > > >      /* Chain up to the parent class */
> > > >      if (G_OBJECT_CLASS(spice_gtk_session_parent_class)-
> > > > >finalize)
> > > >          G_OBJECT_CLASS(spice_gtk_session_parent_class)-
> > > > > finalize(gobject);
> > > > 
> > > > @@ -1094,6 +1103,21 @@ static void
> > > > clipboard_release(SpiceMainChannel *main,
> > > > guint selection,
> > > >      s->clipboard_by_guest[selection] = FALSE;
> > > >  }
> > > >  
> > > > +static void cursor_set(SpiceCursorChannel *channel,
> > > > +                       gint width, gint height,
> > > > +                       gint hot_x G_GNUC_UNUSED, gint hot_y
> > > > G_GNUC_UNUSED,
> > > > +                       gpointer rgba, gpointer data)
> > > > +{
> > > > +    SpiceGtkSession *self = data;
> > > > +    SpiceGtkSessionPrivate *s = self->priv;
> > > > +    if (s->last_cursor.rgba != NULL) {
> > > > +        g_free(s->last_cursor.rgba);
> > > > +    }
> > > > +    s->last_cursor.width = width;
> > > > +    s->last_cursor.height = height;
> > > > +    s->last_cursor.rgba = g_memdup(rgba, width * height * 4);
> > > > +}
> > > > +
> > > >  static void channel_new(SpiceSession *session, SpiceChannel
> > > > *channel,
> > > >                          gpointer user_data)
> > > >  {
> > > > @@ -1117,6 +1141,11 @@ static void channel_new(SpiceSession
> > > > *session,
> > > > SpiceChannel *channel,
> > > >                                        G_CALLBACK(guest_modifi
> > > > ers_
> > > > changed),
> > > >                                        self, 0);
> > > >          spice_gtk_session_sync_keyboard_modifiers_for_channel
> > > > (sel
> > > > f,
> > > >          SPICE_INPUTS_CHANNEL(channel), TRUE);
> > > >      }
> > > > +    if (SPICE_IS_CURSOR_CHANNEL(channel)) {
> > > > +        spice_g_signal_connect_object(channel, "cursor-set",
> > > > +                                      G_CALLBACK(cursor_set),
> > > > self, 0);
> > > > +        return;
> > > > +    }
> > > >  }
> > > >  
> > > >  static void channel_destroy(SpiceSession *session,
> > > > SpiceChannel
> > > > *channel,
> > > > @@ -1338,3 +1367,17 @@ gboolean
> > > > spice_gtk_session_get_mouse_has_pointer(SpiceGtkSession *self)
> > > >  
> > > >      return self->priv->mouse_has_pointer;
> > > >  }
> > > > +
> > > > +G_GNUC_INTERNAL
> > > > +gboolean spice_gtk_session_get_last_cursor(SpiceGtkSession
> > > > *self,
> > > > +                                           gint *width, gint
> > > > *height,
> > > > +                                           gpointer *rgba)
> > > > +{
> > > > +    g_return_val_if_fail(SPICE_IS_GTK_SESSION(self), FALSE);
> > > > +
> > > > +    *width = self->priv->last_cursor.width;
> > > > +    *height = self->priv->last_cursor.height;
> > > > +    *rgba = self->priv->last_cursor.rgba;
> > > > +
> > > > +    return (*rgba != NULL);
> > > > +}
> > > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > > > index 72fbbc8..d3af7ac 100644
> > > > --- a/src/spice-widget.c
> > > > +++ b/src/spice-widget.c
> > > > @@ -119,6 +119,9 @@ static void release_keys(SpiceDisplay
> > > > *display);
> > > >  static void size_allocate(GtkWidget *widget, GtkAllocation
> > > > *conf,
> > > > gpointer
> > > >  data);
> > > >  static gboolean draw_event(GtkWidget *widget, cairo_t *cr,
> > > > gpointer data);
> > > >  static void update_size_request(SpiceDisplay *display);
> > > > +static void cursor_set(SpiceCursorChannel *channel,
> > > > +                       gint width, gint height, gint hot_x,
> > > > gint
> > > > hot_y,
> > > > +                       gpointer rgba, gpointer data);
> > > >  
> > > >  /* ----------------------------------------------------------
> > > > ----
> > > > -- */
> > > >  
> > > > @@ -689,6 +692,8 @@
> > > > spice_display_constructor(GType                  gtype,
> > > >      SpiceDisplayPrivate *d;
> > > >      GList *list;
> > > >      GList *it;
> > > > +    gint cursor_width, cursor_height;
> > > > +    gpointer cursor_rgba;
> > > >  
> > > >      {
> > > >          /* Always chain up to the parent constructor */
> > > > @@ -724,6 +729,11 @@
> > > > spice_display_constructor(GType                  gtype,
> > > >                                    G_CALLBACK(session_inhibit_
> > > > keyb
> > > > oard_grab_changed),
> > > >                                    display, 0);
> > > >  
> > > > +    if (spice_gtk_session_get_last_cursor(d->gtk_session,
> > > > +                                          &cursor_width,
> > > > &cursor_height,
> > > > &cursor_rgba)) {
> > > > +        cursor_set(NULL, cursor_width, cursor_height, 0, 0,
> > > > cursor_rgba,
> > > > display);
> > > > +    }
> > > > +
> > > >      return obj;
> > > >  }
> > > >  
> > > > @@ -2579,7 +2589,9 @@ static void
> > > > cursor_set(SpiceCursorChannel
> > > > *channel,
> > > >          }
> > > >      }
> > > >  
> > > > -    g_object_unref(d->mouse_cursor);
> > > > +    if (d->mouse_cursor != NULL) {
> > > > +        g_object_unref(d->mouse_cursor);
> > > > +    }
> > > >      d->mouse_cursor = cursor;
> > > >  
> > > >      update_mouse_pointer(display);
> > > > --
> > > > 2.11.0
> > > > 
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > 
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 


More information about the Spice-devel mailing list