[Spice-devel] [PATCH spice-gtk] channel-main: Fix monitors_align to not shuffle monitor order

[Hans de Goede]

Before this patch monitor_align was calling qsort directly on the
VDAgentMonConfig monitors array, but VDAgentMonConfig does not contain
an id, so the order matters!

This fixes (for example) 2 issues with having 3 windows/monitors on a row
numbered 1-3, ordered left-to-right as 1-2-3, and then changing the
ordering to 1-3-2:
1) Window 3 would be resized to the size of window 2, and window 2 would
   get resized to the size of window 3.
2) Dragging a window on monitor 1 over its right edge, makes the part over
   the edge show up on the right monitor, rather then on the middle.
This is happening because the agent is configuring qxl-1 (which is monitor 2)
with the monitors[1] data, which after the qsort contains the size and
coordinates of monitor 3.

Note this only happens with virt-viewer fixed to properly send window
coordinates, as before that all monitors had x and y set to 0 making the
sort a nop.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 gtk/channel-main.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index dfd2245..a64aa89 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -48,7 +48,7 @@
 #define SPICE_MAIN_CHANNEL_GET_PRIVATE(obj)                             \
     (G_TYPE_INSTANCE_GET_PRIVATE((obj), SPICE_TYPE_MAIN_CHANNEL, SpiceMainChannelPrivate))
 
-#define MAX_DISPLAY 16
+#define MAX_DISPLAY 16 /* Note must fit in a guint32, see monitors_align */
 
 typedef struct spice_migrate spice_migrate;
 
@@ -977,22 +977,37 @@ static int monitors_cmp(const void *p1, const void *p2)
 
 static void monitors_align(VDAgentMonConfig *monitors, int nmonitors)
 {
-    gint i, x = 0;
+    gint i, j, x = 0;
+    guint32 used = 0;
+    VDAgentMonConfig *sorted_monitors;
 
     if (nmonitors == 0)
         return;
 
     /* sort by distance from origin */
-    qsort(monitors, nmonitors, sizeof(VDAgentMonConfig), monitors_cmp);
+    sorted_monitors = g_memdup(monitors, nmonitors * sizeof(VDAgentMonConfig));
+    qsort(sorted_monitors, nmonitors, sizeof(VDAgentMonConfig), monitors_cmp);
 
     /* super-KISS ltr alignment, feel free to improve */
     for (i = 0; i < nmonitors; i++) {
-        monitors[i].x = x;
-        monitors[i].y = 0;
-        x += monitors[i].width;
-        g_debug("#%d +%d+%d-%dx%d", i, monitors[i].x, monitors[i].y,
-                monitors[i].width, monitors[i].height);
-    }
+        /* Find where this monitor is in the sorted order */
+        for (j = 0; j < nmonitors; j++) {
+            /* Avoid using the same entry twice, this happens with older
+               virt-viewer versions which always set x and y to 0 */
+            if (used & (1 << j))
+                continue;
+            if (memcmp(&monitors[j], &sorted_monitors[i],
+                       sizeof(VDAgentMonConfig)) == 0)
+                break;
+        }
+        used |= 1 << j;
+        monitors[j].x = x;
+        monitors[j].y = 0;
+        x += monitors[j].width;
+        g_debug("#%d +%d+%d-%dx%d", j, monitors[j].x, monitors[j].y,
+                monitors[j].width, monitors[j].height);
+    }
+    g_free(sorted_monitors);
 }
 
 
-- 
1.8.0.2