From c5205c50d246e033265b77d2b746a72c1137e7e1 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 15 Jul 2010 11:32:08 +0100 Subject: [PATCH] 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. --- cogl/cogl-bitmap-pixbuf.c | 41 +++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/cogl/cogl-bitmap-pixbuf.c b/cogl/cogl-bitmap-pixbuf.c index bda210f67..559574fc3 100644 --- a/cogl/cogl-bitmap-pixbuf.c +++ b/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,