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

Jonathon Jongsma jjongsma at redhat.com
Wed Oct 9 20:59:44 UTC 2013



> >
> > The problem with the primary monitor is solved by this earlier patch:
> >
> >   https://www.redhat.com/archives/virt-tools-list/2013-October/msg00047.html
> 
> which is already acked, but you should have put the rationale there,
> not in a different thread.

Yes, I should have mentioned this issue in that patch, sorry about that.

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

Well, the purpose of my statement was to admit that the previous patch wasn't a complete fix. I'd like to analyze how to fix it so that even if an extra zoom-level change notification does get emitted for some reason, it won't cause our auto-conf stuff to break.  I didn't think this was a completely obvious statement, but maybe it is ;)


> > - 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).
> 
> ok, can't it be blocked at this setup time? I think the rationale
> should be in the commit, if it's fixed by the following commit.


Perhaps it can be blocked here somehow, but I didn't want to break things for non-fullscreen-auto-conf cases.  And I couldn't be certain that blocking allocation-handling at this point wouldn't negatively affect some other case.  And we also already have a mechanism to block allocation-based resizing (the fullscreen_changed() handler + auto_resize state variable), so I decided to try to simply make that mechanism work properly.


> > 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?
> 
> No, any ideas (even high level) to make it "less fragile"? :)
>

Well, I had some vague ideas about introducing the concept of two separate application states: startup vs running.  During startup state, perhaps some handlers should be supressed, etc.  But I'm not sure that there's a simple way to determine when we've concluded 'startup state', and it seems like a fairly invasive change so I haven't pursued it yet.

Jonathon




More information about the virt-tools-list mailing list