[Cogl] [PATCH] Allow lazy texture storage allocation

Neil Roberts neil at linux.intel.com
Fri Dec 7 07:13:35 PST 2012


Hi,

I think this patch needs to be a bit more careful about how it cleans up
the texture when there is an error. It looks like currently if the
texture is freed before it it allocated then gl_texture will be left
uninitialised and the free function will delete a random texture. Also
if the allocation fails then it doesn't delete the texture object. If
another operation causes Cogl to attempt to allocate the texture again
then it will leak the GL texture. I'm attaching a diff to fix this.

I'm not sure if it makes sense to have a CoglError argument in the
cogl_texture_*_new_with_size functions. Why don't they do the can_create
check in the allocate function instead of the new_with_size?

Regards,
- Neil

diff --git a/cogl/cogl-texture-3d.c b/cogl/cogl-texture-3d.c
index aa9671c..95a13da 100644
--- a/cogl/cogl-texture-3d.c
+++ b/cogl/cogl-texture-3d.c
@@ -95,7 +95,8 @@ _cogl_texture_3d_gl_flush_legacy_texobj_wrap_modes (CoglTexture *tex,
 static void
 _cogl_texture_3d_free (CoglTexture3D *tex_3d)
 {
-  _cogl_delete_gl_texture (tex_3d->gl_texture);
+  if (tex_3d->gl_texture)
+    _cogl_delete_gl_texture (tex_3d->gl_texture);
 
   /* Chain up */
   _cogl_texture_free (COGL_TEXTURE (tex_3d));
@@ -122,6 +123,8 @@ _cogl_texture_3d_create_base (CoglContext *ctx,
 
   _cogl_texture_init (tex, ctx, width, height, &cogl_texture_3d_vtable);
 
+  tex_3d->gl_texture = 0;
+
   tex_3d->depth = depth;
   tex_3d->mipmaps_dirty = TRUE;
   tex_3d->auto_mipmap = TRUE;
@@ -234,6 +237,7 @@ _cogl_texture_3d_allocate (CoglTexture *tex,
   GLenum gl_format;
   GLenum gl_type;
   GLenum gl_error;
+  GLenum gl_texture;
 
   ctx->driver_vtable->pixel_format_to_gl (ctx,
                                           tex_3d->internal_format,
@@ -241,10 +245,10 @@ _cogl_texture_3d_allocate (CoglTexture *tex,
                                           &gl_format,
                                           &gl_type);
 
-  tex_3d->gl_texture =
+  gl_texture =
     ctx->texture_driver->gen (ctx, GL_TEXTURE_3D, tex_3d->internal_format);
   _cogl_bind_gl_texture_transient (GL_TEXTURE_3D,
-                                   tex_3d->gl_texture,
+                                   gl_texture,
                                    FALSE);
   /* Clear any GL errors */
   while ((gl_error = ctx->glGetError ()) != GL_NO_ERROR)
@@ -255,7 +259,13 @@ _cogl_texture_3d_allocate (CoglTexture *tex,
                      0, gl_format, gl_type, NULL);
 
   if (_cogl_gl_util_catch_out_of_memory (ctx, error))
-    return FALSE;
+    {
+      GE( ctx, glDeleteTextures (1, &gl_texture) );
+      return FALSE;
+    }
+
+  tex_3d->gl_texture = gl_texture;
+  tex_3d->gl_format = gl_intformat;
 
   return TRUE;
 }
diff --git a/cogl/cogl-texture-rectangle.c b/cogl/cogl-texture-rectangle.c
index adda70e..ecb48cd 100644
--- a/cogl/cogl-texture-rectangle.c
+++ b/cogl/cogl-texture-rectangle.c
@@ -102,7 +102,7 @@ _cogl_texture_rectangle_gl_flush_legacy_texobj_wrap_modes (CoglTexture *tex,
 static void
 _cogl_texture_rectangle_free (CoglTextureRectangle *tex_rect)
 {
-  if (!tex_rect->is_foreign)
+  if (!tex_rect->is_foreign && tex_rect->gl_texture)
     _cogl_delete_gl_texture (tex_rect->gl_texture);
 
   /* Chain up */
@@ -173,6 +173,8 @@ _cogl_texture_rectangle_create_base (CoglContext *ctx,
 
   _cogl_texture_init (tex, ctx, width, height, &cogl_texture_rectangle_vtable);
 
+  tex_rect->gl_texture = 0;
+
   /* We default to GL_LINEAR for both filters */
   tex_rect->gl_legacy_texobj_min_filter = GL_LINEAR;
   tex_rect->gl_legacy_texobj_mag_filter = GL_LINEAR;
@@ -217,6 +219,7 @@ _cogl_texture_rectangle_allocate (CoglTexture *tex,
   GLenum gl_format;
   GLenum gl_type;
   GLenum gl_error;
+  GLenum gl_texture;
 
   ctx->driver_vtable->pixel_format_to_gl (ctx,
                                           tex_rect->internal_format,
@@ -224,12 +227,12 @@ _cogl_texture_rectangle_allocate (CoglTexture *tex,
                                           &gl_format,
                                           &gl_type);
 
-  tex_rect->gl_texture =
+  gl_texture =
     ctx->texture_driver->gen (ctx,
                               GL_TEXTURE_RECTANGLE_ARB,
                               tex_rect->internal_format);
   _cogl_bind_gl_texture_transient (GL_TEXTURE_RECTANGLE_ARB,
-                                   tex_rect->gl_texture,
+                                   gl_texture,
                                    tex_rect->is_foreign);
 
   /* Clear any GL errors */
@@ -240,7 +243,13 @@ _cogl_texture_rectangle_allocate (CoglTexture *tex,
                      tex->width, tex->height, 0, gl_format, gl_type, NULL);
 
   if (_cogl_gl_util_catch_out_of_memory (ctx, error))
-    return FALSE;
+    {
+      GE( ctx, glDeleteTextures (1, &gl_texture) );
+      return FALSE;
+    }
+
+  tex_rect->gl_texture = gl_texture;
+  tex_rect->gl_format = gl_intformat;
 
   return TRUE;
 }
diff --git a/cogl/driver/gl/cogl-texture-2d-gl.c b/cogl/driver/gl/cogl-texture-2d-gl.c
index 11d7169..98e39e3 100644
--- a/cogl/driver/gl/cogl-texture-2d-gl.c
+++ b/cogl/driver/gl/cogl-texture-2d-gl.c
@@ -43,7 +43,7 @@
 void
 _cogl_texture_2d_gl_free (CoglTexture2D *tex_2d)
 {
-  if (!tex_2d->is_foreign)
+  if (!tex_2d->is_foreign && tex_2d->gl_texture)
     _cogl_delete_gl_texture (tex_2d->gl_texture);
 }
 
@@ -79,6 +79,8 @@ _cogl_texture_2d_gl_can_create (CoglContext *ctx,
 void
 _cogl_texture_2d_gl_init (CoglTexture2D *tex_2d)
 {
+  tex_2d->gl_texture = 0;
+
   /* We default to GL_LINEAR for both filters */
   tex_2d->gl_legacy_texobj_min_filter = GL_LINEAR;
   tex_2d->gl_legacy_texobj_mag_filter = GL_LINEAR;
@@ -98,6 +100,7 @@ _cogl_texture_2d_gl_allocate (CoglTexture *tex,
   GLenum gl_format;
   GLenum gl_type;
   GLenum gl_error;
+  GLuint gl_texture;
 
   ctx->driver_vtable->pixel_format_to_gl (ctx,
                                           tex_2d->internal_format,
@@ -105,13 +108,11 @@ _cogl_texture_2d_gl_allocate (CoglTexture *tex,
                                           &gl_format,
                                           &gl_type);
 
-  tex_2d->gl_texture =
+  gl_texture =
     ctx->texture_driver->gen (ctx, GL_TEXTURE_2D, tex_2d->internal_format);
 
-  tex_2d->gl_internal_format = gl_intformat;
-
   _cogl_bind_gl_texture_transient (GL_TEXTURE_2D,
-                                   tex_2d->gl_texture,
+                                   gl_texture,
                                    tex_2d->is_foreign);
 
   /* Clear any GL errors */
@@ -122,7 +123,13 @@ _cogl_texture_2d_gl_allocate (CoglTexture *tex,
                      tex->width, tex->height, 0, gl_format, gl_type, NULL);
 
   if (_cogl_gl_util_catch_out_of_memory (ctx, error))
-    return FALSE;
+    {
+      GE( ctx, glDeleteTextures (1, &gl_texture) );
+      return FALSE;
+    }
+
+  tex_2d->gl_texture = gl_texture;
+  tex_2d->gl_internal_format = gl_intformat;
 
   return TRUE;
 }
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.



More information about the Cogl mailing list