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 735927 - Background windows animate over current one when closing
Background windows animate over current one when closing
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
3.13.x
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 736770 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2014-09-02 21:05 UTC by Bastien Nocera
Modified: 2014-09-16 20:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window-actor: Keep in compositor's window list until destroyed (3.93 KB, patch)
2014-09-10 22:43 UTC, Florian Müllner
reviewed Details | Review
window-actor: Consider needs_destroy in is_destroyed() (1.08 KB, patch)
2014-09-10 22:43 UTC, Florian Müllner
committed Details | Review
window-actor: Skip frame-sync when the corresponding window is gone (1.90 KB, patch)
2014-09-10 22:43 UTC, Florian Müllner
committed Details | Review
global: Filter out destroyed actors from get_window_actors() (1.66 KB, patch)
2014-09-10 22:44 UTC, Florian Müllner
committed Details | Review
window-actor: Keep in compositor's window list until destroyed (3.10 KB, patch)
2014-09-11 09:19 UTC, Florian Müllner
accepted-commit_now Details | Review
window-actor: Keep in compositor's window list until destroyed (2.72 KB, patch)
2014-09-11 10:18 UTC, Florian Müllner
committed Details | Review

Description Bastien Nocera 2014-09-02 21:05:36 UTC
gnome-shell 3.13.90

The new window closing animations are great, but if the window closing is behind the currently focused one, one sees a horrible flash, the animation of the window closing on top of the current one, which is unwelcome.

Reproducer:
$ cat cat test.sh 
zenity --text-info --width=1920 --height=1000 &
sleep 10
killall -9 zenity
$ sh test.sh
<switch to another fullscreen window and wait for zenity to close>
<flash of white texture in front of your eyes>
Comment 1 Giovanni Campagna 2014-09-02 22:18:02 UTC
I believe this is a stacking problem in mutter, the shell just animates the window opacity.

Unfortunately, the stacking code is too hairy for me to understand, so I don't know if and where there is a bug.
Comment 2 Jasper St. Pierre (not reading bugmail) 2014-09-02 22:23:49 UTC
Yeah, the compositor restacks all known windows by shoving the known ones to the bottom, which means that any unknown actors bubble to the top. And window actors during a destroy effect are "unknown". We can make them known by deferring the removal until the actual window actor is destroyed, but lots of code in gnome-shell calls get_window_actors() when it really wants to list all windows, and we don't want to return the half-dead windows in there.
Comment 3 Giovanni Campagna 2014-09-03 06:11:37 UTC
Now that display.list_windows () is available, can we fix the shell?
Comment 4 Florian Müllner 2014-09-10 22:43:42 UTC
Created attachment 285870 [details] [review]
window-actor: Keep in compositor's window list until destroyed

When a window is destroyed, the corresponding actor may still be
kept around for the destroy effect. But as the actor is removed
from the compositor's stack list immediately, the compositor will
always stack it above "valid" window actors - this is not what we
want, so only update the compositor's list when the actor is
actually destroyed.
Comment 5 Florian Müllner 2014-09-10 22:43:48 UTC
Created attachment 285871 [details] [review]
window-actor: Consider needs_destroy in is_destroyed()

According to the documentation, the method returns "whether the X window
that the actor was displaying has been destroyed" - that is very much
true when we delay the actual actor destruction for a destroy animation,
so update the method accordingly.
Comment 6 Florian Müllner 2014-09-10 22:43:54 UTC
Created attachment 285872 [details] [review]
window-actor: Skip frame-sync when the corresponding window is gone
Comment 7 Florian Müllner 2014-09-10 22:44:14 UTC
Created attachment 285873 [details] [review]
global: Filter out destroyed actors from get_window_actors()

All current code assumes that the list of window actors corresponds to the
list of windows; however as the list returned by meta_get_window_actors()
now includes actors during the destroy animation, that assumption breaks.

Eventually we should make everyone move to a more appropriate API, but
for now make it work again by returning a filtered list of "good"
window actors.
Comment 8 Jasper St. Pierre (not reading bugmail) 2014-09-11 01:48:24 UTC
Review of attachment 285870 [details] [review]:

::: src/compositor/meta-window-actor.c
@@ +771,3 @@
+  compositor->windows = g_list_remove (compositor->windows, (gconstpointer) self);
+
+  CLUTTER_ACTOR_CLASS (meta_window_actor_parent_class)->destroy (self);

Is there a reason we need this instead of just relying on the existing g_list_remove in dispose?
Comment 9 Florian Müllner 2014-09-11 09:19:50 UTC
Created attachment 285894 [details] [review]
window-actor: Keep in compositor's window list until destroyed

(In reply to comment #8)
> Is there a reason we need this instead of just relying on the existing
> g_list_remove in dispose?

No.
Comment 10 Florian Müllner 2014-09-11 10:18:07 UTC
Created attachment 285899 [details] [review]
window-actor: Keep in compositor's window list until destroyed

(Remove spurious whitespace change left-over from previous patch version)
Comment 11 Jasper St. Pierre (not reading bugmail) 2014-09-11 15:37:24 UTC
Review of attachment 285894 [details] [review]:

OK. This matches the patch I had locally :)
Comment 12 Jasper St. Pierre (not reading bugmail) 2014-09-11 15:37:48 UTC
Review of attachment 285899 [details] [review]:

Whoops, wrong one. This one's good.
Comment 13 Jasper St. Pierre (not reading bugmail) 2014-09-11 15:40:25 UTC
Review of attachment 285871 [details] [review]:

We only seem to use this function in one place: to minimize some extra work we do in js/ui/windowManager.js. I get what you want to do, but I'd point out that nothing important uses this function, so it won't fix anything immediately, it's just preparation for our changes to shell_global_get_window_actors.
Comment 14 Jasper St. Pierre (not reading bugmail) 2014-09-11 15:41:06 UTC
Review of attachment 285872 [details] [review]:

OK.
Comment 15 Jasper St. Pierre (not reading bugmail) 2014-09-11 15:41:48 UTC
Review of attachment 285873 [details] [review]:

OK.
Comment 16 Florian Müllner 2014-09-11 15:51:48 UTC
Attachment 285871 [details] pushed as d50f8af - window-actor: Consider needs_destroy in is_destroyed()
Attachment 285872 [details] pushed as 8d53ae8 - window-actor: Skip frame-sync when the corresponding window is gone
Attachment 285899 [details] pushed as 98fa343 - window-actor: Keep in compositor's window list until destroyed
Comment 17 Florian Müllner 2014-09-11 15:52:52 UTC
Attachment 285873 [details] pushed as 93205ee - global: Filter out destroyed actors from get_window_actors()
Comment 18 Florian Müllner 2014-09-16 20:08:55 UTC
*** Bug 736770 has been marked as a duplicate of this bug. ***