[Cogl] [PATCH] Allow lazy texture storage allocation
Robert Bragg
robert at sixbynine.org
Fri Dec 7 10:21:31 PST 2012
On Fri, Dec 7, 2012 at 3:13 PM, Neil Roberts <neil at linux.intel.com> wrote:
> 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.
Ah right, thanks for the diff, I can fold those into the patch.
>
> 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?
Right, I'm fairly sure we discussed this in meat space at the time I
was doing this since it's not entirely clear which way is better. At
the time I think we agreed it can be useful be able determine hardware
limitations such as size limitations and errors calculating slicing
geometry before actually trying to allocate the storage of a texture
which is why I kept the CoglError args here.
Potentially we could get rid of the CoglError args as you say, though
I can still see some value in being able to catch hardware limitations
early before allocating the textures.
regards,
- Robert
>
> 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.
>
> _______________________________________________
> Cogl mailing list
> Cogl at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/cogl
More information about the Cogl
mailing list