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

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

 



Hi

----- Original Message -----
> 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.
> 

Although silly, in theory the protocol permits it. Who knows how the protocol is being used, we have been surprised in the past (for ex export several VMs in the same session for?)

> 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@xxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > > > 
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > > 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
_______________________________________________
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]