box: Use the ActorIter API

And remove a useless override of the pick() virtual function while we're
at it.

https://bugzilla.gnome.org/show_bug.cgi?id=668669

clutter/clutter-actor.c

@@ -395,6 +395,14 @@ struct _ClutterActorPrivate
 
   gint n_children;
 
+  /* tracks whenever the children of an actor are changed; the
+   * age is incremented by 1 whenever an actor is added or
+   * removed. the age is not incremented when the first or the
+   * last child pointers are changed, or when grandchildren of
+   * an actor are changed.
+   */
+  gint age;
+
   gchar *name; /* a non-unique name, used for debugging */
   guint32 id; /* unique id, used for backward compatibility */
 
@@ -3500,6 +3508,8 @@ clutter_actor_remove_child_internal (ClutterActor                 *self,
 
   self->priv->n_children -= 1;
 
+  self->priv->age += 1;
+
   /* clutter_actor_reparent() will emit ::parent-set for us */
   if (emit_parent_set && !CLUTTER_ACTOR_IN_REPARENT (child))
     g_signal_emit (child, actor_signals[PARENT_SET], 0, self);
@@ -10040,6 +10050,8 @@ clutter_actor_add_child_internal (ClutterActor              *self,
 
   self->priv->n_children += 1;
 
+  self->priv->age += 1;
+
   /* if push_internal() has been called then we automatically set
    * the flag on the actor
    */
@@ -15770,3 +15782,177 @@ clutter_actor_get_last_child (ClutterActor *self)
 
   return self->priv->last_child;
 }
+
+/* easy way to have properly named fields instead of the dummy ones
+ * we use in the public structure
+ */
+typedef struct _RealActorIter
+{
+  ClutterActor *root;           /* dummy1 */
+  ClutterActor *current;        /* dummy2 */
+  gpointer padding_1;           /* dummy3 */
+  gint age;                     /* dummy4 */
+  gpointer padding_2;           /* dummy5 */
+} RealActorIter;
+
+/**
+ * clutter_actor_iter_init:
+ * @iter: a #ClutterActorIter
+ * @root: a #ClutterActor
+ *
+ * Initializes a #ClutterActorIter, which can then be used to iterate
+ * efficiently over a section of the scene graph, and associates it
+ * with @root.
+ *
+ * Modifying the scene graph section that contains @root will invalidate
+ * the iterator.
+ *
+ * |[
+ *   ClutterActorIter iter;
+ *   ClutterActor *child;
+ *
+ *   clutter_actor_iter_init (&iter, container);
+ *   while (clutter_actor_iter_next (&iter, &child))
+ *     {
+ *       /* do something with child */
+ *     }
+ * ]|
+ *
+ * Since: 1.10
+ */
+void
+clutter_actor_iter_init (ClutterActorIter *iter,
+                         ClutterActor     *root)
+{
+  RealActorIter *ri = (RealActorIter *) iter;
+
+  g_return_if_fail (iter != NULL);
+  g_return_if_fail (CLUTTER_IS_ACTOR (root));
+
+  ri->root = root;
+  ri->current = NULL;
+  ri->age = root->priv->age;
+}
+
+/**
+ * clutter_actor_iter_next:
+ * @iter: a #ClutterActorIter
+ * @child: (out): return location for a #ClutterActor
+ *
+ * Advances the @iter and retrieves the next child of the root #ClutterActor
+ * that was used to initialize the #ClutterActorIterator.
+ *
+ * If the iterator can advance, this function returns %TRUE and sets the
+ * @child argument.
+ *
+ * If the iterator cannot advance, this function returns %FALSE, and
+ * the contents of @child are undefined.
+ *
+ * Return value: %TRUE if the iterator could advance, and %FALSE otherwise.
+ *
+ * Since: 1.10
+ */
+gboolean
+clutter_actor_iter_next (ClutterActorIter  *iter,
+                         ClutterActor     **child)
+{
+  RealActorIter *ri = (RealActorIter *) iter;
+
+  g_return_val_if_fail (iter != NULL, FALSE);
+  g_return_val_if_fail (ri->root != NULL, FALSE);
+#ifndef G_DISABLE_ASSERT
+  g_return_val_if_fail (ri->age == ri->root->priv->age, FALSE);
+#endif
+
+  if (ri->current == NULL)
+    ri->current = ri->root->priv->first_child;
+  else
+    ri->current = ri->current->priv->next_sibling;
+
+  if (child != NULL)
+    *child = ri->current;
+
+  return ri->current != NULL;
+}
+
+/**
+ * clutter_actor_iter_next:
+ * @iter: a #ClutterActorIter
+ * @child: (out): return location for a #ClutterActor
+ *
+ * Advances the @iter and retrieves the previous child of the root
+ * #ClutterActor that was used to initialize the #ClutterActorIterator.
+ *
+ * If the iterator can advance, this function returns %TRUE and sets the
+ * @child argument.
+ *
+ * If the iterator cannot advance, this function returns %FALSE, and
+ * the contents of @child are undefined.
+ *
+ * Return value: %TRUE if the iterator could advance, and %FALSE otherwise.
+ *
+ * Since: 1.10
+ */
+gboolean
+clutter_actor_iter_prev (ClutterActorIter  *iter,
+                         ClutterActor     **child)
+{
+  RealActorIter *ri = (RealActorIter *) iter;
+
+  g_return_val_if_fail (iter != NULL, FALSE);
+  g_return_val_if_fail (ri->root != NULL, FALSE);
+#ifndef G_DISABLE_ASSERT
+  g_return_val_if_fail (ri->age == ri->root->priv->age, FALSE);
+#endif
+
+  if (ri->current == NULL)
+    ri->current = ri->root->priv->last_child;
+  else
+    ri->current = ri->current->priv->prev_sibling;
+
+  if (child != NULL)
+    *child = ri->current;
+
+  return ri->current != NULL;
+}
+
+/**
+ * clutter_actor_iter_remove:
+ * @iter: a #ClutterActorIter
+ *
+ * Safely removes the #ClutterActor currently pointer to by the iterator
+ * from its parent.
+ *
+ * This function can only be called after clutter_actor_iter_next() or
+ * clutter_actor_iter_prev() returned %TRUE, and cannot be called more
+ * than once for the same actor.
+ *
+ * This function will call clutter_actor_remove_child() internally.
+ *
+ * Since: 1.10
+ */
+void
+clutter_actor_iter_remove (ClutterActorIter *iter)
+{
+  RealActorIter *ri = (RealActorIter *) iter;
+  ClutterActor *cur;
+
+  g_return_if_fail (iter != NULL);
+  g_return_if_fail (ri->root != NULL);
+#ifndef G_DISABLE_ASSERT
+  g_return_if_fail (ri->age == ri->root->priv->age);
+#endif
+  g_return_if_fail (ri->current != NULL);
+
+  cur = ri->current;
+
+  if (cur != NULL)
+    {
+      ri->current = cur->priv->prev_sibling;
+
+      clutter_actor_remove_child_internal (ri->root, cur,
+                                           REMOVE_CHILD_DEFAULT_FLAGS);
+
+      ri->age += 1;
+    }
+}

clutter/clutter-actor.h

@@ -263,6 +263,27 @@ struct _ClutterActorClass
   gpointer _padding_dummy[28];
 };
 
+/**
+ * ClutterActorIter:
+ *
+ * An iterator structure that allows to efficiently iterate over a
+ * section of the scene graph.
+ *
+ * The contents of the <structname>ClutterActorIter</structname> structure
+ * are private and should only be accessed using the provided API.
+ *
+ * Since: 1.10
+ */
+struct _ClutterActorIter
+{
+  /*< private >*/
+  gpointer CLUTTER_PRIVATE_FIELD (dummy1);
+  gpointer CLUTTER_PRIVATE_FIELD (dummy2);
+  gpointer CLUTTER_PRIVATE_FIELD (dummy3);
+  gint     CLUTTER_PRIVATE_FIELD (dummy4);
+  gpointer CLUTTER_PRIVATE_FIELD (dummy5);
+};
+
 GType clutter_actor_get_type (void) G_GNUC_CONST;
 
 ClutterActor *        clutter_actor_new                       (void);
@@ -486,6 +507,14 @@ void                  clutter_actor_set_child_at_index        (ClutterActor
                                                                ClutterActor          *child,
                                                                gint                   index_);
 
+void                  clutter_actor_iter_init                 (ClutterActorIter      *iter,
+                                                               ClutterActor          *root);
+gboolean              clutter_actor_iter_next                 (ClutterActorIter      *iter,
+                                                               ClutterActor         **child);
+gboolean              clutter_actor_iter_prev                 (ClutterActorIter      *iter,
+                                                               ClutterActor         **child);
+void                  clutter_actor_iter_remove               (ClutterActorIter      *iter);
+
 /* Transformations */
 gboolean              clutter_actor_is_rotated                (ClutterActor          *self);
 gboolean              clutter_actor_is_scaled                 (ClutterActor          *self);

