units: Cache the pixels value inside Units

When computing the pixels value of a ClutterUnits value we should
be caching the value to avoid recomputing for every call of
clutter_units_to_pixels(). We already have a flag telling us to
return the cached value, but we miss the mechanism to evict the
cache whenever the Backend settings affecting the conversion, that
is default font and resolution, change.

In order to implement the eviction we can use a "serial"; the
Backend will have an internal serial field which we retrieve and
put inside the ClutterUnits structure (we split one of the two
64 bit padding fields into two 32 bit fields to maintain ABI); every
time we call clutter_units_to_pixels() we compare the units serial
with that of the Backend; if they match and pixels_set is set to
TRUE then we just return the stored pixels value. If the serials
do not match then we unset the pixels_set flag and recompute the
pixels value.

We can verify this by adding a simple test unit checking that
by changing the resolution of ClutterBackend we get different
pixel values for 1 em.

http://bugzilla.openedhand.com/show_bug.cgi?id=1843

diff --git a/.gitignore b/.gitignore
index 1ca16fe..1984386 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -212,6 +212,7 @@ TAGS
 /tests/conform/test-conformance-result.xml
 /tests/conform/test-fixed-size
 /tests/conform/test-preferred-size
+/tests/conform/test-units-cache
 /tests/micro-bench/test-text-perf
 /tests/micro-bench/test-text
 /tests/micro-bench/test-picking

diff --git a/clutter/clutter-backend.c b/clutter/clutter-backend.c
index c95ac9b..96b2221 100644 (file)
--- a/clutter/clutter-backend.c
+++ b/clutter/clutter-backend.c
@@ -63,6 +63,7 @@ struct _ClutterBackendPrivate
   gdouble resolution;
 
   gfloat units_per_em;
+  gint32 units_serial;
 
   cairo_font_options_t *font_options;
 
@@ -160,7 +161,10 @@ get_units_per_em (ClutterBackend       *backend,
 static void
 clutter_backend_real_resolution_changed (ClutterBackend *backend)
 {
-  backend->priv->units_per_em = get_units_per_em (backend, NULL);
+  ClutterBackendPrivate *priv = backend->priv;
+
+  priv->units_per_em = get_units_per_em (backend, NULL);
+  priv->units_serial += 1;
 
   CLUTTER_NOTE (BACKEND, "Units per em: %.2f", backend->priv->units_per_em);
 }
@@ -168,7 +172,10 @@ clutter_backend_real_resolution_changed (ClutterBackend *backend)
 static void
 clutter_backend_real_font_changed (ClutterBackend *backend)
 {
-  backend->priv->units_per_em = get_units_per_em (backend, NULL);
+  ClutterBackendPrivate *priv = backend->priv;
+
+  priv->units_per_em = get_units_per_em (backend, NULL);
+  priv->units_serial += 1;
 
   CLUTTER_NOTE (BACKEND, "Units per em: %.2f", backend->priv->units_per_em);
 }
@@ -212,7 +219,9 @@ clutter_backend_init (ClutterBackend *backend)
   priv = backend->priv = CLUTTER_BACKEND_GET_PRIVATE (backend);
 
   priv->resolution = -1.0;
+
   priv->units_per_em = -1.0;
+  priv->units_serial = 1;
 }
 
 void
@@ -721,3 +730,11 @@ clutter_backend_get_font_name (ClutterBackend *backend)
 
   return priv->font_name;
 }
+
+gint32
+_clutter_backend_get_units_serial (ClutterBackend *backend)
+{
+  g_return_val_if_fail (CLUTTER_IS_BACKEND (backend), 0);
+
+  return backend->priv->units_serial;
+}

diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h
index bd6f5db..57ec048 100644 (file)
--- a/clutter/clutter-private.h
+++ b/clutter/clutter-private.h
@@ -255,6 +255,8 @@ void _clutter_actor_set_enable_paint_unmapped (ClutterActor *self,
 
 void _clutter_run_repaint_functions (void);
 
+gint32 _clutter_backend_get_units_serial (ClutterBackend *backend);
+
 G_END_DECLS
 
 #endif /* _HAVE_CLUTTER_PRIVATE_H */

diff --git a/clutter/clutter-units.c b/clutter/clutter-units.c
index 595f0a5..4831fd1 100644 (file)
--- a/clutter/clutter-units.c
+++ b/clutter/clutter-units.c
@@ -150,12 +150,17 @@ void
 clutter_units_from_mm (ClutterUnits *units,
                        gfloat        mm)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_MM;
   units->value      = mm;
   units->pixels     = units_mm_to_pixels (mm);
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -171,12 +176,17 @@ void
 clutter_units_from_cm (ClutterUnits *units,
                        gfloat        cm)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_CM;
   units->value      = cm;
   units->pixels     = units_cm_to_pixels (cm);
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -192,12 +202,17 @@ void
 clutter_units_from_pt (ClutterUnits *units,
                        gfloat        pt)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_POINT;
   units->value      = pt;
   units->pixels     = units_pt_to_pixels (pt);
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -214,12 +229,17 @@ void
 clutter_units_from_em (ClutterUnits *units,
                        gfloat        em)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_EM;
   units->value      = em;
   units->pixels     = units_em_to_pixels (NULL, em);
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -237,12 +257,17 @@ clutter_units_from_em_for_font (ClutterUnits *units,
                                 const gchar  *font_name,
                                 gfloat        em)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_EM;
   units->value      = em;
   units->pixels     = units_em_to_pixels (font_name, em);
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -258,12 +283,17 @@ void
 clutter_units_from_pixels (ClutterUnits *units,
                            gint          px)
 {
+  ClutterBackend *backend;
+
   g_return_if_fail (units != NULL);
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type  = CLUTTER_UNIT_PIXEL;
   units->value      = px;
   units->pixels     = px;
   units->pixels_set = TRUE;
+  units->serial     = _clutter_backend_get_units_serial (backend);
 }
 
 /**
@@ -353,8 +383,18 @@ clutter_units_free (ClutterUnits *units)
 gfloat
 clutter_units_to_pixels (ClutterUnits *units)
 {
+  ClutterBackend *backend;
+
   g_return_val_if_fail (units != NULL, 0.0);
 
+  /* if the backend settings changed we evict the cached value */
+  backend = clutter_get_default_backend ();
+  if (units->serial != _clutter_backend_get_units_serial (backend))
+    units->pixels_set = FALSE;
+
+  if (units->pixels_set)
+    return units->pixels;
+
   switch (units->unit_type)
     {
     case CLUTTER_UNIT_MM:
@@ -379,6 +419,7 @@ clutter_units_to_pixels (ClutterUnits *units)
     }
 
   units->pixels_set = TRUE;
+  units->serial = _clutter_backend_get_units_serial (backend);
 
   return units->pixels;
 }
@@ -431,6 +472,7 @@ gboolean
 clutter_units_from_string (ClutterUnits *units,
                            const gchar  *str)
 {
+  ClutterBackend *backend;
   ClutterUnitType unit_type;
   gfloat value;
 
@@ -503,9 +545,12 @@ clutter_units_from_string (ClutterUnits *units,
   if (*str != '\0')
     return FALSE;
 
+  backend = clutter_get_default_backend ();
+
   units->unit_type = unit_type;
   units->value = value;
   units->pixels_set = FALSE;
+  units->serial = _clutter_backend_get_units_serial (backend);
 
   return TRUE;
 }

diff --git a/clutter/clutter-units.h b/clutter/clutter-units.h
index e67af95..a770d1f 100644 (file)
--- a/clutter/clutter-units.h
+++ b/clutter/clutter-units.h
@@ -77,11 +77,18 @@ struct _ClutterUnits
   gfloat value;
 
   /* pre-filled by the provided constructors */
+
+  /* cached pixel value */
   gfloat pixels;
+
+  /* whether the :pixels field is set */
   guint pixels_set;
 
+  /* the serial coming from the backend, used to evict the cache */
+  gint32 serial;
+
   /* padding for eventual expansion */
-  gint64 __padding_1;
+  gint32 __padding_1;
   gint64 __padding_2;
 };
 

diff --git a/tests/conform/test-clutter-units.c b/tests/conform/test-clutter-units.c
index a685d1a..105c454 100644 (file)
--- a/tests/conform/test-clutter-units.c
+++ b/tests/conform/test-clutter-units.c
@@ -4,6 +4,29 @@
 #include "test-conform-common.h"
 
 void
+test_units_cache (TestConformSimpleFixture *fixture,
+                  gconstpointer data)
+{
+  ClutterUnits units;
+  ClutterBackend *backend;
+  gfloat pixels;
+  gdouble dpi;
+
+  backend = clutter_get_default_backend ();
+
+  dpi = clutter_backend_get_resolution (backend);
+
+  clutter_units_from_em (&units, 1.0);
+  pixels = clutter_units_to_pixels (&units);
+
+  clutter_backend_set_resolution (backend, dpi + 10);
+  g_assert_cmpfloat (clutter_units_to_pixels (&units), !=, pixels);
+
+  clutter_backend_set_resolution (backend, dpi);
+  g_assert_cmpfloat (clutter_units_to_pixels (&units), ==, pixels);
+}
+
+void
 test_units_constructors (TestConformSimpleFixture *fixture,
                          gconstpointer data)
 {

diff --git a/tests/conform/test-conform-main.c b/tests/conform/test-conform-main.c
index d8d4b1c..2b26244 100644 (file)
--- a/tests/conform/test-conform-main.c
+++ b/tests/conform/test-conform-main.c
@@ -167,6 +167,7 @@ main (int argc, char **argv)
 
   TEST_CONFORM_SIMPLE ("/units", test_units_constructors);
   TEST_CONFORM_SIMPLE ("/units", test_units_string);
+  TEST_CONFORM_SIMPLE ("/units", test_units_cache);
 
   TEST_CONFORM_SIMPLE ("/group", test_group_depth_sorting);