cogl-atlas-texture: Don't let textures be destroyed during migration

If an atlas texture's last reference is held by the journal or by the
last flushed pipeline then if an atlas migration is started it can
cause a crash. This is because the atlas migration will cause a
journal flush and can sometimes change the current pipeline which
means that the texture would be destroyed during migration.

This patch adds an extra 'post_reorganize' callback to the existing
'reorganize' callback (which is now renamed to 'pre_reorganize'). The
pre_reorganize callback is now called before the atlas grabs a list of
the current textures instead of after so that it doesn't matter if the
journal flush destroys some of those textures. The pre_reorganize
callback for CoglAtlasTexture grabs a reference to all of the textures
so that they can not be destroyed when the migration changes the
pipeline. In the post_reorganize callback the reference is removed
again.

http://bugzilla.clutter-project.org/show_bug.cgi?id=2538

diff --git a/clutter/cogl/cogl/cogl-atlas-texture.c b/clutter/cogl/cogl/cogl-atlas-texture.c
index df06006..6ac97c0 100644 (file)
--- a/clutter/cogl/cogl/cogl-atlas-texture.c
+++ b/clutter/cogl/cogl/cogl-atlas-texture.c
@@ -3,7 +3,7 @@
  *
  * An object oriented GL/GLES Abstraction/Utility Layer
  *
- * Copyright (C) 2009,2010 Intel Corporation.
+ * Copyright (C) 2009,2010,2011 Intel Corporation.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -83,18 +83,25 @@ _cogl_atlas_texture_update_position_cb (gpointer user_data,
 }
 
 static void
-_cogl_atlas_texture_reorganize_foreach_cb (const CoglRectangleMapEntry *entry,
-                                           void *rectangle_data,
-                                           void *user_data)
+_cogl_atlas_texture_pre_reorganize_foreach_cb
+                                         (const CoglRectangleMapEntry *entry,
+                                          void *rectangle_data,
+                                          void *user_data)
 {
+  CoglAtlasTexture *atlas_tex = rectangle_data;
+
+  /* Keep a reference to the texture because we don't want it to be
+     destroyed during the reorganization */
+  cogl_handle_ref (atlas_tex);
+
   /* Notify cogl-pipeline.c that the texture's underlying GL texture
    * storage is changing so it knows it may need to bind a new texture
    * if the CoglTexture is reused with the same texture unit. */
-  _cogl_pipeline_texture_storage_change_notify (rectangle_data);
+  _cogl_pipeline_texture_storage_change_notify (COGL_TEXTURE (atlas_tex));
 }
 
 static void
-_cogl_atlas_texture_reorganize_cb (void *data)
+_cogl_atlas_texture_pre_reorganize_cb (void *data)
 {
   CoglAtlas *atlas = data;
 
@@ -109,10 +116,63 @@ _cogl_atlas_texture_reorganize_cb (void *data)
 
   if (atlas->map)
     _cogl_rectangle_map_foreach (atlas->map,
-                                 _cogl_atlas_texture_reorganize_foreach_cb,
+                                 _cogl_atlas_texture_pre_reorganize_foreach_cb,
                                  NULL);
 }
 
+typedef struct
+{
+  CoglAtlasTexture **textures;
+  /* Number of textures found so far */
+  unsigned int n_textures;
+} CoglAtlasTextureGetRectanglesData;
+
+static void
+_cogl_atlas_texture_get_rectangles_cb (const CoglRectangleMapEntry *entry,
+                                       void *rectangle_data,
+                                       void *user_data)
+{
+  CoglAtlasTextureGetRectanglesData *data = user_data;
+
+  data->textures[data->n_textures++] = rectangle_data;
+}
+
+static void
+_cogl_atlas_texture_post_reorganize_cb (void *user_data)
+{
+  CoglAtlas *atlas = user_data;
+
+  if (atlas->map)
+    {
+      CoglAtlasTextureGetRectanglesData data;
+      unsigned int i;
+
+      data.textures = g_new (CoglAtlasTexture *,
+                             _cogl_rectangle_map_get_n_rectangles (atlas->map));
+      data.n_textures = 0;
+
+      /* We need to remove all of the references that we took during
+         the preorganize callback. We have to get a separate array of
+         the textures because CoglRectangleMap doesn't support
+         removing rectangles during iteration */
+      _cogl_rectangle_map_foreach (atlas->map,
+                                   _cogl_atlas_texture_get_rectangles_cb,
+                                   &data);
+
+      for (i = 0; i < data.n_textures; i++)
+        {
+          /* Ignore textures that don't have an atlas yet. This will
+             happen when a new texture is added because we allocate
+             the structure for the texture so that it can get stored
+             in the atlas but it isn't a valid object yet */
+          if (data.textures[i]->atlas)
+            cogl_object_unref (data.textures[i]);
+        }
+
+      g_free (data.textures);
+    }
+}
+
 static void
 _cogl_atlas_texture_atlas_destroyed_cb (void *user_data)
 {
@@ -136,7 +196,8 @@ _cogl_atlas_texture_create_atlas (void)
                            _cogl_atlas_texture_update_position_cb);
 
   _cogl_atlas_add_reorganize_callback (atlas,
-                                       _cogl_atlas_texture_reorganize_cb,
+                                       _cogl_atlas_texture_pre_reorganize_cb,
+                                       _cogl_atlas_texture_post_reorganize_cb,
                                        atlas);
 
   ctx->atlases = g_slist_prepend (ctx->atlases, atlas);
