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