[virt-tools-list] [PATCH] Do all display alignment in virt-viewer

Jonathon Jongsma jjongsma at redhat.com
Thu Oct 31 22:35:15 UTC 2013


Oh, I should also mention that this patch depends on the following patch to spice-gtk:

http://lists.freedesktop.org/archives/spice-devel/2013-October/015217.html


----- Original Message -----
> From: "Jonathon Jongsma" <jjongsma at redhat.com>
> To: virt-tools-list at redhat.com
> Sent: Thursday, October 31, 2013 5:28:52 PM
> Subject: [virt-tools-list] [PATCH] Do all display alignment in virt-viewer
> 
> Don't rely on spice-gtk to do any alignment of displays.  This patch sets the
> disable-display-align property on the SpiceMainChannel, and makes the
> VirtViewerSessionSpice in charge of doing all alignment. This means that
> every
> display has to communicate to the VirtViewerSessionSpice when it needs to
> reconfigure its display on the guest, rather than sending it directly to the
> Main Channel. The session will then align the displays (if necessary), and
> send
> down new configuration for all displays at once.  This solves a couple of
> problems:
> 
> 1. It allows the session to send down absolute coordinates only in the case
>    where *all* windows are fullscreen (so that we can still support
>    vertically-stacked displays, etc).  But it auto-aligns displays if only a
>    subset of the displays are in fullscreen mode. This solves the problem of
>    overlapping regions on different displays when one monitor is in
>    fullscreen
>    because only one monitor's configuration was updated and the others were
>    not
>    aligned.
> 2. Allows us to always align based on the current position of each display.
> This
>    contrasts with the earlier behavior where the position used for alignment
>    was
>    the window's position at the time when it was last resized. This caused
>    displays to be arranged in a seemingly non-deterministic manner if one
>    window
>    was moved and then another window was resized (causing a display
>    re-configuration).
> 
> Solves rhbz#1002156
> ---
> 
> NOTE:
> 
> This patch is an alternate approach to the spice-gtk patch posted here:
> http://lists.freedesktop.org/archives/spice-devel/2013-October/015147.html
> 
> It turned out to be a bit more work than anticipated to do all of the work in
> virt-viewer, but it does solve the mentioned bug, and provides a couple other
> benefits as well.
> 
> The display align code is basically copied from spice-gtk, but modified
> slightly. Eventually it could probably be made a bit smarter.
> 
>  src/virt-viewer-display-spice.c | 129
>  ++++++++++++++++++++--------------------
>  src/virt-viewer-display-spice.h |   1 +
>  src/virt-viewer-display.c       |   6 ++
>  src/virt-viewer-display.h       |   1 +
>  src/virt-viewer-session-spice.c |  98 ++++++++++++++++++++++++++++--
>  src/virt-viewer-session-spice.h |   1 +
>  src/virt-viewer-session.c       |   5 ++
>  src/virt-viewer-session.h       |   1 +
>  8 files changed, 171 insertions(+), 71 deletions(-)
> 
> diff --git a/src/virt-viewer-display-spice.c
> b/src/virt-viewer-display-spice.c
> index 54c1672..3190867 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -97,20 +97,10 @@ get_main(VirtViewerDisplay *self)
>  static void
>  show_hint_changed(VirtViewerDisplay *self)
>  {
> -    SpiceMainChannel *main_channel = get_main(self);
> -    guint enabled = TRUE;
> -    guint nth, hint = virt_viewer_display_get_show_hint(self);
> +    VirtViewerSessionSpice *session = NULL;
>  
> -    /* this may happen when finalizing */
> -    if (!main_channel)
> -        return;
> -
> -    g_object_get(self, "nth-display", &nth, NULL);
> -    if (!(hint & VIRT_VIEWER_DISPLAY_SHOW_HINT_SET) ||
> -        hint & VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED)
> -        enabled = FALSE;
> -
> -    spice_main_set_display_enabled(main_channel, nth, enabled);
> +    session =
> VIRT_VIEWER_SESSION_SPICE(virt_viewer_display_get_session(VIRT_VIEWER_DISPLAY(self)));
> +    virt_viewer_session_spice_update_displays(session);
>  }
>  
>  static void
> @@ -181,71 +171,83 @@ virt_viewer_display_spice_mouse_grab(SpiceDisplay
> *display G_GNUC_UNUSED,
>  }
>  
>  
> -static void
> -virt_viewer_display_spice_resize(VirtViewerDisplaySpice *self,
> -                                 GtkAllocation *allocation,
> -                                 gboolean resize_guest)
> +
> +void
> +virt_viewer_display_spice_get_requested_size(VirtViewerDisplaySpice *self,
> GdkRectangle* rect)
>  {
> -    gdouble dw = allocation->width, dh = allocation->height;
> -    guint zoom = 100;
> -    guint nth;
> -    gint x = 0, y = 0;
> -    gboolean disable_display_position = TRUE;
> +    GtkWidget *top = NULL;
> +    gint topx = 0, topy = 0;
>  
> -    if (virt_viewer_display_get_auto_resize(VIRT_VIEWER_DISPLAY(self)) ==
> FALSE)
> -        return;
> +    g_return_if_fail(rect != NULL);
>  
> -    if (virt_viewer_display_get_show_hint(VIRT_VIEWER_DISPLAY(self)) &
> VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED)
> +    if (!virt_viewer_display_get_enabled(VIRT_VIEWER_DISPLAY(self))) {
> +        rect->width = 0;
> +        rect->height = 0;
> +        rect->x = 0;
> +        rect->y = 0;
>          return;
> +    }
>  
> -    if (self->priv->auto_resize == AUTO_RESIZE_FULLSCREEN) {
> -        GdkRectangle monitor;
> -        GdkScreen *screen = gtk_widget_get_screen(GTK_WIDGET(self));
> -        int n = virt_viewer_display_get_monitor(VIRT_VIEWER_DISPLAY(self));
> -        if (n == -1)
> -            n = gdk_screen_get_monitor_at_window(screen,
> -
> gtk_widget_get_window(GTK_WIDGET(self)));
> -        gdk_screen_get_monitor_geometry(screen, n, &monitor);
> -        disable_display_position = FALSE;
> -        x = monitor.x;
> -        y = monitor.y;
> -        dw = monitor.width;
> -        dh = monitor.height;
> +    top = gtk_widget_get_toplevel(GTK_WIDGET(self));
> +    gtk_window_get_position(GTK_WINDOW(top), &topx, &topy);
> +    topx = MAX(topx, 0);
> +    topy = MAX(topy, 0);
> +
> +    if (virt_viewer_display_get_auto_resize(VIRT_VIEWER_DISPLAY(self)) ==
> FALSE) {
> +        rect->width = spice_display_get_width(self->priv->display);
> +        rect->height = spice_display_get_height(self->priv->display);
> +        rect->x = topx;
> +        rect->y = topy;
>      } else {
> -        GtkWidget *top = gtk_widget_get_toplevel(GTK_WIDGET(self));
> -        gtk_window_get_position(GTK_WINDOW(top), &x, &y);
> -        if (x < 0)
> -            x = 0;
> -        if (y < 0)
> -            y = 0;
> +        if (virt_viewer_display_get_fullscreen(VIRT_VIEWER_DISPLAY(self))) {
> +            GdkRectangle monitor;
> +            GdkScreen *screen = gtk_widget_get_screen(GTK_WIDGET(self));
> +            int n =
> virt_viewer_display_get_monitor(VIRT_VIEWER_DISPLAY(self));
> +            if (n == -1)
> +                n = gdk_screen_get_monitor_at_window(screen,
> +
> gtk_widget_get_window(GTK_WIDGET(self)));
> +            gdk_screen_get_monitor_geometry(screen, n, &monitor);
> +            rect->x = monitor.x;
> +            rect->y = monitor.y;
> +            rect->width = monitor.width;
> +            rect->height = monitor.height;
> +        } else {
> +            gtk_widget_get_allocation(GTK_WIDGET(self), rect);
> +            rect->x = topx;
> +            rect->y = topy;
> +        }
> +
> +        if (virt_viewer_display_get_zoom(VIRT_VIEWER_DISPLAY(self))) {
> +            guint zoom =
> virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
> +
> +            rect->width = round(rect->width * 100 / zoom);
> +            rect->height = round(rect->height * 100 / zoom);
> +        }
>      }
> +}
>  
> -    if (virt_viewer_display_get_zoom(VIRT_VIEWER_DISPLAY(self))) {
> -        zoom =
> virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
> +static void
> +virt_viewer_display_spice_resize(VirtViewerDisplaySpice *self)
> +{
> +    VirtViewerSessionSpice *session = NULL;
>  
> -        dw = round(dw * 100 / zoom);
> -        dh = round(dh * 100 / zoom);
> -    }
> +    if (virt_viewer_display_get_auto_resize(VIRT_VIEWER_DISPLAY(self)) ==
> FALSE)
> +        return;
>  
> -    g_object_get(self, "nth-display", &nth, NULL);
> +    if (virt_viewer_display_get_show_hint(VIRT_VIEWER_DISPLAY(self)) &
> VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED)
> +        return;
>  
> -    if (resize_guest) {
> -        g_object_set(get_main(VIRT_VIEWER_DISPLAY(self)),
> -                     "disable-display-position", disable_display_position,
> -                     "disable-display-align", !disable_display_position,
> -                     NULL);
> -        spice_main_set_display(get_main(VIRT_VIEWER_DISPLAY(self)),
> -                               nth, x, y, dw, dh);
> -    }
> +    session =
> VIRT_VIEWER_SESSION_SPICE(virt_viewer_display_get_session(VIRT_VIEWER_DISPLAY(self)));
> +    virt_viewer_session_spice_update_displays(session);
>  }
>  
>  static void
>  virt_viewer_display_spice_size_allocate(VirtViewerDisplaySpice *self,
> -                                        GtkAllocation *allocation,
> +                                        GtkAllocation *allocation
> G_GNUC_UNUSED,
>                                          gpointer data G_GNUC_UNUSED)
>  {
> -    virt_viewer_display_spice_resize(self, allocation,
> -                                     self->priv->auto_resize !=
> AUTO_RESIZE_NEVER);
> +    if (self->priv->auto_resize != AUTO_RESIZE_NEVER)
> +        virt_viewer_display_spice_resize(self);
>  
>      if (self->priv->auto_resize == AUTO_RESIZE_FULLSCREEN)
>          self->priv->auto_resize = AUTO_RESIZE_NEVER;
> @@ -256,13 +258,10 @@ zoom_level_changed(VirtViewerDisplaySpice *self,
>                     GParamSpec *pspec G_GNUC_UNUSED,
>                     VirtViewerApp *app G_GNUC_UNUSED)
>  {
> -    GtkAllocation allocation;
> -
>      if (self->priv->auto_resize != AUTO_RESIZE_NEVER)
>          return;
>  
> -    gtk_widget_get_allocation(GTK_WIDGET(self), &allocation);
> -    virt_viewer_display_spice_resize(self, &allocation, TRUE);
> +    virt_viewer_display_spice_resize(self);
>  }
>  
>  static void
> diff --git a/src/virt-viewer-display-spice.h
> b/src/virt-viewer-display-spice.h
> index c2013ec..e3bb467 100644
> --- a/src/virt-viewer-display-spice.h
> +++ b/src/virt-viewer-display-spice.h
> @@ -67,6 +67,7 @@ struct _VirtViewerDisplaySpiceClass {
>  GType virt_viewer_display_spice_get_type(void);
>  
>  GtkWidget* virt_viewer_display_spice_new(VirtViewerSessionSpice *session,
>  SpiceChannel *channel, gint monitorid);
> +void virt_viewer_display_spice_get_requested_size(VirtViewerDisplaySpice
> *self, GdkRectangle* rect);
>  
>  G_END_DECLS
>  
> diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
> index b6ef018..c6f8f82 100644
> --- a/src/virt-viewer-display.c
> +++ b/src/virt-viewer-display.c
> @@ -668,6 +668,12 @@ void virt_viewer_display_set_enabled(VirtViewerDisplay
> *self, gboolean enabled)
>      virt_viewer_display_set_show_hint(self,
>      VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED, !enabled);
>  }
>  
> +gboolean virt_viewer_display_get_enabled(VirtViewerDisplay *self)
> +{
> +    return ((self->priv->show_hint & VIRT_VIEWER_DISPLAY_SHOW_HINT_SET) &&
> +        !(self->priv->show_hint & VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED));
> +}
> +
>  VirtViewerSession* virt_viewer_display_get_session(VirtViewerDisplay *self)
>  {
>      g_return_val_if_fail(VIRT_VIEWER_IS_DISPLAY(self), NULL);
> diff --git a/src/virt-viewer-display.h b/src/virt-viewer-display.h
> index 99844c4..d9535ac 100644
> --- a/src/virt-viewer-display.h
> +++ b/src/virt-viewer-display.h
> @@ -124,6 +124,7 @@ void virt_viewer_display_release_cursor(VirtViewerDisplay
> *display);
>  
>  void virt_viewer_display_close(VirtViewerDisplay *display);
>  void virt_viewer_display_set_enabled(VirtViewerDisplay *display, gboolean
>  enabled);
> +gboolean virt_viewer_display_get_enabled(VirtViewerDisplay *display);
>  gboolean virt_viewer_display_get_selectable(VirtViewerDisplay *display);
>  void virt_viewer_display_queue_resize(VirtViewerDisplay *display);
>  
> diff --git a/src/virt-viewer-session-spice.c
> b/src/virt-viewer-session-spice.c
> index b42d48e..ce64874 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -24,6 +24,7 @@
>  
>  #include <config.h>
>  
> +#include <math.h>
>  #include <spice-audio.h>
>  #include <glib/gi18n.h>
>  
> @@ -675,6 +676,10 @@ virt_viewer_session_spice_channel_new(SpiceSession *s,
>          g_signal_connect(channel, "channel-event",
>                           G_CALLBACK(virt_viewer_session_spice_main_channel_event),
>                           self);
>          self->priv->main_channel = SPICE_MAIN_CHANNEL(channel);
> +        g_object_set(G_OBJECT(channel),
> +                     "disable-display-position", FALSE,
> +                     "disable-display-align", TRUE,
> +                     NULL);
>  
>          g_signal_connect(channel, "notify::agent-connected",
>          G_CALLBACK(agent_connected_changed), self);
>          virt_viewer_session_spice_fullscreen_auto_conf(self);
> @@ -742,12 +747,6 @@
> virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self)
>          return FALSE;
>      }
>  
> -    DEBUG_LOG("Performing full screen auto-conf, %d host monitors",
> -              gdk_screen_get_n_monitors(screen));
> -    g_object_set(G_OBJECT(cmain),
> -                 "disable-display-position", FALSE,
> -                 "disable-display-align", TRUE,
> -                 NULL);
>      spice_main_set_display_enabled(cmain, -1, FALSE);
>      for (i = 0; i < gdk_screen_get_n_monitors(screen); i++) {
>          gdk_screen_get_monitor_geometry(screen, i, &dest);
> @@ -845,6 +844,93 @@
> virt_viewer_session_spice_smartcard_remove(VirtViewerSession *session
> G_GNUC_UNU
>      spice_smartcard_manager_remove_card(spice_smartcard_manager_get());
>  }
>  
> +static int displays_cmp(const void *p1, const void *p2, gpointer user_data)
> +{
> +    GHashTable *displays = user_data;
> +    guint i = *(guint*)p1;
> +    guint j = *(guint*)p2;
> +    GdkRectangle *m1 = g_hash_table_lookup(displays, GINT_TO_POINTER(i));
> +    GdkRectangle *m2 = g_hash_table_lookup(displays, GINT_TO_POINTER(j));
> +    double d1 = sqrt(m1->x * m1->x + m1->y * m1->y);
> +    double d2 = sqrt(m2->x * m2->x + m2->y * m2->y);
> +    int diff = d1 - d2;
> +
> +    return diff == 0 ? i - j : diff;
> +}
> +
> +static void align_displays(GHashTable *displays)
> +{
> +    gint i, j, x = 0;
> +    guint *sorted_displays;
> +    guint ndisplays = 0;
> +
> +    g_return_if_fail(displays != NULL);
> +
> +    ndisplays = g_hash_table_size(displays);
> +
> +    if (ndisplays == 0)
> +        return;
> +
> +    /* sort by distance from origin */
> +    sorted_displays = g_new0(guint, ndisplays);
> +    for (i = 0; i < ndisplays; i++)
> +        sorted_displays[i] = i;
> +    g_qsort_with_data(sorted_displays, ndisplays, sizeof(guint),
> displays_cmp, displays);
> +
> +    for (i = 0; i < ndisplays; i++) {
> +        j = sorted_displays[i];
> +        g_assert(j < ndisplays);
> +        GdkRectangle *rect = g_hash_table_lookup(displays,
> GINT_TO_POINTER(j));
> +        rect->x = x;
> +        rect->y = 0;
> +        x += rect->width;
> +        if (rect->width || rect->height)
> +            DEBUG_LOG("%s: monitor config: #%d %dx%d+%d+%d", G_STRFUNC,
> +                      j, rect->width, rect->height, rect->x, rect->y);
> +    }
> +    g_free(sorted_displays);
> +}
> +
> +void
> +virt_viewer_session_spice_update_displays(VirtViewerSessionSpice *self)
> +{
> +    gboolean all_fullscreen = TRUE;
> +    GHashTable *display_rects = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_free);
> +    GHashTableIter iter;
> +    gpointer key, value;
> +    GList *displays =
> virt_viewer_session_get_displays(VIRT_VIEWER_SESSION(self));
> +
> +    for (GList *l = displays; l; l = l->next) {
> +        GdkRectangle *rect = g_new0(GdkRectangle, 1);
> +        VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
> +        gint nth;
> +        gboolean enabled = virt_viewer_display_get_enabled(d);
> +
> +        g_object_get(d, "nth-display", &nth, NULL);
> +
> +        spice_main_set_display_enabled(self->priv->main_channel, nth,
> enabled);
> +        if (enabled && !virt_viewer_display_get_fullscreen(d))
> +            all_fullscreen = FALSE;
> +
> +
> virt_viewer_display_spice_get_requested_size(VIRT_VIEWER_DISPLAY_SPICE(d),
> rect);
> +        DEBUG_LOG("%s: Got requested size for display %d: (%dx%d) @
> (%d,%d)",
> +                  G_STRFUNC, nth, rect->width, rect->height, rect->x,
> rect->y);
> +        g_hash_table_insert(display_rects, GINT_TO_POINTER(nth), rect);
> +    }
> +
> +    if (!all_fullscreen)
> +        align_displays(display_rects);
> +
> +    g_hash_table_iter_init(&iter, display_rects);
> +    while (g_hash_table_iter_next(&iter, &key, &value)) {
> +        gint nth = GPOINTER_TO_INT(key);
> +        GdkRectangle* rect = (GdkRectangle*) value;
> +        spice_main_set_display(self->priv->main_channel, nth, rect->x,
> +                               rect->y, rect->width, rect->height);
> +    }
> +    g_hash_table_unref(display_rects);
> +}
> +
>  /*
>   * Local variables:
>   *  c-indent-level: 4
> diff --git a/src/virt-viewer-session-spice.h
> b/src/virt-viewer-session-spice.h
> index 95bdcdf..c0a6a79 100644
> --- a/src/virt-viewer-session-spice.h
> +++ b/src/virt-viewer-session-spice.h
> @@ -67,6 +67,7 @@ GType virt_viewer_session_spice_get_type(void);
>  
>  VirtViewerSession* virt_viewer_session_spice_new(VirtViewerApp *app,
>  GtkWindow *main_window);
>  SpiceMainChannel*
>  virt_viewer_session_spice_get_main_channel(VirtViewerSessionSpice *self);
> +void virt_viewer_session_spice_update_displays(VirtViewerSessionSpice
> *self);
>  
>  G_END_DECLS
>  
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index 595dcd0..630cad8 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -547,6 +547,11 @@ VirtViewerFile*
> virt_viewer_session_get_file(VirtViewerSession *self)
>      return self->priv->file;
>  }
>  
> +GList* virt_viewer_session_get_displays(VirtViewerSession *self)
> +{
> +    return self->priv->displays;
> +}
> +
>  /*
>   * Local variables:
>   *  c-indent-level: 4
> diff --git a/src/virt-viewer-session.h b/src/virt-viewer-session.h
> index 0467724..9f135df 100644
> --- a/src/virt-viewer-session.h
> +++ b/src/virt-viewer-session.h
> @@ -128,6 +128,7 @@ VirtViewerApp*
> virt_viewer_session_get_app(VirtViewerSession *self);
>  gchar* virt_viewer_session_get_uri(VirtViewerSession *self);
>  void virt_viewer_session_set_file(VirtViewerSession *self, VirtViewerFile
>  *file);
>  VirtViewerFile* virt_viewer_session_get_file(VirtViewerSession *self);
> +GList* virt_viewer_session_get_displays(VirtViewerSession *self);
>  
>  G_END_DECLS
>  
> --
> 1.8.3.1
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
> 




More information about the virt-tools-list mailing list