Created attachment 236581 [details] [review]
layout: Fix regression

Commit 6b4f52462071dc9af5b8915afebc8bc340b49ca2 removed the layer checks
_updateFullscreen ... this causes corruption when alt-tabbing out
of a fullscreen window so restore the check.

Review of attachment 236581 [details] [review]:

Uhm... isn't meta_window_is_monitor_sized() doing those same checks?

Pushed updated version after IRC review.

@Giovanni: No it does not check the layers.

For the record IRC conversation:

-----
<halfline_home> hmm
<halfline_home> so you're going from
<halfline_home> -            if (layer == Meta.StackLayer.FULLSCREEN) {
<halfline_home> +            if (window.is_monitor_sized()) {
<halfline_home> to effectively (if you combine them together)
<halfline_home> -            if (layer == Meta.StackLayer.FULLSCREEN) {
<halfline_home> +            if (layer == Meta.StackLayer.FULLSCREEN ||
<halfline_home> +               (layer == Meta.StackLayer.OVERRIDE_REDIRECT && metaWindow.is_monitor_sized())) {
<halfline_home> and the commit message is:
<halfline_home> layout: Use window.is_monitor_sized
<halfline_home>     Mutter now provides a method for this, so use it.
<halfline_home> which sounds like is_monitor_sized is supposed to replace the old case not augment it
<drago01> halfline: the motivation for window.is_monitor_sized was to remove the size checks
<drago01> halfline: actually there is an other bug
* michele (~michele@host107-103-dynamic.51-79-r.retail.telecomitalia.it) hat #gnome-shell betreten
<drago01> halfline: the screen sized case
<drago01> halfline: we no longer set all monitors to fullscreen
<halfline_home> ah i see
* michele ist jetzt bekannt als netvandal
<halfline_home> too bad there isn't a is_screen_sized() :-)
<drago01> I can add one
<halfline_home> would it get used more than there?
<halfline_home> does mutter itself ever check for that?
<drago01> yes when unredirecting windows
<drago01> but it does use monitor_sized (which implies screen sized currently)
<halfline_home> oh it does
<halfline_home> so there's not another bug
<halfline_home> basically your patch is fine?
<drago01> which one?
* ricotz (~rico@wf0994.dip.tu-dresden.de) hat #gnome-shell betreten
<drago01> the first problem is this
<drago01> open a terminal 
<drago01> press F11
<drago01> alt tab away
<drago01> the menu bar remains ontop of the panel
<drago01> readding the layer checks
<drago01> fixes that
<drago01> the second one is
<drago01> that in the screen sized case
<drago01> we only mark one monitor as inFullscreen
<drago01> instead of all
<halfline_home> right
<halfline_home> so are you going to add an
<halfline_home> else
<halfline_home> to where line 27 of your patch is
<halfline_home> here http://paste.fedoraproject.org/3260/
<halfline_home> that that loops through and sets it for all?
<drago01> yes
<drago01> halfline_home: http://paste.fedoraproject.org/3262/ mutter patch
* mastroWork (~mastro@2-229-44-241.ip195.fastwebnet.it) hat #gnome-shell betreten
<drago01> halfline_home: updated gnome-shell patch http://paste.fedoraproject.org/3264
<halfline_home> re the mutter patch
<halfline_home> does window->fullscreen ever get set to true when the window is as big as all monitors ?
* alon (~alon@nat-pool-brq-u.redhat.com) hat #gnome-shell betreten
<halfline_home> oh i guess it does if _NET_WM_FULLSCREEN_MONITORS is setup
<drago01> yeah for the NET_WM_FULLSCREEN_MONITORS case
* sobhan1 (~Thunderbi@2.176.61.151) hat #gnome-shell betreten
* nullie hat die Verbindung getrennt (Read error: 145 (Connection timed out))
* langdon hat die Verbindung getrennt (Ping timeout: 600 seconds)
<halfline_home> drago01: but line 50 in that patch
<halfline_home> has if (window->fullscreen) return TRUE;
* nullie (~nullie@2a02:6b8:0:2807:5e26:aff:fe6e:b58d) hat #gnome-shell betreten
<drago01> halfline: that's in meta_window_is_monitor_sized
* sobhan hat die Verbindung getrennt (Ping timeout: 600 seconds)
<halfline_home> but if you call meta_window_is_monitor_sized with a window that's fullscreen across some monitors you're going to get a different answer than if you call meta_window_is_screen_sized on it
<halfline_home> oh i guess that's right
<halfline_home> well it really depends on if we want is_monitor_sized to mean in the case where a window is fullscreen covers more than one monitor, but doesn't cover all monitors
* mastro__ hat die Verbindung getrennt (Ping timeout: 600 seconds)
<halfline_home> drago01: and then on the shell side
<halfline_home> apparently it's wrong to set every monitor to fullscreen
<halfline_home> we should only be setting the fullscreen_monitors monitors
<drago01> halfline: well the doc says "%TRUE if the window is occupies an entire monitor or the whole screen." multiple but not all qulifies as "an entire monitor"
<halfline_home> sure, i can buy that
<drago01> halfline: well it is not wrong for the screen sized case but it is wrong for fullscreen_monitors where it is only a subset of the available montiors
* drago01 fixes the patch
* hughsie ist jetzt bekannt als hughsie-afk-lunch
<drago01> halfline: 
<drago01> specs says
<drago01>  data.l[0] = the monitor whose top edge defines the top edge of the fullscreen window
<drago01>   data.l[1] = the monitor whose bottom edge defines the bottom edge of the fullscreen window
<drago01>   data.l[2] = the monitor whose left edge defines the left edge of the fullscreen window
<drago01>   data.l[3] = the monitor whose right edge defines the right edge of the fullscreen window
<drago01> so this depends on the monitor geometry
<drago01> there isn't just a list of monitors
<halfline_home> on the other hand this was all broken before right?
<drago01> yes
<drago01> well not *that* broken
<drago01> the primary use case of that check is
<drago01> to hide the chrome
<drago01> when something is ontop of it
<drago01> and this should work as long as the primary case is detected
* tacg hat die Verbindung getrennt (Ping timeout: 600 seconds)
* hyperair (~hyperair@49.128.60.68) hat #gnome-shell betreten
<halfline_home> is message try on primary always?
<drago01> yes
* tacg_ hat die Verbindung getrennt (Ping timeout: 600 seconds)
<halfline_home> i thought we put it on the lowest monitor
<halfline_home> even if primary is a top monitor
* drago01 checks
<drago01> I though we put all "chrome" on primary
<drago01> alt-tab, ws switcher etc
* halfline_home looks
<drago01> halfline_home: yeah mt is on bottom
<drago01> you are right
<hadess> and i thought we put the osds on the screen where the mouse was
<halfline_home> ok
<hadess> or maybe keyboard focus
<drago01> hadess: which osds?
<hadess> window switcher, input method switcher
<drago01> not sure about the later
<drago01> but the window switcher is on primary
<drago01> (used to be the "focused" in the past)
* wunterslash_ (~magentar@ip-178-202-29-27.unitymediagroup.de) hat #gnome-shell betreten
<drago01> hadess: https://bugzilla.gnome.org/show_bug.cgi?id=619854
<Services> Bug 619854: normal, Normal, ---, gnome-shell-maint, RESOLVED FIXED, Always show Alt-tab display on the primary display
<halfline_home> but basically looks like fullscreen_monitors wasn't handled before these changes
<halfline_home> so seems a bit inconsiderate to throw it in your lap now...
<drago01> yeah should file a bug that it does not get forgotten though
<halfline_home> makes sense
<drago01> ok, so fine to push the two patches?
<halfline_home> well does this check make sense? if (window->override_redirect || window->fullscreen)
<halfline_home> should it just unconditionally do those checks?
<hadess> halfline_home, ofourdan added fullscreen_monitors support in GTK+ for the wacom calibration tool
<drago01> halfline_home: yeah can remove that can't think of a case where a non or / fs window will pass the checks but no reason to care at this point
* drago01 removes
<drago01> halfline_home: http://paste.fedoraproject.org/3266/
<halfline_home> hadess: that's okay.  the bug is we end up marking monitors outsize the fullscreen_monitors area as inFullscreen too
<halfline_home> hadess: which means we up hiding the panel even if it's on a window that's not one of hte fullscreen ones i guess
<halfline_home> drago01: looks good to me
* wunterslash hat die Verbindung getrennt (Ping timeout: 600 seconds)
<drago01> halfline_home: ok
<drago01> halfline_home: gs patch too?
<halfline_home> heh gcampax already chimed in on the bug
<halfline_home> gs patch looks good, except else it on its own line
<halfline_home> which i don't thing gs does
* tacg (~tiago@2.80.116.163) hat #gnome-shell betreten
* tacg_ (~tiago@2.80.116.163) hat #gnome-shell betreten
<halfline_home> i gotta get ready for work though, running late this morning
<drago01> halfline_home: oh ok 
* drago01 fixes else
-----