[cogl/wip/pbo-read-pixels: 9/18] bitmap: Add a function to convert into an existing buffer

[Neil Roberts <nroberts src gnome org>]

commit 57bd12498df08b7b054d081332d77ff6b2d9cf03
Author: Neil Roberts <neil linux intel com>
Date:   Thu Mar 1 14:44:41 2012 +0000

    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.

 cogl/cogl-bitmap-conversion.c |   96 ++++++++++++++++++++++++++++++----------
 cogl/cogl-bitmap-private.h    |    6 ++-
 cogl/cogl-bitmap.c            |   11 ++++-
 cogl/cogl.c                   |   16 +------
 4 files changed, 88 insertions(+), 41 deletions(-)
---
diff --git a/cogl/cogl-bitmap-conversion.c b/cogl/cogl-bitmap-conversion.c
index 5bdb7cc..9732059 100644
--- a/cogl/cogl-bitmap-conversion.c
+++ b/cogl/cogl-bitmap-conversion.c
@@ -282,29 +282,34 @@ _cogl_bitmap_can_premult (CoglPixelFormat format)
     }
 }
 
-CoglBitmap *
-_cogl_bitmap_convert (CoglBitmap      *src_bmp,
-                      CoglPixelFormat  dst_format)
+gboolean
+_cogl_bitmap_convert_into_bitmap (CoglBitmap *src_bmp,
+                                  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 &&
@@ -316,21 +321,40 @@ _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 &&
-          !_cogl_bitmap_convert_premult_status (dst_bmp, dst_format))
+      if (need_premult)
         {
-          cogl_object_unref (dst_bmp);
-          return NULL;
+          if ((dst_format & COGL_PREMULT_BIT))
+            {
+              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;
+    }
 
   /* If it looks like the format is using more than 8 bits per
      component then we'll unpack into a 16-bit per component buffer
@@ -340,14 +364,7 @@ _cogl_bitmap_convert (CoglBitmap      *src_bmp,
             (!_cogl_pixel_format_is_endian_dependant (dst_format) &&
              _cogl_pixel_format_get_bytes_per_pixel (dst_format) > 2));
 
-  dst_bpp = _cogl_pixel_format_get_bytes_per_pixel (dst_format);
-
-  /* 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 */
+  /* Allocate a buffer to hold a temporary RGBA row */
   tmp_row = g_malloc (width *
                       (use_16 ? sizeof (guint16) : sizeof (guint8)) * 4);
 
@@ -388,14 +405,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,
-                                     dst_format,
-                                     width, height, dst_rowstride,
-                                     (CoglBitmapDestroyNotify) g_free,
-                                     NULL);
+  return TRUE;
+}
+
+CoglBitmap *
+_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
diff --git a/cogl/cogl-bitmap-private.h b/cogl/cogl-bitmap-private.h
index d393041..f552035 100644
--- a/cogl/cogl-bitmap-private.h
+++ b/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,
diff --git a/cogl/cogl-bitmap.c b/cogl/cogl-bitmap.c
index c165e36..2cbd126 100644
--- a/cogl/cogl-bitmap.c
+++ b/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
diff --git a/cogl/cogl.c b/cogl/cogl.c
index 9764578..dffd5f7 100644
--- a/cogl/cogl.c
+++ b/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
-         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
+      if (!_cogl_bitmap_convert_into_bitmap (tmp_bmp, bmp))
         {
           /* FIXME: there's no way to report an error here so we'll
              just have to leave the data initialised */