clutter/clutter-stage.c

@@ -234,17 +234,13 @@ clutter_stage_real_foreach (ClutterContainer *container,
                             ClutterCallback   callback,
                             gpointer          user_data)
 {
-  ClutterActor *iter;
+  ClutterActorIter iter;
+  ClutterActor *child;
 
-  iter = clutter_actor_get_first_child (CLUTTER_ACTOR (container));
-  while (iter != NULL)
-    {
-      ClutterActor *next = clutter_actor_get_next_sibling (iter);
+  clutter_actor_iter_init (&iter, CLUTTER_ACTOR (container));
 
-      callback (iter, user_data);
-
-      iter = next;
-    }
+  while (clutter_actor_iter_next (&iter, &child))
+    callback (child, user_data);
 }
 
 static void
@@ -678,6 +674,7 @@ clutter_stage_paint (ClutterActor *self)
   ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv;
   CoglBufferBit clear_flags;
   CoglColor stage_color;
+  ClutterActorIter iter;
   ClutterActor *child;
   guint8 real_alpha;
 
@@ -732,30 +729,25 @@ clutter_stage_paint (ClutterActor *self)
     cogl_disable_fog ();
 #endif
 
-  for (child = clutter_actor_get_first_child (self);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
-    {
-      clutter_actor_paint (child);
-    }
+  clutter_actor_iter_init (&iter, self);
+  while (clutter_actor_iter_next (&iter, &child))
+    clutter_actor_paint (child);
 }
 
 static void
 clutter_stage_pick (ClutterActor       *self,
 		    const ClutterColor *color)
 {
+  ClutterActorIter iter;
   ClutterActor *child;
 
   /* Note: we don't chain up to our parent as we don't want any geometry
    * emitted for the stage itself. The stage's pick id is effectively handled
    * by the call to cogl_clear done in clutter-main.c:_clutter_do_pick_async()
    */
-  for (child = clutter_actor_get_first_child (self);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
-    {
-      clutter_actor_paint (child);
-    }
+  clutter_actor_iter_init (&iter, self);
+  while (clutter_actor_iter_next (&iter, &child))
+    clutter_actor_paint (child);
 }
 
 static gboolean
@@ -819,14 +811,12 @@ static void
 clutter_stage_show (ClutterActor *self)
 {
   ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv;
+  ClutterActorIter iter;
   ClutterActor *child;
 
-  for (child = clutter_actor_get_first_child (self);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
-    {
-      clutter_actor_show (child);
-    }
+  clutter_actor_iter_init (&iter, self);
+  while (clutter_actor_iter_next (&iter, &child))
+    clutter_actor_show (child);
 
   CLUTTER_ACTOR_CLASS (clutter_stage_parent_class)->show (self);
 
@@ -842,17 +832,15 @@ static void
 clutter_stage_hide (ClutterActor *self)
 {
   ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv;
+  ClutterActorIter iter;
   ClutterActor *child;
 
   g_assert (priv->impl != NULL);
   _clutter_stage_window_hide (priv->impl);
 
-  for (child = clutter_actor_get_first_child (self);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
-    {
-      clutter_actor_show (child);
-    }
+  clutter_actor_iter_init (&iter, self);
+  while (clutter_actor_iter_next (&iter, &child))
+    clutter_actor_show (child);
 
   CLUTTER_ACTOR_CLASS (clutter_stage_parent_class)->hide (self);
 }

clutter/clutter-types.h

@@ -48,6 +48,18 @@ G_BEGIN_DECLS
 #define CLUTTER_DEPRECATED_FOR(f) G_DEPRECATED_FOR(f)
 #endif
 
+/* some structures are meant to be opaque and still be allocated on the stack;
+ * in order to avoid people poking at their internals, we use this macro to
+ * ensure that users don't accidentally access a struct private members.
+ *
+ * we use the CLUTTER_COMPILATION define to allow us easier access, though.
+ */
+#ifdef CLUTTER_COMPILATION
+#define CLUTTER_PRIVATE_FIELD(x)        x
+#else
+#define CLUTTER_PRIVATE_FIELD(x)        clutter_private_ ## x
+#endif
+
 #define CLUTTER_TYPE_ACTOR_BOX          (clutter_actor_box_get_type ())
 #define CLUTTER_TYPE_FOG                (clutter_fog_get_type ())
 #define CLUTTER_TYPE_GEOMETRY           (clutter_geometry_get_type ())
@@ -65,6 +77,7 @@ typedef struct _ClutterChildMeta        ClutterChildMeta;
 typedef struct _ClutterLayoutMeta       ClutterLayoutMeta;
 typedef struct _ClutterActorMeta        ClutterActorMeta;
 typedef struct _ClutterLayoutManager    ClutterLayoutManager;
+typedef struct _ClutterActorIter        ClutterActorIter;
 
 typedef struct _ClutterAlpha            ClutterAlpha;
 typedef struct _ClutterAnimatable       ClutterAnimatable; /* dummy */

clutter/clutter.symbols

@@ -157,6 +157,10 @@ clutter_actor_insert_child_below
 clutter_actor_is_in_clone_paint
 clutter_actor_is_rotated
 clutter_actor_is_scaled
+clutter_actor_iter_init
+clutter_actor_iter_next
+clutter_actor_iter_prev
+clutter_actor_iter_remove
 clutter_actor_lower
 clutter_actor_lower_bottom
 clutter_actor_map

clutter/deprecated/clutter-box.c

@@ -128,6 +128,7 @@ clutter_box_real_get_paint_volume (ClutterActor       *actor,
                                    ClutterPaintVolume *volume)
 {
   gboolean retval = FALSE;
+  ClutterActorIter iter;
   ClutterActor *child;
 
   /* if we have a background color, and an allocation, then we need to
@@ -144,9 +145,8 @@ clutter_box_real_get_paint_volume (ClutterActor       *actor,
   /* otherwise, union the paint volumes of our children, in case
    * any one of them decides to paint outside the parent's allocation
    */
-  for (child = clutter_actor_get_first_child (actor);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
+  clutter_actor_iter_init (&iter, actor);
+  while (clutter_actor_iter_next (&iter, &child))
     {
       const ClutterPaintVolume *child_volume;
 
@@ -162,22 +162,6 @@ clutter_box_real_get_paint_volume (ClutterActor       *actor,
   return retval;
 }
 
-static void
-clutter_box_real_pick (ClutterActor       *actor,
-                       const ClutterColor *pick)
-{
-  ClutterActor *child;
-
-  CLUTTER_ACTOR_CLASS (clutter_box_parent_class)->pick (actor, pick);
-
-  for (child = clutter_actor_get_first_child (actor);
-       child != NULL;
-       child = clutter_actor_get_next_sibling (child))
-    {
-      clutter_actor_paint (child);
-    }
-}
-
 static void
 clutter_box_set_property (GObject      *gobject,
                           guint         prop_id,
@@ -240,7 +224,6 @@ clutter_box_class_init (ClutterBoxClass *klass)
   g_type_class_add_private (klass, sizeof (ClutterBoxPrivate));
 
   actor_class->get_paint_volume = clutter_box_real_get_paint_volume;
-  actor_class->pick = clutter_box_real_pick;
 
   gobject_class->set_property = clutter_box_set_property;
   gobject_class->get_property = clutter_box_get_property;

doc/reference/clutter/clutter-sections.txt

@@ -403,6 +403,13 @@ clutter_actor_set_child_at_index
 clutter_actor_set_child_below_sibling
 clutter_actor_contains
 clutter_actor_get_stage
+ClutterActorIter
+clutter_actor_iter_init
+clutter_actor_iter_next
+clutter_actor_iter_prev
+clutter_actor_iter_remove
+
+<SUBSECTION>
 clutter_actor_push_internal
 clutter_actor_pop_internal
 clutter_actor_set_parent

tests/conform/Makefile.am

@@ -62,6 +62,7 @@ units_sources += \
 	test-actor-graph.c		\
 	test-actor-destroy.c		\
 	test-actor-invariants.c 	\
+	test-actor-iter.c		\
 	test-actor-layout.c		\
 	test-actor-size.c		\
 	test-anchors.c                  \

tests/conform/test-actor-iter.c

@@ -0,0 +1,129 @@
+#include <glib.h>
+#include <clutter/clutter.h>
+#include "test-conform-common.h"
+
+void
+actor_iter_traverse_children (TestConformSimpleFixture *fixture G_GNUC_UNUSED,
+                              gconstpointer dummy G_GNUC_UNUSED)
+{
+  ClutterActorIter iter;
+  ClutterActor *actor;
+  ClutterActor *child;
+  int i, n_actors;
+
+  actor = clutter_actor_new ();
+  clutter_actor_set_name (actor, "root");
+  g_object_ref_sink (actor);
+
+  n_actors = g_random_int_range (10, 50);
+  for (i = 0; i < n_actors; i++)
+    {
+      char *name;
+
+      name = g_strdup_printf ("actor%d", i);
+      child = clutter_actor_new ();
+      clutter_actor_set_name (child, name);
+
+      clutter_actor_add_child (actor, child);
+
+      g_free (name);
+    }
+
+  g_assert_cmpint (clutter_actor_get_n_children (actor), ==, n_actors);
+
+  i = 0;
+  clutter_actor_iter_init (&iter, actor);
+  while (clutter_actor_iter_next (&iter, &child))
+    {
+      g_assert (CLUTTER_IS_ACTOR (child));
+      g_assert (clutter_actor_get_parent (child) == actor);
+
+      if (g_test_verbose ())
+        g_print ("actor %d = '%s'\n", i, clutter_actor_get_name (child));
+
+      if (i == 0)
+        g_assert (child == clutter_actor_get_first_child (actor));
+
+      if (i == (n_actors - 1))
+        g_assert (child == clutter_actor_get_last_child (actor));
+
+      i += 1;
+    }
+
+  g_assert_cmpint (i, ==, n_actors);
+
+  i = 0;
+  clutter_actor_iter_init (&iter, actor);
+  while (clutter_actor_iter_prev (&iter, &child))
+    {
+      g_assert (CLUTTER_IS_ACTOR (child));
+      g_assert (clutter_actor_get_parent (child) == actor);
+
+      if (g_test_verbose ())
+        g_print ("actor %d = '%s'\n", i, clutter_actor_get_name (child));
+
+      if (i == 0)
+        g_assert (child == clutter_actor_get_last_child (actor));
+
+      if (i == (n_actors - 1))
+        g_assert (child == clutter_actor_get_first_child (actor));
+
+      i += 1;
+    }
+
+  g_object_unref (actor);
+}
+
+void
+actor_iter_traverse_remove (TestConformSimpleFixture *fixture G_GNUC_UNUSED,
+                            gconstpointer dummy G_GNUC_UNUSED)
+{
+  ClutterActorIter iter;
+  ClutterActor *actor;
+  ClutterActor *child;
+  int i, n_actors;
+
+  actor = clutter_actor_new ();
+  clutter_actor_set_name (actor, "root");
+  g_object_ref_sink (actor);
+
+  n_actors = g_random_int_range (10, 50);
+  for (i = 0; i < n_actors; i++)
+    {
+      char *name;
+
+      name = g_strdup_printf ("actor%d", i);
+      child = clutter_actor_new ();
+      clutter_actor_set_name (child, name);
+
+      clutter_actor_add_child (actor, child);
+
+      g_free (name);
+    }
+
+  g_assert_cmpint (clutter_actor_get_n_children (actor), ==, n_actors);
+
+  i = 0;
+  clutter_actor_iter_init (&iter, actor);
+  while (clutter_actor_iter_next (&iter, &child))
+    {
+      g_assert (CLUTTER_IS_ACTOR (child));
+      g_assert (clutter_actor_get_parent (child) == actor);
+
+      if (g_test_verbose ())
+        g_print ("actor %d = '%s'\n", i, clutter_actor_get_name (child));
+
+      if (i == 0)
+        g_assert (child == clutter_actor_get_first_child (actor));
+
+      if (i == (n_actors - 1))
+        g_assert (child == clutter_actor_get_last_child (actor));
+
+      clutter_actor_iter_remove (&iter);
+
+      i += 1;
+    }
+
+  g_assert_cmpint (i, ==, n_actors);
+  g_assert_cmpint (0, ==, clutter_actor_get_n_children (actor));
+}

tests/conform/test-conform-main.c

@@ -145,20 +145,23 @@ main (int argc, char **argv)
   TEST_CONFORM_SIMPLE ("/actor", test_offscreen_redirect);
   TEST_CONFORM_SIMPLE ("/actor", test_shader_effect);
 
-  TEST_CONFORM_SIMPLE ("/invariants", test_initial_state);
-  TEST_CONFORM_SIMPLE ("/invariants", test_shown_not_parented);
-  TEST_CONFORM_SIMPLE ("/invariants", test_realized);
-  TEST_CONFORM_SIMPLE ("/invariants", test_realize_not_recursive);
-  TEST_CONFORM_SIMPLE ("/invariants", test_map_recursive);
-  TEST_CONFORM_SIMPLE ("/invariants", test_mapped);
-  TEST_CONFORM_SIMPLE ("/invariants", test_show_on_set_parent);
-  TEST_CONFORM_SIMPLE ("/invariants", test_clone_no_map);
-  TEST_CONFORM_SIMPLE ("/invariants", test_contains);
-  TEST_CONFORM_SIMPLE ("/invariants", default_stage);
+  TEST_CONFORM_SIMPLE ("/actor/iter", actor_iter_traverse_children);
+  TEST_CONFORM_SIMPLE ("/actor/iter", actor_iter_traverse_remove);
 
-  TEST_CONFORM_SIMPLE ("/opacity", test_label_opacity);
-  TEST_CONFORM_SIMPLE ("/opacity", test_rectangle_opacity);
-  TEST_CONFORM_SIMPLE ("/opacity", test_paint_opacity);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_initial_state);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_shown_not_parented);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_realized);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_realize_not_recursive);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_map_recursive);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_mapped);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_show_on_set_parent);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_clone_no_map);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", test_contains);
+  TEST_CONFORM_SIMPLE ("/actor/invariants", default_stage);
+
+  TEST_CONFORM_SIMPLE ("/actor/opacity", test_label_opacity);
+  TEST_CONFORM_SIMPLE ("/actor/opacity", test_rectangle_opacity);
+  TEST_CONFORM_SIMPLE ("/actor/opacity", test_paint_opacity);
 
   TEST_CONFORM_SIMPLE ("/text", text_utf8_validation);
   TEST_CONFORM_SIMPLE ("/text", text_set_empty);