Reuse the numeric id's used for picking actors to avoid the potential

of overflowing the id numbers when continusly creating and destroying
actors on long running applications for 16bpp.
* clutter/clutter-private.h: replaced hashtable with GArray and a
GSList for available slots in the array.
* clutter/clutter-actor.c: (create_actor_id): function to create an
actor->id mapping, (release_actor_id): function to mark an existing id
as available for reuse.
* clutter/clutter-main.c: (clutter_context_free): added utility
function for cleaning up the context, 
(clutter_get_actor_by_gid): use the GArray for looking up actors.

ChangeLog

@@ -1,3 +1,18 @@
+2008-02-29  Øyvind Kolås  <pippin@o-hand.com>
+
+	Reuse the numeric id's used for picking actors to avoid the potential
+	of overflowing the id numbers when continusly creating and destroying
+	actors on long running applications for 16bpp.
+
+	* clutter/clutter-private.h: replaced hashtable with GArray and a
+	GSList for available slots in the array.
+	* clutter/clutter-actor.c: (create_actor_id): function to create an
+	actor->id mapping, (release_actor_id): function to mark an existing id
+	as available for reuse.
+	* clutter/clutter-main.c: (clutter_context_free): added utility
+	function for cleaning up the context, 
+	(clutter_get_actor_by_gid): use the GArray for looking up actors.
+
 2008-02-26  Emmanuele Bassi  <ebassi@openedhand.com>
 
 	* clutter/clutter-alpha.c:

clutter/clutter-actor.c

@@ -151,8 +151,6 @@
 #include "clutter-units.h"
 #include "cogl.h"
 
-static guint32 __id = 0;
-
 typedef struct _ShaderData ShaderData;
 
 #define CLUTTER_ACTOR_GET_PRIVATE(obj) \
@@ -275,6 +273,54 @@ G_DEFINE_ABSTRACT_TYPE_WITH_CODE (ClutterActor,
                                   G_IMPLEMENT_INTERFACE (CLUTTER_TYPE_SCRIPTABLE,
                                                          clutter_scriptable_iface_init));
 
+
+static guint32
+create_actor_id (ClutterActor *actor)
+{
+  ClutterMainContext *context = CLUTTER_CONTEXT();
+  ClutterActor      **array;
+  guint32             id;
+
+  context = clutter_context_get_default ();
+
+  g_return_val_if_fail (context != NULL, 0);
+  g_return_val_if_fail (context->actor_array != NULL, 0);
+
+  /* There are items on our freelist */
+  if (context->free_actor_ids)
+    {
+      array = (void*) context->actor_array->data;
+      id = GPOINTER_TO_UINT (context->free_actor_ids->data);
+
+      context->free_actor_ids = g_slist_remove (context->free_actor_ids,
+                                                context->free_actor_ids->data);
+      array[id] = actor;
+      return id;
+    }
+
+  /* Allocate new id */
+  id = context->actor_array->len;
+  g_array_append_val (context->actor_array, actor);
+  return id;
+}
+
+static void
+release_actor_id (guint32 id)
+{
+  ClutterMainContext *context = CLUTTER_CONTEXT();
+  ClutterActor      **array;
+
+  context = clutter_context_get_default ();
+  g_return_if_fail (context != NULL);
+  g_return_if_fail (context->actor_array != NULL);
+
+  array = (void*) context->actor_array->data;
+  array[id] = (void*)0xdecafbad;
+
+  context->free_actor_ids = g_slist_prepend (context->free_actor_ids,
+                                             GUINT_TO_POINTER (id));
+}
+
 static gboolean
 redraw_update_idle (gpointer data)
 {
@@ -1627,6 +1673,7 @@ clutter_actor_finalize (GObject *object)
 		g_type_name (G_OBJECT_TYPE (actor)));
 
   g_free (actor->priv->name);
+  release_actor_id (actor->priv->id);
 
   G_OBJECT_CLASS (clutter_actor_parent_class)->finalize (object);
 }
@@ -2272,10 +2319,10 @@ clutter_actor_init (ClutterActor *self)
   priv->parent_actor = NULL;
   priv->has_clip     = FALSE;
   priv->opacity      = 0xff;
