group: Restore previous implementation

Making Group just a proxy to Actor broke some behaviour that application
and toolkit code was relying on. Let's keep Group around to fight
another day.

This commit fixes gnome-shell as far as I can test it.

clutter/clutter-actor.c

@@ -10438,12 +10438,12 @@ clutter_actor_reparent (ClutterActor *self,
       /* we emit the ::parent-set signal once */
       g_signal_emit (self, actor_signals[PARENT_SET], 0, old_parent);
 
-      g_object_unref (self);
-
       CLUTTER_UNSET_PRIVATE_FLAGS (self, CLUTTER_IN_REPARENT);
 
       /* the IN_REPARENT flag suspends state updates */
       clutter_actor_update_map_state (self, MAP_STATE_CHECK);
+
+      g_object_unref (self);
    }
 }
 

clutter/deprecated/clutter-group.c

@@ -53,6 +53,7 @@
 #define CLUTTER_DISABLE_DEPRECATION_WARNINGS
 #include "clutter-group.h"
 
+#include "clutter-actor.h"
 #include "clutter-container.h"
 #include "clutter-fixed-layout.h"
 #include "clutter-main.h"
@@ -63,57 +64,344 @@
 
 #include "cogl/cogl.h"
 
-#define CLUTTER_GROUP_GET_PRIVATE(obj) \
+#define CLUTTER_GROUP_GET_PRIVATE(obj)  (G_TYPE_INSTANCE_GET_PRIVATE ((obj), CLUTTER_TYPE_GROUP, ClutterGroupPrivate))
-  (G_TYPE_INSTANCE_GET_PRIVATE ((obj), CLUTTER_TYPE_GROUP, ClutterGroupPrivate))
 
