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 <robert@linux.intel.com>

6 files changed, 87 insertions, 56 deletions

diff --git a/cogl/cogl-atlas-texture.c b/cogl/cogl-atlas-texture.c
index 986707f4..08e1db90 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 0775277c..f5926173 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 71ff44ad..f800fcb4 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 767069a4..01fba91b 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 e98be8d9..ca12fcb0 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 165811f9..8a6fbd2a 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;
 }