Hi Jonathon, > > NACK. I'm quite sure that this is essentially the same problem I've been > trying to solve in https://bugzilla.redhat.com/show_bug.cgi?id=1200750. > Comments below. > > > On Fri, 2015-04-24 at 17:12 +0200, Pavel Grunt wrote: > > When running in fullscreen it is possible to end up in a situation > > where we have more displays enabled than monitors. This can happen > > if displays that were enabled in the previous connection to the guest > > doesn't match displays we request when entering the fullscreen mode. > > In other words when running in the fullscreen mode we request enabling > > displays #1, #2 if we have two monitors. However at the moment the > > first "notify::monitors" is recieved, the displays that were used in > > the previous session (e.g. #1, #3) are enabled. As we leave fullscreen > > the display #3 shows up. > > > > This commit solves the problem by marking requested displays as > > fullscreen and only these can be enabled in the fullscreen mode. > > > > Resolves: rhbz#1212802 > > --- > > I'm not sure wheter this can have impact on rhbz#1200750 > > Thanks in advance for any comments. > > It might be interesting to test this bug with my (rejected) patches for > rhbz#1200750 applied to see if it fixes this issue. If it does, then I > think that it's safe to assume that the root cause is the same, even > though we don't have an acceptable fix for that bug yet. > I started working on it, because your patches for rhbz#1200750 didn't affect this bug. > > --- > > src/virt-viewer-session-spice.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/src/virt-viewer-session-spice.c > > b/src/virt-viewer-session-spice.c > > index b69faa6..326dfd9 100644 > > --- a/src/virt-viewer-session-spice.c > > +++ b/src/virt-viewer-session-spice.c > > @@ -58,6 +58,7 @@ struct _VirtViewerSessionSpicePrivate { > > gboolean has_sw_smartcard_reader; > > guint pass_try; > > gboolean did_auto_conf; > > + guint fullscreen_displays; > > }; > > > > #define VIRT_VIEWER_SESSION_SPICE_GET_PRIVATE(o) > > (G_TYPE_INSTANCE_GET_PRIVATE((o), VIRT_VIEWER_TYPE_SESSION_SPICE, > > VirtViewerSessionSpicePrivate)) > > @@ -702,6 +703,8 @@ virt_viewer_session_spice_display_monitors(SpiceChannel > > *channel, > > GPtrArray *displays = NULL; > > GtkWidget *display; > > guint i, monitors_max; > > + gboolean fullscreen_mode = > > + > > virt_viewer_app_get_fullscreen(virt_viewer_session_get_app(VIRT_VIEWER_SESSION(self))); > > > > g_object_get(channel, > > "monitors", &monitors, > > @@ -726,6 +729,11 @@ > > virt_viewer_session_spice_display_monitors(SpiceChannel *channel, > > display = virt_viewer_display_spice_new(self, channel, i); > > g_debug("creating spice display (#:%d)", i); > > g_ptr_array_index(displays, i) = g_object_ref_sink(display); > > + if (fullscreen_mode) { > > + gint nth = > > virt_viewer_display_get_nth(VIRT_VIEWER_DISPLAY(display)); > > + > > virt_viewer_display_set_fullscreen(VIRT_VIEWER_DISPLAY(display), > > + nth < > > self->priv->fullscreen_displays); > > This is not correct. Due to user-specified fullscreen configuration, > it's possible to have e.g. 2 fullscreen displays enabled that are not > sequential: #1 and #3. So checking that nth is less than the number of > displays is not a valid solution. > My bad, thank you. > > + } > > virt_viewer_session_add_display(VIRT_VIEWER_SESSION(self), > > VIRT_VIEWER_DISPLAY(display)); > > } > > @@ -739,6 +747,12 @@ > > virt_viewer_session_spice_display_monitors(SpiceChannel *channel, > > if (monitor->width == 0 || monitor->height == 0) > > continue; > > > > + if (fullscreen_mode && > > !virt_viewer_display_get_fullscreen(VIRT_VIEWER_DISPLAY(display))) { > > + g_debug("display #%d is not fullscreen, ignoring", > > + > > virt_viewer_display_get_nth(VIRT_VIEWER_DISPLAY(display))); > > + continue; > > + } > > + > > I don't think that this is really an acceptable solution. What you seem > to be doing here is acknowledging that the guest display N is enabled, > but you're just ignoring it rather than showing it to the user. That > leaves an extra display (which may contain application windows, etc) > enabled on the guest but inaccessible to the user. This would be very > confusing to the user who wonders why they can't access one of their > applications that they know is open. It's even possible that the hidden > display might be the one with the start menu on it. > Right, I should make sure that the display will be disabled on the guest side. I probably didn't have the problem you described because the display was set disabled when the monitor-geometry-changed. > I think that we need the comprehensive fix that I've been struggling to > find for rhbz#1200750, assuming that these are related. But even if > these bugs are not related, I'm afraid this is not the right solution > here. > My assumption was that if we know which displays should be disabled, we can keep them disabled. Thanks a lot for your comments, Pavel _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
