From c9b7efec96636e9368a8dfb76440d1b89c618c99 Mon Sep 17 00:00:00 2001 From: Johan Bilien Date: Wed, 11 Nov 2009 20:40:57 -0500 Subject: [PATCH 1/3] Add a cache of size requests clutter_actor_get_preferred_width/height currently caches only one size requests, for a given height / width. It's common for a layout manager to call get_preferred_width with 2 different heights during the same allocation cycle. Typically once in the size request, once in the allocation. If clutter_actor_get_preferred_width is called alternatively with 2 different for_height, the cache is totally inefficient, and we end up always querying the actor size even when the actor does not need a re-allocation. http://bugzilla.openedhand.com/show_bug.cgi?id=1876 Signed-off-by: Emmanuele Bassi --- clutter/clutter-actor.c | 137 ++++++++++++++++++++++++++++++++++------ 1 file changed, 117 insertions(+), 20 deletions(-) diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c index fcab0b6a1..b81a26e47 100644 --- a/clutter/clutter-actor.c +++ b/clutter/clutter-actor.c @@ -234,6 +234,18 @@ struct _AnchorCoord } v; }; +/* 3 entries should be a good compromise, few layout managers + * will ask for 3 different preferred size in each allocation cycle */ +#define N_CACHED_SIZE_REQUESTS 3 +typedef struct _SizeRequest SizeRequest; +struct _SizeRequest +{ + guint age; + gfloat for_size; + gfloat min_size; + gfloat natural_size; +}; + /* Internal enum used to control mapped state update. This is a hint * which indicates when to do something other than just enforce * invariants. @@ -262,14 +274,17 @@ struct _ClutterActorPrivate /* request mode */ ClutterRequestMode request_mode; - /* our cached request width is for this height */ - gfloat request_width_for_height; - gfloat request_min_width; - gfloat request_natural_width; + /* our cached size requests for different width / height */ + SizeRequest width_requests[N_CACHED_SIZE_REQUESTS]; + SizeRequest height_requests[N_CACHED_SIZE_REQUESTS]; - /* our cached request height is for this width */ - gfloat request_height_for_width; + /* An age of 0 means the entry is not set */ + guint cached_height_age; + guint cached_width_age; + + gfloat request_min_width; gfloat request_min_height; + gfloat request_natural_width; gfloat request_natural_height; ClutterActorBox allocation; @@ -1663,6 +1678,12 @@ clutter_actor_real_queue_relayout (ClutterActor *self) priv->needs_height_request = TRUE; priv->needs_allocation = TRUE; + /* reset the cached size requests */ + memset (priv->width_requests, 0, + N_CACHED_SIZE_REQUESTS * sizeof (SizeRequest)); + memset (priv->height_requests, 0, + N_CACHED_SIZE_REQUESTS * sizeof (SizeRequest)); + /* always repaint also (no-op if not mapped) */ clutter_actor_queue_redraw (self); @@ -4392,6 +4413,9 @@ clutter_actor_init (ClutterActor *self) priv->needs_height_request = TRUE; priv->needs_allocation = TRUE; + priv->cached_width_age = 1; + priv->cached_height_age = 1; + priv->opacity_parent = NULL; priv->enable_model_view_transform = TRUE; @@ -4586,6 +4610,40 @@ clutter_actor_get_preferred_size (ClutterActor *self, *natural_height_p = natural_height; } +/* looks for a cached size request for this for_size. If not + * found, returns the oldest entry so it can be overwritten */ +static gboolean +_clutter_actor_get_cached_size_request (gfloat for_size, + SizeRequest *cached_size_requests, + SizeRequest **result) +{ + gboolean found_free_cache; + guint i; + + found_free_cache = FALSE; + *result = &cached_size_requests[0]; + + for (i = 0; i < N_CACHED_SIZE_REQUESTS; i++) + { + SizeRequest *sr; + + sr = &cached_size_requests[i]; + + if (sr->age > 0 && + sr->for_size == for_size) + { + *result = sr; + return TRUE; + } + else if (sr->age < (*result)->age) + { + *result = sr; + } + } + + return FALSE; +} + /** * clutter_actor_get_preferred_width: * @self: A #ClutterActor @@ -4616,14 +4674,23 @@ clutter_actor_get_preferred_width (ClutterActor *self, { ClutterActorClass *klass; ClutterActorPrivate *priv; + gboolean found_in_cache; + SizeRequest *cached_size_request; g_return_if_fail (CLUTTER_IS_ACTOR (self)); klass = CLUTTER_ACTOR_GET_CLASS (self); priv = self->priv; - if (priv->needs_width_request || - priv->request_width_for_height != for_height) + found_in_cache = FALSE; + cached_size_request = &priv->width_requests[0]; + + if (!priv->needs_width_request) + found_in_cache = _clutter_actor_get_cached_size_request (for_height, + priv->width_requests, + &cached_size_request); + + if (!found_in_cache) { gfloat min_width, natural_width; @@ -4641,16 +4708,21 @@ clutter_actor_get_preferred_width (ClutterActor *self, if (natural_width < min_width) natural_width = min_width; - if (!priv->min_width_set) - priv->request_min_width = min_width; + cached_size_request->min_size = min_width; + cached_size_request->natural_size = natural_width; + cached_size_request->for_size = for_height; + cached_size_request->age = priv->cached_width_age; - if (!priv->natural_width_set) - priv->request_natural_width = natural_width; - - priv->request_width_for_height = for_height; + priv->cached_width_age ++; priv->needs_width_request = FALSE; } + if (!priv->min_width_set) + priv->request_min_width = cached_size_request->min_size; + + if (!priv->natural_width_set) + priv->request_natural_width = cached_size_request->natural_size; + if (min_width_p) *min_width_p = priv->request_min_width; @@ -4687,20 +4759,29 @@ clutter_actor_get_preferred_height (ClutterActor *self, { ClutterActorClass *klass; ClutterActorPrivate *priv; + gboolean found_in_cache; + SizeRequest *cached_size_request; g_return_if_fail (CLUTTER_IS_ACTOR (self)); klass = CLUTTER_ACTOR_GET_CLASS (self); priv = self->priv; - if (priv->needs_height_request || - priv->request_height_for_width != for_width) + found_in_cache = FALSE; + cached_size_request = &priv->height_requests[0]; + + if (!priv->needs_height_request) + found_in_cache = _clutter_actor_get_cached_size_request (for_width, + priv->height_requests, + &cached_size_request); + + if (!found_in_cache) { gfloat min_height, natural_height; min_height = natural_height = 0; - CLUTTER_NOTE (LAYOUT, "Width request for %.2f px", for_width); + CLUTTER_NOTE (LAYOUT, "Height request for %.2f px", for_width); klass->get_preferred_height (self, for_width, &min_height, @@ -4713,15 +4794,31 @@ clutter_actor_get_preferred_height (ClutterActor *self, natural_height = min_height; if (!priv->min_height_set) - priv->request_min_height = min_height; + { + priv->request_min_height = min_height; + } if (!priv->natural_height_set) - priv->request_natural_height = natural_height; + { + priv->request_natural_height = natural_height; + } + + cached_size_request->min_size = min_height; + cached_size_request->natural_size = natural_height; + cached_size_request->for_size = for_width; + cached_size_request->age = priv->cached_height_age; + + priv->cached_height_age ++; - priv->request_height_for_width = for_width; priv->needs_height_request = FALSE; } + if (!priv->min_height_set) + priv->request_min_height = cached_size_request->min_size; + + if (!priv->natural_height_set) + priv->request_natural_height = cached_size_request->natural_size; + if (min_height_p) *min_height_p = priv->request_min_height; From cf62b8fe4ab0d7cca360077843a9e213540e2c4d Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Fri, 4 Dec 2009 17:38:26 +0000 Subject: [PATCH 2/3] actor: Add debugging notes for size cache Add a note for cache hits, and another one for cache misses. --- clutter/clutter-actor.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c index b81a26e47..9e7fee728 100644 --- a/clutter/clutter-actor.c +++ b/clutter/clutter-actor.c @@ -4632,6 +4632,7 @@ _clutter_actor_get_cached_size_request (gfloat for_size, if (sr->age > 0 && sr->for_size == for_size) { + CLUTTER_NOTE (LAYOUT, "Size cache hit for size: %.2f", for_size); *result = sr; return TRUE; } @@ -4641,6 +4642,8 @@ _clutter_actor_get_cached_size_request (gfloat for_size, } } + CLUTTER_NOTE (LAYOUT, "Size cache miss for size: %.2f", for_size); + return FALSE; } From b33b6287a1d3d3b07bbb845e79dfc4e2162c5c77 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Fri, 4 Dec 2009 17:39:04 +0000 Subject: [PATCH 3/3] tests: Clean up the BoxLayout interactive test --- tests/interactive/test-box-layout.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/tests/interactive/test-box-layout.c b/tests/interactive/test-box-layout.c index e45631880..a031e269d 100644 --- a/tests/interactive/test-box-layout.c +++ b/tests/interactive/test-box-layout.c @@ -32,6 +32,8 @@ "Press q\t\342\236\236\tQuit" static ClutterActor *hover_actor = NULL; +static ClutterActor *box = NULL; +static ClutterActor *label = NULL; static guint last_index = 0; static void @@ -88,7 +90,7 @@ leave_event (ClutterActor *actor, static gboolean button_release_event (ClutterActor *actor, ClutterEvent *event, - ClutterBoxLayout *box) + ClutterBoxLayout *layout) { gboolean xfill, yfill; ClutterBoxAlignment xalign, yalign; @@ -98,14 +100,14 @@ button_release_event (ClutterActor *actor, if (button == 1) { - clutter_box_layout_get_fill (box, actor, &xfill, &yfill); - clutter_box_layout_set_fill (box, actor, + clutter_box_layout_get_fill (layout, actor, &xfill, &yfill); + clutter_box_layout_set_fill (layout, actor, xfill ? FALSE : TRUE, yfill ? FALSE : TRUE); } else { - clutter_box_layout_get_alignment (box, actor, &xalign, &yalign); + clutter_box_layout_get_alignment (layout, actor, &xalign, &yalign); if (xalign < 2) xalign += 1; @@ -117,14 +119,14 @@ button_release_event (ClutterActor *actor, else yalign = 0; - clutter_box_layout_set_alignment (box, actor, xalign, yalign); + clutter_box_layout_set_alignment (layout, actor, xalign, yalign); } return TRUE; } static void -add_actor (ClutterBoxLayout *box, +add_actor (ClutterBoxLayout *layout, guint index_) { ClutterActor *rect; @@ -138,7 +140,7 @@ add_actor (ClutterBoxLayout *box, rect = clutter_rectangle_new_with_color (&color); clutter_actor_set_size (rect, 32, 64); - clutter_box_layout_pack (box, rect, expand, + clutter_box_layout_pack (layout, rect, expand, FALSE, /* x-fill */ FALSE, /* y-fill */ CLUTTER_BOX_ALIGNMENT_CENTER, @@ -152,7 +154,7 @@ add_actor (ClutterBoxLayout *box, g_signal_connect (rect, "leave-event", G_CALLBACK (leave_event), NULL); g_signal_connect (rect, "button-release-event", G_CALLBACK (button_release_event), - box); + layout); expand = !expand; } @@ -207,18 +209,21 @@ static void stage_size_changed_cb (ClutterActor *stage, const ClutterActorBox *allocation, ClutterAllocationFlags flags, - ClutterActor *box) + gpointer dummy G_GNUC_UNUSED) { gfloat width, height; clutter_actor_box_get_size (allocation, &width, &height); clutter_actor_set_size (box, width - 100, height - 100); + + clutter_actor_set_y (label, + height - clutter_actor_get_height (label) - 8); } G_MODULE_EXPORT int test_box_layout_main (int argc, char *argv[]) { - ClutterActor *stage, *box, *label; + ClutterActor *stage; ClutterLayoutManager *layout; gint i; @@ -242,7 +247,7 @@ test_box_layout_main (int argc, char *argv[]) layout); g_signal_connect (stage, "allocation-changed", G_CALLBACK (stage_size_changed_cb), - box); + NULL); label = clutter_text_new_with_text ("Sans 12px", INSTRUCTIONS); clutter_container_add_actor (CLUTTER_CONTAINER (stage), label);