From 043f804452a9ac12e59b1aed3bf1f2b4fad56a43 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Mon, 11 Apr 2011 14:11:39 +0100 Subject: [PATCH] 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 56189bbf8..138e9cfe1 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_actor_get_stage_internal (self); + stage = CLUTTER_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 3704ee635..2728f0346 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 6854700d5..bb81b7e22 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 7e7b395d4..d53849ff0 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 5634353fa..872b240d0 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 ac3d76c67..44f2a66b2 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);