[virt-tools-list] [PATCH] Do all display alignment in virt-viewer
Marc-André Lureau
mlureau at redhat.com
Tue Nov 5 18:17:00 UTC 2013
----- Original Message -----
> 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
> VirtViewerSession in charge of doing all alignment. This means that every
> display has to tell the VirtViewerSession when its "virtual monitor" has
> changed
> configuration (and wants 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 the spice session will 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
> ---
>
> This patch is an attempt to address Marc-Andre's concerns with the previous
> patch. Most of the alignment logic is moved down to the Session base class.
> It
> uses an array instead of a GHashTable.
>
> It still does some extra at display-reconfiguration, but this seems
> preferable
> to me to pushing the aligned rects into e.g. VirtViewerDisplay, for a couple
> of
> reasons:
> - the aligned rects are only used by the session to send down new geometry to
> the spice server. They have no significance to the display object. The
> display
> object will get updated with its proper geometry when the server sends back
> a
> monitors configuration message (after the monitors have been successfully
> configured).
> - display configuration is not something that happens particularly often, nor
> is
> this a performance-critical path, so a bit of extra allocation doesn't hurt
> anything, and it keeps all of the data encapsulated in one place.
>
> src/virt-viewer-display-spice.c | 90
> +++++++++--------------------------------
> src/virt-viewer-display.c | 72 +++++++++++++++++++++++++++++++++
> src/virt-viewer-display.h | 2 +
> src/virt-viewer-session-spice.c | 26 +++++++++---
> src/virt-viewer-session.c | 89
> ++++++++++++++++++++++++++++++++++++++++
> src/virt-viewer-session.h | 1 +
> 6 files changed, 203 insertions(+), 77 deletions(-)
>
> diff --git a/src/virt-viewer-display-spice.c
> b/src/virt-viewer-display-spice.c
> index 54c1672..fd85e29 100644
> --- a/src/virt-viewer-display-spice.c
> +++ b/src/virt-viewer-display-spice.c
> @@ -95,22 +95,31 @@ get_main(VirtViewerDisplay *self)
> }
>
> static void
> +virt_viewer_display_spice_monitor_geometry_changed(VirtViewerDisplaySpice
> *self)
> +{
> +
> + if (virt_viewer_display_get_auto_resize(VIRT_VIEWER_DISPLAY(self)) ==
> FALSE)
> + return;
> +
> + g_signal_emit_by_name(self, "monitor-geometry-changed", NULL);
> +
> +}
> +
> +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);
> + guint enabled = virt_viewer_display_get_enabled(self);
> + guint nth;
>
> /* 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);
> +
> +
> virt_viewer_display_spice_monitor_geometry_changed(VIRT_VIEWER_DISPLAY_SPICE(self));
> }
>
> static void
> @@ -182,70 +191,12 @@ 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)
> -{
> - gdouble dw = allocation->width, dh = allocation->height;
> - guint zoom = 100;
> - guint nth;
> - gint x = 0, y = 0;
> - gboolean disable_display_position = TRUE;
> -
> - if (virt_viewer_display_get_auto_resize(VIRT_VIEWER_DISPLAY(self)) ==
> FALSE)
> - return;
> -
> - if (virt_viewer_display_get_show_hint(VIRT_VIEWER_DISPLAY(self)) &
> VIRT_VIEWER_DISPLAY_SHOW_HINT_DISABLED)
> - 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;
> - } 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_zoom(VIRT_VIEWER_DISPLAY(self))) {
> - zoom =
> virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
> -
> - dw = round(dw * 100 / zoom);
> - dh = round(dh * 100 / zoom);
> - }
> -
> - g_object_get(self, "nth-display", &nth, NULL);
> -
> - 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);
> - }
> -}
> -
> -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_monitor_geometry_changed(self);
>
> if (self->priv->auto_resize == AUTO_RESIZE_FULLSCREEN)
> self->priv->auto_resize = AUTO_RESIZE_NEVER;
> @@ -256,13 +207,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_monitor_geometry_changed(self);
> }
>
> static void
> diff --git a/src/virt-viewer-display.c b/src/virt-viewer-display.c
> index b6ef018..feefcca 100644
> --- a/src/virt-viewer-display.c
> +++ b/src/virt-viewer-display.c
> @@ -255,6 +255,16 @@ virt_viewer_display_class_init(VirtViewerDisplayClass
> *class)
> G_TYPE_NONE,
> 0);
>
> + g_signal_new("monitor-geometry-changed",
> + G_OBJECT_CLASS_TYPE(object_class),
> + G_SIGNAL_RUN_LAST | G_SIGNAL_NO_HOOKS,
> + 0,
> + NULL,
> + NULL,
> + g_cclosure_marshal_VOID__VOID,
> + G_TYPE_NONE,
> + 0);
> +
> g_type_class_add_private(class, sizeof(VirtViewerDisplayPrivate));
> }
>
> @@ -668,6 +678,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);
> @@ -759,6 +775,62 @@ gboolean
> virt_viewer_display_get_fullscreen(VirtViewerDisplay *self)
> return self->priv->fullscreen;
> }
>
> +void virt_viewer_display_get_preferred_monitor_geometry(VirtViewerDisplay*
> self,
> + GdkRectangle*
> preferred)
> +{
> + GtkWidget *top = NULL;
> + gint topx = 0, topy = 0;
> +
> + g_return_if_fail(preferred != NULL);
> +
> + if (!virt_viewer_display_get_enabled(VIRT_VIEWER_DISPLAY(self))) {
> + preferred->width = 0;
> + preferred->height = 0;
> + preferred->x = 0;
> + preferred->y = 0;
> + return;
> + }
> +
> + 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) {
> + guint w, h;
> + virt_viewer_display_get_desktop_size(self, &w, &h);
> + preferred->width = w;
> + preferred->height = h;
> + preferred->x = topx;
> + preferred->y = topy;
> + } else {
> + if (virt_viewer_display_get_fullscreen(VIRT_VIEWER_DISPLAY(self))) {
> + GdkRectangle physical_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, &physical_monitor);
> + preferred->x = physical_monitor.x;
> + preferred->y = physical_monitor.y;
> + preferred->width = physical_monitor.width;
> + preferred->height = physical_monitor.height;
> + } else {
> + gtk_widget_get_allocation(GTK_WIDGET(self), preferred);
> + preferred->x = topx;
> + preferred->y = topy;
> + }
> +
> + if (virt_viewer_display_get_zoom(VIRT_VIEWER_DISPLAY(self))) {
> + guint zoom =
> virt_viewer_display_get_zoom_level(VIRT_VIEWER_DISPLAY(self));
> +
> + preferred->width = round(preferred->width * 100 / zoom);
> + preferred->height = round(preferred->height * 100 / zoom);
> + }
> + }
> +}
> +
> /*
> * Local variables:
> * c-indent-level: 4
> diff --git a/src/virt-viewer-display.h b/src/virt-viewer-display.h
> index 99844c4..195eeee 100644
> --- a/src/virt-viewer-display.h
> +++ b/src/virt-viewer-display.h
> @@ -124,8 +124,10 @@ 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);
> +void virt_viewer_display_get_preferred_monitor_geometry(VirtViewerDisplay
> *self, GdkRectangle* preferred);
>
> G_END_DECLS
>
> diff --git a/src/virt-viewer-session-spice.c
> b/src/virt-viewer-session-spice.c
> index b42d48e..21a0b38 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -80,6 +80,7 @@ static void
> virt_viewer_session_spice_channel_destroy(SpiceSession *s,
> static void virt_viewer_session_spice_smartcard_insert(VirtViewerSession
> *session);
> static void virt_viewer_session_spice_smartcard_remove(VirtViewerSession
> *session);
> static gboolean
> virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice
> *self);
> +static void
> virt_viewer_session_spice_monitor_geometry_changed(VirtViewerSession *self,
> GdkRectangle *monitors, guint nmonitors);
>
> static void
> virt_viewer_session_spice_get_property(GObject *object, guint property_id,
> @@ -155,6 +156,7 @@
> virt_viewer_session_spice_class_init(VirtViewerSessionSpiceClass *klass)
> dclass->smartcard_insert = virt_viewer_session_spice_smartcard_insert;
> dclass->smartcard_remove = virt_viewer_session_spice_smartcard_remove;
> dclass->mime_type = virt_viewer_session_spice_mime_type;
> + dclass->monitor_geometry_changed =
> virt_viewer_session_spice_monitor_geometry_changed;
>
> g_type_class_add_private(klass, sizeof(VirtViewerSessionSpicePrivate));
>
> @@ -675,6 +677,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 +748,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 +845,20 @@
> virt_viewer_session_spice_smartcard_remove(VirtViewerSession *session
> G_GNUC_UNU
> spice_smartcard_manager_remove_card(spice_smartcard_manager_get());
> }
>
> +void
> +virt_viewer_session_spice_monitor_geometry_changed(VirtViewerSession
> *session, GdkRectangle *monitors, guint nmonitors)
> +{
> + guint i;
> + VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);
> +
> + for (i = 0; i < nmonitors; i++) {
> + GdkRectangle* rect = &monitors[i];
> +
> + spice_main_set_display(self->priv->main_channel, i, rect->x,
> + rect->y, rect->width, rect->height);
> + }
> +}
> +
> /*
> * Local variables:
> * c-indent-level: 4
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index 595dcd0..e82ed5d 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -25,6 +25,7 @@
> #include <config.h>
>
> #include <locale.h>
> +#include <math.h>
>
> #include "virt-viewer-session.h"
> #include "virt-viewer-util.h"
> @@ -337,6 +338,90 @@ virt_viewer_session_init(VirtViewerSession *session)
> session->priv = VIRT_VIEWER_SESSION_GET_PRIVATE(session);
> }
>
> +/* simple sorting of monitors. Primary sort left-to-right, secondary sort
> from
> + * top-to-bottom, finally by monitor id */
> +static int
> +displays_cmp(const void *p1, const void *p2, gpointer user_data)
> +{
> + guint diff;
> + GdkRectangle *displays = user_data;
> + guint i = *(guint*)p1;
> + guint j = *(guint*)p2;
> + GdkRectangle *m1 = &displays[i];
> + GdkRectangle *m2 = &displays[j];
> + diff = m1->x - m2->x;
> + if (diff == 0)
> + diff = m1->y - m2->y;
> + if (diff == 0)
> + diff = i - j;
> +
> + return diff;
> +}
> +
> +static void
> +virt_viewer_session_align_monitors_linear(GdkRectangle *displays, guint
> ndisplays)
> +{
> + gint i, x = 0;
> + guint *sorted_displays;
> +
> + g_return_if_fail(displays != NULL);
> +
> + if (ndisplays == 0)
> + return;
> +
> + 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);
> +
> + /* adjust monitor positions so that there's no gaps or overlap between
> + * monitors */
> + for (i = 0; i < ndisplays; i++) {
> + guint nth = sorted_displays[i];
> + g_assert(nth < ndisplays);
> + GdkRectangle *rect = &displays[nth];
> + rect->x = x;
> + rect->y = 0;
> + x += rect->width;
> + }
> + g_free(sorted_displays);
> +}
> +
> +static void
> +virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession* self,
> + VirtViewerDisplay* display
> G_GNUC_UNUSED)
> +{
> + VirtViewerSessionClass *klass;
> + gboolean all_fullscreen = TRUE;
> + guint nmonitors = g_list_length(self->priv->displays);
> + GdkRectangle *monitors = g_new0(GdkRectangle, nmonitors);
> +
> + klass = VIRT_VIEWER_SESSION_GET_CLASS(self);
> + if (!klass->monitor_geometry_changed)
> + return;
> +
> + for (GList *l = self->priv->displays; l; l = l->next) {
> + VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
> + guint nth = 0;
> + GdkRectangle *rect = NULL;
> +
> + g_object_get(d, "nth-display", &nth, NULL);
> + g_return_if_fail(nth < nmonitors);
> + rect = &monitors[nth];
> + virt_viewer_display_get_preferred_monitor_geometry(d, rect);
> +
> + if (virt_viewer_display_get_enabled(d) &&
> + !virt_viewer_display_get_fullscreen(d))
> + all_fullscreen = FALSE;
> + }
> +
> + if (!all_fullscreen)
> + virt_viewer_session_align_monitors_linear(monitors, nmonitors);
> +
> + klass->monitor_geometry_changed(self, monitors, nmonitors);
I guess this code path shouldn't be reached by vnc backend. A g_return_if_fail(klass->monitor_geometry_changed) wouldn't hurt though.
Also to help showing different phases of monitor_geometry_changed, perhaps "apply_changes" or "commit_config", would help mixing the various callbacks and signals.
The rest looks good to me
> + g_free(monitors);
> +}
> +
> void virt_viewer_session_add_display(VirtViewerSession *session,
> VirtViewerDisplay *display)
> {
> @@ -346,6 +431,10 @@ void virt_viewer_session_add_display(VirtViewerSession
> *session,
> session->priv->displays = g_list_append(session->priv->displays,
> display);
> g_object_ref(display);
> g_signal_emit_by_name(session, "session-display-added", display);
> +
> + virt_viewer_signal_connect_object(display, "monitor-geometry-changed",
> +
> G_CALLBACK(virt_viewer_session_on_monitor_geometry_changed),
> session,
> + G_CONNECT_SWAPPED);
> }
>
>
> diff --git a/src/virt-viewer-session.h b/src/virt-viewer-session.h
> index 0467724..f12cc1c 100644
> --- a/src/virt-viewer-session.h
> +++ b/src/virt-viewer-session.h
> @@ -94,6 +94,7 @@ struct _VirtViewerSessionClass {
> void (*session_cut_text)(VirtViewerSession *session, const gchar *str);
> void (*session_bell)(VirtViewerSession *session);
> void (*session_cancelled)(VirtViewerSession *session);
> + void (*monitor_geometry_changed)(VirtViewerSession *session,
> GdkRectangle* monitors, guint nmonitors);
> };
>
> GType virt_viewer_session_get_type(void);
> --
> 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