diff --git a/ChangeLog b/ChangeLog index 67bcda6d6..a0d79a9de 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,25 @@ +2008-09-12 Neil Roberts + + Bug 1010 - ClutterLabel does not update the layout (again) + + * clutter/clutter-label.c: Bring back layout caching. This time it + will cache up to three different layouts. The width for which each + layout was generated is stored so that if the same width is + requested again it can use the cached version. If no cached + version is available it will try to replace the oldest + layout. 'Age' is determined by a counter which is incremented + every time a layout is created. The cache is cleared after any + property changes that might affect the layout. + (struct _ClutterLabelPrivate): Removed extents_width, + extents_height and context because they weren't used anywhere. + + * tests/test-label-cache.c: Add a test which checks whether the + label is using a different layout when properties are + changed. Also compares the extents of the label's layout with an + independent layout and fails if they aren't the same. + + * tests/Makefile.am (noinst_PROGRAMS): Add test-label-cache + 2008-09-10 Neil Roberts Bug 1137 - Setting the anchor point does not trigger a re-paint diff --git a/clutter/clutter-label.c b/clutter/clutter-label.c index 89cc922d6..2ab3d0c73 100644 --- a/clutter/clutter-label.c +++ b/clutter/clutter-label.c @@ -68,9 +68,30 @@ enum #define CLUTTER_LABEL_GET_PRIVATE(obj) \ (G_TYPE_INSTANCE_GET_PRIVATE ((obj), CLUTTER_TYPE_LABEL, ClutterLabelPrivate)) +typedef struct _ClutterLabelCachedLayout ClutterLabelCachedLayout; + +struct _ClutterLabelCachedLayout +{ + /* Cached layout. Pango internally caches the computed extents when + they are requested so there is no need to cache that as well */ + PangoLayout *layout; + /* The width that used to generate this layout */ + ClutterUnit width; + /* A number representing the age of this cache (so that when a new + layout is needed the last used cache is replaced) */ + guint age; +}; + +/* We need at least three cached layouts to run the allocation without + regenerating a new layout. First the layout will be generated at + full width to get the preferred width, then it will be generated at + the preferred width to get the preferred height and then it might + be regenerated at a different width to get the height for the + actual allocated width */ +#define CLUTTER_LABEL_N_CACHED_LAYOUTS 3 + struct _ClutterLabelPrivate { - PangoContext *context; PangoFontDescription *font_desc; ClutterColor fgcol; @@ -78,9 +99,6 @@ struct _ClutterLabelPrivate gchar *text; gchar *font_name; - gint extents_width; - gint extents_height; - guint alignment : 2; guint wrap : 1; guint use_underline : 1; @@ -92,11 +110,9 @@ struct _ClutterLabelPrivate PangoAttrList *attrs; PangoAttrList *effective_attrs; - PangoLayout *layout; - ClutterUnit layout_width; - ClutterUnit layout_height; - guint layout_dirty : 1; + ClutterLabelCachedLayout cached_layouts[CLUTTER_LABEL_N_CACHED_LAYOUTS]; + guint cache_age; }; /* @@ -179,26 +195,83 @@ clutter_label_create_layout_no_cache (ClutterLabel *label, return layout; } +static void +clutter_label_dirty_cache (ClutterLabel *label) +{ + ClutterLabelPrivate *priv = label->priv; + int i; + + /* Delete the cached layouts so they will be recreated the next time + they are needed */ + for (i = 0; i < CLUTTER_LABEL_N_CACHED_LAYOUTS; i++) + if (priv->cached_layouts[i].layout) + { + g_object_unref (priv->cached_layouts[i].layout); + priv->cached_layouts[i].layout = NULL; + } +} + /* * clutter_label_create_layout: * @label: a #ClutterLabel * @allocation_width: the allocation width * - * Link clutter_label_create_layout_no_cache(), but will also - * ensure the glyphs cache. This function should be called by - * clutter_label_allocate(). + * Like clutter_label_create_layout_no_cache(), but will also ensure + * the glyphs cache. If a previously cached layout generated using the + * same width is available then that will be used instead of + * generating a new one. */ static PangoLayout * clutter_label_create_layout (ClutterLabel *label, ClutterUnit allocation_width) { - PangoLayout *retval; + ClutterLabelPrivate *priv = label->priv; + int i; + ClutterLabelCachedLayout *oldest_cache = priv->cached_layouts; + gboolean found_free_cache = FALSE; - retval = clutter_label_create_layout_no_cache (label, allocation_width); + /* Search for a cached layout with the same width and keep track of + the oldest one */ + for (i = 0; i < CLUTTER_LABEL_N_CACHED_LAYOUTS; i++) + { + if (priv->cached_layouts[i].layout == NULL) + { + /* Always prefer free cache spaces */ + found_free_cache = TRUE; + oldest_cache = priv->cached_layouts + i; + } + /* If this cached layout is using the same width then we can + just return that directly */ + else if (priv->cached_layouts[i].width == allocation_width) + { + CLUTTER_NOTE (ACTOR, "ClutterLabel: %p: cache hit for width %i", + label, CLUTTER_UNITS_TO_DEVICE (allocation_width)); + return priv->cached_layouts[i].layout; + } + else if (!found_free_cache && (priv->cached_layouts[i].age + < oldest_cache->age)) + oldest_cache = priv->cached_layouts + i; + } - pango_clutter_ensure_glyph_cache_for_layout (retval); + CLUTTER_NOTE (ACTOR, "ClutterLabel: %p: cache miss for width %i", + label, CLUTTER_UNITS_TO_DEVICE (allocation_width)); - return retval; + /* If we make it here then we didn't have a cached version so we + need to recreate the layout */ + if (oldest_cache->layout) + g_object_unref (oldest_cache->layout); + + oldest_cache->layout + = clutter_label_create_layout_no_cache (label, allocation_width); + + pango_clutter_ensure_glyph_cache_for_layout (oldest_cache->layout); + + /* Mark the 'time' this cache was created and advance the time */ + oldest_cache->age = priv->cache_age++; + + oldest_cache->width = allocation_width; + + return oldest_cache->layout; } static void @@ -206,7 +279,9 @@ clutter_label_paint (ClutterActor *self) { ClutterLabel *label = CLUTTER_LABEL (self); ClutterLabelPrivate *priv = label->priv; + PangoLayout *layout; ClutterColor color = { 0, }; + ClutterActorBox alloc = { 0, }; if (priv->font_desc == NULL || priv->text == NULL) { @@ -218,21 +293,13 @@ clutter_label_paint (ClutterActor *self) CLUTTER_NOTE (PAINT, "painting label (text:`%s')", priv->text); - /* XXX - this should never happen, as the layout is always - * recreated when the label allocation changes - */ - if (G_UNLIKELY (!priv->layout)) - { - ClutterActorBox alloc = { 0, }; - - clutter_actor_get_allocation_box (self, &alloc); - priv->layout = clutter_label_create_layout (label, alloc.x2 - alloc.x1); - } + clutter_actor_get_allocation_box (self, &alloc); + layout = clutter_label_create_layout (label, alloc.x2 - alloc.x1); memcpy (&color, &priv->fgcol, sizeof (ClutterColor)); color.alpha = clutter_actor_get_paint_opacity (self); - pango_clutter_render_layout (priv->layout, 0, 0, &color, 0); + pango_clutter_render_layout (layout, 0, 0, &color, 0); } static void @@ -247,13 +314,7 @@ clutter_label_get_preferred_width (ClutterActor *self, PangoLayout *layout; ClutterUnit layout_width; - /* we create a layout to compute the width request; we ignore the - * passed height because ClutterLabel is a height-for-width actor - * - * the layout is destroyed soon after, so it's cheap to do - * computations with it - */ - layout = clutter_label_create_layout_no_cache (label, -1); + layout = clutter_label_create_layout (label, -1); pango_layout_get_extents (layout, NULL, &logical_rect); @@ -271,8 +332,6 @@ clutter_label_get_preferred_width (ClutterActor *self, if (natural_width_p) *natural_width_p = layout_width; - - g_object_unref (layout); } static void @@ -297,10 +356,7 @@ clutter_label_get_preferred_height (ClutterActor *self, PangoRectangle logical_rect = { 0, }; ClutterUnit height; - /* we create a new layout to compute the height for the - * given width and then discard it - */ - layout = clutter_label_create_layout_no_cache (label, for_width); + layout = clutter_label_create_layout (label, for_width); pango_layout_get_extents (layout, NULL, &logical_rect); height = CLUTTER_UNITS_FROM_PANGO_UNIT (logical_rect.height); @@ -310,8 +366,6 @@ clutter_label_get_preferred_height (ClutterActor *self, if (natural_height_p) *natural_height_p = height; - - g_object_unref (layout); } } @@ -321,36 +375,12 @@ clutter_label_allocate (ClutterActor *self, gboolean origin_changed) { ClutterLabel *label = CLUTTER_LABEL (self); - ClutterLabelPrivate *priv = label->priv; ClutterActorClass *parent_class; - /* we try really hard not to recreate the layout unless - * stricly necessary to avoid hitting the GL machinery - * too hard. if the allocation did not really change and - * no other detail of the layout that might affect it - * did change as well, then we simply bail out - */ - if (priv->layout) - { - if (!priv->layout_dirty && - (box->x2 - box->x1) == priv->layout_width && - (box->y2 - box->y1) == priv->layout_height) - { - goto out; - } - - g_object_unref (priv->layout); - priv->layout = NULL; - } + /* Ensure that there is a cached label with the right width so that + we don't need to create the label during the paint run */ + clutter_label_create_layout (label, box->x2 - box->x1); - CLUTTER_NOTE (ACTOR, "Creating the PangoLayout"); - priv->layout = clutter_label_create_layout (label, box->x2 - box->x1); - - priv->layout_width = box->x2 - box->x1; - priv->layout_height = box->y2 - box->y1; - priv->layout_dirty = FALSE; - -out: parent_class = CLUTTER_ACTOR_CLASS (clutter_label_parent_class); parent_class->allocate (self, box, origin_changed); } @@ -363,17 +393,8 @@ clutter_label_dispose (GObject *object) priv = self->priv; - if (priv->layout) - { - g_object_unref (priv->layout); - priv->layout = NULL; - } - - if (priv->context) - { - g_object_unref (priv->context); - priv->context = NULL; - } + /* Get rid of the cached layouts */ + clutter_label_dirty_cache (self); G_OBJECT_CLASS (clutter_label_parent_class)->dispose (object); } @@ -606,6 +627,7 @@ static void clutter_label_init (ClutterLabel *self) { ClutterLabelPrivate *priv; + int i; self->priv = priv = CLUTTER_LABEL_GET_PRIVATE (self); @@ -620,7 +642,9 @@ clutter_label_init (ClutterLabel *self) priv->use_markup = FALSE; priv->justify = FALSE; - priv->layout = NULL; + for (i = 0; i < CLUTTER_LABEL_N_CACHED_LAYOUTS; i++) + priv->cached_layouts[i].layout = NULL; + priv->text = NULL; priv->attrs = NULL; @@ -631,8 +655,6 @@ clutter_label_init (ClutterLabel *self) priv->font_name = g_strdup (DEFAULT_FONT_NAME); priv->font_desc = pango_font_description_from_string (priv->font_name); - - priv->layout_dirty = TRUE; } /** @@ -727,7 +749,8 @@ clutter_label_set_text (ClutterLabel *label, g_free (priv->text); priv->text = g_strdup (text); - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -797,7 +820,8 @@ clutter_label_set_font_name (ClutterLabel *label, pango_font_description_free (priv->font_desc); priv->font_desc = desc; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); if (label->priv->text && label->priv->text[0] != '\0') clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -892,7 +916,8 @@ clutter_label_set_ellipsize (ClutterLabel *label, if ((PangoEllipsizeMode) priv->ellipsize != mode) { priv->ellipsize = mode; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -946,7 +971,8 @@ clutter_label_set_line_wrap (ClutterLabel *label, if (priv->wrap != wrap) { priv->wrap = wrap; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -997,7 +1023,8 @@ clutter_label_set_line_wrap_mode (ClutterLabel *label, if (priv->wrap_mode != wrap_mode) { priv->wrap_mode = wrap_mode; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -1041,9 +1068,13 @@ clutter_label_get_line_wrap_mode (ClutterLabel *label) PangoLayout * clutter_label_get_layout (ClutterLabel *label) { + ClutterUnit width; + g_return_val_if_fail (CLUTTER_IS_LABEL (label), NULL); - return label->priv->layout; + width = clutter_actor_get_widthu (CLUTTER_ACTOR (label)); + + return clutter_label_create_layout (label, width); } static inline void @@ -1092,7 +1123,8 @@ clutter_label_set_attributes (ClutterLabel *label, } priv->attrs = attrs; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); g_object_notify (G_OBJECT (label), "attributes"); @@ -1140,7 +1172,8 @@ clutter_label_set_use_markup (ClutterLabel *label, if (priv->use_markup != setting) { priv->use_markup = setting; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -1190,7 +1223,8 @@ clutter_label_set_alignment (ClutterLabel *label, if (priv->alignment != alignment) { priv->alignment = alignment; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); @@ -1240,7 +1274,8 @@ clutter_label_set_justify (ClutterLabel *label, if (priv->justify != justify) { priv->justify = justify; - priv->layout_dirty = TRUE; + + clutter_label_dirty_cache (label); clutter_actor_queue_relayout (CLUTTER_ACTOR (label)); diff --git a/tests/Makefile.am b/tests/Makefile.am index a8ca07444..d6a75336c 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -14,7 +14,7 @@ noinst_PROGRAMS = test-textures test-events test-offscreen test-scale \ test-cogl-tex-polygon test-stage-read-pixels \ test-random-text test-clip test-paint-wrapper \ test-texture-quality test-entry-auto test-layout \ - test-invariants + test-invariants test-label-cache if X11_TESTS noinst_PROGRAMS += test-pixmap @@ -72,5 +72,6 @@ test_entry_auto_SOURCES = test-entry-auto.c test_layout_SOURCES = test-layout.c test_invariants_SOURCES = test-invariants.c test_devices_SOURCES = test-devices.c +test_label_cache_SOURCES = test-label-cache.c EXTRA_DIST = redhand.png test-script.json diff --git a/tests/test-label-cache.c b/tests/test-label-cache.c new file mode 100644 index 000000000..793a12720 --- /dev/null +++ b/tests/test-label-cache.c @@ -0,0 +1,257 @@ +#include +#include + +#define TEST_FONT "Sans 10" + +static const char long_text[] = + "This is some REALLY " + "long text that contains markup for testing the use_markup " + "property and to test word-wrapping, justification and alignment."; + +typedef struct _CallbackData CallbackData; + +struct _CallbackData +{ + ClutterActor *stage; + ClutterActor *label; + + PangoLayout *old_layout; + gboolean layout_changed; + PangoRectangle label_extents; + + PangoLayout *test_layout; + + gboolean test_failed; +}; + +static void +on_paint (ClutterActor *label, CallbackData *data) +{ + PangoLayout *new_layout; + + /* Check whether the layout used for this paint is different from + the layout used for the last paint */ + new_layout = clutter_label_get_layout (CLUTTER_LABEL (data->label)); + data->layout_changed = data->old_layout != new_layout; + + if (data->old_layout) + g_object_unref (data->old_layout); + /* Keep a reference to the old layout so we can be sure it won't + just reallocate a new layout with the same address */ + data->old_layout = g_object_ref (new_layout); + + pango_layout_get_extents (new_layout, NULL, &data->label_extents); +} + +static void +force_redraw (CallbackData *data) +{ + clutter_redraw (CLUTTER_STAGE (clutter_actor_get_stage (data->label))); +} + +static gboolean +check_result (CallbackData *data, const char *note, + gboolean layout_should_change) +{ + PangoRectangle test_extents; + gboolean fail = FALSE; + + printf ("%s: ", note); + + /* Force a redraw to get the on_paint handler to run */ + force_redraw (data); + + /* Compare the extents from the label with the extents from our test + layout */ + pango_layout_get_extents (data->test_layout, NULL, &test_extents); + if (memcmp (&test_extents, &data->label_extents, sizeof (PangoRectangle))) + { + printf ("extents are different, "); + fail = TRUE; + } + else + printf ("extents are the same, "); + + if (data->layout_changed) + printf ("layout changed, "); + else + printf ("layout did not change, "); + + if (data->layout_changed != layout_should_change) + fail = TRUE; + + if (fail) + { + printf ("FAIL\n"); + data->test_failed = TRUE; + } + else + printf ("pass\n"); + + return fail; +} + +static gboolean +do_tests (CallbackData *data) +{ + PangoFontDescription *fd; + static const ClutterColor red = { 0xff, 0x00, 0x00, 0xff }; + PangoAttrList *attr_list, *attr_list_copy; + PangoAttribute *attr; + + /* TEST 1: change the text */ + clutter_label_set_text (CLUTTER_LABEL (data->label), "Counter 0"); + pango_layout_set_text (data->test_layout, "Counter 0", -1); + check_result (data, "Change text", TRUE); + + /* TEST 2: change a single character */ + clutter_label_set_text (CLUTTER_LABEL (data->label), "Counter 1"); + pango_layout_set_text (data->test_layout, "Counter 1", -1); + check_result (data, "Change a single character", TRUE); + + /* TEST 3: move the label */ + clutter_actor_set_position (data->label, 10, 0); + check_result (data, "Move the label", FALSE); + + /* TEST 4: change the font */ + clutter_label_set_font_name (CLUTTER_LABEL (data->label), "Serif 15"); + fd = pango_font_description_from_string ("Serif 15"); + pango_layout_set_font_description (data->test_layout, fd); + pango_font_description_free (fd); + check_result (data, "Change the font", TRUE); + + /* TEST 5: change the color */ + clutter_label_set_color (CLUTTER_LABEL (data->label), &red); + check_result (data, "Change the color", FALSE); + + /* TEST 6: change the attributes */ + attr = pango_attr_weight_new (PANGO_WEIGHT_BOLD); + attr->start_index = 0; + attr->end_index = 2; + attr_list = pango_attr_list_new (); + pango_attr_list_insert (attr_list, attr); + attr_list_copy = pango_attr_list_copy (attr_list); + clutter_label_set_attributes (CLUTTER_LABEL (data->label), attr_list); + pango_layout_set_attributes (data->test_layout, attr_list_copy); + pango_attr_list_unref (attr_list_copy); + pango_attr_list_unref (attr_list); + check_result (data, "Change the attributes", TRUE); + + /* TEST 7: change the text again */ + clutter_label_set_attributes (CLUTTER_LABEL (data->label), NULL); + clutter_label_set_text (CLUTTER_LABEL (data->label), long_text); + pango_layout_set_attributes (data->test_layout, NULL); + pango_layout_set_text (data->test_layout, long_text, -1); + check_result (data, "Change the text again", TRUE); + + /* TEST 8: enable markup */ + clutter_label_set_use_markup (CLUTTER_LABEL (data->label), TRUE); + pango_layout_set_markup (data->test_layout, long_text, -1); + check_result (data, "Enable markup", TRUE); + + /* This part can't be a test because Clutter won't restrict the + width if wrapping and ellipsizing is disabled so the extents will + be different, but we still want to do it for the later tests */ + clutter_actor_set_width (data->label, 200); + pango_layout_set_width (data->test_layout, 200 * PANGO_SCALE); + /* Force a redraw so that changing the width won't affect the + results */ + force_redraw (data); + + /* TEST 9: enable ellipsize */ + clutter_label_set_ellipsize (CLUTTER_LABEL (data->label), + PANGO_ELLIPSIZE_END); + pango_layout_set_ellipsize (data->test_layout, PANGO_ELLIPSIZE_END); + check_result (data, "Enable ellipsize", TRUE); + clutter_label_set_ellipsize (CLUTTER_LABEL (data->label), + PANGO_ELLIPSIZE_NONE); + pango_layout_set_ellipsize (data->test_layout, PANGO_ELLIPSIZE_NONE); + force_redraw (data); + + /* TEST 10: enable line wrap */ + clutter_label_set_line_wrap (CLUTTER_LABEL (data->label), TRUE); + pango_layout_set_wrap (data->test_layout, PANGO_WRAP_WORD); + check_result (data, "Enable line wrap", TRUE); + + /* TEST 11: change wrap mode */ + clutter_label_set_line_wrap_mode (CLUTTER_LABEL (data->label), + PANGO_WRAP_CHAR); + pango_layout_set_wrap (data->test_layout, PANGO_WRAP_CHAR); + check_result (data, "Change wrap mode", TRUE); + + /* TEST 12: enable justify */ + clutter_label_set_justify (CLUTTER_LABEL (data->label), TRUE); + pango_layout_set_justify (data->test_layout, TRUE); + /* Pango appears to have a bug which means that you can't change the + justification after setting the text but this fixes it. + See http://bugzilla.gnome.org/show_bug.cgi?id=551865 */ + pango_layout_context_changed (data->test_layout); + check_result (data, "Enable justify", TRUE); + + /* TEST 13: change alignment */ + clutter_label_set_alignment (CLUTTER_LABEL (data->label), PANGO_ALIGN_RIGHT); + pango_layout_set_alignment (data->test_layout, PANGO_ALIGN_RIGHT); + check_result (data, "Change alignment", TRUE); + + clutter_main_quit (); + + return FALSE; +} + +static PangoLayout * +make_layout_like_label (ClutterLabel *label) +{ + PangoLayout *label_layout, *new_layout; + PangoContext *context; + PangoFontDescription *fd; + + /* Make another layout using the same context as the layout from the + label */ + label_layout = clutter_label_get_layout (label); + context = pango_layout_get_context (label_layout); + new_layout = pango_layout_new (context); + fd = pango_font_description_from_string (TEST_FONT); + pango_layout_set_font_description (new_layout, fd); + pango_font_description_free (fd); + + return new_layout; +} + +int +main (int argc, char **argv) +{ + CallbackData data; + int ret = 0; + + memset (&data, 0, sizeof (data)); + + clutter_init (&argc, &argv); + + data.stage = clutter_stage_get_default (); + + data.label = clutter_label_new_with_text (TEST_FONT, ""); + + data.test_layout = make_layout_like_label (CLUTTER_LABEL (data.label)); + + g_signal_connect (data.label, "paint", G_CALLBACK (on_paint), &data); + + clutter_container_add (CLUTTER_CONTAINER (data.stage), data.label, NULL); + + clutter_actor_show (data.stage); + + clutter_threads_add_idle ((GSourceFunc) do_tests, &data); + + clutter_main (); + + printf ("\nOverall result: "); + + if (data.test_failed) + { + printf ("FAIL\n"); + ret = 1; + } + else + printf ("pass\n"); + + return ret; +}