On Wed, May 16, 2018 at 10:43:20AM +0200, Christophe Fergeau wrote:
> On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote:
> > > > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > > > index 73db593..251b29c 100644
> > > > --- a/src/spice-widget.c
> > > > +++ b/src/spice-widget.c
> > > > +static void* prepare_streaming_mode(SpiceChannel *channel, bool streaming_mode, gpointer data)
> > > > +{
> > > > +#ifdef GDK_WINDOWING_X11
> > > > + SpiceDisplay *display = data;
> > > > + SpiceDisplayPrivate *d = display->priv;
> > > > +
> > > > + /* GstVideoOverlay will currently be used only under x */
> > > > + if (!g_getenv("DISABLE_GSTVIDEOOVERLAY") &&
> > > > + streaming_mode &&
> > > > + GDK_IS_X11_DISPLAY(gdk_display_get_default())) {
> > > > + GdkWindow *window;
> > > > +
> > > > + window = gtk_widget_get_window(GTK_WIDGET(display));
> > > > + if (window) {
> > > > +#if GTK_CHECK_VERSION(2,18,0)
> > > > + if (gdk_window_ensure_native (window))
> > > > +#endif
> > > > + {
> > > > + gtk_stack_set_visible_child_name(d->stack, "gst-area");
> > > > + return (void*)GDK_WINDOW_XID(window);
> > > > + }
> > > > + }
> > > > + }
> > > > +#endif
> > > > + return NULL;
> > > > +}
> > > > +
> > > > static void invalidate(SpiceChannel *channel,
> > > > gint x, gint y, gint w, gint h, gpointer data)
> > > > {
> > > > @@ -2962,6 +2993,8 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, gpointer data)
> > > > spice_g_signal_connect_object(channel, "notify::monitors",
> > > > G_CALLBACK(spice_display_widget_update_monitor_area),
> > > > display, G_CONNECT_AFTER | G_CONNECT_SWAPPED);
> > > > + spice_g_signal_connect_object(channel, "streaming-mode",
> > > > + G_CALLBACK(prepare_streaming_mode), display, G_CONNECT_AFTER);
> > > Rather than having this signal which is used by ChannelDisplay to call a
> > > method on a GtkWidget it should not have any access to, have you
> > > considered adding code to spice-widget.c::realize() and
> > > spice-widget.c::unrealize() which would call
> > > channel_display_set_window_handle(display->priv->display, handle);
> > > ?
> >
> > Sound a bit similar to a previous version I've tried, iiuc your
> > suggestion is to make widget call a function in channel_display
> > to set the handle (are we doing something like that already?).
>
> Yes, indeed, this is similar to what you were doing in earlier
> iterations.
>
> > I personally really like the current flow of the request for handle
> > using the signal and getting it as a response, avoiding of setting
> > and getting an handle from different components.
>
> Using signals as some generic way of calling a getter on some unknown
> class is rather unusual, and feels like something you should not really
> be using signals for.
Just realized, this change is most likely an ABI break:
diff --git a/src/channel-display.h b/src/channel-display.h
index 5b48d2ff..9c51aa2a 100644
--- a/src/channel-display.h
+++ b/src/channel-display.h
@@ -140,6 +141,8 @@ struct _SpiceDisplayChannelClass {
gint x, gint y, gint w, gint h);
void (*display_mark)(SpiceChannel *channel,
gboolean mark);
+ void (*streaming_mode)(SpiceChannel *channel,
+ gboolean streaming_mode);
/*< private >*/
};
However, I think you don't need to add a field to the class in order to
define a signal, so this added field can probably be removed somehow.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel