actor: Turn push/pop_internal into Actor methods

Since the "internal" state is global, it will leak onto actors that you
didn't intend for it to, because it applies not just to the actors you
create, but also to any actors *they* create. Eg, if you have a dialog
box class, you might push/pop_internal around creating its buttons, so
that those buttons get marked as internal to the dialog box. But
ctx->internal_child will still be set during the *button*'s constructor
as well, and so, eg, the label and icon inside the button actor will
*also* be marked as internal children, even if that isn't what the
button class wanted.

The least intrusive change at this point is to make push_internal() and
pop_internal() two methods of the Actor class, and take a ClutterActor
pointer as the argument - thus moving the locality of the internal_child
counter to the Actor itself.

http://bugzilla.openedhand.com/show_bug.cgi?id=1990

diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c
index fb4441a..b3e7a29 100644 (file)
--- a/clutter/clutter-actor.c
+++ b/clutter/clutter-actor.c
@@ -351,6 +351,8 @@ struct _ClutterActorPrivate
   ClutterActor   *opacity_parent;
 
   ClutterTextDirection text_direction;
+
+  gint internal_child;
 };
 
 enum
@@ -6782,7 +6784,6 @@ clutter_actor_set_parent (ClutterActor *self,
 {
   ClutterActorPrivate *priv;
   ClutterTextDirection text_dir;
-  ClutterMainContext *ctx;
 
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (CLUTTER_IS_ACTOR (parent));
@@ -6812,12 +6813,10 @@ clutter_actor_set_parent (ClutterActor *self,
   g_object_ref_sink (self);
   priv->parent_actor = parent;
 
-  ctx = _clutter_context_get_default ();
-
   /* if push_internal() has been called then we automatically set
    * the flag on the actor
    */
-  if (ctx->internal_child)
+  if (parent->priv->internal_child)
     CLUTTER_SET_PRIVATE_FLAGS (self, CLUTTER_ACTOR_INTERNAL_CHILD);
 
   /* clutter_actor_reparent() will emit ::parent-set for us */
@@ -9723,6 +9722,7 @@ clutter_actor_get_text_direction (ClutterActor *self)
 
 /**
  * clutter_actor_push_internal:
+ * @self: a #ClutterActor
  *
  * Should be used by actors implementing the #ClutterContainer and with
  * internal children added through clutter_actor_set_parent(), for instance:
@@ -9733,7 +9733,7 @@ clutter_actor_get_text_direction (ClutterActor *self)
  *   {
  *     self->priv = SELF_ACTOR_GET_PRIVATE (self);
  *
- *     clutter_actor_push_internal ();
+ *     clutter_actor_push_internal (CLUTTER_ACTOR (self));
  *
  *     /* calling clutter_actor_set_parent() now will result in
  *      * the internal flag being set on a child of MyActor
@@ -9749,7 +9749,7 @@ clutter_actor_get_text_direction (ClutterActor *self)
  *     clutter_actor_set_parent (self->priv->label,
  *                               CLUTTER_ACTOR (self));
  *
- *     clutter_actor_pop_internal ();
+ *     clutter_actor_pop_internal (CLUTTER_ACTOR (self));
  *
  *     /* calling clutter_actor_set_parent() now will not result in
  *      * the internal flag being set on a child of MyActor
@@ -9771,26 +9771,31 @@ clutter_actor_get_text_direction (ClutterActor *self)
  * Since: 1.2
  */
 void
-clutter_actor_push_internal (void)
+clutter_actor_push_internal (ClutterActor *self)
 {
-  ClutterMainContext *ctx = _clutter_context_get_default ();
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
 
-  ctx->internal_child += 1;
+  self->priv->internal_child += 1;
 }
 
 /**
  * clutter_actor_pop_internal:
+ * @self: a #ClutterActor
  *
  * Disables the effects of clutter_actor_pop_internal()
  *
  * Since: 1.2
  */
 void
-clutter_actor_pop_internal (void)
+clutter_actor_pop_internal (ClutterActor *self)
 {
-  ClutterMainContext *ctx = _clutter_context_get_default ();
+  ClutterActorPrivate *priv;
+
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
+
+  priv = self->priv;
 
-  if (ctx->internal_child == 0)
+  if (priv->internal_child == 0)
     {
       g_warning ("Mismatched %s: you need to call "
                  "clutter_actor_push_composite() at least once before "
@@ -9798,7 +9803,7 @@ clutter_actor_pop_internal (void)
       return;
     }
 
-  ctx->internal_child -= 1;
+  priv->internal_child -= 1;
 }
 
 /**

diff --git a/clutter/clutter-actor.h b/clutter/clutter-actor.h
index acb1603..96f4e9f 100644 (file)
--- a/clutter/clutter-actor.h
+++ b/clutter/clutter-actor.h
@@ -538,8 +538,8 @@ void                 clutter_actor_set_text_direction (ClutterActor         *sel
                                                        ClutterTextDirection  text_dir);
 ClutterTextDirection clutter_actor_get_text_direction (ClutterActor         *self);
 
-void                 clutter_actor_push_internal      (void);
-void                 clutter_actor_pop_internal       (void);
+void                 clutter_actor_push_internal      (ClutterActor         *self);
+void                 clutter_actor_pop_internal       (ClutterActor         *self);
 
 G_END_DECLS
 

diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h
index c20bfdc..fda8b4b 100644 (file)
--- a/clutter/clutter-private.h
+++ b/clutter/clutter-private.h
@@ -180,8 +180,6 @@ struct _ClutterMainContext
   gulong redraw_count;
 
   GList *repaint_funcs;
-
-  gint internal_child;
 };
 
 #define CLUTTER_CONTEXT()      (_clutter_context_get_default ())

diff --git a/tests/conform/test-actor-destroy.c b/tests/conform/test-actor-destroy.c
index 8cc0d48..6c49e25 100644 (file)
--- a/tests/conform/test-actor-destroy.c
+++ b/tests/conform/test-actor-destroy.c
@@ -131,7 +131,7 @@ test_destroy_class_init (TestDestroyClass *klass)
 static void
 test_destroy_init (TestDestroy *self)
 {
-  clutter_actor_push_internal ();
+  clutter_actor_push_internal (CLUTTER_ACTOR (self));
 
   if (g_test_verbose ())
     g_print ("Adding internal children...\n");
@@ -144,7 +144,7 @@ test_destroy_init (TestDestroy *self)
   clutter_actor_set_parent (self->label, CLUTTER_ACTOR (self));
   clutter_actor_set_name (self->label, "Label");
 
-  clutter_actor_pop_internal ();
+  clutter_actor_pop_internal (CLUTTER_ACTOR (self));
 
   self->tex = clutter_texture_new ();
   clutter_actor_set_parent (self->tex, CLUTTER_ACTOR (self));