[clutter] actor: Do not unmap/unrealize twice on destruction

[Emmanuele Bassi <ebassi src gnome org>]

commit 11239d8da650ecf5d467fd8a131b30691e28dcbd
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Tue Jan 31 12:35:17 2012 +0000

    actor: Do not unmap/unrealize twice on destruction
    
    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.

 clutter/clutter-actor.c |   18 ++++++++----------
 1 files changed, 8 insertions(+), 10 deletions(-)
---
diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c
index 4dc97b9..ab262f8 100644
--- a/clutter/clutter-actor.c
+++ b/clutter/clutter-actor.c
@@ -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);