[virt-tools-list] [PATCH virt-viewer v6] session-spice: Disable extra displays in fullscreen mode

[Pavel Grunt]

On Fri, 2015-10-16 at 12:08 -0500, Jonathon Jongsma wrote:
> On Fri, 2015-10-16 at 17:20 +0200, Pavel Grunt wrote:
> > When running in fullscreen it is possible to end up in a situation
> > where we have more displays enabled than monitors. This can happen
> > if displays that were enabled in the previous connection to the guest
> > doesn't match displays requested when entering the fullscreen mode.
> > 
> > This commit solves the problem by disabling displays that should not
> > enabled in the fullscreen mode.
> 
> missing word here: "should not *be* enabled"

thanks, fixed
> 
> 
> > 
> > Resolves: rhbz#1212802
> > ---
> > v1: https://www.redhat.com/archives/virt-tools-list/2015-April/msg001
> > 84.html
> > v2: - nth is not used to determine which display should be enabled,
> >       but the list of fullscreen displays is used
> >     - the extra display is disabled instead of being ignored
> > v3: - added missing check for self->priv->did_auto_conf to fix hidden
> > display
> >       when --reconnect
> > v4: - due to PATCH 2/3 it is possible to use
> > virt_viewer_app_get_initial_displays()
> >       to get fullscreen mode displays
> > v5: - fixed the leak of virt_viewer_app_get_initial_displays()
> >     - https://www.redhat.com/archives/virt-tools-list/2015-May/msg000
> > 14.html
> > v6: - rebased
> >     - changed debug to warning
> >     - due to recent changes
> > (aff6c79ae080db286e4cb853cdfa02f2da0d0398, thanks Jonathon)
> >       other patches from the series are no longer needed 
> > ---
> >  src/virt-viewer-session-spice.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer
> > -session-spice.c
> > index eb0761d..a25e600 100644
> > --- a/src/virt-viewer-session-spice.c
> > +++ b/src/virt-viewer-session-spice.c
> > @@ -838,6 +838,19 @@ destroy_display(gpointer data)
> >      g_object_unref(display);
> >  }
> >  
> > +static gboolean
> > +display_is_in_fullscreen_mode(VirtViewerSessionSpice *self,
> > +                              VirtViewerDisplay *display)
> > +{
> > +    gconstpointer nth =
> > GINT_TO_POINTER(virt_viewer_display_get_nth(display));
> > +    VirtViewerApp *app =
> > virt_viewer_session_get_app(VIRT_VIEWER_SESSION(self));
> > +    GList *fullscreen_displays =
> > virt_viewer_app_get_initial_displays(app);
> > +    gboolean is_fullscreen = g_list_index(fullscreen_displays, nth)
> > != -1;
> > +
> > +    g_list_free(fullscreen_displays);
> > +    return is_fullscreen;
> > +}
> 
> Couldn't we simply use this?
> 
> return (virt_viewer_app_get_initial_monitor_for_display(app, nth) != 
> -1);
> 
> That way we don't have to allocate a new GList and then free it.

Unfortunately we cannot use it now. It would only work when the user monitor
mapping for the vm exists (in ~/.config/virt-viewer/settings) . If it doesn't
exists then virt_viewer_app_get_initial_monitor_for_display() just maps display
= monitor, so it never returns -1. I will send a patch to change it.

Pavel

> 
> 
> > +
> >  static void
> >  virt_viewer_session_spice_display_monitors(SpiceChannel *channel,
> >                                             GParamSpec *pspec
> > G_GNUC_UNUSED,
> > @@ -847,6 +860,8 @@
> > virt_viewer_session_spice_display_monitors(SpiceChannel *channel,
> >      GPtrArray *displays = NULL;
> >      GtkWidget *display;
> >      guint i, monitors_max;
> > +    gboolean fullscreen_mode =
> > +       
> >  virt_viewer_app_get_fullscreen(virt_viewer_session_get_app(VIRT_VIEW
> > ER_SESSION(self)));
> >  
> >      g_object_get(channel,
> >                   "monitors", &monitors,
> > @@ -883,6 +898,16 @@
> > virt_viewer_session_spice_display_monitors(SpiceChannel *channel,
> >          display = g_ptr_array_index(displays, monitor->id);
> >          g_return_if_fail(display != NULL);
> >  
> > +        if (!disabled && fullscreen_mode && self->priv
> > ->did_auto_conf &&
> > +            !display_is_in_fullscreen_mode(self,
> > VIRT_VIEWER_DISPLAY(display))) {
> > +            g_warning("display %d should not be enabled, disabling",
> > +                     
> >  virt_viewer_display_get_nth(VIRT_VIEWER_DISPLAY(display)));
> > +           
> >  spice_main_set_display_enabled(virt_viewer_session_spice_get_main_ch
> > annel(self),
> > +                                          
> >  virt_viewer_display_get_nth(VIRT_VIEWER_DISPLAY(display)),
> > +                                           FALSE);
> > +            disabled = TRUE;
> > +        }
> > +
> >         
> >  virt_viewer_display_set_enabled(VIRT_VIEWER_DISPLAY(display),
> > !disabled);
> >  
> >          if (disabled)
> 
> ACK otherwise.