[Spice-devel] [spice-gtk PATCH] Keep internal monitor config up-to-date

[Jonathon Jongsma]

On Fri, 2015-04-10 at 11:39 -0500, Jonathon Jongsma wrote:
> On Fri, 2015-04-10 at 15:34 +0200, Marc-André Lureau wrote:
> > 
> > On Thu, Apr 9, 2015 at 11:40 PM, Jonathon Jongsma
> > <jjongsma at redhat.com> wrote:
> >         When the server sends a new monitors config message to the
> >         client, it
> >         comes through the display channel. However, when the client
> >         wants to
> >         change the monitor config on the server, it does so via the
> >         main channel
> >         (i.e. the agent). The main channel keeps a set of structs that
> >         represent
> >         the current state each display. An application can update a
> >         single 
> >         
> >         display by calling spice_main_update_display(). The main
> >         channel will
> >         update that particular struct and then schedule a new
> >         MonitorsConfig
> >         message based on the values of all of these structs. This
> >         generally
> >         works fine, but it relies on the application to remember to
> >         update each
> >         display properly. It would be less error-prone if spice-gtk
> >         tracked
> >         incoming display updates from the server rather than having
> >         spice-gtk
> >         pass them up to the application and have the application pass
> >         them back
> >         down.
> >         
> > 
> > 
> > That's simply because we shouldn't mix (unless it is somehow safe
> > enough) what is the monitor config requested (main channel config),
> > and what is the actual monitor config (display channels)
> > 
> > 
> >         
> >         Note that when spice-gtk updates its internal state, it does
> >         not trigger
> >         a new monitor configuration to be sent down to the server. If
> >         the
> >         application wants to send down a new configuration, it still
> >         has to call
> >         spice_main_update_display() as before. This change simply
> >         means that the
> >         application won't need to call this function in response to
> >         informational updates from the server.
> > 
> > 
> > It would change the user requested config if a display channel monitor
> > config would reset it (on main channel) and the user would set/resize
> > another monitor and send the last config for example.
> > 
> > 
> > 
> > nack
> 
> 
> If you want it this way that's fine. But I think that it would be
> perfectly reasonable (and in fact would probably result in more expected
> behavior) to say that once we get a display configuration update from
> the server, all previous requests are "reset". If the app wishes to
> request a different size for a particular display, it must expicitly
> request it after the last display update, rather than using a
> configuration request that was set some time in the past and never
> updated. I cannot think of a situation where this behavior would be
> desired:
> 
> - client requests display 1 and 2 to be both 800x600
> - server sends display update with display 1 and 2 at 800x600
> - For whatever reason (perhaps resolution was change within the guest
> control panel), the server sends a new display update with both displays
> at 1280x720
> - client windows are resized to 1280x720
> - user maximizes display 1 (e.g. 1600x900) which causes app to tell
> spice-gtk to request new size for display 1
> - spice-gtk sends down new monitor configuration request: display 1 =
> 1600x900, display 2 = 800x600
> 
> I think most people would expect display 2 to remain at 1280x720 rather
> than shrinking unexpectedly to 800x600.  
> 
> (And, yes, you could achieve this by making sure that the app calls into
> spice-gtk to tell spice-gtk that your display size changed every time
> that spice-gtk tells the app that the the display size changed, but this
> is both more verbose and more prone to error when the app doesn't do
> this properly.)
> 
> Jonathon


To be clear, the above is mostly an explanation for why I sent the patch
(and why I still don't think it would be harmful). I don't feel
particularly strongly about it, so if you're opposed, I'm perfectly
happy to drop it. But I thought it was worth a proposal.

Jonathon