@@ -582,6 +643,9 @@ _cogl_atlas_texture_new_from_bitmap (CoglBitmap      *bmp,
   /* We need to allocate the texture now because we need the pointer
      to set as the data for the rectangle in the atlas */
   atlas_tex = g_new (CoglAtlasTexture, 1);
+  /* Mark it as having no atlas so we don't try to unref it in
+     _cogl_atlas_texture_post_reorganize_cb */
+  atlas_tex->atlas = NULL;
 
   _cogl_texture_init (COGL_TEXTURE (atlas_tex),
                       &cogl_atlas_texture_vtable);

diff --git a/clutter/cogl/cogl/cogl-atlas.c b/clutter/cogl/cogl/cogl-atlas.c
index 6f99f3b..bb1cf8a 100644 (file)
--- a/clutter/cogl/cogl/cogl-atlas.c
+++ b/clutter/cogl/cogl/cogl-atlas.c
@@ -3,7 +3,7 @@
  *
  * An object oriented GL/GLES Abstraction/Utility Layer
  *
- * Copyright (C) 2010 Intel Corporation.
+ * Copyright (C) 2010,2011 Intel Corporation.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -58,7 +58,8 @@ _cogl_atlas_new (CoglPixelFormat texture_format,
   atlas->texture = NULL;
   atlas->flags = flags;
   atlas->texture_format = texture_format;
-  _cogl_callback_list_init (&atlas->reorganize_callbacks);
+  _cogl_callback_list_init (&atlas->pre_reorganize_callbacks);
+  _cogl_callback_list_init (&atlas->post_reorganize_callbacks);
 
   return _cogl_atlas_object_new (atlas);
 }
@@ -73,7 +74,8 @@ _cogl_atlas_free (CoglAtlas *atlas)
   if (atlas->map)
     _cogl_rectangle_map_free (atlas->map);
 
-  _cogl_callback_list_destroy (&atlas->reorganize_callbacks);
+  _cogl_callback_list_destroy (&atlas->pre_reorganize_callbacks);
+  _cogl_callback_list_destroy (&atlas->post_reorganize_callbacks);
 
   g_free (atlas);
 }
@@ -308,9 +310,15 @@ _cogl_atlas_compare_size_cb (const void *a,
 }
 
 static void
-_cogl_atlas_notify_reorganize (CoglAtlas *atlas)
+_cogl_atlas_notify_pre_reorganize (CoglAtlas *atlas)
 {
-  _cogl_callback_list_invoke (&atlas->reorganize_callbacks);
+  _cogl_callback_list_invoke (&atlas->pre_reorganize_callbacks);
+}
+
+static void
+_cogl_atlas_notify_post_reorganize (CoglAtlas *atlas)
+{
+  _cogl_callback_list_invoke (&atlas->post_reorganize_callbacks);
 }
 
 gboolean
@@ -349,8 +357,13 @@ _cogl_atlas_reserve_space (CoglAtlas             *atlas,
       return TRUE;
     }
 
-  /* We need to reorganise the atlas so we'll get an array of all the
-     textures currently in the atlas. */
+  /* If we make it here then we need to reorganize the atlas. First
+     we'll notify any users of the atlas that this is going to happen
+     so that for example in CoglAtlasTexture it can notify that the
+     storage has changed and cause a flush */
+  _cogl_atlas_notify_pre_reorganize (atlas);
+
+  /* Get an array of all the textures currently in the atlas. */
   data.n_textures = 0;
   if (atlas->map == NULL)
     data.textures = g_malloc (sizeof (CoglAtlasRepositionData));