-G_DEFINE_TYPE (ClutterGroup, clutter_group, CLUTTER_TYPE_ACTOR)
+struct _ClutterGroupPrivate
+{
+  GList *children;
+
+  ClutterLayoutManager *layout;
+};
+
+enum
+{
+  ADD,
+  REMOVE,
+
+  LAST_SIGNAL
+};
+
+static void clutter_container_iface_init (ClutterContainerIface *iface);
+
+G_DEFINE_TYPE_WITH_CODE (ClutterGroup, clutter_group, CLUTTER_TYPE_ACTOR,
+                         G_IMPLEMENT_INTERFACE (CLUTTER_TYPE_CONTAINER,
+                                                clutter_container_iface_init));
+
+static gint
+sort_by_depth (gconstpointer a,
+               gconstpointer b)
+{
+  gfloat depth_a = clutter_actor_get_depth (CLUTTER_ACTOR(a));
+  gfloat depth_b = clutter_actor_get_depth (CLUTTER_ACTOR(b));
+
+  if (depth_a < depth_b)
+    return -1;
+
+  if (depth_a > depth_b)
+    return 1;
+
+  return 0;
+}
+
+static void
+clutter_group_real_add (ClutterContainer *container,
+                        ClutterActor     *actor)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (container)->priv;
+
+  g_object_ref (actor);
+
+  priv->children = g_list_append (priv->children, actor);
+  clutter_actor_set_parent (actor, CLUTTER_ACTOR (container));
+
+  /* queue a relayout, to get the correct positioning inside
+   * the ::actor-added signal handlers
+   */
+  clutter_actor_queue_relayout (CLUTTER_ACTOR (container));
+
+  g_signal_emit_by_name (container, "actor-added", actor);
+
+  clutter_container_sort_depth_order (container);
+
+  g_object_unref (actor);
+}
+
+static void
+clutter_group_real_remove (ClutterContainer *container,
+                           ClutterActor     *actor)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (container)->priv;
+
+  g_object_ref (actor);
+
+  priv->children = g_list_remove (priv->children, actor);
+  clutter_actor_unparent (actor);
+
+  /* queue a relayout, to get the correct positioning inside
+   * the ::actor-removed signal handlers
+   */
+  clutter_actor_queue_relayout (CLUTTER_ACTOR (container));
+
+  /* at this point, the actor passed to the "actor-removed" signal
+   * handlers is not parented anymore to the container but since we
+   * are holding a reference on it, it's still valid
+   */
+  g_signal_emit_by_name (container, "actor-removed", actor);
+
+  clutter_actor_queue_redraw (CLUTTER_ACTOR (container));
+
+  g_object_unref (actor);
+}
+
+static void
+clutter_group_real_foreach (ClutterContainer *container,
+                            ClutterCallback   callback,
+                            gpointer          user_data)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (container)->priv;
+
+  /* Using g_list_foreach instead of iterating the list manually
+     because it has better protection against the current node being
+     removed. This will happen for example if someone calls
+     clutter_container_foreach(container, clutter_actor_destroy) */
+  g_list_foreach (priv->children, (GFunc) callback, user_data);
+}
+
+static void
+clutter_group_real_raise (ClutterContainer *container,
+                          ClutterActor     *actor,
+                          ClutterActor     *sibling)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (container)->priv;
+
+  priv->children = g_list_remove (priv->children, actor);
+
+  /* Raise at the top */
+  if (!sibling)
+    {
+      GList *last_item;
+
+      last_item = g_list_last (priv->children);
+
+      if (last_item)
+	sibling = last_item->data;
+
+      priv->children = g_list_append (priv->children, actor);
+    }
+  else
+    {
+      gint index_ = g_list_index (priv->children, sibling) + 1;
+
+      priv->children = g_list_insert (priv->children, actor, index_);
+    }
+
+  /* set Z ordering a value below, this will then call sort
+   * as values are equal ordering shouldn't change but Z
+   * values will be correct.
+   *
+   * FIXME: optimise
+   */
+  if (sibling &&
+      clutter_actor_get_depth (sibling) != clutter_actor_get_depth (actor))
+    {
+      clutter_actor_set_depth (actor, clutter_actor_get_depth (sibling));
+    }
+
+  clutter_actor_queue_redraw (CLUTTER_ACTOR (container));
+}
+
+static void
+clutter_group_real_lower (ClutterContainer *container,
+                          ClutterActor     *actor,
+                          ClutterActor     *sibling)
+{
+  ClutterGroup *self = CLUTTER_GROUP (container);
+  ClutterGroupPrivate *priv = self->priv;
+
+  priv->children = g_list_remove (priv->children, actor);
+
+  /* Push to bottom */
+  if (!sibling)
+    {
+      GList *last_item;
+
+      last_item = g_list_first (priv->children);
+
+      if (last_item)
+	sibling = last_item->data;
+
+      priv->children = g_list_prepend (priv->children, actor);
+    }
+  else
+    {
+      gint index_ = g_list_index (priv->children, sibling);
+
+      priv->children = g_list_insert (priv->children, actor, index_);
+    }
+
+  /* See comment in group_raise for this */
+  if (sibling &&
+      clutter_actor_get_depth (sibling) != clutter_actor_get_depth (actor))
+    {
+      clutter_actor_set_depth (actor, clutter_actor_get_depth (sibling));
+    }
+
+  clutter_actor_queue_redraw (CLUTTER_ACTOR (container));
+}
+
+static void
+clutter_group_real_sort_depth_order (ClutterContainer *container)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (container)->priv;
+
+  priv->children = g_list_sort (priv->children, sort_by_depth);
+
+  clutter_actor_queue_redraw (CLUTTER_ACTOR (container));
+}
+
+static void
+clutter_container_iface_init (ClutterContainerIface *iface)
+{
+  iface->add = clutter_group_real_add;
+  iface->remove = clutter_group_real_remove;
+  iface->foreach = clutter_group_real_foreach;
+  iface->raise = clutter_group_real_raise;
+  iface->lower = clutter_group_real_lower;
+  iface->sort_depth_order = clutter_group_real_sort_depth_order;
+}
+
+static void
+clutter_group_real_paint (ClutterActor *actor)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
+
+  CLUTTER_NOTE (PAINT, "ClutterGroup paint enter '%s'",
+                clutter_actor_get_name (actor) ? clutter_actor_get_name (actor)
+                                               : "unknown");
+
+  g_list_foreach (priv->children, (GFunc) clutter_actor_paint, NULL);
+
+  CLUTTER_NOTE (PAINT, "ClutterGroup paint leave '%s'",
+                clutter_actor_get_name (actor) ? clutter_actor_get_name (actor)
+                                               : "unknown");
+}
+
+static void
+clutter_group_real_pick (ClutterActor       *actor,
+                         const ClutterColor *pick)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
+
+  /* Chain up so we get a bounding box pained (if we are reactive) */
+  CLUTTER_ACTOR_CLASS (clutter_group_parent_class)->pick (actor, pick);
+
+  g_list_foreach (priv->children, (GFunc) clutter_actor_paint, NULL);
+}
+
+static void
+clutter_group_real_get_preferred_width (ClutterActor *actor,
+                                        gfloat        for_height,
+                                        gfloat       *min_width,
+                                        gfloat       *natural_width)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
+
+  clutter_layout_manager_get_preferred_width (priv->layout,
+                                              CLUTTER_CONTAINER (actor),
+                                              for_height,
+                                              min_width, natural_width);
+}
+
+static void
+clutter_group_real_get_preferred_height (ClutterActor *actor,
+                                         gfloat        for_width,
+                                         gfloat       *min_height,
+                                         gfloat       *natural_height)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
+
+  clutter_layout_manager_get_preferred_height (priv->layout,
+                                               CLUTTER_CONTAINER (actor),
+                                               for_width,
+                                               min_height, natural_height);
+}
+
+static void
+clutter_group_real_allocate (ClutterActor           *actor,
+                             const ClutterActorBox  *allocation,
+                             ClutterAllocationFlags  flags)
+{
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
+  ClutterActorClass *klass;
+
+  klass = CLUTTER_ACTOR_CLASS (clutter_group_parent_class);
+  klass->allocate (actor, allocation, flags);
+
+  if (priv->children == NULL)
+    return;
+
+  clutter_layout_manager_allocate (priv->layout,
+                                   CLUTTER_CONTAINER (actor),
+                                   allocation, flags);
+}
+
+static void
+clutter_group_dispose (GObject *object)
+{
+  ClutterGroup *self = CLUTTER_GROUP (object);
+  ClutterGroupPrivate *priv = self->priv;
+
+  /* Note: we are careful to consider that destroying children could
+   * have the side-effect of destroying other children so
+   * priv->children may be modified during clutter_actor_destroy. */
+  while (priv->children != NULL)
+    {
+      ClutterActor *child = priv->children->data;
+      priv->children = g_list_delete_link (priv->children, priv->children);
+      clutter_actor_destroy (child);
+    }
+
+  if (priv->layout)
+    {
+      clutter_layout_manager_set_container (priv->layout, NULL);
+      g_object_unref (priv->layout);
+      priv->layout = NULL;
+    }
+
+  G_OBJECT_CLASS (clutter_group_parent_class)->dispose (object);
+}
 
 static void
 clutter_group_real_show_all (ClutterActor *actor)
 {
-  ClutterActor *iter;
+  clutter_container_foreach (CLUTTER_CONTAINER (actor),
-
+                             CLUTTER_CALLBACK (clutter_actor_show),
-  for (iter = clutter_actor_get_first_child (actor);
+                             NULL);
-       iter != NULL;
-       iter = clutter_actor_get_next_sibling (iter))
-    clutter_actor_show (iter);
-
   clutter_actor_show (actor);
 }
 
 static void
 clutter_group_real_hide_all (ClutterActor *actor)
 {
-  ClutterActor *iter;
-
   clutter_actor_hide (actor);
-
+  clutter_container_foreach (CLUTTER_CONTAINER (actor),
-  for (iter = clutter_actor_get_first_child (actor);
+                             CLUTTER_CALLBACK (clutter_actor_hide),
-       iter != NULL;
+                             NULL);
-       iter = clutter_actor_get_next_sibling (iter))
-    clutter_actor_hide (iter);
 }
 
 static gboolean
