On Thu, 2015-10-08 at 16:03 -0500, Jonathon Jongsma wrote:
> When starting virt-viewer in fullscreen mode, we generally try to
> arrange guest displays exactly the same as client monitors. So if a
> client machine has two monitors, we'll try to enable display 0 and 1 on
> the guest (in that order). However, when using the configuration file to
> map fullscreen displays to different monitors, the guest displays may
> not be sequential, or there may be displays missing. For example,
> consider the following configuration:
>
> monitor-mapping=1:2;2:1
>
> In virt_viewer_session_spice_fullscreen_auto_conf(), we were building an
> array of GdkRectangles for the initial monitors that we want to enable
> on the guest. We then configured the guest displays using the index of
> the array for the as the id of the guest display. But when displays
> are sparse or are out-of-sequence, the array index will not match the
> intended display ID> This created problems where displays were arranged
> incorrectly. By changing the simple array into a GHashTable, we can keep
> the display ID together with the GdkRectangle until we need to use it,
> and things will be configured correctly.
>
Yes, ack.
> This regression was introduced by c586dc8c.
>
> Fixes: rhbz#1269918
and rhbz#1267184
Thanks,
Pavel
> ---
> src/virt-viewer-session-spice.c | 43 +++++++++++++++++-------------
> src/virt-viewer-session.c | 30 ++++++---------------
> src/virt-viewer-session.h | 3 ++-
> src/virt-viewer-util.c | 58 +++++++++++++++++++++++++++-------------
> -
> src/virt-viewer-util.h | 4 +--
> 5 files changed, 76 insertions(+), 62 deletions(-)
>
> diff --git a/src/virt-viewer-session-spice.c b/src/virt-viewer-session-spice.c
> index f792294..eb0761d 100644
> --- a/src/virt-viewer-session-spice.c
> +++ b/src/virt-viewer-session-spice.c
> @@ -85,7 +85,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_apply_monitor_geometry(VirtViewerSession *self,
> GdkRectangle *monitors, guint nmonitors);
> +static void
> virt_viewer_session_spice_apply_monitor_geometry(VirtViewerSession *self,
> GHashTable *monitors);
>
> static void virt_viewer_session_spice_clear_displays(VirtViewerSessionSpice
> *self)
> {
> @@ -966,9 +966,10 @@
> 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 *displays;
> + GHashTable *displays;
> + GHashTableIter iter;
> + gpointer key, value;
> gboolean agent_connected;
> - gint i;
> GList *initial_displays, *l;
> guint ndisplays;
>
> @@ -1003,29 +1004,32 @@
> virt_viewer_session_spice_fullscreen_auto_conf(VirtViewerSessionSpice *self)
> initial_displays = virt_viewer_app_get_initial_displays(app);
> ndisplays = g_list_length(initial_displays);
> g_debug("Performing full screen auto-conf, %u host monitors", ndisplays);
> - displays = g_new0(GdkRectangle, ndisplays);
> + displays = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
> g_free);
>
> - for (i = 0, l = initial_displays; l != NULL; l = l->next, i++) {
> - GdkRectangle* rect = &displays[i];
> + for (l = initial_displays; l != NULL; l = l->next) {
> + GdkRectangle* rect = g_new0(GdkRectangle, 1);;
> gint j = virt_viewer_app_get_initial_monitor_for_display(app,
> GPOINTER_TO_INT(l->data));
> if (j == -1)
> continue;
>
> gdk_screen_get_monitor_geometry(screen, j, rect);
> + g_hash_table_insert(displays, l->data, rect);
> }
> - g_list_free(initial_displays);
>
> - virt_viewer_shift_monitors_to_origin(displays, ndisplays);
> + virt_viewer_shift_monitors_to_origin(displays);
>
> - for (i = 0; i < ndisplays; i++) {
> - GdkRectangle *rect = &displays[i];
> + g_hash_table_iter_init(&iter, displays);
> + while (g_hash_table_iter_next(&iter, &key, &value)) {
> + GdkRectangle *rect = value;
> + gint j = GPOINTER_TO_INT(key);
>
> - spice_main_set_display(cmain, i, rect->x, rect->y, rect->width, rect-
> >height);
> - spice_main_set_display_enabled(cmain, i, TRUE);
> + spice_main_set_display(cmain, j, rect->x, rect->y, rect->width, rect-
> >height);
> + spice_main_set_display_enabled(cmain, j, TRUE);
> g_debug("Set SPICE display %d to (%d,%d)-(%dx%d)",
> - i, rect->x, rect->y, rect->width, rect->height);
> + j, rect->x, rect->y, rect->width, rect->height);
> }
> - g_free(displays);
> + g_list_free(initial_displays);
> + g_hash_table_unref(displays);
>
> spice_main_send_monitor_config(cmain);
> self->priv->did_auto_conf = TRUE;
> @@ -1109,13 +1113,16 @@
> virt_viewer_session_spice_smartcard_remove(VirtViewerSession *session
> G_GNUC_UNU
> }
>
> static void
> -virt_viewer_session_spice_apply_monitor_geometry(VirtViewerSession *session,
> GdkRectangle *monitors, guint nmonitors)
> +virt_viewer_session_spice_apply_monitor_geometry(VirtViewerSession *session,
> GHashTable *monitors)
> {
> - guint i;
> + GHashTableIter iter;
> + gpointer key = NULL, value = NULL;
> VirtViewerSessionSpice *self = VIRT_VIEWER_SESSION_SPICE(session);
>
> - for (i = 0; i < nmonitors; i++) {
> - GdkRectangle* rect = &monitors[i];
> + g_hash_table_iter_init(&iter, monitors);
> + while (g_hash_table_iter_next(&iter, &key, &value)) {
> + gint i = GPOINTER_TO_INT(key);
> + GdkRectangle* rect = value;
>
> spice_main_set_display(self->priv->main_channel, i, rect->x,
> rect->y, rect->width, rect->height);
> diff --git a/src/virt-viewer-session.c b/src/virt-viewer-session.c
> index 9405281..2699f41 100644
> --- a/src/virt-viewer-session.c
> +++ b/src/virt-viewer-session.c
> @@ -404,49 +404,35 @@
> virt_viewer_session_on_monitor_geometry_changed(VirtViewerSession* self,
> {
> VirtViewerSessionClass *klass;
> gboolean all_fullscreen = TRUE;
> - guint nmonitors = 0;
> - GdkRectangle *monitors = NULL;
> + /* GHashTable<gint, GdkRectangle*> */
> + GHashTable *monitors = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_free);
> GList *l;
>
> klass = VIRT_VIEWER_SESSION_GET_CLASS(self);
> if (!klass->apply_monitor_geometry)
> return;
>
> - /* find highest monitor ID so we can create the sparse array */
> for (l = self->priv->displays; l; l = l->next) {
> VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
> guint nth = 0;
> - g_object_get(d, "nth-display", &nth, NULL);
> -
> - nmonitors = MAX(nth + 1, nmonitors);
> - }
> -
> - if (nmonitors == 0)
> - return;
> -
> - monitors = g_new0(GdkRectangle, nmonitors);
> - for (l = self->priv->displays; l; l = l->next) {
> - VirtViewerDisplay *d = VIRT_VIEWER_DISPLAY(l->data);
> - guint nth = 0;
> - GdkRectangle *rect = NULL;
> + GdkRectangle *rect = g_new0(GdkRectangle, 1);
>
> 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;
> + g_hash_table_insert(monitors, GINT_TO_POINTER(nth), rect);
> }
>
> if (!all_fullscreen)
> - virt_viewer_align_monitors_linear(monitors, nmonitors);
> + virt_viewer_align_monitors_linear(monitors);
>
> - virt_viewer_shift_monitors_to_origin(monitors, nmonitors);
> + virt_viewer_shift_monitors_to_origin(monitors);
>
> - klass->apply_monitor_geometry(self, monitors, nmonitors);
> - g_free(monitors);
> + klass->apply_monitor_geometry(self, monitors);
> + g_hash_table_unref(monitors);
> }
>
> void virt_viewer_session_add_display(VirtViewerSession *session,
> diff --git a/src/virt-viewer-session.h b/src/virt-viewer-session.h
> index 85f17cc..d3a9ccc 100644
> --- a/src/virt-viewer-session.h
> +++ b/src/virt-viewer-session.h
> @@ -94,7 +94,8 @@ struct _VirtViewerSessionClass {
> void (*session_cut_text)(VirtViewerSession *session, const gchar *str);
> void (*session_bell)(VirtViewerSession *session);
> void (*session_cancelled)(VirtViewerSession *session);
> - void (*apply_monitor_geometry)(VirtViewerSession *session, GdkRectangle*
> monitors, guint nmonitors);
> + /* monitors = GHashTable<int, GdkRectangle*> */
> + void (*apply_monitor_geometry)(VirtViewerSession *session, GHashTable*
> monitors);
> gboolean (*can_share_folder)(VirtViewerSession *session);
> gboolean (*can_retry_auth)(VirtViewerSession *session);
> };
> diff --git a/src/virt-viewer-util.c b/src/virt-viewer-util.c
> index 19a475e..e9f771b 100644
> --- a/src/virt-viewer-util.c
> +++ b/src/virt-viewer-util.c
> @@ -536,11 +536,11 @@ static int
> displays_cmp(const void *p1, const void *p2, gpointer user_data)
> {
> guint diff;
> - GdkRectangle *displays = user_data;
> + GHashTable *displays = user_data;
> guint i = *(guint*)p1;
> guint j = *(guint*)p2;
> - GdkRectangle *m1 = &displays[i];
> - GdkRectangle *m2 = &displays[j];
> + GdkRectangle *m1 = g_hash_table_lookup(displays, GINT_TO_POINTER(i));
> + GdkRectangle *m2 = g_hash_table_lookup(displays, GINT_TO_POINTER(j));
> diff = m1->x - m2->x;
> if (diff == 0)
> diff = m1->y - m2->y;
> @@ -550,28 +550,44 @@ displays_cmp(const void *p1, const void *p2, gpointer
> user_data)
> return diff;
> }
>
> +static void find_max_id(gpointer key,
> + gpointer value G_GNUC_UNUSED,
> + gpointer user_data)
> +{
> + guint *max_id = user_data;
> + guint id = GPOINTER_TO_INT(key);
> + *max_id = MAX(*max_id, id);
> +}
> +
> void
> -virt_viewer_align_monitors_linear(GdkRectangle *displays, guint ndisplays)
> +virt_viewer_align_monitors_linear(GHashTable *displays)
> {
> gint i, x = 0;
> guint *sorted_displays;
> + guint max_id = 0;
> + GHashTableIter iter;
> + gpointer key, value;
>
> g_return_if_fail(displays != NULL);
>
> - if (ndisplays == 0)
> + if (g_hash_table_size(displays) == 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);
> + g_hash_table_foreach(displays, find_max_id, &max_id);
> + sorted_displays = g_new0(guint, max_id);
> +
> + g_hash_table_iter_init(&iter, displays);
> + while (g_hash_table_iter_next(&iter, &key, &value))
> + sorted_displays[GPOINTER_TO_INT(key)] = GPOINTER_TO_INT(key);
> +
> + g_qsort_with_data(sorted_displays, max_id, sizeof(guint), displays_cmp,
> displays);
>
> /* adjust monitor positions so that there's no gaps or overlap between
> * monitors */
> - for (i = 0; i < ndisplays; i++) {
> + for (i = 0; i < max_id; i++) {
> guint nth = sorted_displays[i];
> - g_assert(nth < ndisplays);
> - GdkRectangle *rect = &displays[nth];
> + g_assert(nth < max_id);
> + GdkRectangle *rect = g_hash_table_lookup(displays,
> GINT_TO_POINTER(nth));
> rect->x = x;
> rect->y = 0;
> x += rect->width;
> @@ -591,16 +607,19 @@ virt_viewer_align_monitors_linear(GdkRectangle
> *displays, guint ndisplays)
> * screen of that size.
> */
> void
> -virt_viewer_shift_monitors_to_origin(GdkRectangle *displays, guint ndisplays)
> +virt_viewer_shift_monitors_to_origin(GHashTable *displays)
> {
> gint xmin = G_MAXINT;
> gint ymin = G_MAXINT;
> - gint i;
> + GHashTableIter iter;
> + gpointer key, value;
>
> - g_return_if_fail(ndisplays > 0);
> + if (g_hash_table_size(displays) == 0)
> + return;
>
> - for (i = 0; i < ndisplays; i++) {
> - GdkRectangle *display = &displays[i];
> + g_hash_table_iter_init(&iter, displays);
> + while (g_hash_table_iter_next(&iter, &key, &value)) {
> + GdkRectangle *display = value;
> if (display->width > 0 && display->height > 0) {
> xmin = MIN(xmin, display->x);
> ymin = MIN(ymin, display->y);
> @@ -610,8 +629,9 @@ virt_viewer_shift_monitors_to_origin(GdkRectangle
> *displays, guint ndisplays)
>
> 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];
> + g_hash_table_iter_init(&iter, displays);
> + while (g_hash_table_iter_next(&iter, &key, &value)) {
> + GdkRectangle *display = value;
> if (display->width > 0 && display->height > 0) {
> display->x -= xmin;
> display->y -= ymin;
> diff --git a/src/virt-viewer-util.h b/src/virt-viewer-util.h
> index 98badd2..f1cb08b 100644
> --- a/src/virt-viewer-util.h
> +++ b/src/virt-viewer-util.h
> @@ -57,8 +57,8 @@ gchar* spice_hotkey_to_gtk_accelerator(const gchar *key);
> gint virt_viewer_compare_buildid(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);
> +void virt_viewer_align_monitors_linear(GHashTable *displays);
> +void virt_viewer_shift_monitors_to_origin(GHashTable *displays);
>
> #endif
>
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
