[virt-tools-list] [PATCH virt-viewer v2 5/7] Configure display before adding it to the session

[Pavel Grunt]

Hi,

Ack. Thanks a lot for this, seems that it fixed all the issues I reported.
I have just a small cosmetic comment below, feel free to ignore it.

> 
> Due to the fullscreen auto-conf changes that cause the application to
> wait to process monitor updates until the server configuration
> matches
> the client's requested configuration, The display channel's "ready"
> property can become TRUE before the VirtViewerDisplay object gets
> created. When this happens, virt_viewer_session_add_display() can
> trigger a notify::show-hint on the display, which will cause us to
> calculate its minimum zoom level before any of the desktop sizes for
> the
> display have been set. This results in an erroneous zoom level, which
> results in undesirable behavior. For example, leaving fullscreen mode
> caused the first display to unexpectedly resize to approximately half
> the width of the client monitor because its minimum zoom level was
> 190%.
> To avoid this, we guarantee that all displays are configured
> (_set_desktop(), _set_enabled()) before we add the display to the
> session.
> ---
>  src/virt-viewer-session-spice.c | 33
>  +++++++++++++++++++--------------
>  1 file changed, 19 insertions(+), 14 deletions(-)
> 
> diff --git a/src/virt-viewer-session-spice.c
> b/src/virt-viewer-session-spice.c
> index d0a7ab5..9a9a337 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -731,28 +731,33 @@
> virt_viewer_session_spice_process_monitors_for_display_channel(VirtViewerSession
>      g_ptr_array_set_size(displays, monitors_max);
>  
>      for (i = 0; i < monitors_max; i++) {
> +        gboolean create = FALSE;
>          display = g_ptr_array_index(displays, i);
> -        if (display == NULL) {
> +
> +        create = (display == NULL);

It is possible to move the 'display' variable here and have just this:

        GtkWidget *display = g_ptr_array_index(displays, i);
        gboolean create = (display == NULL);

>
> +        if (create) {
>              display = virt_viewer_display_spice_new(self, channel,
>              i);
>              g_debug("creating spice display (#:%d)", i);
>              g_ptr_array_index(displays, i) =
>              g_object_ref_sink(display);
> -
>            virt_viewer_session_add_display(VIRT_VIEWER_SESSION(self),
> -
>                                            VIRT_VIEWER_DISPLAY(display));
>          }
> -    }
> -
> -    for (i = 0; i < monitors->len; i++) {
> -        SpiceDisplayMonitorConfig *monitor =
> &g_array_index(monitors, SpiceDisplayMonitorConfig, i);
> -        display = g_ptr_array_index(displays, monitor->id);
>          g_return_if_fail(display != NULL);
>  
> -        if (monitor->width == 0 || monitor->height == 0)
> -            continue;
> +        if (i < monitors->len) {
> +            SpiceDisplayMonitorConfig *monitor =
> &g_array_index(monitors, SpiceDisplayMonitorConfig, i);
>  
> -
>        virt_viewer_display_set_enabled(VIRT_VIEWER_DISPLAY(display),
> TRUE);
> -
>        virt_viewer_display_spice_set_desktop(VIRT_VIEWER_DISPLAY(display),
> -                                              monitor->x,
> monitor->y,
> -                                              monitor->width,
> monitor->height);
> +            if (monitor->width == 0 || monitor->height == 0)
> +                continue;
> +
> +
>            virt_viewer_display_spice_set_desktop(VIRT_VIEWER_DISPLAY(display),
> +                                                  monitor->x,
> monitor->y,
> +                                                  monitor->width,
> monitor->height);
> +
>            virt_viewer_display_set_enabled(VIRT_VIEWER_DISPLAY(display),
> TRUE);
> +        }
> +
> +        if (create) {
> +
>            virt_viewer_session_add_display(VIRT_VIEWER_SESSION(self),
> +
>                                            VIRT_VIEWER_DISPLAY(display));
> +        }
>      }
>  
>      g_clear_pointer(&monitors, g_array_unref);
> --
> 2.1.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 

Thanks,
Pavel