-clutter_group_get_paint_volume (ClutterActor *actor,
+clutter_group_real_get_paint_volume (ClutterActor       *actor,
-                                ClutterPaintVolume *volume)
+                                     ClutterPaintVolume *volume)
 {
-  ClutterActor *child;
+  ClutterGroupPrivate *priv = CLUTTER_GROUP (actor)->priv;
-  gboolean retval;
+  GList *l;
 
-  /* bail out early if we don't have any child */
+  if (priv->children == NULL)
-  if (clutter_actor_get_n_children (actor) == 0)
+    return TRUE;
-    return FALSE;
 
-  retval = TRUE;
+  for (l = priv->children; l != NULL; l = l->next)
-
-  /* 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))
     {
+      ClutterActor *child = l->data;
       const ClutterPaintVolume *child_volume;
 
       /* This gets the paint volume of the child transformed into the
@@ -125,24 +413,39 @@ clutter_group_get_paint_volume (ClutterActor *actor,
       clutter_paint_volume_union (volume, child_volume);
     }
 
-  return retval;
+  return TRUE;
 }
 
 static void
 clutter_group_class_init (ClutterGroupClass *klass)
 {
+  GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
   ClutterActorClass *actor_class = CLUTTER_ACTOR_CLASS (klass);
 
+  g_type_class_add_private (klass, sizeof (ClutterGroupPrivate));
+
+  actor_class->get_preferred_width = clutter_group_real_get_preferred_width;
+  actor_class->get_preferred_height = clutter_group_real_get_preferred_height;
+  actor_class->allocate = clutter_group_real_allocate;
+  actor_class->paint = clutter_group_real_paint;
+  actor_class->pick = clutter_group_real_pick;
   actor_class->show_all = clutter_group_real_show_all;
   actor_class->hide_all = clutter_group_real_hide_all;
-  actor_class->get_paint_volume = clutter_group_get_paint_volume;
+  actor_class->get_paint_volume = clutter_group_real_get_paint_volume;
+
+  gobject_class->dispose = clutter_group_dispose;
 }
 
 static void
 clutter_group_init (ClutterGroup *self)
 {
-  clutter_actor_set_layout_manager (CLUTTER_ACTOR (self),
+  self->priv = CLUTTER_GROUP_GET_PRIVATE (self);
-                                    clutter_fixed_layout_new ());
+
+  self->priv->layout = clutter_fixed_layout_new ();
+  g_object_ref_sink (self->priv->layout);
+
+  clutter_layout_manager_set_container (self->priv->layout,
+                                        CLUTTER_CONTAINER (self));
 }
 
 /**