GNOME Bugzilla – Bug 775478
wl_surface.leave is not emitted consistently
Last modified: 2016-12-02 11:14:40 UTC
At the moment mutter only rechecks the outputs a surface is over at paint time. This means that these checks will not (consistently at least) kick in when a surface stops being repainted, eg. due to switching to another workspace. This results in surfaces not consistently receiving wl_surface.leave in these situations. I'm attaching a couple of patches that improve this situation by triggering the output checks after ClutterActor::unmap happens on the surface actor. One oddity I see with this though: if the actors are cloned (eg. when showing the overview), the actors seem to be temporarily mapped/unmapped while drawing, this results in surfaces getting enter/leave pairs while drawing those. Any better idea about how to do this perhaps? I'm also thinking that checking the actor position alone in the output checks is kinda wrong, given clones/transforms/whatnot.
Created attachment 341165 [details] [review] compositor: Add MetaSurfaceActorWayland::unmapped signal This allows us to hook actions to the surface actors being unmapped.
Created attachment 341166 [details] [review] wayland: Check surface outputs after unmap So they consistently receive wl_surface.leave after the surface is not visible anymore.
Review of attachment 341165 [details] [review]: We could use "notify:mapped" as well I think. I leave it to you to decide whether it's better to add a designated signal fro this though.
Review of attachment 341166 [details] [review]: Looks good. One (my) sanity check comment only. ::: src/wayland/meta-wayland-surface.c @@ +1320,3 @@ + "unmapped", + G_CALLBACK (surface_actor_unmapped), + surface, 0); self-sanity check: this only means there is an extra MetaWaylandSurface ref during signal emission right? I.e. if the surface is unmapped as part of surface destruction, we'll still always be the only owner at the end of wl_surface_destructor.
(In reply to Jonas Ådahl from comment #3) > Review of attachment 341165 [details] [review] [review]: > > We could use "notify:mapped" as well I think. I leave it to you to decide > whether it's better to add a designated signal fro this though. Oh, I saw the mapped state is stored in internal flags, but didn't get to see the property for it (I guess I just stared at clutter_actor_set_property while this is readonly). Sounds indeed better to just use that, which renders this patch unneeded. (In reply to Jonas Ådahl from comment #4) > Review of attachment 341166 [details] [review] [review]: > > Looks good. One (my) sanity check comment only. > > ::: src/wayland/meta-wayland-surface.c > @@ +1320,3 @@ > + "unmapped", > + G_CALLBACK (surface_actor_unmapped), > + surface, 0); > > self-sanity check: this only means there is an extra MetaWaylandSurface ref > during signal emission right? I.e. if the surface is unmapped as part of > surface destruction, we'll still always be the only owner at the end of > wl_surface_destructor. Yes exactly. This is mostly for the cases where the object is unref'ed inside the signal handler, so it's preserved alive until after the callback is dispatched. But it doesn't add any extra permanent ref. I'll rework the last patch to use notify::mapped and drop the first one.
Created attachment 341231 [details] [review] wayland: Check surface outputs after mapped state changes So they consistently receive wl_surface.leave after the surface is not visible anymore.
Attachment 341231 [details] pushed as 5eb5f72 - wayland: Check surface outputs after mapped state changes