From 0201e5fa97549ffb2d20a1180e63e5a7a73fa7e2 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Fri, 17 Dec 2010 16:30:23 +0000 Subject: [PATCH] cogl: Don't assume that CoglBitmaps are allocated to height*rowstride Cogl no longer has any code that assumes the buffer in a CoglBitmap is allocated to the full size of height*rowstride. We should comment that this is the case so that we remember to keep it that way. This is important for cogl_texture_new_from_data because the application may have created the data from a sub-region of a larger image and in that case it's not safe to read the full rowstride of the last row when the sub region contains the last row of the larger image. http://bugzilla.clutter-project.org/show_bug.cgi?id=2491 --- clutter/cogl/cogl/cogl-bitmap-pixbuf.c | 60 ++++--------------------- clutter/cogl/cogl/cogl-bitmap-private.h | 8 ++++ 2 files changed, 17 insertions(+), 51 deletions(-) diff --git a/clutter/cogl/cogl/cogl-bitmap-pixbuf.c b/clutter/cogl/cogl/cogl-bitmap-pixbuf.c index 80ed00ce4..f0bd0eee8 100644 --- a/clutter/cogl/cogl/cogl-bitmap-pixbuf.c +++ b/clutter/cogl/cogl/cogl-bitmap-pixbuf.c @@ -198,13 +198,8 @@ _cogl_bitmap_from_file (const char *filename, int width; int height; int rowstride; - int aligned_rowstride; int bits_per_sample; int n_channels; - guint8 *pixels; - guint8 *out_data; - guint8 *out; - int r; g_return_val_if_fail (error == NULL || *error == NULL, FALSE); @@ -247,55 +242,18 @@ _cogl_bitmap_from_file (const char *filename, return FALSE; } - /* Work out what the rowstride would be if it was packing the data - but aligned to 4 bytes */ - aligned_rowstride = (width * n_channels + 3) & ~3; - - /* The documentation for GdkPixbuf states that is not safe to read - all of the data as height*rowstride because the buffer might not - be allocated to include the full length of the rowstride for the - last row so arguably we should always copy the buffer when - rowstride != width*bpp because some places in Cogl assume that it - can memcpy(height*rowstride). However that rule is probably only - in place so that GdkPixbuf can implement gdk_pixbuf_new_subpixbuf - by just sharing the data and setting a large rowstride. That does - not apply in this case because we are just creating a new buffer - for a file. It seems very unlikely that GdkPixbuf would not - allocate the full rowstride in this case and it is highly - desirable to avoid copying the buffer. This instead just assumes - that whatever buffer pixbuf points into will always be allocated - to a 4-byte aligned buffer so we can avoid copying unless the - rowstride is unusually large */ - if (rowstride <= aligned_rowstride) - 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 (aligned_rowstride * height); - out = out_data; - - for (r = 0; r < height; ++r) - { - memcpy (out, pixels, n_channels * width); - pixels += rowstride; - out += aligned_rowstride; - } - - /* Destroy GdkPixbuf object */ - g_object_unref (pixbuf); - - return _cogl_bitmap_new_from_data (out_data, + /* We just use the data directly from the pixbuf so that we don't + have to copy to a seperate buffer. Note that Cogl is expected not + to read past the end of bpp*width on the last row even if the + rowstride is much larger so we don't need to worry about + GdkPixbuf's semantics that it may under-allocate the buffer. */ + return _cogl_bitmap_new_from_data (gdk_pixbuf_get_pixels (pixbuf), pixel_format, width, height, - aligned_rowstride, - (CoglBitmapDestroyNotify) g_free, - NULL); + rowstride, + _cogl_bitmap_unref_pixbuf, + pixbuf); } #else diff --git a/clutter/cogl/cogl/cogl-bitmap-private.h b/clutter/cogl/cogl/cogl-bitmap-private.h index bf21a5732..a1f306631 100644 --- a/clutter/cogl/cogl/cogl-bitmap-private.h +++ b/clutter/cogl/cogl/cogl-bitmap-private.h @@ -180,6 +180,14 @@ _cogl_bitmap_get_height (CoglBitmap *bitmap); int _cogl_bitmap_get_rowstride (CoglBitmap *bitmap); +/* Maps the bitmap so that the pixels can be accessed directly or if + the bitmap is just a memory bitmap then it just returns the pointer + to memory. Note that the bitmap isn't guaranteed to allocated to + the full size of rowstride*height so it is not safe to read up to + the rowstride of the last row. This will be the case if the user + uploads data using gdk_pixbuf_new_subpixbuf with a sub region + containing the last row of the pixbuf because in that case the + rowstride can be much larger than the width of the image */ guint8 * _cogl_bitmap_map (CoglBitmap *bitmap, CoglBufferAccess access,