Yeah, as I mentioned in an email just a few minutes ago, I do still have the same concern, but it's a minor one and arguably a very rare corner case. So probably it shouldn't hold up this patch. On Wed, 2016-03-23 at 20:39 +0100, Pavel Grunt wrote: > Hi, > > So the timer is the main problem ? > > It also solves/avoids problems with a guest running on wayland when the > "resize-guest" property is TRUE. > See bug https://bugzilla.redhat.com/show_bug.cgi?id=1266484 ;; > It avoids destroying the primary surface when the display configuration > has not changed hmm. so, the suggestion from my previous email (only compare with previous config and return early from update_display() if there's currently a pending config message scheduled) would probably break this case again (if I understand the situation correctly). Do you know *why* we're repeatedly updating to the same monitor config under wayland? > > I would like to hear from Jonathon what he thinks about it. There were > some concerns / discussion whether it should be compared to the "real" > state of displays in the guest. > https://lists.freedesktop.org/archives/spice-devel/2015-October/022785. > html > > Maybe some check should be done on the server side because it is > "closer" to the guest. I was not advocating doing anything on the server side. What I had in mind was to simply have spice-gtk cache the last received monitor config from the server. And use this cached value to determine whether the new config is the same as the current setting. > > Pavel > > On Wed, 2016-03-23 at 18:02 +0100, Marc-André Lureau wrote: > > With virgl, set_monitor_ready() may be called each time the scanout > > is > > updated to set the monitor area. This will call > > spice_main_update_display(), and keep the timer postponed even if the > > monitor configuration didn't change. Treat unchanged configuration > > has a > > no-op and keep configuration timer unchanged. This fixes monitor > > autoconfig with virgl (when the display is regularly updated). > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > > --- > > src/channel-main.c | 29 ++++++++++++++++++----------- > > 1 file changed, 18 insertions(+), 11 deletions(-) > > > > diff --git a/src/channel-main.c b/src/channel-main.c > > index 1c19de1..b4875f6 100644 > > --- a/src/channel-main.c > > +++ b/src/channel-main.c > > @@ -121,6 +121,14 @@ typedef enum { > > DISPLAY_ENABLED, > > } SpiceDisplayState; > > > > +typedef struct { > > + int x; > > + int y; > > + int width; > > + int height; > > + SpiceDisplayState display_state; > > +} SpiceDisplayConfig; > > + > > struct _SpiceMainChannelPrivate { > > enum SpiceMouseMode mouse_mode; > > bool agent_connected; > > @@ -140,13 +148,7 @@ 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]; > > + SpiceDisplayConfig display[MAX_DISPLAY]; > > gint timer_id; > > GQueue *agent_msg_queue; > > GHashTable *file_xfer_tasks; > > @@ -2686,10 +2688,15 @@ 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; > > + SpiceDisplayConfig display = { > > + .x = x, .y = y, .width = width, .height = height, > > + .display_state = c->display[id].display_state > > + }; > > + > > + if (memcmp(&display, &c->display[id], > > sizeof(SpiceDisplayConfig)) == 0) > > + return; > > + > > + c->display[id] = display; > > > > if (update) > > update_display_timer(channel, 1); _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel