[Spice-devel] [PATCH spice-gtk] main: Send monitor config only when it changes
Pavel Grunt
pgrunt at redhat.com
Fri Oct 23 08:25:16 PDT 2015
When the guest recieves the monitor configuration message, it replies
(through spice-server) by destroying the primary surface, which makes
the SpiceDisplay disabled if its "resize-guest" property is used.
This change of the display state (disabled/enabled) leads to sending
a new monitor config message (containing the disabled display) after
a second. Then spice-server sends the monitor configuration message,
spice-gtk replies back and we may end up with a loop of monitor
configuration messages even if no real change of monitor configuration
happens.
To avoid the loop, the new monitor configuration messages are only sent
if they are different from the current monitor configuration.
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=1266484
---
The problem can be seen in spicy, gnome-boxes and virt-manager w/ fedora23 wayland as the guest
---
src/channel-main.c | 67 ++++++++++++++++++++++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 17 deletions(-)
diff --git a/src/channel-main.c b/src/channel-main.c
index 0eb40b9..64d4e4a 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -123,6 +123,14 @@ typedef enum {
DISPLAY_ENABLED,
} SpiceDisplayState;
+typedef struct SpiceMonitor {
+ int x;
+ int y;
+ int width;
+ int height;
+ SpiceDisplayState display_state;
+} SpiceMonitor;
+
struct _SpiceMainChannelPrivate {
enum SpiceMouseMode mouse_mode;
bool agent_connected;
@@ -142,13 +150,8 @@ struct _SpiceMainChannelPrivate {
guint agent_msg_pos;
uint8_t agent_msg_size;
uint32_t agent_caps[VD_AGENT_CAPS_SIZE];
- struct {
- int x;
- int y;
- int width;
- int height;
- SpiceDisplayState display_state;
- } display[MAX_DISPLAY];
+ SpiceMonitor display[MAX_DISPLAY]; /* current configuration of spice-widgets */
+ SpiceMonitor display_to_be_sent[MAX_DISPLAY]; /* new requested configuration */
gint timer_id;
GQueue *agent_msg_queue;
GHashTable *file_xfer_tasks;
@@ -1149,7 +1152,7 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
} else {
monitors = 0;
for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
- if (c->display[i].display_state == DISPLAY_ENABLED)
+ if (c->display_to_be_sent[i].display_state == DISPLAY_ENABLED)
monitors += 1;
}
}
@@ -1165,6 +1168,7 @@ gboolean spice_main_send_monitor_config(SpiceMainChannel *channel)
CHANNEL_DEBUG(channel, "sending new monitors config to guest");
j = 0;
for (i = 0; i < SPICE_N_ELEMENTS(c->display); i++) {
+ c->display[i] = c->display_to_be_sent[i]; /* current display is now display_to_be_sent */
if (c->display[i].display_state != DISPLAY_ENABLED) {
if (spice_main_agent_test_capability(channel,
VD_AGENT_CAP_SPARSE_MONITORS_CONFIG))
@@ -1513,13 +1517,37 @@ static gboolean any_display_has_dimensions(SpiceMainChannel *channel)
c = channel->priv;
for (i = 0; i < MAX_DISPLAY; i++) {
- if (c->display[i].width > 0 && c->display[i].height > 0)
+ if (c->display_to_be_sent[i].width > 0 && c->display_to_be_sent[i].height > 0)
return TRUE;
}
return FALSE;
}
+static gboolean spice_monitor_equal(SpiceMonitor *mon_a, SpiceMonitor *mon_b)
+{
+ return mon_a->x == mon_b->x &&
+ mon_a->y == mon_b->y &&
+ mon_a->width == mon_b->width &&
+ mon_a->height == mon_b->height &&
+ mon_a->display_state == mon_b->display_state;
+}
+
+static gboolean spice_monitor_configuration_changed(SpiceMainChannel *channel)
+{
+ SpiceMainChannelPrivate *c;
+ guint i;
+
+ g_return_val_if_fail(SPICE_IS_MAIN_CHANNEL(channel), FALSE);
+ c = channel->priv;
+
+ for (i = 0; i < MAX_DISPLAY; i++) {
+ if (!spice_monitor_equal(&c->display[i], &c->display_to_be_sent[i]))
+ return TRUE;
+ }
+ return FALSE;
+}
+
/* main context*/
static gboolean timer_set_display(gpointer data)
{
@@ -1532,6 +1560,11 @@ static gboolean timer_set_display(gpointer data)
if (!c->agent_connected)
return FALSE;
+ if (!spice_monitor_configuration_changed(channel)) {
+ SPICE_DEBUG("Not sending monitors config, configuration has not change");
+ return FALSE;
+ }
+
if (!any_display_has_dimensions(channel)) {
SPICE_DEBUG("Not sending monitors config, at least one monitor must have dimensions");
return FALSE;
@@ -1543,7 +1576,7 @@ static gboolean timer_set_display(gpointer data)
/* ensure we have an explicit monitor configuration at least for
number of display channels */
for (i = 0; i < spice_session_get_n_display_channels(session); i++)
- if (c->display[i].display_state == DISPLAY_UNDEFINED) {
+ if (c->display_to_be_sent[i].display_state == DISPLAY_UNDEFINED) {
SPICE_DEBUG("Not sending monitors config, missing monitors");
return FALSE;
}
@@ -2697,10 +2730,10 @@ void spice_main_update_display(SpiceMainChannel *channel, int id,
g_return_if_fail(id < SPICE_N_ELEMENTS(c->display));
- c->display[id].x = x;
- c->display[id].y = y;
- c->display[id].width = width;
- c->display[id].height = height;
+ c->display_to_be_sent[id].x = x;
+ c->display_to_be_sent[id].y = y;
+ c->display_to_be_sent[id].width = width;
+ c->display_to_be_sent[id].height = height;
if (update)
update_display_timer(channel, 1);
@@ -2903,13 +2936,13 @@ void spice_main_update_display_enabled(SpiceMainChannel *channel, int id, gboole
if (id == -1) {
gint i;
for (i = 0; i < G_N_ELEMENTS(c->display); i++) {
- c->display[i].display_state = display_state;
+ c->display_to_be_sent[i].display_state = display_state;
}
} else {
g_return_if_fail(id < G_N_ELEMENTS(c->display));
- if (c->display[id].display_state == display_state)
+ if (c->display_to_be_sent[id].display_state == display_state)
return;
- c->display[id].display_state = display_state;
+ c->display_to_be_sent[id].display_state = display_state;
}
if (update)
--
2.5.0
More information about the Spice-devel
mailing list