GNOME Bugzilla – Bug 782183
wayland: new window momentarily displays on wrong monitor
Last modified: 2017-05-22 07:28:47 UTC
When running an application under Valgrind, I notice a behavior with a 2-monitor setup. I'm running on gnome-shell 3.24.1, with my terminal/etc on the secondary monitor. Running my app under Valgrind like "valgrind ./some-test-app", the window briefly displays on the primary monitor before jumping to the secondary monitor where my mouse cursor is. I assume this always happens, but it's just too quick for me to notice in the general case.
I don't see that here with 3.22 on F25... But anyhow this cannot be gtk+ as clients have no control on toplevel positioning under Wayland so this can only be a compositor issue... That sounds like bug 750552 which was fixed long time ago though (see also bug 780820)
Created attachment 351165 [details] Screencast I tried hard to reproduce on F26 with mutter 3.24 on an old and slow laptop, and all is fine with gtk3-demo. Usually, things like that show well is a screencast (which makes things even slower) and here I see no problem at all in the screencast. I do notice though that when an application starts either maximized or fullscreen (like totem replying the screencast) it does show up first on the primary monitor *then* gets relocated to the other monitor. Maybe that's your issue? Can you tell a bit more about the issue you're seeing, maybe try to see if you can capture a screencast, or even better a log of mutter with MUTTER_VERBOSE set?
(In reply to Olivier Fourdan from comment #2) > I do notice though that when an application starts either maximized or > fullscreen (like totem replying the screencast) it does show up first on the > primary monitor *then* gets relocated to the other monitor. This is exactly the issue, I forgot to mention the app is maximized before gtk_window_present() is called.
Created attachment 351410 [details] MUTTER_VERBOSE=1 logs MUTTER_VERBOSE=1 of the issue when it happens. We clearly see the window is first shown on the first monitor: PLACEMENT: Putting window W2 on active workspace WINDOW_STATE: Putting W2 in the move_resize queue WINDOW_STATE: Putting W2 in the calc_showing queue ... GEOMETRY: Clearing the move_resize queue WINDOW_STATE: Removing W2 (Videos) from the move_resize queue GEOMETRY: Constraining W2 (Videos) in move from 0,0 0x0 to 0,0 0x0 WORKAREA: Window W2 (Videos) monitor 0 has work area 0,0 1024 x 768 ... and then shortly after moved to the second monitor: ... PLACEMENT: Placing window W2 (Videos) XINERAMA: Natural monitor is 1024,0 +1024,768 WORKAREA: Window W2 (Videos) monitor 1 has work area 1024,0 1024 x 768 WORKAREA: Window W2 (Videos) monitor 1 has work area 1024,0 1024 x 768 WINDOW_OPS: Maximizing W2 (Videos)
What happens (it seems) is that the window is first moved to a "wrong" location where it shows and when the set_maximized() is received, it gets moved to the "active" monitor which in Wayland happens to be the onw with the pointer. What places the window at the wrong position initially is the initial apply_pending_state() which places the surface at (-26,-23) (the shadows offset):
+ Trace 237438
Then the animation is played from that "wrong" location and the window is finally showed at its final location on the "other" monitor.
Created attachment 351522 [details] [review] [RFC PATCH] wayland: place window if moved before placement If a client changes the state of a surface to issue a set_maximize or a set_fullscreen, the surface is resized before the window is actually placed by mutter, which will show the initial animation at the wrong location, before moving the window to the current monitor which is where the pointer is placed. Set the internal flag "placed" when this occurs so that mutter won't relocate the window.
Review of attachment 351522 [details] [review]: Won't work, it breaks placement entirely as pretty much all clients issue a resize before showing.
Oh, looks like we show the window "too early" before it's placed:
+ Trace 237441
So we play a "map effect" at the wrong position, then the window is placed appropriately and so moved to its final destination on the other monitor...
Created attachment 351955 [details] [review] [PATCH] wayland: place window if maximized before placement If a client changes the state of a surface to issue a set_maximize, this causes the window to be showing early due to apply_pending_state() being called before placement. If the monitor on which the window is to be shown initially is different from the one where the pointer is placed, this causes the effect to be played at the wrong location before the window reaches its monitor location. Force the window to be placed prior to change its state to maximized in xdg-shell so that mutter won't relocate the window afterwards.
Review of attachment 351955 [details] [review]: ::: src/wayland/meta-wayland-xdg-shell.c @@ +352,3 @@ + * this has no effect... + */ + meta_window_force_placement (surface->window); I think I understand why this helps, but my understanding doesn't seem to be what is described in the commit message... 1) a side effect of calling this is that window->placed is set to TRUE, which in turn avoids the defer-maximize-to-post-placement. For example, the issue is also fixed either by commenting out that early-out in meta_window_maximize() or changing the above line here to "window->placed = TRUE;". 2) this means the call below will actually do the move_resize call which will constrain etc resulting in meta_wayland_xdg_toplevel_send_configure() being called with the maximize dimensions. 3) this means that the meta_wayland_xdg_toplevel_send_configure(0,0) call in xdg_toplevel_role_commit() won't be called. As far as I can tell, the incorrect call send_configure(0,0) was the issue here, as it meant the client first sent a non-maximized window geometry, which was used when showing and initial placing. After placing, the correct configure was sent, and then when the client had redrawn, the actual maximized surface content was provided. So... our window management code is quite frustrating, and this fix has an after taste of work-around. I wonder if we can't fix this some better way. First thing I can think of is to move part of the maximize code behind a MetaWindowClass vfunc, as there are multiple things in meta_window_maximize() that is X11 only (such as unshading, and this defer-maximize thing). Another idea, is to get rid of that deferred-maximize thing all together. It seems to originate from 2003 with no explanation as to why, and I don't know how to trigger it either. When checking how it works with the panel-gtk test case under X11, window-props.c handles initial maximization in a different way. Given that meta_window_maximize() seems to be called mostly by interactive window management operations, the two places I think needs consideration are: meta_window_apply_session_info() (should probably set the defer-maximize variables, as done in window-props.c), and meta_window_x11_client_message().
(In reply to Jonas Ådahl from comment #10) > > [...] > > First thing I can think of is to move part of the maximize code behind a > MetaWindowClass vfunc, as there are multiple things in > meta_window_maximize() that is X11 only (such as unshading, and this > defer-maximize thing). > > Another idea, is to get rid of that deferred-maximize thing all together. It > seems to originate from 2003 with no explanation as to why, and I don't know > how to trigger it either. When checking how it works with the panel-gtk test > case under X11, window-props.c handles initial maximization in a different > way. Given that meta_window_maximize() seems to be called mostly by > interactive window management operations, the two places I think needs > consideration are: meta_window_apply_session_info() (should probably set the > defer-maximize variables, as done in window-props.c), and > meta_window_x11_client_message(). I am not sure I follow, what is it you call "deferred-maximize", is it maximize_horizontally_after_placement and maximize_vertically_after_placement?
Assuming this is maximize_*_after_placement, then this would be originally commit 010e620 for bug 111902
(In reply to Olivier Fourdan from comment #11) > (In reply to Jonas Ådahl from comment #10) ... > I am not sure I follow, what is it you call "deferred-maximize", is it > maximize_horizontally_after_placement and > maximize_vertically_after_placement? Yes. As in, it's deferred and not implemented until after placing.
I think there is another thing to consider, if we remove the "maximize_*_after_placement" logic, the windows will most likely be misplaced when eventually they're de-maximized (as maximization is supposed to save the previous size/position so that de-maximizing a window restores the previous pre-maximization location and size on screen).
(In reply to Olivier Fourdan from comment #14) > I think there is another thing to consider, if we remove the > "maximize_*_after_placement" logic, the windows will most likely be > misplaced when eventually they're de-maximized (as maximization is supposed > to save the previous size/position so that de-maximizing a window restores > the previous pre-maximization location and size on screen). AFAICS, with force_place() before maximize() will end up not setting any old rect. I think that also actually makes sense. We should probably not have mutter pretend to know what to restore to, as in Wayland that is up to the client. Initially we send 0x0, and I guess when restoring to something unknonw, we should send 0x0 too.
For the size, sure the client is in charge, but the location still needs to be set/restored, and placement depends on the size. BTW, I think there is more to it than just maximize_*_after_placement, if you remove that from meta_window_maximize() (as a test), the initial map animation still shows on the first monitor when the window is maximized on the second monitor (which is this bug. different from bug 781353 even if related) To reproduce: 1. MUTTER_DEBUG_NUM_DUMMY_MONITORS=2 jhbuild run mutter --wayland --nested 2. Place the mouse pointer on the (fake) screen on the right 3. Launch an application that starts maximized (e.g. totem if closed while maximized) => The inital map animation plays on the monitor on the left and the window eventually shows on the monitor on the right (where the pointer is located, as this makes the "current" monitor)
Review of attachment 351955 [details] [review]: ::: src/wayland/meta-wayland-xdg-shell.c @@ +351,3 @@ + /* Make sure the window is placed first, and if it's already placed, + * this has no effect... + */ We have been discussing this issue on IRC, and concluded that its probably not the right time to fix things "too" properly, and as this work around fixes the issue AFAIK without introducing any new issues, I think it's best to land it. This comment seems a bit unnecessary though, as it states what is stated below. The reason why is probably a better thing to document here, but its also documented in the commit message so, maybe its not necessary anyway. BTW, I think the commit message is a bit incorrect. Nothing is causing anything to be "showing early", as showing still will strictly be delayed until a buffer is attached.
Comment on attachment 351955 [details] [review] [PATCH] wayland: place window if maximized before placement attachment 351955 [details] [review] pushed to git master as commit 561d71b - wayland: place window if maximized before placement attachment 351955 [details] [review] pushed to branch gnome-3-24 as commit 7801df7 - wayland: place window if maximized before placement