[virt-tools-list] [PATCH virt-viewer 2/2] RFC: resize: simplify and isolate fullscreen aspect logic

Wed Mar 12 11:56:07 UTC 2014

----- Original Message -----
> On Tue, Mar 11, 2014 at 09:49:12PM +0100, Marc-André Lureau wrote:
> > On Tue, Mar 11, 2014 at 5:18 PM, Daniel P. Berrange <berrange at redhat.com>
> > wrote:
> > > On Tue, Mar 11, 2014 at 04:38:46PM +0100, Marc-André Lureau wrote:
> > >> From: Marc-André Lureau <marcandre.lureau at redhat.com>
> > >>
> > >> Tbh, I don't understand the purpose of this code (both spice and vnc
> > >> widgets keep the aspect ration), I would welcome some comments. I am not
> > >> sure why we check fullscreen display resolution in window mode either,
> > >> so I moved the code in if (priv->fullscreen)
> > >>
> > >> If it's possible, this code should be removed (surrouding with if 0
> > >> doesn't seem to change anything here with spice or vnc). Please help me
> > >> to understand that logic.
> > >> ---
> > >>  src/virt-viewer-window.c | 63
> > >>  +++++++++++++++++++++++-------------------------
> > >>  1 file changed, 30 insertions(+), 33 deletions(-)
> > >
> > > When the guest resolution exceeds the host resolution we need to scale
> > > to ensure we can see the full extent of the guest desktop. If I remove
> > > this code, then as you say, there doesn't appear to be any functional
> > > impact. When the guest resizes, the window resizes sensibly every time,
> > > scaling when too large.
> > > The caveat is that this only works corretly with GTK-3.0 If I build
> > > virt-viewer for GTK-2.0 then the window size isn't limited by GTK
> > > and exceeds the size of the desktop. So this code is basically
> > > working around a limitation fo GTK-2.0
> > 
> > Ok, I am not sure that auto-scaling behaviour is always desirable
> > though... If I explicitely change the resolution in the guest to
> > something higher that doesn't fit on screen, it will be reverted back
> > with Spice, because the main window is resized to fit the screen.
> >
> > Here, I just maximized, and I would want the guest resolution to be
> > 1600x776, to get 1:1, no scaling necessary. But the guest resolution
> > is changed to 1472x713, with annoying scale bluriness:
> > 
> >  Decided todo 1472x713 (desktop is 1600x776, fullscreen is 1600x900
> > 
> > (this is not specific to maximizing, as you can imagine, but this
> > makes the problem even more annoying)
> > 
> > I don't see how we could have both this auto-scaling code to fit
> > display, and 1:1 resolution.
> >
> > In the case of Spice, we should not need any scaling (unless there is
> > a zoom factor), because guest can be resized to whatever we need.
> > 
> > I would ask that given the current code is broken, and causes some
> > extra pain, we disable/comment that logic until we have a solution.
> 
> First of all we can't assume that automatic guest resize always
> works. Even if you have SPICE + QXL present we can't assume it
> since the guest agent isn't ported to every guest OS a user might
> have, not to mention use of VNC which lacks this entirely. Even
> when it does work, we can't assume that users actually always want
> that enabled.
> 
> The auto-scaling code is intended to only be used when we know
> the guest resolution is too large for the host display. At that
> point you have a choice of resizing the guest display, having
> part of the guest display be off-screen in the host, or scaling
> the guest display. If SPICE guest agent is enabled, then the
> guest display will resize so we have no problem to worry about.
> So we're really talking about the choice between having part of
> the guest display be offscreen or scaling the result. It is a
> no brainer that we want to scale the result in that case.

This is also a no-brainer that user can zoom the display manually.

> 
> > > That said, this is stil broken as I mentioned wrt your previous
> > > patch. I tested this patch which appeared to make it work properly
> > > with GTK-2
> > >
> > > diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> > > index 05d5fe7..c2551d4 100644
> > > --- a/src/virt-viewer-window.c
> > > +++ b/src/virt-viewer-window.c
> > > @@ -446,11 +446,11 @@ virt_viewer_window_resize(VirtViewerWindow *self,
> > > gboolean keep_win_size)
> > >          /* Doesn't fit native res, so go as large as possible
> > >             maintaining aspect ratio */
> > >          if (screenAspect > desktopAspect) {
> > > -            width = desktopHeight * desktopAspect;
> > > -            height = desktopHeight;
> > > +            width = (fullscreen.height - 128) * desktopAspect;
> > > +            height = (fullscreen.height - 128);
> > >          } else {
> > > -            width = desktopWidth;
> > > -            height = desktopWidth / desktopAspect;
> > > +            width = (fullscreen.width - 128);
> > > +            height = (fullscreen.width - 128) / desktopAspect;
> > >          }
> > 
> > However, when there are rounding issues between widget size and
> > desktop size, you get nasty blurry effects.
> 
> Of course, but the alternative is that part of the guest display
> is invisible since it is off screen in the host display. If you
> don't want scaling then make sure the guest display is small
> enough to actually fit in the host display resoltion.
> 
> I for one do actually make use of the case where the guest display
> is greater than the host display in some of my guests, where I have
> (poorly designed) applications which actually need a higher resolution
> than my host has in order to fit all their GUI on screen. If I allowed
> SPICE to auto-change the guest resolution it wouldn't help because then
> the app wouldn't fit in the guest display, let alone host. Allowing
> scaling makes it possible for me to use these applications in the least
> sucky manner.

I understand scaling is useful, but that auto-scaling code is quite broken and hacky currently. And it gets even worse with the change you propose.

All I would like is to have a configured Spice guest to not be scaled automatically if the window fits on the screen. With the current code (and the change you proposed), this doesn't work.

See also https://bugzilla.redhat.com/show_bug.cgi?id=1056041 (rhel6 high/high)

> Regards,
> Daniel
> --
> |: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
> |: http://libvirt.org              -o-             http://virt-manager.org :|
> |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
> |: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|
> 