@@ -423,8 +436,6 @@ _cogl_atlas_reserve_space (CoglAtlas             *atlas,
     {
       int waste;
 
-      _cogl_atlas_notify_reorganize (atlas);
-
       COGL_NOTE (ATLAS,
                  "%p: Atlas %s with size %ix%i",
                  atlas,
@@ -476,6 +487,8 @@ _cogl_atlas_reserve_space (CoglAtlas             *atlas,
 
   g_free (data.textures);
 
+  _cogl_atlas_notify_post_reorganize (atlas);
+
   return ret;
 }
 
@@ -529,17 +542,30 @@ _cogl_atlas_copy_rectangle (CoglAtlas        *atlas,
 
 void
 _cogl_atlas_add_reorganize_callback (CoglAtlas            *atlas,
-                                     CoglCallbackListFunc  callback,
+                                     CoglCallbackListFunc  pre_callback,
+                                     CoglCallbackListFunc  post_callback,
                                      void                 *user_data)
 {
-  _cogl_callback_list_add (&atlas->reorganize_callbacks, callback, user_data);
+  if (pre_callback)
+    _cogl_callback_list_add (&atlas->pre_reorganize_callbacks,
+                             pre_callback,
+                             user_data);
+  if (post_callback)
+    _cogl_callback_list_add (&atlas->post_reorganize_callbacks,
+                             post_callback,
+                             user_data);
 }
 
 void
 _cogl_atlas_remove_reorganize_callback (CoglAtlas            *atlas,
-                                        CoglCallbackListFunc  callback,
+                                        CoglCallbackListFunc  pre_callback,
+                                        CoglCallbackListFunc  post_callback,
                                         void                 *user_data)
 {
-  _cogl_callback_list_remove (&atlas->reorganize_callbacks,
-                              callback, user_data);
+  if (pre_callback)
+    _cogl_callback_list_remove (&atlas->pre_reorganize_callbacks,
+                                pre_callback, user_data);
+  if (post_callback)
+    _cogl_callback_list_remove (&atlas->post_reorganize_callbacks,
+                                post_callback, user_data);
 }

diff --git a/clutter/cogl/cogl/cogl-atlas.h b/clutter/cogl/cogl/cogl-atlas.h
index 1c5ae30..72eb8ad 100644 (file)
--- a/clutter/cogl/cogl/cogl-atlas.h
+++ b/clutter/cogl/cogl/cogl-atlas.h
@@ -3,7 +3,7 @@
  *
  * An object oriented GL/GLES Abstraction/Utility Layer
  *
- * Copyright (C) 2010 Intel Corporation.
+ * Copyright (C) 2010,2011 Intel Corporation.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -55,7 +55,8 @@ struct _CoglAtlas
 
   CoglAtlasUpdatePositionCallback update_position_cb;
 
-  CoglCallbackList reorganize_callbacks;
+  CoglCallbackList pre_reorganize_callbacks;
+  CoglCallbackList post_reorganize_callbacks;
 };
 
 CoglAtlas *
@@ -84,12 +85,14 @@ _cogl_atlas_copy_rectangle (CoglAtlas        *atlas,
 
 void
 _cogl_atlas_add_reorganize_callback (CoglAtlas            *atlas,
-                                     CoglCallbackListFunc  callback,
+                                     CoglCallbackListFunc  pre_callback,
+                                     CoglCallbackListFunc  post_callback,
                                      void                 *user_data);
 
 void
 _cogl_atlas_remove_reorganize_callback (CoglAtlas            *atlas,
-                                        CoglCallbackListFunc  callback,
+                                        CoglCallbackListFunc  pre_callback,
+                                        CoglCallbackListFunc  post_callback,
                                         void                 *user_data);
 
 #endif /* __COGL_ATLAS_H */

diff --git a/clutter/cogl/pango/cogl-pango-glyph-cache.c b/clutter/cogl/pango/cogl-pango-glyph-cache.c
index c7f09bd..d90c384 100644 (file)
--- a/clutter/cogl/pango/cogl-pango-glyph-cache.c
+++ b/clutter/cogl/pango/cogl-pango-glyph-cache.c
@@ -241,7 +241,7 @@ cogl_pango_glyph_cache_lookup (CoglPangoGlyphCache *cache,
             }
 
           _cogl_atlas_add_reorganize_callback
-            (atlas, cogl_pango_glyph_cache_reorganize_cb, cache);
+            (atlas, cogl_pango_glyph_cache_reorganize_cb, NULL, cache);
 
           cache->atlases = g_slist_prepend (cache->atlases, atlas);
         }