-  priv->id           = __id++;
+  priv->id           = create_actor_id (self);
   priv->scale_x      = CFX_ONE;
   priv->scale_y      = CFX_ONE;
-  priv->shader_data     = NULL;
+  priv->shader_data  = NULL;
 
   memset (priv->clip, 0, sizeof (ClutterUnit) * 4);
 
@@ -3968,9 +4015,6 @@ clutter_actor_set_parent (ClutterActor *self,
       return;
     }
 
-  g_hash_table_insert (clutter_context->actor_hash,
-		       GUINT_TO_POINTER (clutter_actor_get_gid (self)),
-		       (gpointer)self);
 
   g_object_ref_sink (self);
   self->priv->parent_actor = parent;
@@ -4039,8 +4083,6 @@ clutter_actor_unparent (ClutterActor *self)
   self->priv->parent_actor = NULL;
   g_signal_emit (self, actor_signals[PARENT_SET], 0, old_parent);
 
-  g_hash_table_remove (clutter_context->actor_hash,
-		       GUINT_TO_POINTER (clutter_actor_get_gid (self)));
 
   g_object_unref (self);
 }

clutter/clutter-main.c

@@ -269,6 +269,23 @@ _clutter_do_pick (ClutterStage   *stage,
   return clutter_get_actor_by_gid (id);
 }
 
+static void
+clutter_context_free (ClutterMainContext *context)
+{
+  /* this will take care of destroying the stage */
+  g_object_unref (context->backend);
+  context->backend = NULL;
+  g_array_free (context->actor_array, TRUE);
+  context->actor_array = NULL;
+  g_slist_free (context->free_actor_ids);
+  context->free_actor_ids = NULL;
+
+  /* XXX: The cleaning up of the event queue should be moved here from
+          the backend base class. */
+
+  g_free (context);
+}
+
 /**
  * clutter_main_quit:
  *
@@ -335,11 +352,7 @@ clutter_main (void)
 
   if (clutter_main_loop_level == 0)
     {
-      /* this will take care of destroying the stage */
-      g_object_unref (context->backend);
-      context->backend = NULL;
-
-      g_free (context);
+      clutter_context_free (context);
     }
 
   CLUTTER_MARK ();
@@ -799,8 +812,9 @@ pre_parse_hook (GOptionContext  *context,
   clutter_context->font_map = PANGO_FT2_FONT_MAP (pango_ft2_font_map_new ());
   pango_ft2_font_map_set_resolution (clutter_context->font_map, 96.0, 96.0);
 
-
-  clutter_context->actor_hash = g_hash_table_new (NULL, NULL);
+  clutter_context->actor_array = g_array_sized_new (FALSE, FALSE,
+                                                    sizeof (guint32), 256);
+  clutter_context->free_actor_ids = NULL;
 
   backend = clutter_context->backend;
   g_assert (CLUTTER_IS_BACKEND (backend));
@@ -1554,13 +1568,18 @@ ClutterActor*
 clutter_get_actor_by_gid (guint32 id)
 {
   ClutterMainContext *context;
+  ClutterActor      **array;
 
   context = clutter_context_get_default ();
 
   g_return_val_if_fail (context != NULL, NULL);
-  g_return_val_if_fail (context->actor_hash != NULL, NULL);
+  g_return_val_if_fail (context->actor_array != NULL, NULL);
 
-  return g_hash_table_lookup (context->actor_hash, GUINT_TO_POINTER (id));
+  g_assert (id < context->actor_array->len);
+  g_return_val_if_fail (id < context->actor_array->len, NULL);
+
+  array = (void*) context->actor_array->data;
+  return array[id];
 }
 
 void

clutter/clutter-private.h

@@ -83,7 +83,8 @@ struct _ClutterMainContext
   guint            motion_frequency;   /* Motion events per second */
   gint             num_reactives;      /* Num of reactive actors */
 
-  GHashTable      *actor_hash;	       /* Hash of all actors mapped to id */
+  GArray          *actor_array;        /* Array of ClutterActors        */
+  GSList          *free_actor_ids;     /* A stack of released actor ids */
 
   guint            frame_rate;         /* Default FPS */