actor: Make Actor.add_child and Container.add_actor idempotent

And make sure that overriding Container and calling
clutter_actor_add_child() will result in the same sequence of operations
as the current set_parent()+queue_relayout()+signal_emit pattern.

Existing containers can continue using:

        clutter_actor_set_parent (child, CLUTTER_ACTOR (container));
        clutter_actor_queue_relayout (CLUTTER_ACTOR (container));
        g_signal_emit_by_name (container, "actor-added", child);

and newly written containers overriding Container.add() can simply call:

        clutter_actor_add_child (CLUTTER_ACTOR (container), child);

instead.

clutter/clutter-actor.c

@@ -9230,6 +9230,8 @@ clutter_actor_add_child_internal (ClutterActor             *self,
  * This function will take into consideration the #ClutterActor:depth
  * of @child, and will keep the list of children sorted.
  *
+ * This function will emit the #ClutterContainer::actor-added signal.
+ *
  * Since: 1.10
  */
 void
@@ -9239,10 +9241,13 @@ clutter_actor_add_child (ClutterActor *self,
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (CLUTTER_IS_ACTOR (child));
   g_return_if_fail (self != child);
+  g_return_if_fail (child->priv->parent_actor == NULL);
 
+  clutter_container_create_child_meta (CLUTTER_CONTAINER (self), child);
   clutter_actor_add_child_internal (self, child,
                                     insert_child_at_depth,
                                     NULL);
+  g_signal_emit_by_name (self, "actor-added", child);
 }
 
 /**
@@ -9260,6 +9265,8 @@ clutter_actor_add_child (ClutterActor *self,
  * This function will not take into consideration the #ClutterActor:depth
  * of @child.
  *
+ * This function will emit the #ClutterContainer::actor-added signal.
+ *
  * Since: 1.10
  */
 void
@@ -9269,12 +9276,15 @@ clutter_actor_insert_child_at_index (ClutterActor *self,
 {
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (CLUTTER_IS_ACTOR (child));
+  g_return_if_fail (self != child);
   g_return_if_fail (child->priv->parent_actor == NULL);
   g_return_if_fail (index_ < self->priv->n_children);
 
+  clutter_container_create_child_meta (CLUTTER_CONTAINER (self), child);
   clutter_actor_add_child_internal (self, child,
                                     insert_child_at_index,
                                     GINT_TO_POINTER (index_));
+  g_signal_emit_by_name (self, "actor-added", child);
 }
 
 /**
@@ -9293,6 +9303,8 @@ clutter_actor_insert_child_at_index (ClutterActor *self,
  * This function will not take into consideration the #ClutterActor:depth
  * of @child.
  *
+ * This function will emit the #ClutterContainer::actor-added signal.
+ *
  * Since: 1.10
  */
 void
@@ -9302,14 +9314,18 @@ clutter_actor_insert_child_above (ClutterActor *self,
 {
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (CLUTTER_IS_ACTOR (child));
+  g_return_if_fail (self != child);
+  g_return_if_fail (child != sibling);
   g_return_if_fail (child->priv->parent_actor == NULL);
   g_return_if_fail (sibling == NULL ||
                     (CLUTTER_IS_ACTOR (sibling) &&
                      sibling->priv->parent_actor == self));
 
+  clutter_container_create_child_meta (CLUTTER_CONTAINER (self), child);
   clutter_actor_add_child_internal (self, child,
                                     insert_child_above,
                                     sibling);
+  g_signal_emit_by_name (self, "actor-added", child);
 }
 
 /**
@@ -9328,6 +9344,8 @@ clutter_actor_insert_child_above (ClutterActor *self,
  * This function will not take into consideration the #ClutterActor:depth
  * of @child.
  *
+ * This function will emit the #ClutterContainer::actor-added signal.
+ *
  * Since: 1.10
  */
 void
@@ -9337,14 +9355,18 @@ clutter_actor_insert_child_below (ClutterActor *self,
 {
   g_return_if_fail (CLUTTER_IS_ACTOR (self));
   g_return_if_fail (CLUTTER_IS_ACTOR (child));
+  g_return_if_fail (self != child);
+  g_return_if_fail (child != sibling);
   g_return_if_fail (child->priv->parent_actor == NULL);
   g_return_if_fail (sibling == NULL ||
                     (CLUTTER_IS_ACTOR (sibling) &&
                      sibling->priv->parent_actor == self));
 
+  clutter_container_create_child_meta (CLUTTER_CONTAINER (self), child);
   clutter_actor_add_child_internal (self, child,
                                     insert_child_below,
                                     sibling);
+  g_signal_emit_by_name (self, "actor-added", child);
 }
 
 /**
@@ -9354,13 +9376,27 @@ clutter_actor_insert_child_below (ClutterActor *self,
  *
  * Sets the parent of @self to @parent.
  *
- * This function just calls clutter_actor_add_child().
+ * This function will result in @parent acquiring a reference on @self,
+ * eventually by sinking its floating reference first. The reference
+ * will be released by clutter_actor_unparent().
+ *
+ * This function should only be called by legacy #ClutterActor<!-- -->s
+ * implementing the #ClutterContainer interface.
+ *
+ * Newly written code should use clutter_actor_add_child() instead.
  */
 void
 clutter_actor_set_parent (ClutterActor *self,
 		          ClutterActor *parent)
 {
-  clutter_actor_add_child (parent, self);
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
+  g_return_if_fail (CLUTTER_IS_ACTOR (parent));
+  g_return_if_fail (self != parent);
+  g_return_if_fail (self->priv->parent_actor == NULL);
+
+  clutter_actor_add_child_internal (parent, self,
+                                    insert_child_at_depth,
+                                    NULL);
 }
 
 /**
@@ -9417,33 +9453,12 @@ invalidate_queue_redraw_entry (ClutterActor *self,
   return CLUTTER_ACTOR_TRAVERSE_VISIT_CONTINUE;
 }
 
-/**
+static void
- * clutter_actor_remove_child:
+clutter_actor_remove_child_internal (ClutterActor *self,
- * @self: a #ClutterActor
+                                     ClutterActor *child)
- * @child: a #ClutterActor
- *
- * Removes @child from the children of @self.
- *
- * This function will release the reference added by
- * clutter_actor_add_child().
- *
- * Since: 1.10
- */
-void
-clutter_actor_remove_child (ClutterActor *self,
-                            ClutterActor *child)
 {
   gboolean was_mapped;
 
-  g_return_if_fail (CLUTTER_IS_ACTOR (self));
-  g_return_if_fail (CLUTTER_IS_ACTOR (child));
-
-  if (child->priv->parent_actor == NULL)
-    return;
-
-  if (child->priv->parent_actor != self)
-    return;
-
   was_mapped = CLUTTER_ACTOR_IS_MAPPED (child);
 
   /* we need to unrealize *before* we set parent_actor to NULL,
@@ -9503,20 +9518,71 @@ clutter_actor_remove_child (ClutterActor *self,
   g_object_unref (child);
 }
 
+/**
+ * clutter_actor_remove_child:
+ * @self: a #ClutterActor
+ * @child: a #ClutterActor
+ *
+ * Removes @child from the children of @self.
+ *
+ * This function will release the reference added by
+ * clutter_actor_add_child(), so if you want to keep using @child
+ * you will have to acquire a referenced on it before calling this
+ * function.
+ *
+ * This function will emit the #ClutterContainer::actor-removed
+ * signal.
+ *
+ * Since: 1.10
+ */
+void
+clutter_actor_remove_child (ClutterActor *self,
+                            ClutterActor *child)
+{
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
+  g_return_if_fail (CLUTTER_IS_ACTOR (child));
+  g_return_if_fail (self != child);
+  g_return_if_fail (child->priv->parent_actor != NULL);
+  g_return_if_fail (child->priv->parent_actor == self);
+
+  g_object_ref (child);
+
+  clutter_container_destroy_child_meta (CLUTTER_CONTAINER (self), child);
+
+  clutter_actor_remove_child_internal (self, child);
+
+  g_signal_emit_by_name (self, "actor-removed", child);
+
+  g_object_unref (child);
+}
+
 /**
  * clutter_actor_unparent:
  * @self: a #ClutterActor
  *
  * Removes the parent of @self.
  *
- * This function just calls clutter_actor_remove_child().
+ * This will cause the parent of @self to release the reference
+ * acquired when calling clutter_actor_set_parent(), so if you
+ * want to keep @self you will have to acquire a reference of
+ * your own, through g_object_ref().
+ *
+ * This function should only be called by legacy #ClutterActor<!-- -->s
+ * implementing the #ClutterContainer interface.
+ *
+ * Newly written code should use clutter_actor_remove_child() instead.
  *
  * Since: 0.1.1
  */
 void
 clutter_actor_unparent (ClutterActor *self)
 {
-  clutter_actor_remove_child (self->priv->parent_actor, self);
+  g_return_if_fail (CLUTTER_IS_ACTOR (self));
+
+  if (self->priv->parent_actor == NULL)
+    return;
+
+  clutter_actor_remove_child_internal (self->priv->parent_actor, self);
 }
 
 /**
@@ -10120,23 +10186,13 @@ container_add_actor (ClutterContainer *container,
                      ClutterActor     *actor)
 {
   clutter_actor_add_child (CLUTTER_ACTOR (container), actor);
-  clutter_actor_queue_relayout (CLUTTER_ACTOR (container));
-
-  g_signal_emit_by_name (container, "actor-added", actor);
 }
 
 static void
 container_remove_actor (ClutterContainer *container,
                         ClutterActor     *actor)
 {
-  g_object_ref (actor);
-
   clutter_actor_remove_child (CLUTTER_ACTOR (container), actor);
-  clutter_actor_queue_relayout (CLUTTER_ACTOR (container));
-
-  g_signal_emit_by_name (container, "actor-removed", actor);
-
-  g_object_unref (actor);
 }
 
 typedef struct {

tests/conform/test-actor-layout.c

@@ -218,17 +218,17 @@ basic_layout (TestConformSimpleFixture *fixture,
   flower[0] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Red);
   clutter_actor_set_size (flower[0], 100, 100);
   clutter_actor_set_name (flower[0], "Red Flower");
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[0]);
+  clutter_actor_add_child (vase, flower[0]);
 
   flower[1] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Yellow);
   clutter_actor_set_size (flower[1], 100, 100);
   clutter_actor_set_name (flower[1], "Yellow Flower");
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[1]);
+  clutter_actor_add_child (vase, flower[1]);
 
   flower[2] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Green);
   clutter_actor_set_size (flower[2], 100, 100);
   clutter_actor_set_name (flower[2], "Green Flower");
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[2]);
+  clutter_actor_add_child (vase, flower[2]);
 
   clutter_actor_show_all (stage);
 
@@ -258,21 +258,21 @@ margin_layout (TestConformSimpleFixture *fixture,
   flower[0] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Red);
   clutter_actor_set_size (flower[0], 100, 100);
   clutter_actor_set_name (flower[0], "Red Flower");
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[0]);
+  clutter_actor_add_child (vase, flower[0]);
 
   flower[1] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Yellow);
   clutter_actor_set_size (flower[1], 100, 100);
   clutter_actor_set_name (flower[1], "Yellow Flower");
   clutter_actor_set_margin_right (flower[1], 6);
   clutter_actor_set_margin_left (flower[1], 6);
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[1]);
+  clutter_actor_add_child (vase, flower[1]);
 
   flower[2] = clutter_rectangle_new_with_color (CLUTTER_COLOR_Green);
   clutter_actor_set_size (flower[2], 100, 100);
   clutter_actor_set_name (flower[2], "Green Flower");
   clutter_actor_set_margin_top (flower[2], 6);
   clutter_actor_set_margin_bottom (flower[2], 6);
-  clutter_container_add_actor (CLUTTER_CONTAINER (vase), flower[2]);
+  clutter_actor_add_child (vase, flower[2]);
 
   clutter_actor_show_all (stage);