After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 764844 - [WAYLAND] Workspace thumbnail flipping between 2 different window stacking order
[WAYLAND] Workspace thumbnail flipping between 2 different window stacking order
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
3.20.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 758335 758345 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-04-10 12:26 UTC by Lionel Landwerlin
Modified: 2016-09-01 06:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Video deomstrating the issue (1.41 MB, video/webm)
2016-04-30 09:52 UTC, Timm Bäder
  Details
[PATCH] stack: Do not restack hidden X11 windows on Wayland (1.82 KB, patch)
2016-05-10 13:22 UTC, Olivier Fourdan
none Details | Review
stack: Stack both wayland and X hidden windows below the guard window (3.35 KB, patch)
2016-05-12 16:58 UTC, Rui Matos
committed Details | Review
stack: Don't add wayland windows to _NET_CLIENT_LIST (3.39 KB, patch)
2016-05-12 16:59 UTC, Rui Matos
committed Details | Review

Description Lionel Landwerlin 2016-04-10 12:26:17 UTC
In this video : https://www.youtube.com/watch?v=5YebtCZOtzo

I switch between 2 different workspaces and the stacking order used to paint windows in the workspace thumbnail seems to flip between 2 different states.

I haven't seen that on X11 so I suspect it's a Wayland issue.
In particular the window flipping is Chromium (which is an X11 client as opposed to other clients), so this might be hint as to where the bug lies.
Comment 1 Timm Bäder 2016-04-30 09:52:40 UTC
Created attachment 327069 [details]
Video deomstrating the issue


