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
This commit is contained in:
Jonas Dreßler 2020-07-06 15:35:14 +02:00 committed by verdre
parent 826573ccce
commit ae83a61e67
3 changed files with 33 additions and 4 deletions

View File

@ -2201,11 +2201,19 @@ unrealize_actor_after_children_cb (ClutterActor *self,
int depth, int depth,
void *user_data) void *user_data)
{ {
ClutterActor *stage = _clutter_actor_get_stage_internal (self);
/* We want to unset the realized flag only _after_ /* We want to unset the realized flag only _after_
* child actors are unrealized, to maintain invariants. * child actors are unrealized, to maintain invariants.
*/ */
CLUTTER_ACTOR_UNSET_FLAGS (self, CLUTTER_ACTOR_REALIZED); CLUTTER_ACTOR_UNSET_FLAGS (self, CLUTTER_ACTOR_REALIZED);
g_object_notify_by_pspec (G_OBJECT (self), obj_props[PROP_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; return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE;
} }

View File

@ -141,6 +141,9 @@ void clutter_stage_presented (ClutterStage *stag
void clutter_stage_queue_actor_relayout (ClutterStage *stage, void clutter_stage_queue_actor_relayout (ClutterStage *stage,
ClutterActor *actor); ClutterActor *actor);
void clutter_stage_dequeue_actor_relayout (ClutterStage *stage,
ClutterActor *actor);
GList * clutter_stage_get_views_for_rect (ClutterStage *stage, GList * clutter_stage_get_views_for_rect (ClutterStage *stage,
const graphene_rect_t *rect); const graphene_rect_t *rect);

View File

@ -1202,6 +1202,28 @@ clutter_stage_queue_actor_relayout (ClutterStage *stage,
g_object_ref (actor)); 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 void
clutter_stage_maybe_relayout (ClutterActor *actor) 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 */ if (CLUTTER_ACTOR_IN_RELAYOUT (queued_actor)) /* avoid reentrancy */
continue; 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) if (queued_actor == actor)
CLUTTER_NOTE (ACTOR, " Deep relayout of stage %s", CLUTTER_NOTE (ACTOR, " Deep relayout of stage %s",
_clutter_actor_get_debug_name (queued_actor)); _clutter_actor_get_debug_name (queued_actor));