bitmap: Add a function to convert into an existing buffer

This adds _cogl_bitmap_convert_into_bitmap which is the same as
_cogl_bitmap_convert except that it writes into an existing bitmap
instead of allocating a new one. _cogl_bitmap_convert now just
allocates a buffer and calls the new function. This is used in
_cogl_read_pixels to avoid allocating a second intermediate buffer
when the pixel format to store in is not GL_RGBA.

Reviewed-by: Robert Bragg <robert@linux.intel.com>

cogl/cogl-bitmap-conversion.c

@@ -342,29 +342,34 @@ _cogl_bitmap_needs_short_temp_buffer (CoglPixelFormat format)
   g_assert_not_reached ();
 }
 
-CoglBitmap *
+gboolean
-_cogl_bitmap_convert (CoglBitmap      *src_bmp,
+_cogl_bitmap_convert_into_bitmap (CoglBitmap *src_bmp,
-                      CoglPixelFormat  dst_format)
+                                  CoglBitmap *dst_bmp)
 {
   guint8          *src_data;
   guint8          *dst_data;
   guint8          *src;
   guint8          *dst;
   void            *tmp_row;
-  int              dst_bpp;
   int              src_rowstride;
   int              dst_rowstride;
   int              y;
   int              width, height;
   CoglPixelFormat  src_format;
+  CoglPixelFormat  dst_format;
   gboolean         use_16;
   gboolean         need_premult;
 
   src_format = _cogl_bitmap_get_format (src_bmp);
   src_rowstride = _cogl_bitmap_get_rowstride (src_bmp);
+  dst_format = _cogl_bitmap_get_format (dst_bmp);
+  dst_rowstride = _cogl_bitmap_get_rowstride (dst_bmp);
   width = _cogl_bitmap_get_width (src_bmp);
   height = _cogl_bitmap_get_height (src_bmp);
 
+  _COGL_RETURN_VAL_IF_FAIL (width == _cogl_bitmap_get_width (dst_bmp), FALSE);
+  _COGL_RETURN_VAL_IF_FAIL (height == _cogl_bitmap_get_height (dst_bmp), FALSE);
+
   need_premult
     = ((src_format & COGL_PREMULT_BIT) != (dst_format & COGL_PREMULT_BIT) &&
        src_format != COGL_PIXEL_FORMAT_A_8 &&
@@ -376,32 +381,44 @@ _cogl_bitmap_convert (CoglBitmap      *src_bmp,
   if ((src_format & ~COGL_PREMULT_BIT) == (dst_format & ~COGL_PREMULT_BIT) &&
       (!need_premult || _cogl_bitmap_can_premult (dst_format)))
     {
-      CoglBitmap *dst_bmp = _cogl_bitmap_copy (src_bmp);
+      if (!_cogl_bitmap_copy_subregion (src_bmp, dst_bmp,
+                                        0, 0, /* src_x / src_y */
+                                        0, 0, /* dst_x / dst_y */
+                                        width, height))
+        return FALSE;
 
-      if (need_premult &&
+      if (need_premult)
-          !_cogl_bitmap_convert_premult_status (dst_bmp, dst_format))
         {
-          cogl_object_unref (dst_bmp);
+          if ((dst_format & COGL_PREMULT_BIT))
-          return NULL;
+            {
+              if (!_cogl_bitmap_premult (dst_bmp))
+                return FALSE;
+            }
+          else
+            {
+              if (!_cogl_bitmap_unpremult (dst_bmp))
+                return FALSE;
+            }
         }
 
-      return dst_bmp;
+      return TRUE;
     }
 
   src_data = _cogl_bitmap_map (src_bmp, COGL_BUFFER_ACCESS_READ, 0);
   if (src_data == NULL)
-    return NULL;
+    return FALSE;
+  dst_data = _cogl_bitmap_map (dst_bmp,
+                               COGL_BUFFER_ACCESS_WRITE,
+                               COGL_BUFFER_MAP_HINT_DISCARD);
+  if (dst_data == NULL)
+    {
+      _cogl_bitmap_unmap (src_bmp);
+      return FALSE;
+    }
 
   use_16 = _cogl_bitmap_needs_short_temp_buffer (dst_format);
 
-  dst_bpp = _cogl_pixel_format_get_bytes_per_pixel (dst_format);
+  /* Allocate a buffer to hold a temporary RGBA row */
-
-  /* Initialize destination bitmap */
-  dst_rowstride = (sizeof(guint8) * dst_bpp * width + 3) & ~3;
-
-  /* Allocate a new buffer to hold converted data */
-  dst_data = g_malloc (height * dst_rowstride);
-  /* and a buffer to hold a temporary RGBA row */
   tmp_row = g_malloc (width *
                       (use_16 ? sizeof (guint16) : sizeof (guint8)) * 4);
 
@@ -442,14 +459,45 @@ _cogl_bitmap_convert (CoglBitmap      *src_bmp,
     }
 
   _cogl_bitmap_unmap (src_bmp);
+  _cogl_bitmap_unmap (dst_bmp);
 
   g_free (tmp_row);
 
-  return _cogl_bitmap_new_from_data (dst_data,
+  return TRUE;
-                                     dst_format,
+}
-                                     width, height, dst_rowstride,
+
-                                     (CoglBitmapDestroyNotify) g_free,
+CoglBitmap *
-                                     NULL);
+_cogl_bitmap_convert (CoglBitmap *src_bmp,
+                      CoglPixelFormat dst_format)
+{
+  int dst_bpp;
+  int dst_rowstride;
+  guint8 *dst_data;
+  CoglBitmap *dst_bmp;
+  int width, height;
+
+  width = _cogl_bitmap_get_width (src_bmp);
+  height = _cogl_bitmap_get_height (src_bmp);
+  dst_bpp = _cogl_pixel_format_get_bytes_per_pixel (dst_format);
+  dst_rowstride = (sizeof (guint8) * dst_bpp * width + 3) & ~3;
+
+  /* Allocate a new buffer to hold converted data */
+  dst_data = g_malloc (height * dst_rowstride);
+
+  dst_bmp = _cogl_bitmap_new_from_data (dst_data,
+                                        dst_format,
+                                        width, height,
+                                        dst_rowstride,
+                                        (CoglBitmapDestroyNotify) g_free,
+                                        NULL);
+
+  if (!_cogl_bitmap_convert_into_bitmap (src_bmp, dst_bmp))
+    {
+      cogl_object_unref (dst_bmp);
+      return NULL;
+    }
+
+  return dst_bmp;
 }
 
 gboolean

cogl/cogl-bitmap-private.h

@@ -87,6 +87,10 @@ CoglBitmap *
 _cogl_bitmap_convert (CoglBitmap *bmp,
 		      CoglPixelFormat   dst_format);
 
+gboolean
+_cogl_bitmap_convert_into_bitmap (CoglBitmap *src_bmp,
+                                  CoglBitmap *dst_bmp);
+
 CoglBitmap *
 _cogl_bitmap_from_file (const char *filename,
 			GError     **error);
@@ -101,7 +105,7 @@ gboolean
 _cogl_bitmap_convert_premult_status (CoglBitmap      *bmp,
                                      CoglPixelFormat  dst_format);
 
-void
+gboolean
 _cogl_bitmap_copy_subregion (CoglBitmap *src,
 			     CoglBitmap *dst,
 			     int         src_x,

cogl/cogl-bitmap.c

@@ -128,7 +128,7 @@ _cogl_bitmap_copy (CoglBitmap *src_bmp)
   return dst_bmp;
 }
 
-void
+gboolean
 _cogl_bitmap_copy_subregion (CoglBitmap *src,
 			     CoglBitmap *dst,
 			     int         src_x,
@@ -142,9 +142,12 @@ _cogl_bitmap_copy_subregion (CoglBitmap *src,
   guint8 *dstdata;
   int    bpp;
   int    line;
+  gboolean succeeded = FALSE;
 
   /* Intended only for fast copies when format is equal! */
-  g_assert (src->format == dst->format);
+  g_assert ((src->format & ~COGL_PREMULT_BIT) ==
+            (dst->format & ~COGL_PREMULT_BIT));
+
   bpp = _cogl_pixel_format_get_bytes_per_pixel (src->format);
 
   if ((srcdata = _cogl_bitmap_map (src, COGL_BUFFER_ACCESS_READ, 0)))
@@ -161,11 +164,15 @@ _cogl_bitmap_copy_subregion (CoglBitmap *src,
               dstdata += dst->rowstride;
             }
 
+          succeeded = TRUE;
+
           _cogl_bitmap_unmap (dst);
         }
 
       _cogl_bitmap_unmap (src);
     }
+
+  return succeeded;
 }
 
 gboolean

cogl/cogl.c

@@ -484,7 +484,7 @@ _cogl_read_pixels_with_rowstride (int x,
       (gl_format != GL_RGBA || gl_type != GL_UNSIGNED_BYTE ||
        rowstride != 4 * width))
     {
-      CoglBitmap *tmp_bmp, *dst_bmp;
+      CoglBitmap *tmp_bmp;
       guint8 *tmp_data = g_malloc (width * height * 4);
 
       tmp_bmp = _cogl_bitmap_new_from_data (tmp_data,
@@ -500,19 +500,7 @@ _cogl_read_pixels_with_rowstride (int x,
                              GL_RGBA, GL_UNSIGNED_BYTE,
                              tmp_data) );
 
-      /* CoglBitmap doesn't currently have a way to convert without
+      if (!_cogl_bitmap_convert_into_bitmap (tmp_bmp, bmp))
-         allocating its own buffer so we have to copy the data
-         again */
-      if ((dst_bmp = _cogl_bitmap_convert (tmp_bmp, format)))
-        {
-          _cogl_bitmap_copy_subregion (dst_bmp,
-                                       bmp,
-                                       0, 0,
-                                       0, 0,
-                                       width, height);
-          cogl_object_unref (dst_bmp);
-        }
-      else
         {
           /* FIXME: there's no way to report an error here so we'll
              just have to leave the data initialised */