[Spice-commits] 4 commits - gtk/channel-main.c gtk/spice-widget.c spice-common

Hans de Goede jwrdegoede at kemper.freedesktop.org
Sat Jan 19 00:41:35 PST 2013


 gtk/channel-main.c |   51 +++++++++++++++++++++++++++++++++++++--------------
 gtk/spice-widget.c |   15 +++++++++++----
 spice-common       |    2 +-
 3 files changed, 49 insertions(+), 19 deletions(-)

New commits:
commit c62c5af6e6bb3144fa3952221a03db90feb9aeab
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Jan 18 16:01:50 2013 +0100

    channel-main: Fix monitors_align to not shuffle monitor order
    
    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>

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);
 }
 
 
commit 9d80356963c8542e7a8fff6015e6335f07295d2e
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Thu Jan 10 23:42:18 2013 +0100

    channel-main: Add support for VD_AGENT_CAP_SPARSE_MONITORS_CONFIG (rhbz#881072)
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/channel-main.c b/gtk/channel-main.c
index 706c119..dfd2245 100644
--- a/gtk/channel-main.c
+++ b/gtk/channel-main.c
@@ -1019,11 +1019,15 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
     c = channel->priv;
     g_return_val_if_fail(c->agent_connected, FALSE);
 
-    monitors = 0;
-    /* FIXME: fix MonitorConfig to be per display */
-    for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
-        if (c->display[i].enabled)
-            monitors += 1;
+    if (spice_main_agent_test_capability(channel,
+                                     VD_AGENT_CAP_SPARSE_MONITORS_CONFIG)) {
+        monitors = SPICE_N_ELEMENTS(c->display);
+    } else {
+        monitors = 0;
+        for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
+            if (c->display[i].enabled)
+                monitors += 1;
+        }
     }
 
     size = sizeof(VDAgentMonitorsConfig) + sizeof(VDAgentMonConfig) * monitors;
@@ -1036,8 +1040,12 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
 
     j = 0;
     for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
-        if (!c->display[i].enabled)
+        if (!c->display[i].enabled) {
+            if (spice_main_agent_test_capability(channel,
+                                     VD_AGENT_CAP_SPARSE_MONITORS_CONFIG))
+                j++;
             continue;
+        }
         mon->monitors[j].depth  = c->display_color_depth ? c->display_color_depth : 32;
         mon->monitors[j].width  = c->display[j].width;
         mon->monitors[j].height = c->display[j].height;
diff --git a/spice-common b/spice-common
index 81f40cc..8a10919 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit 81f40cca5f930bb256da62760859ac802f11b3a7
+Subproject commit 8a10919658950aa600bd5fcaf12c28b026fd70ad
commit 56460d3c7e44707c9037829ee76cbb2e0e37c49e
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Jan 18 16:24:28 2013 +0100

    spice-widget: update_monitor_area: Fix memory-leak on whole fallback
    
    When we've successfully gotten the monitors display-channel property, but
    still end up falling back to whole-display mode, we still need to free
    the monitors array.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index f1628d9..81670c3 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -302,6 +302,7 @@ static void update_monitor_area(SpiceDisplay *display)
     return;
 
 whole:
+    g_clear_pointer(&monitors, g_array_unref);
     /* by display whole surface */
     update_area(display, 0, 0, d->width, d->height);
     set_monitor_ready(display, true);
commit 5a54a6b00612a558f95ef4744315d3d7ec380442
Author: Hans de Goede <hdegoede at redhat.com>
Date:   Fri Jan 18 14:16:03 2013 +0100

    spice-widget: Search monitor info by display monitor-id
    
    As discussed indexing the display-channel's monitors property by monitor-id
    causes problems with the vdagent's new sparse monitor config support.
    
    Searching the monitors property by monitor-id avoids these problems.
    
    Signed-off-by: Hans de Goede <hdegoede at redhat.com>

diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
index 9cfb683..f1628d9 100644
--- a/gtk/spice-widget.c
+++ b/gtk/spice-widget.c
@@ -265,15 +265,23 @@ static void set_monitor_ready(SpiceDisplay *self, gboolean ready)
 static void update_monitor_area(SpiceDisplay *display)
 {
     SpiceDisplayPrivate *d = SPICE_DISPLAY_GET_PRIVATE(display);
-    SpiceDisplayMonitorConfig *c;
+    SpiceDisplayMonitorConfig *cfg, *c = NULL;
     GArray *monitors = NULL;
+    int i;
 
     SPICE_DEBUG("update monitor area %d:%d", d->channel_id, d->monitor_id);
     if (d->monitor_id < 0)
         goto whole;
 
     g_object_get(d->display, "monitors", &monitors, NULL);
-    if (monitors == NULL || d->monitor_id >= monitors->len) {
+    for (i = 0; monitors != NULL && i < monitors->len; i++) {
+        cfg = &g_array_index(monitors, SpiceDisplayMonitorConfig, i);
+        if (cfg->id == d->monitor_id) {
+           c = cfg;
+           break;
+        }
+    }
+    if (c == NULL) {
         SPICE_DEBUG("update monitor: no monitor %d", d->monitor_id);
         set_monitor_ready(display, false);
         if (spice_channel_test_capability(d->display, SPICE_DISPLAY_CAP_MONITORS_CONFIG)) {
@@ -283,8 +291,6 @@ static void update_monitor_area(SpiceDisplay *display)
         goto whole;
     }
 
-    c = &g_array_index(monitors, SpiceDisplayMonitorConfig, d->monitor_id);
-    g_return_if_fail(c != NULL);
     if (c->surface_id != 0) {
         g_warning("FIXME: only support monitor config with primary surface 0, "
                   "but given config surface %d", c->surface_id);


More information about the Spice-commits mailing list