[clutter] Make the pick id pool per Stage

[Emmanuele Bassi <ebassi src gnome org>]

commit 043f804452a9ac12e59b1aed3bf1f2b4fad56a43
Author: Emmanuele Bassi <ebassi linux intel com>
Date:   Mon Apr 11 14:11:39 2011 +0100

    Make the pick id pool per Stage
    
    The id pool used for the actor's id should be a per-stage field. At some
    point, we might have a Stage mapped to multiple framebuffers, or each
    Stage mapped to a different framebuffer; also, on backend with low
    color precision we don't want to exhaust the size of the available ids
    with a global pool. Finally, it's yet another thing we can remove from
    the global Clutter context.
    
    Having the id pool allocated per-stage, and the pick id assigned on
    Actor:mapped will make the whole pick-id more reliable and future proof.
    
    http://bugzilla.clutter-project.org/show_bug.cgi?id=2633
    
    https://bugzilla.gnome.org/show_bug.cgi?id=647876

 clutter/clutter-actor.c         |   42 ++++++++++++++++++++++++++++++-------
 clutter/clutter-main.c          |   14 ++++++++----
 clutter/clutter-private.h       |    9 ++++---
 clutter/clutter-stage-private.h |    7 ++++++
 clutter/clutter-stage.c         |   44 +++++++++++++++++++++++++++++++++++++-
 tests/conform/test-pick.c       |   14 +++++-------
 6 files changed, 103 insertions(+), 27 deletions(-)
---
diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c
index 56189bb..138e9cf 100644
--- a/clutter/clutter-actor.c
+++ b/clutter/clutter-actor.c
@@ -449,6 +449,8 @@ struct _ClutterActorPrivate
   gchar          *name;
   guint32         id; /* Unique ID */
 
+  gint32 pick_id;
+
   gdouble         scale_x;
   gdouble         scale_y;
   AnchorCoord     scale_center;