I have the same problem, and I can reproduce it by simply opening a xwayland client, another native wayland client, then focusing the xwayland one and switching workspaces. It doesn't happen when I didn't have the xwayland client focused when switching workspaces.
Comment 2 Emmanuele Bassi (:ebassi) 2016-05-09 13:07:00 UTC
Dupe of 756272 ?
Comment 3 Lionel Landwerlin 2016-05-09 13:53:05 UTC
Might be, I'll give it a try when it lands.
Comment 4 Olivier Fourdan 2016-05-10 07:27:02 UTC
(In reply to Emmanuele Bassi (:ebassi) from comment #2)
> Dupe of 756272 ?

Nope, I have the fix for bug 756272 applied to Xwayland and can still reproduce this one.
Comment 5 Olivier Fourdan 2016-05-10 07:59:27 UTC
I reckon it's actually mutter restacking the windows, as captured with the following MUTTER_VERBOSE log between two windows, one Wayland native (named Wayland) and an Xwayland app (using "xlogo").

The xlogo window is actually placed above the Wayland native window, and yet mutter will restack the Wayland above the Xologo as soon as le latter gets unmapped due to the workspace switch:


  VERBOSE: Setting _NET_CURRENT_DESKTOP to 1
  WINDOW_STATE: Putting 0x600003 (xlogo) in the calc_showing queue
  WINDOW_STATE: Putting W12 (Wayland) in the calc_showing queue

  FOCUS: Focusing default window on new workspace
  FOCUS: Focusing MRU window
  FOCUS: No MRU window to focus found; focusing no_focus_window.

  FOCUS: 0x600003 (xlogo) is now the previous focus window due to being focused out or unmapped
  VERBOSE: Setting _NET_WM_STATE with 0 atoms
  STACK: Syncing window stack to server
  STACK: Recomputing layers
  STACK: Window 0x600003 (xlogo) on layer 2 type = 0 has_focus = 0
  STACK: Window W12 (Wayland) on layer 2 type = 0 has_focus = 0
  STACK: Bottom to top: 2:0 - W12 (Wayland) 2:1 - 0x600003 (xlogo) 
  STACK: Restacking 2 windows
  VERBOSE: Grabbing unfocused window buttons for 0x600003 (xlogo)

  FOCUS: * Focus --> NULL with serial 5210
  WINDOW_STATE: Clearing the calc_showing queue
  VERBOSE: Implement showing = 0 for window W12 (Wayland)
  WINDOW_STATE: Hiding window W12 (Wayland)

  STACK: Syncing window stack to server
  STACK: Bottom to top: 2:0 - W12 (Wayland) 2:1 - 0x600003 (xlogo) 
  STACK: Restacking 1 windows

  VERBOSE: Implement showing = 0 for window 0x600003 (xlogo)
  WINDOW_STATE: Hiding window 0x600003 (xlogo)
  STACK: Syncing window stack to server

  STACK: Bottom to top: 2:0 - W12 (Wayland) 2:1 - 0x600003 (xlogo) 
  STACK: Restacking 0 windows
  STACK: Predicting: RAISE_ABOVE(0x2002f7 (xlogo), 0; 5219)
  STACK: MetaStackTracker state (screen=0)
    xserver_serial: 339
    verified_stack:   0x20001b  0x200001  0x200002  0x200003  0x200004  0x200005  0x200006  0x200007  0x200008  0x20000d  0x20000e (gnome-shel)  0x200012  0x200013  0x200017  0x400001  0x400003  0x20001a  0x20001f  0x10000000c (Wayland)  0x2002f7 (xlogo)
    unverified_predictions: [RAISE_ABOVE(0x2002f7 (xlogo), 0; 5219)]

Here ^^^ we have [...] 0x10000000c (Wayland)  0x2002f7 (xlogo)

  VERBOSE: Setting _NET_WM_STATE with 0 atoms
  FOCUS: Focus out event received on 0x600003 (xlogo) 0x600003 (client window) mode NotifyNormal detail NotifyAncestor serial 5210
  VERBOSE: Property notify on 0x600003 (xlogo) for _NET_WM_STATE
  VERBOSE: Requesting 1 properties of 0x600003 at once
  SYNC: 44: Syncing to get 1 GetProperty replies in meta_prop_get_values
  STACK: Stack op event received: RAISE_ABOVE(0x2002f7 (xlogo), 0; 5219)
  STACK: MetaStackTracker state (screen=0)
    xserver_serial: 339
    verified_stack:   0x2002f7 (xlogo)  0x20001b  0x200001  0x200002  0x200003  0x200004  0x200005  0x200006  0x200007  0x200008  0x20000d  0x20000e (gnome-shel)  0x200012  0x200013  0x200017  0x400001  0x400003  0x20001a  0x20001f  0x10000000c (Wayland)
    unverified_predictions: []

And now ^^^ we have 0x2002f7 (xlogo)  0x20001b [...] 0x10000000c (Wayland)

  VERBOSE: Property notify on 0x600003 (xlogo) for WM_STATE
  VERBOSE: Requesting 1 properties of 0x600003 at once
  SYNC: 45: Syncing to get 1 GetProperty replies in meta_prop_get_values
  VERBOSE: Property notify on 0x600003 (xlogo) for _NET_WM_STATE
  VERBOSE: Requesting 1 properties of 0x600003 at once
  SYNC: 46: Syncing to get 1 GetProperty replies in meta_prop_get_values
Comment 6 Olivier Fourdan 2016-05-10 12:50:40 UTC
Looking at the code, I suspect this might be because stack_sync_to_xserver() places X11 hidden windows back to the bottom of the stack:

https://git.gnome.org/browse/mutter/tree/src/core/stack.c#n1084

1084 
1085       /* We don't restack hidden windows along with the rest, though they are
1086        * reflected in the _NET hints. Hidden windows all get pushed below
1087        * the screens fullscreen guard_window. */
1088       if (w->hidden)
1089         {
1090           if (w->client_type == META_WINDOW_CLIENT_TYPE_X11)
1091             g_array_append_val (x11_hidden_stack_ids, top_level_window);
1092           continue;
1093         }
1094 

and:

https://git.gnome.org/browse/mutter/tree/src/core/stack.c#n1114

1114   meta_stack_tracker_restack_at_bottom (stack->screen->stack_tracker,
1115                                         (guint64 *)x11_hidden_stack_ids->data,
1116                                         x11_hidden_stack_ids->len);

This is originally coming from commit 0058271a ("Re-works the approach to supporting live preview to handle stacking") that puts hidden windows back to the bottom of the stack and commit 40e820f5 ("Add support for stacking X and Wayland windows together") that does this specifically for X11 windows only.
Comment 7 Olivier Fourdan 2016-05-10 13:22:18 UTC
Created attachment 327591 [details] [review]
[PATCH] stack: Do not restack hidden X11 windows on Wayland

On X11, hidden windows all get pushed below the screens fullscreen
guard_window.

But when mutter is a Wayland compositor, mixing native Wayland surfaces
with X11 windows running in Xwayland, that breaks the actual stacking
of mixed type windows.

Avoid re-stacking hidden X11 windows on Wayland, the surface is hidden
anyway so this should not be necessary.
Comment 8 Olivier Fourdan 2016-05-10 13:23:59 UTC
Note, this patch seems to fix the issue for me without noticeable ill effects (AFAICS) but this is slightly out of my area of comfort so I am unsure if this is really an appropriate fix...
Comment 9 Rui Matos 2016-05-11 18:16:12 UTC
(In reply to Olivier Fourdan from comment #5)
> The xlogo window is actually placed above the Wayland native window, and yet
> mutter will restack the Wayland above the Xologo as soon as le latter gets
> unmapped due to the workspace switch:

mutter doesn't unmap windows on workspace switches as you note below!

(In reply to Olivier Fourdan from comment #7)
> Avoid re-stacking hidden X11 windows on Wayland, the surface is hidden
> anyway so this should not be necessary.

The surface is hidden on the clutter scene graph yes, but if the X window isn't stacked below the guard window, with this patch, Xwayland might decide to send events to the wrong window because from its POV the "hidden" window might be above another X window.

I think what we want to do here is to remove the client_type == X11 check and restack hidden wayland windows below the guard window too which should keep both types of windows correctly stacked relative to each others.
Comment 10 Olivier Fourdan 2016-05-11 18:38:15 UTC
(In reply to Rui Matos from comment #9)
> I think what we want to do here is to remove the client_type == X11 check
> and restack hidden wayland windows below the guard window too which should
> keep both types of windows correctly stacked relative to each others.

I tried this approach at first (removing the "if (w->client_type == META_WINDOW_CLIENT_TYPE_X11)" line) but that doesn't seem to work, the thumbnails still flip, from what I could see.
Comment 11 Rui Matos 2016-05-12 16:58:10 UTC
Created attachment 327729 [details] [review]
stack: Stack both wayland and X hidden windows below the guard window

Stacking hidden X windows below the guard window is a necessity to
ensure input events aren't delivered to them. Wayland windows don't
need this because the decision to send them input events is done by us
looking at the clutter scene graph.

But, since we don't stack hidden wayland windows along with their X
siblings we lose their relative stack positions while hidden. As
there's no ill side effect to re-stacking hidden wayland windows below
the X guard window we can fix this by just doing it regardless of
window type.
--

This fixes the original issue reported here.
Comment 12 Rui Matos 2016-05-12 16:59:43 UTC
Created attachment 327730 [details] [review]
stack: Don't add wayland windows to _NET_CLIENT_LIST

Window->xwindow is None (i.e. 0) for wayland windows and there's no
point in adding them to these X specific properties.
--

This is a drive-by cleanup. Not sure what, if anything looks at
_NET_CLIENT_LIST but it's unlikely that having a bunch of zeros for
wayland windows there is of any use.
Comment 13 Olivier Fourdan 2016-05-12 17:15:02 UTC
attachment 327729 [details] [review] + attachment 327730 [details] [review] both look good to me and fix the issue.
Comment 14 Florian Müllner 2016-05-12 17:18:43 UTC
Review of attachment 327729 [details] [review]:

LGTM
Comment 15 Florian Müllner 2016-05-12 17:38:58 UTC
Review of attachment 327730 [details] [review]:

::: src/core/stack.c
@@ -145,3 @@
-  /* Remember the window ID to remove it from the stack array.
-   * The macro is safe to use: Window is guaranteed to be 32 bits, and
-   * GUINT_TO_POINTER says it only works on 32 bits.

Maybe it's just me, but the comment still looks useful

@@ -863,3 @@
 
-      old_size = stack->xwindows->len;
-      g_array_set_size (stack->xwindows, old_size + n_added);

Mmmh, there are probably some rare cases where this avoids repeated reallocations compared to multiple append_val() calls (more than 16 windows in stack->added?), but I doubt it's worth the code ...
Comment 16 Rui Matos 2016-05-12 18:51:50 UTC
(In reply to Florian Müllner from comment #15)
> Maybe it's just me, but the comment still looks useful

Ok, I kept the comment.

> Mmmh, there are probably some rare cases where this avoids repeated
> reallocations compared to multiple append_val() calls (more than 16 windows
> in stack->added?), but I doubt it's worth the code ...

Yeah, I don't think it's worth it.

Attachment 327729 [details] pushed as c5637c5 - stack: Stack both wayland and X hidden windows below the guard window
Attachment 327730 [details] pushed as bbb83d4 - stack: Don't add wayland windows to _NET_CLIENT_LIST
Comment 17 Rui Matos 2016-07-18 11:41:02 UTC
*** Bug 758335 has been marked as a duplicate of this bug. ***
Comment 18 Olivier Fourdan 2016-09-01 06:53:24 UTC
*** Bug 758345 has been marked as a duplicate of this bug. ***