GNOME Bugzilla – Bug 646861
_NET_WM_FULLSCREEN_MONITORS is not always respected
Last modified: 2013-02-25 21:07:05 UTC
Created attachment 185236 [details] Test program to toggle _NET_WM_FULLSCREEN_MONITORS Sometimes, when a program attempts to fullscreen across all monitors, the gnome-shell panel still is raised above the programs window. In particular, this seems to happen when focusing another window on a secondary monitor while the program is fullscreen on just one monitor. It may take a few attempts of toggling between fullscreen and all monitors with the test case attached. But seems reproducable on all of my Fedora 15 setups with multi monitors (w/ various graphics drivers). You can compile the attached test case with gcc -o test-all-monitors -g $(pkg-config --cflags --libs gtk+-3.0 gdk-x11-3.0 x11) test-all-monitors.c And run it with ./test-all-monitors. The program will start in fullscreen with a single monitor. Clicking the toggle button will toggle between all-monitors and a single fullscreen monitor.
I am having this issue in 3.5.91. I can't get the above testcase to compile, however I can quite easily reproduce this issue, by loading a VM guest in full-screen with 2 monitors. This happens with both vmware and virtualbox.
Created attachment 223871 [details] xprop - fullscreen - 1 monitor (panel does disappear)
Created attachment 223872 [details] xprop - fullscreen - 2 monitors (panel is displayed)
*** Bug 694083 has been marked as a duplicate of this bug. ***
Created attachment 237255 [details] [review] window: Add is_fullscreen_monitors Add a method that returns whether a window uses fullscreen monitors.
Created attachment 237256 [details] [review] layout: Handle _NET_WM_FULLSCREEN_MONITORS Mark all monitors that fullscreen when window is using the _NET_WM_FULLSCREEN_MONITORS to be fullscreen on multiple monitors.
Created attachment 237257 [details] [review] layout: Handle _NET_WM_FULLSCREEN_MONITORS Mark all monitors that fullscreen when window is using the _NET_WM_FULLSCREEN_MONITORS to be fullscreen on multiple monitors.
Review of attachment 237255 [details] [review]: i don't want to bikeshed too much here, but "is_fullscreen_monitors" doesn't read well to me. maybe it should be "is_fullscreen_over_multiple_monitors" ? Or maybe it should be a more general purpose "get_monitors" that returns what monitors the window is on and the caller could then check is_fullscreen separately.
Review of attachment 237257 [details] [review]: ::: js/ui/layout.js @@ +889,3 @@ + monitor.x + monitor.width >= window.x + window.width && + monitor.y >= window.y && + monitor.y + monitor.height >= window.y + window.height) Right so if it was "get_monitors" instead of "is_fullscreen_monitor" then this logic could be simplified here (at the expense of making it more complicated in mutter). what do you think?
(In reply to comment #9) > Review of attachment 237257 [details] [review]: > > ::: js/ui/layout.js > @@ +889,3 @@ > + monitor.x + monitor.width >= window.x + > window.width && > + monitor.y >= window.y && > + monitor.y + monitor.height >= window.y + > window.height) > > Right so if it was "get_monitors" instead of "is_fullscreen_monitor" then this > logic could be simplified here (at the expense of making it more complicated in > mutter). what do you think? Yeah that makes sense.
Created attachment 237382 [details] [review] window: Add get_monitors Add a method that returns the indicies of the monitors a window is on.
Created attachment 237383 [details] [review] layout: Handle _NET_WM_FULLSCREEN_MONITORS Mark all monitors that fullscreen when window is using the _NET_WM_FULLSCREEN_MONITORS to be fullscreen on multiple monitors.
Review of attachment 237382 [details] [review]: ::: src/core/window.c @@ +3652,3 @@ + * + * Returns: (element-type gint) (transfer full): A #GArray of the monitor + * indices the window is on. I'd prefer this was: Returns: (array length=length) (element-type gint) (transfer container) int *meta_window_get_monitors (MetaWindow *window, gsize *length) so it's less cumbersome to use in the future from C inside mutter, but, of course, your call. @@ +3664,3 @@ + monitors = g_array_new (TRUE, FALSE, sizeof (int)); + + for (i = 0; i < window->screen->n_monitor_infos; i++) { mutter uses gnu style braces i believe @@ +3665,3 @@ + + for (i = 0; i < window->screen->n_monitor_infos; i++) { + MetaRectangle monitor_rect = window->screen->monitor_infos[i].rect; probably should be MetaRectangle *monitor_rect = &.... to avoid the memcpy
Review of attachment 237383 [details] [review]: ::: js/ui/layout.js @@ +889,3 @@ + let monitors = metaWindow.get_monitors(); + for (let i = 0; i < monitors.length; i++) + this.monitors[i].inFullscreen = true; ah that cleaned up really nice. The fact we have get_monitor() and get_monitors() in the api is little awkward. maybe we should name the new one get_all_monitors() or so (again, totally your call)
Created attachment 237387 [details] [review] window: Add get_all_monitors Add a method that returns the indicies of the monitors a window is on.
Created attachment 237388 [details] [review] layout: Handle _NET_WM_FULLSCREEN_MONITORS Mark all monitors that fullscreen when window is using the _NET_WM_FULLSCREEN_MONITORS to be fullscreen on multiple monitors.
Review of attachment 237387 [details] [review]: s/indicies/indices/ This is fine. a few comments below, but take 'em or leave 'em. ::: src/core/window.c @@ +3650,3 @@ /** + * meta_window_get_all_monitors: + * you don't list the arguments here @@ +3662,3 @@ + + meta_window_get_outer_rect (window, &window_rect); + *length = 0; might be better to make it legal to pass NULL for length (and just always add a -1 at the end), but doesn't really matter at this point since we aren't using it in C. @@ +3670,3 @@ + if (meta_rectangle_overlap (&window_rect, monitor_rect)) + { + monitors = g_realloc (monitors, *length + 1); I think i would have kept your GArray code instead of reallocing and just made the last two lines be: if (length) *length = array->len; return g_array_free (array, FALSE);
Review of attachment 237388 [details] [review]: cool
Comment on attachment 237387 [details] [review] window: Add get_all_monitors Pushed with suggested changes. Attachment 237387 [details] pushed as 2a773e0 - window: Add get_all_monitors
Attachment 237388 [details] pushed as a361180 - layout: Handle _NET_WM_FULLSCREEN_MONITORS