Hi, On Fri, May 12, 2017 at 01:00:00PM +0200, Pavel Grunt wrote: > It provides information about the remote cursor similar to the signal > "cursor-set". The benefit over the signal is that the property can be > queried at any time. > > Users of the "cursor-set" signal should migrate to "notify::cursor". > > Related: > https://bugzilla.redhat.com/show_bug.cgi?id=1411380 > --- > doc/reference/spice-gtk-sections.txt | 3 ++ > src/channel-cursor.c | 99 +++++++++++++++++++++++++++++++++++- > src/channel-cursor.h | 25 +++++++++ > src/map-file | 1 + > src/spice-glib-sym-file | 1 + > src/spice-widget.c | 31 ++++++----- > 6 files changed, 144 insertions(+), 16 deletions(-) > > diff --git a/doc/reference/spice-gtk-sections.txt b/doc/reference/spice-gtk-sections.txt > index 6f49df3..82c930e 100644 > --- a/doc/reference/spice-gtk-sections.txt > +++ b/doc/reference/spice-gtk-sections.txt > @@ -180,6 +180,8 @@ SPICE_IS_DISPLAY_CHANNEL_CLASS > SPICE_DISPLAY_CHANNEL_GET_CLASS > SPICE_TYPE_GL_SCANOUT > spice_gl_scanout_get_type > +SPICE_TYPE_CURSOR_SHAPE > +spice_cursor_shape_get_type > <SUBSECTION Private> > SpiceDisplayChannelPrivate > </SECTION> > @@ -189,6 +191,7 @@ SpiceDisplayChannelPrivate > <TITLE>SpiceCursorChannel</TITLE> > SpiceCursorChannel > SpiceCursorChannelClass > +SpiceCursorShape > <SUBSECTION Standard> > SPICE_CURSOR_CHANNEL > SPICE_IS_CURSOR_CHANNEL > diff --git a/src/channel-cursor.c b/src/channel-cursor.c > index 609243b..25ba366 100644 > --- a/src/channel-cursor.c > +++ b/src/channel-cursor.c > @@ -36,8 +36,9 @@ > * The Spice protocol defines a set of messages for controlling cursor > * shape and position on the remote display area. The cursor changes > * that should be reflected on the display are notified by > - * signals. See for example #SpiceCursorChannel::cursor-set > - * #SpiceCursorChannel::cursor-move signals. > + * signals. See for example #SpiceCursorChannel::cursor-set and > + * #SpiceCursorChannel::cursor-move signals and the #SpiceCursorChannel:cursor > + * property. > */ > > #define SPICE_CURSOR_CHANNEL_GET_PRIVATE(obj) \ > @@ -55,6 +56,13 @@ struct display_cursor { > struct _SpiceCursorChannelPrivate { > display_cache *cursors; > gboolean init_done; > + SpiceCursorShape last_cursor; > +}; > + > +/* Properties */ > +enum { > + PROP_0, > + PROP_CURSOR, > }; > > enum { > @@ -74,7 +82,30 @@ static void channel_set_handlers(SpiceChannelClass *klass); > > G_DEFINE_TYPE(SpiceCursorChannel, spice_cursor_channel, SPICE_TYPE_CHANNEL) > > +static SpiceCursorShape *spice_cursor_shape_copy(const SpiceCursorShape *cursor); > +static void spice_cursor_shape_free(SpiceCursorShape *cursor); > + > +G_DEFINE_BOXED_TYPE(SpiceCursorShape, spice_cursor_shape, > + (GBoxedCopyFunc)spice_cursor_shape_copy, > + (GBoxedFreeFunc)spice_cursor_shape_free) > + > /* ------------------------------------------------------------------ */ > +static SpiceCursorShape *spice_cursor_shape_copy(const SpiceCursorShape *cursor) > +{ g_return_val_if_fail(cursor != NULL, NULL); > + SpiceCursorShape *new_cursor = g_new(SpiceCursorShape, 1); > + *new_cursor = *cursor; > + new_cursor->data = g_memdup(cursor->data, cursor->width * cursor->height * 4); > + > + return new_cursor; > +} > + > +static void spice_cursor_shape_free(SpiceCursorShape *cursor) > +{ > + g_return_if_fail(cursor != NULL); > + > + g_free(cursor->data); > + g_free(cursor); > +} > > static void spice_cursor_channel_init(SpiceCursorChannel *channel) > { > @@ -107,15 +138,60 @@ static void spice_cursor_channel_reset(SpiceChannel *channel, gboolean migrating > SPICE_CHANNEL_CLASS(spice_cursor_channel_parent_class)->channel_reset(channel, migrating); > } > > +static void spice_cursor_channel_dispose(GObject *object) > +{ > + SpiceCursorChannelPrivate *c = SPICE_CURSOR_CHANNEL(object)->priv; > + > + g_clear_pointer(&c->last_cursor.data, g_free); Out of curiosity, putting on _finalize was not enough? > + > + if (G_OBJECT_CLASS(spice_cursor_channel_parent_class)->dispose) > + G_OBJECT_CLASS(spice_cursor_channel_parent_class)->dispose(object); > +} > + > +static void spice_cursor_channel_get_property(GObject *object, > + guint prop_id, > + GValue *value, > + GParamSpec *pspec) > +{ > + SpiceCursorChannel *channel = SPICE_CURSOR_CHANNEL(object); > + SpiceCursorChannelPrivate *c = channel->priv; > + > + switch (prop_id) { > + case PROP_CURSOR: > + g_value_set_static_boxed(value, c->last_cursor.data ? &c->last_cursor : NULL); > + break; > + default: > + G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec); > + break; > + } > +} > + > static void spice_cursor_channel_class_init(SpiceCursorChannelClass *klass) > { > GObjectClass *gobject_class = G_OBJECT_CLASS(klass); > SpiceChannelClass *channel_class = SPICE_CHANNEL_CLASS(klass); > > + gobject_class->dispose = spice_cursor_channel_dispose; > gobject_class->finalize = spice_cursor_channel_finalize; > + gobject_class->get_property = spice_cursor_channel_get_property; > channel_class->channel_reset = spice_cursor_channel_reset; > > /** > + * SpiceCursorChannel:cursor: > + * > + * The last #SpiceCursorShape received. > + * > + * Since: 0.34 > + */ > + g_object_class_install_property > + (gobject_class, PROP_CURSOR, > + g_param_spec_boxed("cursor", > + "Last cursor shape", > + "Last cursor shape received from the server", > + SPICE_TYPE_CURSOR_SHAPE, > + G_PARAM_READABLE | > + G_PARAM_STATIC_STRINGS)); > + /** > * SpiceCursorChannel::cursor-set: > * @cursor: the #SpiceCursorChannel that emitted the signal > * @width: width of the shape > @@ -128,6 +204,8 @@ static void spice_cursor_channel_class_init(SpiceCursorChannelClass *klass) > * > * The #SpiceCursorChannel::cursor-set signal is emitted to modify > * cursor aspect and position on the display area. > + * > + * Deprecated: 0.34: Use #SpiceCursorChannel:cursor notify instead. > **/ > signals[SPICE_CURSOR_SET] = > g_signal_new("cursor-set", > @@ -399,7 +477,24 @@ cache_add: > /* coroutine context */ > static void emit_cursor_set(SpiceChannel *channel, display_cursor *cursor) > { > + SpiceCursorChannelPrivate *c; > + > g_return_if_fail(cursor != NULL); > + > + c = SPICE_CURSOR_CHANNEL(channel)->priv; > + Remove trailing space > + c->last_cursor.type = cursor->hdr.type; > + c->last_cursor.width = cursor->hdr.width; > + c->last_cursor.height = cursor->hdr.height; > + c->last_cursor.hot_spot_x = cursor->hdr.hot_spot_x; > + c->last_cursor.hot_spot_y = cursor->hdr.hot_spot_y; > + g_clear_pointer(&c->last_cursor.data, g_free); > + if (cursor->data != NULL) { > + c->last_cursor.data = g_memdup(cursor->data, > + cursor->hdr.width * cursor->hdr.height * 4); > + } Documentation says that g_memdup() will return NULL if cursor->data is null, so you can remove the if(). (Looking at the g_memdup() code, it does will not warn/critical so removing if() is fine) > + > + g_coroutine_object_notify(G_OBJECT(channel), "cursor"); > g_coroutine_signal_emit(channel, signals[SPICE_CURSOR_SET], 0, > cursor->hdr.width, cursor->hdr.height, > cursor->hdr.hot_spot_x, cursor->hdr.hot_spot_y, > diff --git a/src/channel-cursor.h b/src/channel-cursor.h > index 20f0380..a90c739 100644 > --- a/src/channel-cursor.h > +++ b/src/channel-cursor.h > @@ -37,6 +37,29 @@ typedef struct _SpiceCursorChannel SpiceCursorChannel; > typedef struct _SpiceCursorChannelClass SpiceCursorChannelClass; > typedef struct _SpiceCursorChannelPrivate SpiceCursorChannelPrivate; > > +#define SPICE_TYPE_CURSOR_SHAPE (spice_cursor_shape_get_type ()) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ Extra space! To be honest, that's pretty common with _get_type() -> grep -rniI "spice_.*get_type (" src/ | wc -l ==> 22 -> grep -rniI "spice_.*get_type(" src/ | wc -l ==> 57 But without space wins (besides or coding style) > +/** > + * SpiceCursorShape: > + * @type: a #SpiceCursorType of @data > + * @width: a width of the remote cursor > + * @height: a height of the remote cursor > + * @hot_spot_x: a 'x' coordinate of the remote cursor > + * @hot_spot_y: a 'y' coordinate of the remote cursor > + * @data: image data of the remote cursor > + * > + * The #SpiceCursorShape structure defines the remote cursor's shape. > + * > + */ > +typedef struct _SpiceCursorShape SpiceCursorShape; > +struct _SpiceCursorShape { > + SpiceCursorType type; > + guint16 width; > + guint16 height; > + guint16 hot_spot_x; > + guint16 hot_spot_y; > + gpointer data; > +}; > + > /** > * SpiceCursorChannel: > * > @@ -76,6 +99,8 @@ struct _SpiceCursorChannelClass { > > GType spice_cursor_channel_get_type(void); > > +GType spice_cursor_shape_get_type(void) G_GNUC_CONST; > + > G_END_DECLS > > #endif /* __SPICE_CLIENT_CURSOR_CHANNEL_H__ */ > diff --git a/src/map-file b/src/map-file > index 31cafc2..668ff41 100644 > --- a/src/map-file > +++ b/src/map-file > @@ -20,6 +20,7 @@ spice_channel_test_common_capability; > spice_channel_type_to_string; > spice_client_error_quark; > spice_cursor_channel_get_type; > +spice_cursor_shape_get_type; > spice_display_change_preferred_compression; > spice_display_change_preferred_video_codec_type; > spice_display_channel_get_type; > diff --git a/src/spice-glib-sym-file b/src/spice-glib-sym-file > index d73f799..e061744 100644 > --- a/src/spice-glib-sym-file > +++ b/src/spice-glib-sym-file > @@ -18,6 +18,7 @@ spice_channel_test_common_capability > spice_channel_type_to_string > spice_client_error_quark > spice_cursor_channel_get_type > +spice_cursor_shape_get_type > spice_display_change_preferred_compression > spice_display_change_preferred_video_codec_type > spice_display_channel_get_type > diff --git a/src/spice-widget.c b/src/spice-widget.c > index 8203d55..b1c8ab1 100644 > --- a/src/spice-widget.c > +++ b/src/spice-widget.c > @@ -2635,29 +2635,32 @@ static void mark(SpiceDisplay *display, gint mark) > } > > static void cursor_set(SpiceCursorChannel *channel, > - gint width, gint height, gint hot_x, gint hot_y, > - gpointer rgba, gpointer data) > + G_GNUC_UNUSED GParamSpec *pspec, > + gpointer data) This changes is to move the callback from cursor-set signal to cursor property. I'd move it to a follow up patch. > { > SpiceDisplay *display = data; > SpiceDisplayPrivate *d = display->priv; > GdkCursor *cursor = NULL; > + SpiceCursorShape *cursor_shape = NULL; Don't assign NULL here. > > cursor_invalidate(display); > > - g_clear_object(&d->mouse_pixbuf); > - > - if (rgba != NULL) { > - d->mouse_pixbuf = gdk_pixbuf_new_from_data(g_memdup(rgba, width * height * 4), > + 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, > - width, > - height, > - width * 4, > - (GdkPixbufDestroyNotify)g_free, NULL); > - d->mouse_hotspot.x = hot_x; > - d->mouse_hotspot.y = hot_y; > + 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(display)), > - d->mouse_pixbuf, hot_x, hot_y); > + d->mouse_pixbuf, > + d->mouse_hotspot.x, > + d->mouse_hotspot.y); > } else > g_warn_if_reached(); It would make sense with the property callback to make this an early return in case cursor_shape is NULL. (/me is almost always looking forward to early returns and less code indentation) > > @@ -2958,7 +2961,7 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data) > if (id != d->channel_id) > return; > d->cursor = SPICE_CURSOR_CHANNEL(channel); > - spice_g_signal_connect_object(channel, "cursor-set", > + spice_g_signal_connect_object(channel, "notify::cursor", > G_CALLBACK(cursor_set), display, 0); Cheers, toso > spice_g_signal_connect_object(channel, "cursor-move", > G_CALLBACK(cursor_move), display, 0); > -- > 2.12.2 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel