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
Attachment:
pgpVsmKZYhWKE.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
