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

actor: Do not unmap/unrealize twice on destruction

Browse Source 
When calling clutter_actor_destroy(), ClutterActor calls
update_map_state() on itself to unset the REALIZED and MAPPED states,
prior to running the dispose() implementation.

The default dispose() will call remove_child() (either directly or
through the Container implementation), which will check for the MAPPED
state and then run update_map_state() again. We use the previously set
MAPPED state to decide whether or not the parent should queue for a
relayout/redraw when removing a visible children.

If the MAPPED flag was cleared prior to remove_child(), though, it'll
always be unset by the time we get to remove_child(), and this will
cause missing redraws/relayouts; we were ignoring this prior the
post-First Apocalypse changes because we were doing:

  if (was_mapped)
    clutter_actor_queue_relayout (parent);

  clutter_actor_queue_redraw (parent);

which is obviously wrong. Once I removed that glaring brain damage from
the remove_child() implementation, bugs started appearing — bugs that
were probably the reason why we introduced that brain damage in the
first place, instead of checking the source of those bugs.

The obvious fix is to avoid clearing up the actor's state on destroy()
until we remove the actor from its parent. This also reduces the amount
of work we do, and the code paths that can potentially go wrong.
This commit is contained in:
Emmanuele Bassi 2012-01-31 12:35:17 +00:00
parent 8ee6d10681
commit 11239d8da6
1 changed files with 8 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
clutter/clutter-actor.c
View File

@ -3464,8 +3464,9 @@ clutter_actor_remove_child_internal (ClutterActor *self,
/* we need to unrealize *before* we set parent_actor to NULL,
* because in an unrealize method actors are dissociating from the
* stage, which means they need to be able to
* clutter_actor_get_stage(). This should unmap and unrealize,
* unless we're reparenting.
* clutter_actor_get_stage().
*
* yhis should unmap and unrealize, unless we're reparenting.
*/
clutter_actor_update_map_state (child, MAP_STATE_MAKE_UNREALIZED);
}
@ -4532,7 +4533,10 @@ clutter_actor_dispose (GObject *object)
ClutterActor *parent = priv->parent;
/* go through the Container implementation unless this
* is an internal child and has been marked as such
* is an internal child and has been marked as such.
*
* removing the actor from its parent will reset the
* realized and mapped states.
*/
if (!CLUTTER_ACTOR_IS_INTERNAL_CHILD (self))
clutter_container_remove_actor (CLUTTER_CONTAINER (parent), self);
@ -4541,7 +4545,7 @@ clutter_actor_dispose (GObject *object)
REMOVE_CHILD_LEGACY_FLAGS);
}
/* parent should be gone */
/* parent must be gone at this point */
g_assert (priv->parent == NULL);
if (!CLUTTER_ACTOR_IS_TOPLEVEL (self))
@ -6379,12 +6383,6 @@ clutter_actor_destroy (ClutterActor *self)
{
CLUTTER_SET_PRIVATE_FLAGS (self, CLUTTER_IN_DESTRUCTION);
/* if we are destroying we want to unrealize ourselves
* first before the dispose run removes the parent
*/
if (!CLUTTER_ACTOR_IS_TOPLEVEL (self))
clutter_actor_update_map_state (self, MAP_STATE_MAKE_UNREALIZED);
g_object_run_dispose (G_OBJECT (self));
CLUTTER_UNSET_PRIVATE_FLAGS (self, CLUTTER_IN_DESTRUCTION);