[virt-tools-list] [PATCH 2/2] Set Spice display to fullscreen if owning window is pending fullscreen

[Jonathon Jongsma]

----- Original Message -----
> From: "Marc-André Lureau" <marcandre.lureau at gmail.com>
> To: "Jonathon Jongsma" <jjongsma at redhat.com>
> Cc: "virt" <virt-tools-list at redhat.com>
> Sent: Wednesday, October 9, 2013 3:29:22 PM
> Subject: Re: [virt-tools-list] [PATCH 2/2] Set Spice display to fullscreen if owning window is pending fullscreen
> 
> To avoid introducing a new pending state, we could set
> 
> priv->fullscreen = TRUE;
> 
> before the delayed map-event, and in the handler, set it back to
> FALSE. That really shouldn't be a problem, and since it's a
> special/temporary case, I think that would be simpler.


I'm not quite sure what you mean by "in the handler, set it back to FALSE".  But in general I don't think it will really work to use a single boolean for this case. virt_viewer_enter_fullscreen() actually has an early return at the start of the function:

    if (priv->fullscreen)
        return;

As far as I can tell, this is here because after we enter fullscreen mode, we end up calling gtk_check_menu_item_set_active(check, TRUE), which can result in another call to _enter_fullscreen(). So this protects us from running this handler twice.  I could work around this in a few ways (e.g. blocking that signal handler while we call gtk_check_menu_item_set_active(), or turning priv->fullscreen into a tri-state variable rather than a boolean), but neither of those seemed obviously better to me than simply adding a new priv->pending_fullscreen state.

Jonathon



> On Wed, Oct 9, 2013 at 10:09 PM, Jonathon Jongsma <jjongsma at redhat.com>
> wrote:
> > When you call virt_viewer_window_enter_fullscreen() on a hidden window, it
> > doesn't actually change its fullscreen state.  Instead, it sets up a
> > map-event
> > handler to enter fullscreen after it is shown. When _set_display() is
> > called on
> > a window that is pending fullscreen status, it initially sets the
> > fullscreen
> > state of the display to FALSE, which can cause an unwanted resize to be
> > sent
> > down to the guest.
> > ---
> >  src/virt-viewer-window.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > index 0f62feb..7108aa0 100644
> > --- a/src/virt-viewer-window.c
> > +++ b/src/virt-viewer-window.c
> > @@ -96,6 +96,7 @@ struct _VirtViewerWindowPrivate {
> >      GSList *accel_list;
> >      gboolean enable_mnemonics_save;
> >      gboolean grabbed;
> > +    gboolean fullscreen_pending;
> >      gint fullscreen_monitor;
> >      gboolean desktop_resize_pending;
> >      gboolean kiosk;
> > @@ -294,6 +295,7 @@ virt_viewer_window_init (VirtViewerWindow *self)
> >      self->priv = GET_PRIVATE(self);
> >      priv = self->priv;
> >
> > +    priv->fullscreen_pending = FALSE;
> >      priv->fullscreen_monitor = -1;
> >      priv->auto_resize = TRUE;
> >      g_value_init(&priv->accel_setting, G_TYPE_STRING);
> > @@ -533,11 +535,13 @@ virt_viewer_window_enter_fullscreen(VirtViewerWindow
> > *self, gint monitor)
> >      priv->fullscreen_monitor = monitor;
> >
> >      if (!gtk_widget_get_mapped(priv->window)) {
> > +        priv->fullscreen_pending = TRUE;
> >          g_signal_connect(priv->window, "map-event", G_CALLBACK(mapped),
> >          self);
> >          return;
> >      }
> >
> >      priv->fullscreen = TRUE;
> > +    priv->fullscreen_pending = FALSE;
> >
> >      gtk_check_menu_item_set_active(check, TRUE);
> >      gtk_widget_hide(menu);
> > @@ -1232,7 +1236,7 @@ virt_viewer_window_set_display(VirtViewerWindow
> > *self, VirtViewerDisplay *displa
> >          virt_viewer_display_set_zoom_level(VIRT_VIEWER_DISPLAY(priv->display),
> >          priv->zoomlevel);
> >          virt_viewer_display_set_auto_resize(VIRT_VIEWER_DISPLAY(priv->display),
> >          priv->auto_resize);
> >          virt_viewer_display_set_monitor(VIRT_VIEWER_DISPLAY(priv->display),
> >          priv->fullscreen_monitor);
> > -
> > virt_viewer_display_set_fullscreen(VIRT_VIEWER_DISPLAY(priv->display),
> > priv->fullscreen);
> > +
> > virt_viewer_display_set_fullscreen(VIRT_VIEWER_DISPLAY(priv->display),
> > priv->fullscreen_pending || priv->fullscreen);
> >
> >          gtk_widget_show_all(GTK_WIDGET(display));
> >          gtk_notebook_append_page(GTK_NOTEBOOK(priv->notebook),
> >          GTK_WIDGET(display), NULL);
> > --
> > 1.8.3.1
> >
> > _______________________________________________
> > virt-tools-list mailing list
> > virt-tools-list at redhat.com
> > https://www.redhat.com/mailman/listinfo/virt-tools-list
> 
> 
> 
> --
> Marc-André Lureau
>