[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[virt-tools-list] --fullscreen=auto-conf issues



I don't know if anybody else has noticed, but --fullscreen=auto-conf is broken
in git. For example, in a two-monitor case, what happens is that two display
windows are created, and they are fullscreened on each monitor, but the
resolution does not match the window size.  They both tend to be very
low-resolution displays that are then zoomed up to fill the full screen.

I've been investigating the cause, and it turns out that the cause is a fairly
complex set of interactions. First of all, these are the commits that broke this
functionality:

9da9305 - Use display fullscreen state instead of app state
 - Causes the secondary displays to be the wrong resolution
323d85d - spice: if zoom-level is changed, resize guest, even in fullscreen
 - Causes the primary display to be the wrong resolution

The problem with the primary monitor is solved by this earlier patch:

  https://www.redhat.com/archives/virt-tools-list/2013-October/msg00047.html

Ideally we should probably make the code less fragile in response to extraneous
zoom-level change notifications.

The reason that the behavior of the primary display window is different than the
secondary (and following) display windows is because the first window is always
shown (and thus full-screened) before any Spice displays are created. All
following windows are created in response to new monitor notifications from the
spice server. Or, to state it more simply: the first VirtViewerDisplay object is
added to an already-mapped and properly-sized fullscreen window, whereas all
future VirtViewerDisplay objects are added to hidden and unconfigured windows.
This turns out to be quite important.

So, here is the sequence of events that results in the secondary displays having
incorrect resolution:

- There is some functionality in VirtViewerDisplaySpice that attempts to prevent
  itself from sending down a resize command to the guest in response to
  allocations when we are in fullscreen=auto-conf mode. Unfortunately, this
  doesn't work properly because it depends on setting a local state variable
  inside of a "notify::fullscreen" signal handler.
- unfortunately, this "notify::fullscreen" signal handler doesn't get called
  before the first allocation, because during the display setup, there's an
  explicit call to gtk_widget_realize() (which causes an allocation) between
  calls to g_object_freeze_notify() and g_object_thaw_notify(). Therefore, the
  first allocation happens before we can synchronize our fullscreen state
  properly, and as a result, we send a resize command down to the server (with a
  resolution of 100x198).
- When the VirtViewerDisplaySpice object is created, a VirtViewerWindow is
  created for it, and the display is added to this window. The window sets
  various properties on the display to match its own state (e.g. zoom level,
  auto-resize, and fullscreen).  Unfortunately, although we called
  virt_viewer_window_enter_fullscreen() on the newly-created window, it hasn't
  taken effect yet.  when you call virt_viewer_window_enter_fullscreen() on a
  hidden window, it doesn't actually change its priv->fullscreen state variable.
  Instead, it sets up a "map-event" handler which tries to enter fullscreen
  after the window is mapped. So, until the window is displayed, it thinks it's
  not in fullscreen mode.  So, when the window attempts to set display's
  fullscreen mode to match its own fullscreen mode, it passes priv->fullscreen,
  which remains FALSE.

The simplest fix for the issue is show by the attached patches, but I'm afraid
that these small changes could result in regressions in other areas, since this
startup code is rather fragile.  Any thoughts?


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]