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

[Jonathon Jongsma]

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"


> 
> 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.


> +
>  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.