Hey, On Tue, Sep 23, 2014 at 04:57:41PM -0500, Jonathon Jongsma wrote: > When using the fullscreen display mapping configuration file, extra > monitors could end up enabled by mistake. This was because > virt_viewer_app_get_initial_monitor_for_display would end up returning > Nmonitor = Ndisplay when the display map hash lookup failed. In > reality, when a display map is specified, but the hash lookup fails, > the display should not be enabled. This function now returns -1 to > distinguish this case, and the display is not enabled when this value is > returned. > --- > > The implementation was apparently incomplete. This patch attempts to fix an > issue that was found during testing: > see https://bugzilla.redhat.com/show_bug.cgi?id=1129477#c9 > > src/virt-viewer-app.c | 22 ++++++++++++---------- > src/virt-viewer-session-spice.c | 3 +++ > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/src/virt-viewer-app.c b/src/virt-viewer-app.c > index a890bf6..12d01eb 100644 > --- a/src/virt-viewer-app.c > +++ b/src/virt-viewer-app.c > @@ -314,8 +314,11 @@ gint > virt_viewer_app_get_initial_monitor_for_display(VirtViewerApp* self, gint d > > if (self->priv->initial_display_map) { > gpointer value = NULL; > - if (g_hash_table_lookup_extended(self->priv->initial_display_map, > GINT_TO_POINTER(display), NULL, &value)) > + if (g_hash_table_lookup_extended(self->priv->initial_display_map, > GINT_TO_POINTER(display), NULL, &value)) { > monitor = GPOINTER_TO_INT(value); > + } else { > + monitor = -1; > + } > }
My understanding is that self->priv->initial_display_map is only set
when a display configuration file is in used?
>
> return monitor;
> @@ -326,13 +329,14 @@ app_window_try_fullscreen(VirtViewerApp *self
> G_GNUC_UNUSED,
> VirtViewerWindow *win, gint nth)
> {
> GdkScreen *screen = gdk_screen_get_default();
> -
> - if (nth >= gdk_screen_get_n_monitors(screen)) {
> - g_debug("skipping display %d", nth);
> + gint monitor = virt_viewer_app_get_initial_monitor_for_display(self,
> nth);
> + if (monitor == -1 || monitor >= gdk_screen_get_n_monitors(screen)) {
> + g_debug("skipping fullscreen for display %d", nth);
> + virt_viewer_window_hide(win);
> return;
> }
>
> - virt_viewer_window_enter_fullscreen(win, nth);
> + virt_viewer_window_enter_fullscreen(win, monitor);
> }
>
>
> @@ -449,8 +453,7 @@ void virt_viewer_app_set_uuid_string(VirtViewerApp *self,
> const gchar *uuid_stri
>
> g_hash_table_iter_init(&iter, self->priv->windows);
> while (g_hash_table_iter_next(&iter, NULL, &value)) {
> - gint monitor =
> virt_viewer_app_get_initial_monitor_for_display(self, i);
> - app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value),
> monitor);
> + app_window_try_fullscreen(self, VIRT_VIEWER_WINDOW(value), i);
> i++;
> }
For what it's worth, the move of
virt_viewer_app_get_initial_monitor_for_display() in
app_window_try_fullscreen() could have been done in a preparatory
commit.
ACK if my understanding about initial_display_map is correct.
Christophe
pgp3katLoKbMQ.pgp
Description: PGP signature
_______________________________________________ virt-tools-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/virt-tools-list
