[Spice-devel] [PATCH spice-gtk v2 3/3] widget: Set cursor during construction

[Pavel Grunt]

On Wed, 2017-05-24 at 09:22 +0200, Victor Toso wrote:
> On Tue, May 23, 2017 at 04:56:26PM +0200, Pavel Grunt wrote:
> > 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 set the initial cursor using the "cursor"
> > property
> > of the SpiceCursorChannel.
> > 
> > Resolves:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1411380
> > ---
> > v2: changed to "early return"
> > ---
> >  src/spice-widget.c | 36 +++++++++++++++++++-----------------
> >  1 file changed, 19 insertions(+), 17 deletions(-)
> > 
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index b1c8ab1..a117484 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -2646,23 +2646,24 @@ static void cursor_set(SpiceCursorChannel
> > *channel,
> >      cursor_invalidate(display);
> > 
> >      g_object_get(G_OBJECT(channel), "cursor", &cursor_shape,
> > NULL);
> > -    if (cursor_shape != NULL && cursor_shape->data != NULL) {
> > -        g_clear_object(&d->mouse_pixbuf);
> > -        d->mouse_pixbuf = gdk_pixbuf_new_from_data(cursor_shape-
> > >data,
> > -                                                   GDK_COLORSPACE
> > _RGB,
> > -                                                   TRUE, 8,
> > -                                                   cursor_shape-
> > >width,
> > -                                                   cursor_shape-
> > >height,
> > -                                                   cursor_shape-
> > >width * 4,
> > -                                                   NULL, NULL);
> > -        d->mouse_hotspot.x = cursor_shape->hot_spot_x;
> > -        d->mouse_hotspot.y = cursor_shape->hot_spot_y;
> > -        cursor =
> > gdk_cursor_new_from_pixbuf(gtk_widget_get_display(GTK_WIDGET(displ
> > ay)),
> > -                                            d->mouse_pixbuf,
> > -                                            d->mouse_hotspot.x,
> > -                                            d->mouse_hotspot.y);
> > -    } else
> > -        g_warn_if_reached();
> > +    if (G_UNLIKELY(cursor_shape == NULL || cursor_shape->data ==
> > NULL)) {
> > +        return;
> > +    }
> 
> Is it unlikely? Do we always receive cursor->data?
It is unlikely, i've never seen this warning. I change it to return,
because it is now called in the widget construction. I can change it
back if it is preferred.

> 
> Before it was g_warn_if_reached() which means we would very likely
> be
> calling update_mouse_pointer() and cursor_invalidate(). Not calling
> this functions now is part of the solution to rhbz#1411380? If so, I
> would mention it in the commit log too.
> 
> Acked-by: Victor Toso <victortoso at redhat.com>
> 
> > +
> > +    g_clear_object(&d->mouse_pixbuf);
> > +    d->mouse_pixbuf = gdk_pixbuf_new_from_data(cursor_shape-
> > >data,
> > +                                               GDK_COLORSPACE_RGB
> > ,
> > +                                               TRUE, 8,
> > +                                               cursor_shape-
> > >width,
> > +                                               cursor_shape-
> > >height,
> > +                                               cursor_shape-
> > >width * 4,
> > +                                               NULL, NULL);
> > +    d->mouse_hotspot.x = cursor_shape->hot_spot_x;
> > +    d->mouse_hotspot.y = cursor_shape->hot_spot_y;
> > +    cursor =
> > gdk_cursor_new_from_pixbuf(gtk_widget_get_display(GTK_WIDGET(displ
> > ay)),
> > +                                        d->mouse_pixbuf,
> > +                                        d->mouse_hotspot.x,
> > +                                        d->mouse_hotspot.y);
> >  s
> >  #if HAVE_EGL
> >      if (egl_enabled(d))
> > @@ -2970,6 +2971,7 @@ static void channel_new(SpiceSession *s,
> > SpiceChannel *channel, gpointer data)
> >          spice_g_signal_connect_object(channel, "cursor-reset",
> >                                        G_CALLBACK(cursor_reset),
> > display, 0);
> >          spice_channel_connect(channel);
> > +        cursor_set(d->cursor, NULL, display);
> >          return;
> >      }
> >  
> > -- 
> > 2.13.0
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel