[Spice-devel] [PATCH spice-gtk 4/4] widget: display a message about required EGL support

[Marc-André Lureau]

On Sat, Jan 5, 2019 at 1:07 PM Victor Toso <victortoso at redhat.com> wrote:
>
> Hi,
>
> On Thu, Jan 03, 2019 at 09:13:26PM +0400, marcandre.lureau at redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau at redhat.com>
> >
> > Add a new "label" stack page. When EGL is unsupported and a GL scanout
> > message is received, switch to that page with an appropriate message
> > hint.
> >
> > At this point, no further switch back to 2d drawing is implemented,
> > although it is theorically possible for a server to switch between GL
> > and 2D, I don't think we need to bother with this corner case.
> >
> > Alternative implementation also considered:
> >
> > - a dialog: problematic, the widget may not have an associated
> >   GtkWindow parent, not very user friendly (one time dialog).
> >
> > - an info-bar: does not fit well inside display widget, may not be
> >   well integrated with client application. Can usually be discarded.
> >
> > - an error signal: would need to be implemented by various client, but
> >   could give more flexibility on how to present various errors.  Could
> >   be added later, and replace the "label" page used here by
> >   default (if a signal handler is detected).
> >
> > - a property: may be more appropriate, since the error should
> >   stick. Although it would more complicated to deal with multiple
> >   errors in that case.
> >
> > The current solution is not perfect, but should be a decent default,
> > not requiring client modification. It can be later refined by adding a
> > way for the client to override the presentation of the message via
> > signal handlers.
>
> Ok
>
> > Fixes:
> > https://gitlab.freedesktop.org/spice/spice-gtk/issues/69
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at redhat.com>
> > ---
> >  src/spice-widget-priv.h |  1 +
> >  src/spice-widget.c      | 21 +++++++++++++++++++--
> >  2 files changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/spice-widget-priv.h b/src/spice-widget-priv.h
> > index 0264577..54fe177 100644
> > --- a/src/spice-widget-priv.h
> > +++ b/src/spice-widget-priv.h
> > @@ -61,6 +61,7 @@ struct _SpiceDisplayClass {
> >
> >  struct _SpiceDisplayPrivate {
> >      GtkStack                *stack;
> > +    GtkWidget               *label;
> >      gint                    channel_id;
> >      gint                    monitor_id;
> >
> > diff --git a/src/spice-widget.c b/src/spice-widget.c
> > index 3ff5be9..d35f457 100644
> > --- a/src/spice-widget.c
> > +++ b/src/spice-widget.c
> > @@ -20,6 +20,7 @@
> >  #include <math.h>
> >  #include <glib.h>
> >  #include <gdk/gdk.h>
> > +#include <glib/gi18n-lib.h>
> >
> >  #ifdef HAVE_X11_XKBLIB_H
> >  #include <X11/XKBlib.h>
> > @@ -251,7 +252,9 @@ static void update_ready(SpiceDisplay *display)
> >      SpiceDisplayPrivate *d = display->priv;
> >      gboolean ready = FALSE;
> >
> > -    if (d->monitor_ready) {
> > +    if (gtk_stack_get_visible_child(d->stack) == d->label) {
> > +        ready = true;
> > +    }if (d->monitor_ready) {
>
> \n

yes

>
> >          ready = egl_enabled(d) || d->mark != 0;
> >      }
> >      /* If the 'resize-guest' property is set, the application expects spice-gtk
> > @@ -661,6 +664,9 @@ static void spice_display_init(SpiceDisplay *display)
> >                       "signal::size-allocate", gst_size_allocate, display,
> >                       NULL);
> >  #endif
> > +    d->label = gtk_label_new(NULL);
> > +    gtk_label_set_selectable(GTK_LABEL(d->label), true);
> > +    gtk_stack_add_named(d->stack, d->label, "label");
> >
> >      gtk_widget_show_all(widget);
> >
> > @@ -2988,6 +2994,17 @@ static void gl_draw(SpiceDisplay *display,
> >          spice_display_channel_gl_draw_done(d->display);
> >      }
> >  }
> > +#else
> > +static void spice_display_widget_gl_scanout(SpiceDisplay *display)
> > +{
> > +    SpiceDisplayPrivate *d = display->priv;
> > +
> > +    DISPLAY_DEBUG(display, "%s",  __FUNCTION__);
>
> I would add something more in the debug above as we have two
> possible spice_display_widget_gl_scanout() paths

I added !EGL

> > +    gtk_label_set_label(GTK_LABEL(d->label), _("spice-gtk: The remote requires EGL support."));
> > +    gtk_stack_set_visible_child(d->stack, d->label);
> > +    update_ready(display);
> > +    spice_display_channel_gl_draw_done(d->display);
> > +}
> >  #endif
> >
> >  static void channel_new(SpiceSession *s, SpiceChannel *channel, SpiceDisplay *display)
> > @@ -3028,10 +3045,10 @@ static void channel_new(SpiceSession *s, SpiceChannel *channel, SpiceDisplay *di
> >              mark(display, primary.marked);
> >          }
> >
> > -#if HAVE_EGL
> >          spice_g_signal_connect_object(channel, "notify::gl-scanout",
> >                                        G_CALLBACK(spice_display_widget_gl_scanout),
> >                                        display, G_CONNECT_SWAPPED);
> > +#if HAVE_EGL
> >          spice_g_signal_connect_object(channel, "gl-draw",
> >                                        G_CALLBACK(gl_draw), display, G_CONNECT_SWAPPED);
> >  #endif
> > --
> > 2.20.1.2.gb21ebb671b
> >
> > _______________________________________________
> > 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

thanks

-- 
Marc-André Lureau