[virt-tools-list] [PATCH virt-viewer 2/2] Shift top-left display to origin

[Christophe Fergeau]

Hey, all looks good now, 2 more small nits below,

On Wed, Oct 15, 2014 at 12:42:17PM -0500, Jonathon Jongsma wrote:
> When using a custom fullscreen display configuration, it's possible to
> specify that e.g. a single screen should be fullscreen on client
> monitor #4. Since we send down absolute positions and disable alignment
> when all windows are in fullscreen, we can send configurations with a
> very large offset to the top-left corner. This could result in the guest
> trying to create a screen that was much larger than necessary. For
> example when sending a configuration of 1280x1024+4240+0, the guest
> would need to allocate a screen of size 5520x1024, which might fail if
> video memory was too low. To avoid this issue, we shift all displays
> so that the minimum X coordinate for all screens is at x=0, and the
> minimum y coordinate is at y=0.
> ---
>  src/virt-viewer-session-spice.c | 20 +++++++++++++++-----
>  src/virt-viewer-session.c       |  2 ++
>  src/virt-viewer-util.c          | 41 +++++++++++++++++++++++++++++++++++++++++
>  src/virt-viewer-util.h          |  3 +++
>  4 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index 885399c..66fe939 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -770,7 +770,7 @@ virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self)
>      GdkScreen *screen = gdk_screen_get_default();
>      SpiceMainChannel* cmain = virt_viewer_session_spice_get_main_channel(self);
>      VirtViewerApp *app = NULL;
> -    GdkRectangle dest;
> +    GdkRectangle *displays;
>      gboolean agent_connected;
>      gint i;
>      gsize ndisplays = 0;
> @@ -804,18 +804,28 @@ virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self)
>  
>      ndisplays = virt_viewer_app_get_n_initial_displays(app);
>      g_debug("Performing full screen auto-conf, %" G_GSIZE_FORMAT " host monitors", ndisplays);
> +    displays = g_new0(GdkRectangle, ndisplays);
>  
>      for (i = 0; i < ndisplays; i++) {
> +        GdkRectangle* rect = &displays[i];
>          gint j = virt_viewer_app_get_initial_monitor_for_display(app, i);
>          if (j == -1)
>              continue;
>  
> -        gdk_screen_get_monitor_geometry(screen, j, &dest);
> -        g_debug("Set SPICE display %d to (%d,%d)-(%dx%d)",
> -                  i, dest.x, dest.y, dest.width, dest.height);
> -        spice_main_set_display(cmain, i, dest.x, dest.y, dest.width, dest.height);
> +        gdk_screen_get_monitor_geometry(screen, j, rect);
> +    }
> +
> +    virt_viewer_shift_monitors_to_origin(displays, ndisplays);
> +
> +    for (i = 0; i < ndisplays; i++) {
> +        GdkRectangle *rect = &displays[i];
> +
> +        spice_main_set_display(cmain, i, rect->x, rect->y, rect->width, rect->height);
>          spice_main_set_display_enabled(cmain, i, TRUE);
> +        g_debug("Set SPICE display %d to (%d,%d)-(%dx%d)",
> +                  i, rect->x, rect->y, rect->width, rect->height);
>      }
> +    g_free(displays);
>  
>      spice_main_send_monitor_config(cmain);
>      self->priv->did_auto_conf = TRUE;
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index d050ba1..4262eda 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -381,6 +381,8 @@ virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession* self,
>      if (!all_fullscreen)
>          virt_viewer_align_monitors_linear(monitors, nmonitors);
>  
> +    virt_viewer_shift_monitors_to_origin(monitors, nmonitors);
> +
>      klass->apply_monitor_geometry(self, monitors, nmonitors);
>      g_free(monitors);
>  }
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index c46d07d..234e92a 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -538,6 +538,47 @@ virt_viewer_align_monitors_linear(GdkRectangle *displays, guint ndisplays)
>      g_free(sorted_displays);
>  }
>  
> +/* Shift all displays so that the monitor origin is at (0,0). This reduces the
> + * size of the screen that will be required on the guest when all client
> + * monitors are fullscreen but do not begin at the origin. For example, instead
> + * of sending down the following configuration:
> + *   1280x1024+4240+0
> + * (which implies that the guest screen must be at least 5520x1024), we'd send
> + *   1280x1024+0+0
> + * (which implies the guest screen only needs to be 1280x1024). The first
> + * version might fail if the guest video memory is not large enough to handle a
> + * screen of that size.
> + */
> +void
> +virt_viewer_shift_monitors_to_origin(GdkRectangle *displays, guint ndisplays)
> +{
> +    gint xmin = G_MAXINT;
> +    gint ymin = G_MAXINT;
> +    gint i;
> +
> +    g_return_if_fail(ndisplays > 0);
> +
> +    for (i = 0; i < ndisplays; i++) {
> +        GdkRectangle *display = &displays[i];
> +        if (display->width > 0 && display->height > 0) {
> +            xmin = MIN(xmin, display->x);
> +            ymin = MIN(ymin, display->y);
> +        }
> +    }

Is the case where ndisplays > 0 but all these displays have 0 width/0
height filtered out somewhere? We may end up with xmin/ymin == G_MAXINT
if this can happen, which would not go well. We could have a
g_return_if_fail() to avoid that.


> +
> +    if (xmin > 0 || ymin > 0) {
> +        g_debug("%s: Shifting all monitors by (%i, %i)", G_STRFUNC, xmin, ymin);
> +        for (i = 0; i < ndisplays; i++) {
> +            GdkRectangle *display = &displays[i];
> +            if (display->width > 0 && display->height > 0) {
> +                display->x -= xmin;
> +                display->y -= ymin;
> +            }
> +        }
> +    }
> +}
> +
> +
>  /*
>   * Local variables:
>   *  c-indent-level: 4
> diff --git a/src/virt-viewer-util.h b/src/virt-viewer-util.h
> index 3973dde..8c38739 100644
> --- a/src/virt-viewer-util.h
> +++ b/src/virt-viewer-util.h
> @@ -57,6 +57,9 @@ gint virt_viewer_compare_version(const gchar *s1, const gchar *s2);
>  
>  /* monitor alignment */
>  void virt_viewer_align_monitors_linear(GdkRectangle *displays, guint ndisplays);
> +void virt_viewer_shift_monitors_to_origin(GdkRectangle *displays, guint ndisplays);
> +
> +

Extra whitespace here.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20141022/d66384d4/attachment.sig>