brl/mutter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

clutter/stage: Don't do newly queued relayouts after allocation cycle

Browse Source 
While it's strongly discouraged, it is possible to queue a new relayout
of an actor in the middle of an allocation cycle, we warn about it but
don't forbid it.

With the introduction of the "shallow relayout" API, our handling of
those relayouts silently changed: Before introducing "shallow
relayouts", we'd handle them on the next stage update, but with the
priv->pending_relayouts hashtable and the
priv->pending_relayouts_version counter, we now do them immediately
during the same allocation cycle (the counter is increased by 1 when
queuing the relayout and we switch to a new GHashTableIter after
finishing the current relayout, which means we'll now do the newly
queued relayout).

This change in behavior was probably not intended and wasn't mentioned
in the commit message of 5257c6ecc2, so
switch back to the old behavior, which is more robust in preventing
allocation-loops. To do this, use a GSList instead of GHashTable for the
pending_relayouts list, and simply steal that list before doing the
relayouts in _clutter_stage_maybe_relayout().

https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1267
This commit is contained in:
Jonas Dreßler 2020-05-23 10:31:29 +02:00 committed by Robert Mader
parent e09ac67698
commit 0ba15df57c
1 changed files with 14 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
clutter/clutter/clutter-stage.c
View File

@ -119,8 +119,7 @@ struct _ClutterStagePrivate
ClutterPlane current_clip_planes[4]; ClutterPlane current_clip_planes[4];
GHashTable *pending_relayouts; GSList *pending_relayouts;
unsigned int pending_relayouts_version;
GList *pending_queue_redraws; GList *pending_queue_redraws;
gint sync_delay; gint sync_delay;
@ -1295,7 +1294,7 @@ _clutter_stage_needs_update (ClutterStage *stage)
return (priv->redraw_pending || return (priv->redraw_pending ||
priv->needs_update || priv->needs_update ||
g_hash_table_size (priv->pending_relayouts) > 0); priv->pending_relayouts != NULL);
} }
void void
@ -1304,11 +1303,11 @@ clutter_stage_queue_actor_relayout (ClutterStage *stage,
{ {
ClutterStagePrivate *priv = stage->priv; ClutterStagePrivate *priv = stage->priv;
if (g_hash_table_size (priv->pending_relayouts) == 0) if (priv->pending_relayouts == NULL)
clutter_stage_schedule_update (stage); clutter_stage_schedule_update (stage);
g_hash_table_add (priv->pending_relayouts, g_object_ref (actor)); priv->pending_relayouts = g_slist_prepend (priv->pending_relayouts,
priv->pending_relayouts_version++; g_object_ref (actor));
} }
void void
@ -1316,24 +1315,20 @@ _clutter_stage_maybe_relayout (ClutterActor *actor)
{ {
ClutterStage *stage = CLUTTER_STAGE (actor); ClutterStage *stage = CLUTTER_STAGE (actor);
ClutterStagePrivate *priv = stage->priv; ClutterStagePrivate *priv = stage->priv;
GHashTableIter iter; g_autoptr (GSList) stolen_list = NULL;
gpointer key; GSList *l;
int count = 0; int count = 0;
/* No work to do? Avoid the extraneous debug log messages too. */ /* No work to do? Avoid the extraneous debug log messages too. */
if (g_hash_table_size (priv->pending_relayouts) == 0) if (priv->pending_relayouts == NULL)
return; return;
CLUTTER_NOTE (ACTOR, ">>> Recomputing layout"); CLUTTER_NOTE (ACTOR, ">>> Recomputing layout");
g_hash_table_iter_init (&iter, priv->pending_relayouts); stolen_list = g_steal_pointer (&priv->pending_relayouts);
while (g_hash_table_iter_next (&iter, &key, NULL)) for (l = stolen_list; l; l = l->next)
{ {
g_autoptr (ClutterActor) queued_actor = key; g_autoptr (ClutterActor) queued_actor = l->data;
unsigned int old_version;
g_hash_table_iter_steal (&iter);
priv->pending_relayouts_version++;
if (CLUTTER_ACTOR_IN_RELAYOUT (queued_actor)) /* avoid reentrancy */ if (CLUTTER_ACTOR_IN_RELAYOUT (queued_actor)) /* avoid reentrancy */
continue; continue;
@ -1351,16 +1346,11 @@ _clutter_stage_maybe_relayout (ClutterActor *actor)
CLUTTER_SET_PRIVATE_FLAGS (queued_actor, CLUTTER_IN_RELAYOUT); CLUTTER_SET_PRIVATE_FLAGS (queued_actor, CLUTTER_IN_RELAYOUT);
old_version = priv->pending_relayouts_version;
clutter_actor_allocate_preferred_size (queued_actor); clutter_actor_allocate_preferred_size (queued_actor);
CLUTTER_UNSET_PRIVATE_FLAGS (queued_actor, CLUTTER_IN_RELAYOUT); CLUTTER_UNSET_PRIVATE_FLAGS (queued_actor, CLUTTER_IN_RELAYOUT);
count++; count++;
/* Prevent using an iterator that's been invalidated */
if (old_version != priv->pending_relayouts_version)
g_hash_table_iter_init (&iter, priv->pending_relayouts);
} }
CLUTTER_NOTE (ACTOR, "<<< Completed recomputing layout of %d subtrees", count); CLUTTER_NOTE (ACTOR, "<<< Completed recomputing layout of %d subtrees", count);
@ -1923,7 +1913,9 @@ clutter_stage_dispose (GObject *object)
(GDestroyNotify) free_queue_redraw_entry); (GDestroyNotify) free_queue_redraw_entry);
priv->pending_queue_redraws = NULL; priv->pending_queue_redraws = NULL;
g_clear_pointer (&priv->pending_relayouts, g_hash_table_destroy); g_slist_free_full (priv->pending_relayouts,
(GDestroyNotify) g_object_unref);
priv->pending_relayouts = NULL;
/* this will release the reference on the stage */ /* this will release the reference on the stage */
stage_manager = clutter_stage_manager_get_default (); stage_manager = clutter_stage_manager_get_default ();
@ -2237,10 +2229,6 @@ clutter_stage_init (ClutterStage *self)
clutter_actor_set_background_color (CLUTTER_ACTOR (self), clutter_actor_set_background_color (CLUTTER_ACTOR (self),
&default_stage_color); &default_stage_color);
priv->pending_relayouts = g_hash_table_new_full (NULL,
NULL,
g_object_unref,
NULL);
clutter_stage_queue_actor_relayout (self, CLUTTER_ACTOR (self)); clutter_stage_queue_actor_relayout (self, CLUTTER_ACTOR (self));
clutter_actor_set_reactive (CLUTTER_ACTOR (self), TRUE); clutter_actor_set_reactive (CLUTTER_ACTOR (self), TRUE);