From 503f138fb67f6c0d742055bc7ed9974dd9af2e05 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Wed, 31 Aug 2011 17:22:20 +0100 Subject: [PATCH] texture: Call _cogl_texture_prepare_for_upload in set_region impl Instead of calling _cogl_texutre_prepare_for_upload in cogl_texture_set_region_from_bitmap the call is now deferred to the implementation of the virtual for set_region. This is needed if the texture backend is using a different format for the actual GL texture than what is reported by cogl_texture_get_format. This happens for example with atlas textures which report the original internal format specified when the texture was created but actually always store the data in an RGBA texture. Also when creating an atlas texture from a bitmap it was preparing the bitmap to be uploaded to the original format instead of the format of the actual texture used for the atlas. Then it was using cogl_texture_set_region_from_bitmap to upload the 5 pieces to make the copies of the edge pixels. This would end up converting the image to the actual format 5 times. The atlas textures have now been changed to prepare the bitmap for the right format. https://bugzilla.gnome.org/show_bug.cgi?id=657840 Reviewed-by: Robert Bragg --- cogl/cogl-atlas-texture.c | 83 ++++++++++++++++++++++------------- cogl/cogl-texture-2d-sliced.c | 12 +++-- cogl/cogl-texture-2d.c | 12 +++-- cogl/cogl-texture-private.h | 6 ++- cogl/cogl-texture-rectangle.c | 12 +++-- cogl/cogl-texture.c | 18 +++----- 6 files changed, 87 insertions(+), 56 deletions(-) diff --git a/cogl/cogl-atlas-texture.c b/cogl/cogl-atlas-texture.c index 986707f42..08e1db90f 100644 --- a/cogl/cogl-atlas-texture.c +++ b/cogl/cogl-atlas-texture.c @@ -488,6 +488,52 @@ _cogl_atlas_texture_set_region_with_border (CoglAtlasTexture *atlas_tex, return TRUE; } +static CoglBitmap * +_cogl_atlas_texture_prepare_for_upload (CoglAtlasTexture *atlas_tex, + CoglBitmap *bmp) +{ + CoglPixelFormat internal_format; + CoglBitmap *converted_bmp; + CoglBitmap *override_bmp; + + /* We'll prepare to upload using the format of the actual texture of + the atlas texture instead of the format reported by + cogl_texture_get_format which would be the original internal + format specified when the texture was created. However we'll + preserve the premult status of the internal format because the + images are all stored in the original premult format of the + orignal format so we do need to trigger the conversion */ + + internal_format = (COGL_PIXEL_FORMAT_RGBA_8888 | + (atlas_tex->format & COGL_PREMULT_BIT)); + + converted_bmp = _cogl_texture_prepare_for_upload (bmp, + internal_format, + NULL, /* dst_format_out */ + NULL, /* glintformat */ + NULL, /* glformat */ + NULL /* gltype */); + + if (converted_bmp == NULL) + return NULL; + + /* We'll create another bitmap which uses the same data but + overrides the format to remove the premult flag so that uploads + to the atlas texture won't trigger the conversion again */ + + override_bmp = + _cogl_bitmap_new_shared (converted_bmp, + _cogl_bitmap_get_format (converted_bmp) & + ~COGL_PREMULT_BIT, + _cogl_bitmap_get_width (converted_bmp), + _cogl_bitmap_get_height (converted_bmp), + _cogl_bitmap_get_rowstride (converted_bmp)); + + cogl_object_unref (converted_bmp); + + return override_bmp; +} + static gboolean _cogl_atlas_texture_set_region (CoglTexture *tex, int src_x, @@ -506,12 +552,8 @@ _cogl_atlas_texture_set_region (CoglTexture *tex, { gboolean ret; - bmp = _cogl_bitmap_new_shared (bmp, - _cogl_bitmap_get_format (bmp) & - ~COGL_PREMULT_BIT, - _cogl_bitmap_get_width (bmp), - _cogl_bitmap_get_height (bmp), - _cogl_bitmap_get_rowstride (bmp)); + bmp = _cogl_atlas_texture_prepare_for_upload (atlas_tex, + bmp); /* Upload the data ignoring the premult bit */ ret = _cogl_atlas_texture_set_region_with_border (atlas_tex, @@ -684,10 +726,6 @@ _cogl_atlas_texture_new_from_bitmap (CoglBitmap *bmp, CoglHandle atlas_tex_handle; CoglAtlasTexture *atlas_tex; CoglBitmap *dst_bmp; - CoglBitmap *override_bmp; - GLenum gl_intformat; - GLenum gl_format; - GLenum gl_type; int bmp_width; int bmp_height; CoglPixelFormat bmp_format; @@ -711,12 +749,8 @@ _cogl_atlas_texture_new_from_bitmap (CoglBitmap *bmp, atlas_tex = atlas_tex_handle; - dst_bmp = _cogl_texture_prepare_for_upload (bmp, - internal_format, - &internal_format, - &gl_intformat, - &gl_format, - &gl_type); + dst_bmp = _cogl_atlas_texture_prepare_for_upload (atlas_tex, + bmp); if (dst_bmp == NULL) { @@ -724,19 +758,8 @@ _cogl_atlas_texture_new_from_bitmap (CoglBitmap *bmp, return COGL_INVALID_HANDLE; } - /* Make another bitmap so that we can override the format */ - override_bmp = _cogl_bitmap_new_shared (dst_bmp, - _cogl_bitmap_get_format (dst_bmp) & - ~COGL_PREMULT_BIT, - _cogl_bitmap_get_width (dst_bmp), - _cogl_bitmap_get_height (dst_bmp), - _cogl_bitmap_get_rowstride (dst_bmp)); - cogl_object_unref (dst_bmp); - /* Defer to set_region so that we can share the code for copying the - edge pixels to the border. We don't want to pass the actual - format of the converted texture because otherwise it will get - unpremultiplied. */ + edge pixels to the border. */ _cogl_atlas_texture_set_region_with_border (atlas_tex, 0, /* src_x */ 0, /* src_y */ @@ -744,9 +767,9 @@ _cogl_atlas_texture_new_from_bitmap (CoglBitmap *bmp, 0, /* dst_y */ bmp_width, /* dst_width */ bmp_height, /* dst_height */ - override_bmp); + dst_bmp); - cogl_object_unref (override_bmp); + cogl_object_unref (dst_bmp); return atlas_tex_handle; } diff --git a/cogl/cogl-texture-2d-sliced.c b/cogl/cogl-texture-2d-sliced.c index 0775277c4..f59261733 100644 --- a/cogl/cogl-texture-2d-sliced.c +++ b/cogl/cogl-texture-2d-sliced.c @@ -1325,10 +1325,12 @@ _cogl_texture_2d_sliced_set_region (CoglTexture *tex, _COGL_GET_CONTEXT (ctx, FALSE); - ctx->texture_driver->pixel_format_to_gl (_cogl_bitmap_get_format (bmp), - NULL, /* internal format */ - &gl_format, - &gl_type); + bmp = _cogl_texture_prepare_for_upload (bmp, + cogl_texture_get_format (tex), + NULL, + NULL, + &gl_format, + &gl_type); /* Send data to GL */ _cogl_texture_2d_sliced_upload_subregion_to_gl (tex_2ds, @@ -1339,6 +1341,8 @@ _cogl_texture_2d_sliced_set_region (CoglTexture *tex, gl_format, gl_type); + cogl_object_unref (bmp); + return TRUE; } diff --git a/cogl/cogl-texture-2d.c b/cogl/cogl-texture-2d.c index 71ff44ad0..f800fcb49 100644 --- a/cogl/cogl-texture-2d.c +++ b/cogl/cogl-texture-2d.c @@ -816,10 +816,12 @@ _cogl_texture_2d_set_region (CoglTexture *tex, _COGL_GET_CONTEXT (ctx, FALSE); - ctx->texture_driver->pixel_format_to_gl (_cogl_bitmap_get_format (bmp), - NULL, /* internal format */ - &gl_format, - &gl_type); + bmp = _cogl_texture_prepare_for_upload (bmp, + cogl_texture_get_format (tex), + NULL, + NULL, + &gl_format, + &gl_type); /* If this touches the first pixel then we'll update our copy */ if (dst_x == 0 && dst_y == 0 && @@ -850,6 +852,8 @@ _cogl_texture_2d_set_region (CoglTexture *tex, tex_2d->mipmaps_dirty = TRUE; + cogl_object_unref (bmp); + return TRUE; } diff --git a/cogl/cogl-texture-private.h b/cogl/cogl-texture-private.h index 767069a4e..01fba91b6 100644 --- a/cogl/cogl-texture-private.h +++ b/cogl/cogl-texture-private.h @@ -67,8 +67,10 @@ struct _CoglTextureVtable backend */ /* This should update the specified sub region of the texture with a - sub region of the given bitmap. The bitmap will have first been - converted to a suitable format for uploading if neccessary. */ + sub region of the given bitmap. The bitmap is not converted + before being passed so the implementation is expected to call + _cogl_texture_prepare_for_upload with a suitable destination + format before uploading */ gboolean (* set_region) (CoglTexture *tex, int src_x, int src_y, diff --git a/cogl/cogl-texture-rectangle.c b/cogl/cogl-texture-rectangle.c index e98be8d95..ca12fcb0c 100644 --- a/cogl/cogl-texture-rectangle.c +++ b/cogl/cogl-texture-rectangle.c @@ -564,10 +564,12 @@ _cogl_texture_rectangle_set_region (CoglTexture *tex, _COGL_GET_CONTEXT (ctx, FALSE); - ctx->texture_driver->pixel_format_to_gl (_cogl_bitmap_get_format (bmp), - NULL, /* internal format */ - &gl_format, - &gl_type); + bmp = _cogl_texture_prepare_for_upload (bmp, + cogl_texture_get_format (tex), + NULL, + NULL, + &gl_format, + &gl_type); /* Send data to GL */ ctx->texture_driver->upload_subregion_to_gl (GL_TEXTURE_RECTANGLE_ARB, @@ -580,6 +582,8 @@ _cogl_texture_rectangle_set_region (CoglTexture *tex, gl_format, gl_type); + cogl_object_unref (bmp); + return TRUE; } diff --git a/cogl/cogl-texture.c b/cogl/cogl-texture.c index 165811f9e..8a6fbd2a3 100644 --- a/cogl/cogl-texture.c +++ b/cogl/cogl-texture.c @@ -862,22 +862,18 @@ cogl_texture_set_region_from_bitmap (CoglHandle handle, CoglBitmap *bmp) { CoglTexture *tex = COGL_TEXTURE (handle); - GLenum closest_gl_format; - GLenum closest_gl_type; gboolean ret; /* Shortcut out early if the image is empty */ if (dst_width == 0 || dst_height == 0) return TRUE; - /* Prepare the bitmap so that it will do the premultiplication - conversion */ - bmp = _cogl_texture_prepare_for_upload (bmp, - cogl_texture_get_format (handle), - NULL, - NULL, - &closest_gl_format, - &closest_gl_type); + /* Note that we don't prepare the bitmap for upload here because + some backends may be internally using a different format for the + actual GL texture than that reported by + cogl_texture_get_format. For example the atlas textures are + always stored in an RGBA texture even if the texture format is + advertised as RGB. */ ret = tex->vtable->set_region (handle, src_x, src_y, @@ -885,8 +881,6 @@ cogl_texture_set_region_from_bitmap (CoglHandle handle, dst_width, dst_height, bmp); - cogl_object_unref (bmp); - return ret; }