From ae83a61e670cd2f25bd3ee6d757cdb64eaba1710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Dre=C3=9Fler?= Date: Mon, 6 Jul 2020 15:35:14 +0200 Subject: [PATCH] clutter/actor: Remove actors from shallow relayout list when unrealizing With the introduction of the shallow relayout mechanism another small but severe regression sneaked into our layout machinery: We might allocate an actor twice during the same allocation cycle, with one allocation happening using the wrong parent. This issue happens when reparenting an actor from a NO_LAYOUT parent to a non-NO_LAYOUT parent, in particular it triggered a bug in gnome-shell when DND reparents a child from the NO_LAYOUT uiGroup to the overviews Workspace actor after a drag ended. The reason the issue happens is the following chain of events: 1. child of a NO_LAYOUT parent queues a relayout, this child is added to the priv->pending_relayouts list maintained by ClutterStage 2. child is reparented to a different parent which doesn't have the NO_LAYOUT flag set, another relayout is queued, this time a different actor is added to the priv->pending_relayouts list 3. the relayout happens and we go through the pending_relayouts list backwards, that means the correct relayout queued during 2. happens first, then the old one happens and we simply call clutter_actor_allocate_preferred_size() on the actor, that allocation overrides the other, correct one. So fix that issue by adding a method to ClutterStage which removes actors from the pending_relayouts list again and call this method as soon as an actor with a NO_LAYOUT parent is detached from the stage. With that in place, we can also remove the check whether an actor is still on stage while looping through pending_relayouts. In case something else is going wrong and the actor is not on stage, clutter_actor_allocate() will warn anyway. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1356 --- clutter/clutter/clutter-actor.c | 8 ++++++++ clutter/clutter/clutter-stage-private.h | 3 +++ clutter/clutter/clutter-stage.c | 26 +++++++++++++++++++++---- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 1bd88bcd3..d3ca1b22d 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -2201,11 +2201,19 @@ unrealize_actor_after_children_cb (ClutterActor *self, int depth, void *user_data) { + ClutterActor *stage = _clutter_actor_get_stage_internal (self); + /* We want to unset the realized flag only _after_ * child actors are unrealized, to maintain invariants. */ CLUTTER_ACTOR_UNSET_FLAGS (self, CLUTTER_ACTOR_REALIZED); g_object_notify_by_pspec (G_OBJECT (self), obj_props[PROP_REALIZED]); + + if (stage != NULL && + self->priv->parent != NULL && + self->priv->parent->flags & CLUTTER_ACTOR_NO_LAYOUT) + clutter_stage_dequeue_actor_relayout (CLUTTER_STAGE (stage), self); + return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE; } diff --git a/clutter/clutter/clutter-stage-private.h b/clutter/clutter/clutter-stage-private.h index 7297d56a6..f2a371c5e 100644 --- a/clutter/clutter/clutter-stage-private.h +++ b/clutter/clutter/clutter-stage-private.h @@ -141,6 +141,9 @@ void clutter_stage_presented (ClutterStage *stag void clutter_stage_queue_actor_relayout (ClutterStage *stage, ClutterActor *actor); +void clutter_stage_dequeue_actor_relayout (ClutterStage *stage, + ClutterActor *actor); + GList * clutter_stage_get_views_for_rect (ClutterStage *stage, const graphene_rect_t *rect); diff --git a/clutter/clutter/clutter-stage.c b/clutter/clutter/clutter-stage.c index e9368fe44..51f738217 100644 --- a/clutter/clutter/clutter-stage.c +++ b/clutter/clutter/clutter-stage.c @@ -1202,6 +1202,28 @@ clutter_stage_queue_actor_relayout (ClutterStage *stage, g_object_ref (actor)); } +void +clutter_stage_dequeue_actor_relayout (ClutterStage *stage, + ClutterActor *actor) +{ + ClutterStagePrivate *priv = stage->priv; + GSList *l; + + for (l = priv->pending_relayouts; l; l = l->next) + { + ClutterActor *relayout_actor = l->data; + + if (relayout_actor == actor) + { + g_object_unref (relayout_actor); + priv->pending_relayouts = + g_slist_delete_link (priv->pending_relayouts, l); + + return; + } + } +} + void clutter_stage_maybe_relayout (ClutterActor *actor) { @@ -1227,10 +1249,6 @@ clutter_stage_maybe_relayout (ClutterActor *actor) if (CLUTTER_ACTOR_IN_RELAYOUT (queued_actor)) /* avoid reentrancy */ continue; - /* An actor may have been destroyed or hidden between queuing and now */ - if (clutter_actor_get_stage (queued_actor) != actor) - continue; - if (queued_actor == actor) CLUTTER_NOTE (ACTOR, " Deep relayout of stage %s", _clutter_actor_get_debug_name (queued_actor));