Try to avoid copying the GdkPixbuf when it is tightly packed

The docs for GdkPixbuf say that the last row of the image won't
necessarily be allocated to the size of the full rowstride. The rest
of Cogl and possibly GL assumes that we can copy the bitmap with
memcpy(height*rowstride) so we previously would copy the pixbuf data
to ensure this. However if the rowstride is the same as bpp*width then
there is no way for the last row to be under-allocated so in this case
we can just directly upload from the gdk pixbuf. Now that CoglBitmap
can be created with a destroy function we can make it keep a reference
to the pixbuf and unref it during its destroy callback. GdkPixbuf
seems to always pack the image with no padding between rows even if it
is RGB so this should end up always avoiding the memcpy.

The fallback code for when we do have to copy the pixbuf is now
simplified so that it copies all of the rows in a single loop. We only
copy the useful region of each row so this should be safe. The
rowstride of the CoglBitmap is now always allocated to bpp*width
regardless of the rowstride of the pixbuf.

diff --git a/clutter/cogl/cogl/cogl-bitmap-pixbuf.c b/clutter/cogl/cogl/cogl-bitmap-pixbuf.c
index bda210f..559574f 100644 (file)
--- a/clutter/cogl/cogl/cogl-bitmap-pixbuf.c
+++ b/clutter/cogl/cogl/cogl-bitmap-pixbuf.c
@@ -180,6 +180,13 @@ _cogl_bitmap_get_size_from_file (const char *filename,
   return FALSE;
 }
 
+static void
+_cogl_bitmap_unref_pixbuf (guint8 *pixels,
+                           void *pixbuf)
+{
+  g_object_unref (pixbuf);
+}
+
 CoglBitmap *
 _cogl_bitmap_from_file (const char   *filename,
                        GError      **error)
@@ -193,7 +200,6 @@ _cogl_bitmap_from_file (const char   *filename,
   int               rowstride;
   int               bits_per_sample;
   int               n_channels;
-  int               last_row_size;
   guint8           *pixels;
   guint8           *out_data;
   guint8           *out;
@@ -215,9 +221,6 @@ _cogl_bitmap_from_file (const char   *filename,
   bits_per_sample = gdk_pixbuf_get_bits_per_sample (pixbuf);
   n_channels      = gdk_pixbuf_get_n_channels (pixbuf);
 
-  /* The docs say this is the right way */
-  last_row_size = width * ((n_channels * bits_per_sample + 7) / 8);
-
   /* According to current docs this should be true and so
    * the translation to cogl pixel format below valid */
   g_assert (bits_per_sample == 8);
@@ -243,30 +246,34 @@ _cogl_bitmap_from_file (const char   *filename,
       return FALSE;
     }
 
-  /* FIXME: Any way to destroy pixbuf but retain pixel data? */
+  /* If the pixbuf is tightly packed then we can create a bitmap that
+     directly keeps a reference to the pixbuf to avoid copying the
+     data. Otherwise we need to copy because according to the
+     GdkPixbuf docs we can't be sure whether the last row will be
+     allocated to the length of the full rowstride. */
+  if (rowstride == n_channels * width)
+    return _cogl_bitmap_new_from_data (gdk_pixbuf_get_pixels (pixbuf),
+                                       pixel_format,
+                                       width,
+                                       height,
+                                       rowstride,
+                                       _cogl_bitmap_unref_pixbuf,
+                                       pixbuf);
 
   pixels   = gdk_pixbuf_get_pixels (pixbuf);
-  out_data = g_malloc (height * rowstride);
+  out_data = g_malloc (height * n_channels * width);
   out      = out_data;
 
-  /* Copy up to last row */
-  for (r = 0; r < height-1; ++r)
+  for (r = 0; r < height; ++r)
     {
-      memcpy (out, pixels, rowstride);
+      memcpy (out, pixels, n_channels * width);
       pixels += rowstride;
-      out += rowstride;
+      out += n_channels * width;
     }
 
-  /* Copy last row */
-  memcpy (out, pixels, last_row_size);
-
   /* Destroy GdkPixbuf object */
   g_object_unref (pixbuf);
 
-  /* Store bitmap info */
-  /* The stored data the same alignment constraints as a gdkpixbuf but
-   * stores a full rowstride in the last scanline
-   */
   return _cogl_bitmap_new_from_data (out_data,
                                      pixel_format,
                                      width,