[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);

As Fabiano mentioned there are some issues, probably because i != monitor->id.
Before this patch there were two cycles, the first one was creating the display#i.
The second cycle was setting the display#monitor->id as enabled, setting its dimensions.
Now the the monitor config of the display#monitor->id is used for the display#i.

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