@@ -1032,12 +1034,23 @@ clutter_actor_update_map_state (ClutterActor  *self,
 static void
 clutter_actor_real_map (ClutterActor *self)
 {
+  ClutterActorPrivate *priv = self->priv;
+  ClutterActor *stage;
   GList *c;
 
   g_assert (!CLUTTER_ACTOR_IS_MAPPED (self));
 
+  CLUTTER_NOTE (ACTOR, "Mapping actor '%s'",
+                _clutter_actor_get_debug_name (self));
+
   CLUTTER_ACTOR_SET_FLAGS (self, CLUTTER_ACTOR_MAPPED);
 
+  stage = _clutter_actor_get_stage_internal (self);
+  priv->pick_id = _clutter_stage_acquire_pick_id (CLUTTER_STAGE (stage), self);
+  CLUTTER_NOTE (ACTOR, "Pick id '%d' for actor '%s'",
+                priv->pick_id,
+                _clutter_actor_get_debug_name (self));
+
   /* notify on parent mapped before potentially mapping
    * children, so apps see a top-down notification.
    */
@@ -1086,11 +1099,15 @@ clutter_actor_map (ClutterActor *self)
 static void
 clutter_actor_real_unmap (ClutterActor *self)
 {
+  ClutterActorPrivate *priv = self->priv;
   GList *c;
 
   g_assert (CLUTTER_ACTOR_IS_MAPPED (self));
 
-  for (c = self->priv->children; c; c = c->next)
+  CLUTTER_NOTE (ACTOR, "Unmapping actor '%s'",
+                _clutter_actor_get_debug_name (self));
+
+  for (c = priv->children; c; c = c->next)
     {
       ClutterActor *child = c->data;
       clutter_actor_unmap (child);
@@ -1101,8 +1118,8 @@ clutter_actor_real_unmap (ClutterActor *self)
   /* clear the contents of the last paint volume, so that hiding + moving +
    * showing will not result in the wrong area being repainted
    */
-  _clutter_paint_volume_init_static (&self->priv->last_paint_volume, NULL);
-  self->priv->last_paint_volume_valid = TRUE;
+  _clutter_paint_volume_init_static (&priv->last_paint_volume, NULL);
+  priv->last_paint_volume_valid = TRUE;
 
   /* notify on parent mapped after potentially unmapping
    * children, so apps see a bottom-up notification.
@@ -1112,14 +1129,19 @@ clutter_actor_real_unmap (ClutterActor *self)
   /* relinquish keyboard focus if we were unmapped while owning it */
   if (!CLUTTER_ACTOR_IS_TOPLEVEL (self))
     {
-      ClutterActor *stage;
+      ClutterStage *stage;
+
+      stage = CLUTTER_STAGE (_clutter_actor_get_stage_internal (self));
 
-      stage = _clutter_actor_get_stage_internal (self);
+      if (stage != NULL)
+        _clutter_stage_release_pick_id (stage, priv->pick_id);
+
+      priv->pick_id = -1;
 
       if (stage != NULL &&
-          clutter_stage_get_key_focus (CLUTTER_STAGE (stage)) == self)
+          clutter_stage_get_key_focus (stage) == self)
         {
-          clutter_stage_set_key_focus (CLUTTER_STAGE (stage), NULL);
+          clutter_stage_set_key_focus (stage, NULL);
         }
     }
 }
@@ -2653,7 +2675,10 @@ actor_has_shader_data (ClutterActor *self)
 guint32
 _clutter_actor_get_pick_id (ClutterActor *self)
 {
-  return self->priv->id;
+  if (self->priv->pick_id < 0)
+    return 0;
+
+  return self->priv->pick_id;
 }
 
 /**
@@ -5003,6 +5028,7 @@ clutter_actor_init (ClutterActor *self)
   priv->has_clip = FALSE;
   priv->opacity = 0xff;
   priv->id = _clutter_context_acquire_id (self);
+  priv->pick_id = -1;
   priv->scale_x = 1.0;
   priv->scale_y = 1.0;
   priv->show_on_set_parent = TRUE;
diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c
index 3704ee6..2728f03 100644
--- a/clutter/clutter-main.c
+++ b/clutter/clutter-main.c
@@ -304,13 +304,17 @@ clutter_get_motion_events_enabled (void)
 }
 
 ClutterActor *
-_clutter_get_actor_by_id (guint32 actor_id)
+_clutter_get_actor_by_id (ClutterStage *stage,
+                          guint32       actor_id)
 {
-  ClutterMainContext *context = _clutter_context_get_default ();
+  if (stage == NULL)
+    {
+      ClutterMainContext *context = _clutter_context_get_default ();
 
-  g_assert (context->id_pool != NULL);
+      return _clutter_id_pool_lookup (context->id_pool, actor_id);
+    }
 
-  return _clutter_id_pool_lookup (context->id_pool, actor_id);
+  return _clutter_stage_get_actor_by_pick_id (stage, actor_id);
 }
 
 void
@@ -2302,7 +2306,7 @@ _clutter_process_event (ClutterEvent *event)
 ClutterActor *
 clutter_get_actor_by_gid (guint32 id_)
 {
-  return _clutter_get_actor_by_id (id_);
+  return _clutter_get_actor_by_id (NULL, id_);
 }
 
 void
diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h
index 6854700..bb81b7e 100644
--- a/clutter/clutter-private.h
+++ b/clutter/clutter-private.h
@@ -194,10 +194,11 @@ G_CONST_RETURN gchar *_clutter_gettext (const gchar *str);
 gboolean      _clutter_feature_init (GError **error);
 
 /* Picking code */
-guint         _clutter_pixel_to_id (guchar pixel[4]);
-void          _clutter_id_to_color (guint id,
-                                    ClutterColor *col);
-ClutterActor *_clutter_get_actor_by_id (guint32 actor_id);
+guint           _clutter_pixel_to_id            (guchar        pixel[4]);
+void            _clutter_id_to_color            (guint         id,
+                                                 ClutterColor *col);
+ClutterActor *  _clutter_get_actor_by_id        (ClutterStage *stage,
+                                                 guint32       actor_id);
 
 /* use this function as the accumulator if you have a signal with
  * a G_TYPE_BOOLEAN return value; this will stop the emission as
diff --git a/clutter/clutter-stage-private.h b/clutter/clutter-stage-private.h
index 7e7b395..d53849f 100644
--- a/clutter/clutter-stage-private.h
+++ b/clutter/clutter-stage-private.h
@@ -96,6 +96,13 @@ gboolean _clutter_stage_get_motion_events_enabled (ClutterStage *stage);
 
 CoglFramebuffer *_clutter_stage_get_active_framebuffer (ClutterStage *stage);
 
+gint32          _clutter_stage_acquire_pick_id          (ClutterStage *stage,
+                                                         ClutterActor *actor);
+void            _clutter_stage_release_pick_id          (ClutterStage *stage,
+                                                         gint32        pick_id);
+ClutterActor *  _clutter_stage_get_actor_by_pick_id     (ClutterStage *stage,
+                                                         gint32        pick_id);
+
 G_END_DECLS
 
 #endif /* __CLUTTER_STAGE_PRIVATE_H__ */
diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c
index 5634353..872b240 100644
--- a/clutter/clutter-stage.c
+++ b/clutter/clutter-stage.c
@@ -66,6 +66,7 @@
 #include "clutter-device-manager-private.h"
 #include "clutter-enum-types.h"
 #include "clutter-event-private.h"
+#include "clutter-id-pool.h"
 #include "clutter-main.h"
 #include "clutter-marshal.h"
 #include "clutter-master-clock.h"
@@ -155,6 +156,8 @@ struct _ClutterStagePrivate
   GTimer *fps_timer;
   gint32 timer_n_frames;
 
+  ClutterIDPool *pick_id_pool;
+
 #ifdef CLUTTER_ENABLE_DEBUG
   gulong redraw_count;
 #endif /* CLUTTER_ENABLE_DEBUG */
@@ -1284,7 +1287,7 @@ _clutter_stage_do_pick (ClutterStage   *stage,
         }
 
       id_ = _clutter_pixel_to_id (pixel);
-      actor = _clutter_get_actor_by_id (id_);
+      actor = _clutter_get_actor_by_id (stage, id_);
       goto result;
     }
 
@@ -1376,7 +1379,7 @@ _clutter_stage_do_pick (ClutterStage   *stage,
     }
 
   id_ = _clutter_pixel_to_id (pixel);
-  actor = _clutter_get_actor_by_id (id_);
+  actor = _clutter_get_actor_by_id (stage, id_);
 
 result:
 
@@ -1598,6 +1601,8 @@ clutter_stage_finalize (GObject *object)
 
   g_hash_table_destroy (priv->devices);
 
+  _clutter_id_pool_free (priv->pick_id_pool);
+
   if (priv->fps_timer != NULL)
     g_timer_destroy (priv->fps_timer);
 
@@ -2034,6 +2039,8 @@ clutter_stage_init (ClutterStage *self)
     g_array_new (FALSE, FALSE, sizeof (ClutterPaintVolume));
 
   priv->devices = g_hash_table_new (NULL, NULL);
+
+  priv->pick_id_pool = _clutter_id_pool_new (256);
 }
 
 /**
@@ -3827,3 +3834,36 @@ _clutter_stage_get_active_framebuffer (ClutterStage *stage)
 {
   return stage->priv->active_framebuffer;
 }
+
+gint32
+_clutter_stage_acquire_pick_id (ClutterStage *stage,
+                                ClutterActor *actor)
+{
+  ClutterStagePrivate *priv = stage->priv;
+
+  g_assert (priv->pick_id_pool != NULL);
+
+  return _clutter_id_pool_add (priv->pick_id_pool, actor);
+}
+
+void
+_clutter_stage_release_pick_id (ClutterStage *stage,
+                                gint32        pick_id)
+{
+  ClutterStagePrivate *priv = stage->priv;
+
+  g_assert (priv->pick_id_pool != NULL);
+
+  _clutter_id_pool_remove (priv->pick_id_pool, pick_id);
+}
+
+ClutterActor *
+_clutter_stage_get_actor_by_pick_id (ClutterStage *stage,
+                                     gint32        pick_id)
+{
+  ClutterStagePrivate *priv = stage->priv;
+
+  g_assert (priv->pick_id_pool != NULL);
+
+  return _clutter_id_pool_lookup (priv->pick_id_pool, pick_id);
+}
diff --git a/tests/conform/test-pick.c b/tests/conform/test-pick.c
index ac3d76c..44f2a66 100644
--- a/tests/conform/test-pick.c
+++ b/tests/conform/test-pick.c
@@ -13,7 +13,7 @@ struct _State
 {
   ClutterActor *stage;
   int y, x;
-  guint32 gids[ACTORS_X * ACTORS_Y];
+  ClutterActor *actors[ACTORS_X * ACTORS_Y];
   guint actor_width, actor_height;
   gboolean pass;
 };
@@ -74,7 +74,6 @@ on_timeout (State *state)
         for (x = 0; x < ACTORS_X; x++)
           {
             gboolean pass = FALSE;
-            guint32 gid;
             ClutterActor *actor
               = clutter_stage_get_actor_at_pos (CLUTTER_STAGE (state->stage),
                                                 CLUTTER_PICK_ALL,
@@ -84,8 +83,8 @@ on_timeout (State *state)
                                                 + state->actor_height / 2);
 
             if (g_test_verbose ())
-              g_print ("% 3i,% 3i / % 4i -> ",
-                       x, y, (int) state->gids[y * ACTORS_X + x]);
+              g_print ("% 3i,% 3i / %p -> ",
+                       x, y, state->actors[y * ACTORS_X + x]);
 
             if (actor == NULL)
               {
@@ -104,15 +103,14 @@ on_timeout (State *state)
               }
             else
               {
-                gid = clutter_actor_get_gid (actor);
-                if (gid == state->gids[y * ACTORS_X + x]
+                if (actor == state->actors[y * ACTORS_X + x]
                     && (test_num != 2
                         || x < 2 || x >= ACTORS_X - 2
                         || y < 2 || y >= ACTORS_Y - 2))
                   pass = TRUE;
 
                 if (g_test_verbose ())
-                  g_print ("% 10i: %s\n", gid, pass ? "pass" : "FAIL");
+                  g_print ("%p: %s\n", actor, pass ? "pass" : "FAIL");
               }
 
             if (!pass)
@@ -152,7 +150,7 @@ actor_picking (void)
 
 	clutter_container_add (CLUTTER_CONTAINER (state.stage), rect, NULL);
 
-	state.gids[y * ACTORS_X + x] = clutter_actor_get_gid (rect);
+	state.actors[y * ACTORS_X + x] = rect;
       }
 
   clutter_actor_show (state.stage);