From 6ea2fbc7297a1f6e3d78d705fdaef8307cf59d97 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 15 Jul 2010 13:05:55 +0100 Subject: [PATCH] Add an internal _cogl_bitmap_new_from_buffer This function creates a CoglBitmap which internally references a CoglBuffer. The map and unmap functions will divert to mapping the buffer. There are also now bind and unbind functions which should be used instead of map and unmap whenever the data doesn't need to be read from the CPU but will instead be passed to GL for packing or unpacking. For bitmaps created from buffers this just binds the bitmap. cogl_texture_new_from_buffer now just uses this function to wrap the buffer in a bitmap rather than trying to bind the buffer immediately. This means that the buffer will be bound only at the point right before the texture data is uploaded. This approach means that using a pixel array will take the fastest upload route if possible, but can still fallback to copying the data by mapping the buffer if some conversion is needed. Previously it would just crash in this case because the texture functions were all passed a NULL pointer. http://bugzilla.clutter-project.org/show_bug.cgi?id=2112 --- clutter/cogl/cogl/cogl-bitmap-private.h | 25 ++++ clutter/cogl/cogl/cogl-bitmap.c | 139 +++++++++++++++++- clutter/cogl/cogl/cogl-texture.c | 37 ++--- .../cogl/cogl/driver/gl/cogl-texture-driver.c | 106 +++++++------ .../cogl/driver/gles/cogl-texture-driver.c | 79 +++++----- 5 files changed, 256 insertions(+), 130 deletions(-) diff --git a/clutter/cogl/cogl/cogl-bitmap-private.h b/clutter/cogl/cogl/cogl-bitmap-private.h index 7aefee631..d14293fad 100644 --- a/clutter/cogl/cogl/cogl-bitmap-private.h +++ b/clutter/cogl/cogl/cogl-bitmap-private.h @@ -82,6 +82,17 @@ _cogl_bitmap_new_shared (CoglBitmap *shared_bmp, int height, int rowstride); +/* This creates a cogl bitmap that internally references a pixel + array. The data is not copied. _cogl_bitmap_map will divert to + mapping the pixel array */ +CoglBitmap * +_cogl_bitmap_new_from_buffer (CoglBuffer *buffer, + CoglPixelFormat format, + int width, + int height, + int rowstride, + int offset); + gboolean _cogl_bitmap_can_convert (CoglPixelFormat src, CoglPixelFormat dst); @@ -173,4 +184,18 @@ _cogl_bitmap_map (CoglBitmap *bitmap, void _cogl_bitmap_unmap (CoglBitmap *bitmap); +/* These two are replacements for map and unmap that should used when + the pointer is going to be passed to GL for pixel packing or + unpacking. The address might not be valid for reading if the bitmap + was created with new_from_buffer but it will however be good to + pass to glTexImage2D for example. The access should be READ for + unpacking and WRITE for packing. It can not be both */ +guint8 * +_cogl_bitmap_bind (CoglBitmap *bitmap, + CoglBufferAccess access, + CoglBufferMapHint hints); + +void +_cogl_bitmap_unbind (CoglBitmap *bitmap); + #endif /* __COGL_BITMAP_H */ diff --git a/clutter/cogl/cogl/cogl-bitmap.c b/clutter/cogl/cogl/cogl-bitmap.c index 313177783..8ea83fd27 100644 --- a/clutter/cogl/cogl/cogl-bitmap.c +++ b/clutter/cogl/cogl/cogl-bitmap.c @@ -28,6 +28,7 @@ #include "cogl.h" #include "cogl-internal.h" #include "cogl-bitmap-private.h" +#include "cogl-buffer-private.h" #include @@ -44,10 +45,15 @@ struct _CoglBitmap void *destroy_fn_data; gboolean mapped; + gboolean bound; /* If this is non-null then 'data' is ignored and instead it is fetched from this shared bitmap. */ CoglBitmap *shared_bmp; + + /* If this is non-null then 'data' is treated as an offset into the + buffer and map will divert to mapping the buffer */ + CoglBuffer *buffer; }; static void _cogl_bitmap_free (CoglBitmap *bmp); @@ -58,6 +64,7 @@ static void _cogl_bitmap_free (CoglBitmap *bmp) { g_assert (!bmp->mapped); + g_assert (!bmp->bound); if (bmp->destroy_fn) bmp->destroy_fn (bmp->data, bmp->destroy_fn_data); @@ -65,6 +72,9 @@ _cogl_bitmap_free (CoglBitmap *bmp) if (bmp->shared_bmp) cogl_object_unref (bmp->shared_bmp); + if (bmp->buffer) + cogl_object_unref (bmp->buffer); + g_slice_free (CoglBitmap, bmp); } @@ -237,7 +247,9 @@ _cogl_bitmap_new_from_data (guint8 *data, bmp->destroy_fn = destroy_fn; bmp->destroy_fn_data = destroy_fn_data; bmp->mapped = FALSE; + bmp->bound = FALSE; bmp->shared_bmp = NULL; + bmp->buffer = NULL; return _cogl_bitmap_object_new (bmp); } @@ -284,6 +296,32 @@ cogl_bitmap_new_from_file (const char *filename, return bmp; } +CoglBitmap * +_cogl_bitmap_new_from_buffer (CoglBuffer *buffer, + CoglPixelFormat format, + int width, + int height, + int rowstride, + int offset) +{ + CoglBitmap *bmp; + + g_return_val_if_fail (cogl_is_buffer (buffer), NULL); + + bmp = _cogl_bitmap_new_from_data (NULL, /* data */ + format, + width, + height, + rowstride, + NULL, /* destroy_fn */ + NULL /* destroy_fn_data */); + + bmp->buffer = cogl_object_ref (buffer); + bmp->data = GINT_TO_POINTER (offset); + + return bmp; +} + CoglPixelFormat _cogl_bitmap_get_format (CoglBitmap *bitmap) { @@ -331,11 +369,32 @@ _cogl_bitmap_map (CoglBitmap *bitmap, return _cogl_bitmap_map (bitmap->shared_bmp, access, hints); g_assert (!bitmap->mapped); - bitmap->mapped = TRUE; - /* Currently the bitmap is always in regular memory so we can just - directly return the pointer */ - return bitmap->data; + if (bitmap->buffer) + { + guint8 *data = cogl_buffer_map (bitmap->buffer, + access, + hints); + + COGL_NOTE (BITMAP, "A pixel array is being mapped from a bitmap. This " + "usually means that some conversion on the pixel array is " + "needed so a sub-optimal format is being used."); + + if (data) + { + bitmap->mapped = TRUE; + + return data + GPOINTER_TO_INT (bitmap->data); + } + else + return NULL; + } + else + { + bitmap->mapped = TRUE; + + return bitmap->data; + } } void @@ -348,6 +407,74 @@ _cogl_bitmap_unmap (CoglBitmap *bitmap) g_assert (bitmap->mapped); bitmap->mapped = FALSE; - /* Currently the bitmap is always in regular memory so we don't need - to do anything */ + if (bitmap->buffer) + cogl_buffer_unmap (bitmap->buffer); +} + +guint8 * +_cogl_bitmap_bind (CoglBitmap *bitmap, + CoglBufferAccess access, + CoglBufferMapHint hints) +{ + guint8 *ptr; + + /* Divert to another bitmap if this data is shared */ + if (bitmap->shared_bmp) + return _cogl_bitmap_bind (bitmap->shared_bmp, access, hints); + + g_assert (!bitmap->bound); + + /* If the bitmap wasn't created from a buffer then the + implementation of bind is the same as map */ + if (bitmap->buffer == NULL) + { + guint8 *data = _cogl_bitmap_map (bitmap, access, hints); + if (data) + bitmap->bound = TRUE; + return data; + } + + bitmap->bound = TRUE; + + /* If buffer is using a malloc fallback then we'll just use the + pointer directly */ + if (COGL_BUFFER_FLAG_IS_SET (bitmap->buffer, BUFFER_OBJECT)) + { + ptr = NULL; + + if (access == COGL_BUFFER_ACCESS_READ) + _cogl_buffer_bind (bitmap->buffer, + COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK); + else if (access == COGL_BUFFER_ACCESS_WRITE) + _cogl_buffer_bind (bitmap->buffer, + COGL_BUFFER_BIND_TARGET_PIXEL_PACK); + else + g_assert_not_reached (); + } + else + ptr = bitmap->buffer->data; + + /* The data pointer actually stores the offset */ + return GPOINTER_TO_INT (bitmap->data) + ptr; +} + +void +_cogl_bitmap_unbind (CoglBitmap *bitmap) +{ + /* Divert to another bitmap if this data is shared */ + if (bitmap->shared_bmp) + return _cogl_bitmap_unbind (bitmap->shared_bmp); + + g_assert (bitmap->bound); + bitmap->bound = FALSE; + + /* If the bitmap wasn't created from a pixel array then the + implementation of unbind is the same as unmap */ + if (bitmap->buffer) + { + if (COGL_BUFFER_FLAG_IS_SET (bitmap->buffer, BUFFER_OBJECT)) + _cogl_buffer_unbind (bitmap->buffer); + } + else + _cogl_bitmap_unmap (bitmap); } diff --git a/clutter/cogl/cogl/cogl-texture.c b/clutter/cogl/cogl/cogl-texture.c index 1a633b278..0f6654fc2 100644 --- a/clutter/cogl/cogl/cogl-texture.c +++ b/clutter/cogl/cogl/cogl-texture.c @@ -560,6 +560,7 @@ cogl_texture_new_from_buffer_EXP (CoglHandle buffer, CoglHandle texture; CoglBuffer *cogl_buffer; CoglPixelArray *pixel_array; + CoglBitmap *bmp; g_return_val_if_fail (cogl_is_buffer (buffer), COGL_INVALID_HANDLE); @@ -588,36 +589,16 @@ cogl_texture_new_from_buffer_EXP (CoglHandle buffer, return COGL_INVALID_HANDLE; } -#if !defined (COGL_HAS_GLES) - if (cogl_features_available (COGL_FEATURE_PBOS)) - { - CoglBitmap *bmp; + /* Wrap the buffer into a bitmap */ + bmp = _cogl_bitmap_new_from_buffer (cogl_buffer, + format, + width, height, + rowstride, + offset); - /* Wrap the data into a bitmap */ - bmp = _cogl_bitmap_new_from_data (GUINT_TO_POINTER (offset), - format, - width, height, - rowstride, - NULL, NULL); + texture = cogl_texture_new_from_bitmap (bmp, flags, internal_format); - _cogl_buffer_bind (cogl_buffer, - COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK); - texture = cogl_texture_new_from_bitmap (bmp, flags, internal_format); - _cogl_buffer_unbind (cogl_buffer); - - cogl_object_unref (bmp); - } - else -#endif - { - texture = cogl_texture_new_from_data (width, - height, - flags, - format, - internal_format, - rowstride, - cogl_buffer->data); - } + cogl_object_unref (bmp); return texture; } diff --git a/clutter/cogl/cogl/driver/gl/cogl-texture-driver.c b/clutter/cogl/cogl/driver/gl/cogl-texture-driver.c index 376584d80..bf742d7c8 100644 --- a/clutter/cogl/cogl/driver/gl/cogl-texture-driver.c +++ b/clutter/cogl/cogl/driver/gl/cogl-texture-driver.c @@ -154,29 +154,27 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum gl_target, GLuint source_gl_type) { guint8 *data; + int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); - if ((data = _cogl_bitmap_map (source_bmp, COGL_BUFFER_ACCESS_READ, 0))) - { - int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); + data = _cogl_bitmap_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0); - /* Setup gl alignment to match rowstride and top-left corner */ - prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), - 0, - src_x, - src_y, - bpp); + /* Setup gl alignment to match rowstride and top-left corner */ + prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), + 0, + src_x, + src_y, + bpp); - _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); + _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); - GE( glTexSubImage2D (gl_target, 0, - dst_x, dst_y, - width, height, - source_gl_format, - source_gl_type, - data) ); + GE( glTexSubImage2D (gl_target, 0, + dst_x, dst_y, + width, height, + source_gl_format, + source_gl_type, + data) ); - _cogl_bitmap_unmap (source_bmp); - } + _cogl_bitmap_unbind (source_bmp); } void @@ -189,28 +187,26 @@ _cogl_texture_driver_upload_to_gl (GLenum gl_target, GLuint source_gl_type) { guint8 *data; + int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); - if ((data = _cogl_bitmap_map (source_bmp, COGL_BUFFER_ACCESS_READ, 0))) - { - int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); + data = _cogl_bitmap_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0); - /* Setup gl alignment to match rowstride and top-left corner */ - prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), - 0, 0, 0, bpp); + /* Setup gl alignment to match rowstride and top-left corner */ + prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), + 0, 0, 0, bpp); - _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); + _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); - GE( glTexImage2D (gl_target, 0, - internal_gl_format, - _cogl_bitmap_get_width (source_bmp), - _cogl_bitmap_get_height (source_bmp), - 0, - source_gl_format, - source_gl_type, - data) ); + GE( glTexImage2D (gl_target, 0, + internal_gl_format, + _cogl_bitmap_get_width (source_bmp), + _cogl_bitmap_get_height (source_bmp), + 0, + source_gl_format, + source_gl_type, + data) ); - _cogl_bitmap_unmap (source_bmp); - } + _cogl_bitmap_unbind (source_bmp); } void @@ -225,34 +221,32 @@ _cogl_texture_driver_upload_to_gl_3d (GLenum gl_target, GLuint source_gl_type) { guint8 *data; + int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); _COGL_GET_CONTEXT (ctx, NO_RETVAL); - if ((data = _cogl_bitmap_map (source_bmp, COGL_BUFFER_ACCESS_READ, 0))) - { - int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp)); + data = _cogl_bitmap_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0); - /* Setup gl alignment to match rowstride and top-left corner */ - prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), - (_cogl_bitmap_get_height (source_bmp) / - depth), - 0, 0, bpp); + /* Setup gl alignment to match rowstride and top-left corner */ + prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp), + (_cogl_bitmap_get_height (source_bmp) / + depth), + 0, 0, bpp); - _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); + _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); - GE( glTexImage3D (gl_target, - 0, /* level */ - internal_gl_format, - _cogl_bitmap_get_width (source_bmp), - height, - depth, - 0, - source_gl_format, - source_gl_type, - data) ); + GE( glTexImage3D (gl_target, + 0, /* level */ + internal_gl_format, + _cogl_bitmap_get_width (source_bmp), + height, + depth, + 0, + source_gl_format, + source_gl_type, + data) ); - _cogl_bitmap_unmap (source_bmp); - } + _cogl_bitmap_unbind (source_bmp); } gboolean diff --git a/clutter/cogl/cogl/driver/gles/cogl-texture-driver.c b/clutter/cogl/cogl/driver/gles/cogl-texture-driver.c index de704cf18..503090c27 100644 --- a/clutter/cogl/cogl/driver/gles/cogl-texture-driver.c +++ b/clutter/cogl/cogl/driver/gles/cogl-texture-driver.c @@ -150,19 +150,18 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum gl_target, 0, 0, width, height); - if ((data = _cogl_bitmap_map (slice_bmp, COGL_BUFFER_ACCESS_READ, 0))) - { - _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); + data = _cogl_bitmap_bind (slice_bmp, COGL_BUFFER_ACCESS_READ, 0); - GE( glTexSubImage2D (gl_target, 0, - dst_x, dst_y, - width, height, - source_gl_format, - source_gl_type, - data) ); + _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); - _cogl_bitmap_unmap (slice_bmp); - } + GE( glTexSubImage2D (gl_target, 0, + dst_x, dst_y, + width, height, + source_gl_format, + source_gl_type, + data) ); + + _cogl_bitmap_unbind (slice_bmp); cogl_object_unref (slice_bmp); } @@ -209,18 +208,17 @@ _cogl_texture_driver_upload_to_gl (GLenum gl_target, _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign); - if ((data = _cogl_bitmap_map (bmp, COGL_BUFFER_ACCESS_READ, 0))) - { - GE( glTexImage2D (gl_target, 0, - internal_gl_format, - bmp_width, bmp_height, - 0, - source_gl_format, - source_gl_type, - data) ); + data = _cogl_bitmap_bind (bmp, COGL_BUFFER_ACCESS_READ, 0); - _cogl_bitmap_unmap (bmp); - } + GE( glTexImage2D (gl_target, 0, + internal_gl_format, + bmp_width, bmp_height, + 0, + source_gl_format, + source_gl_type, + data) ); + + _cogl_bitmap_unbind (bmp); cogl_object_unref (bmp); } @@ -289,29 +287,30 @@ _cogl_texture_driver_upload_to_gl_3d (GLenum gl_target, bmp_width, bmp_height); - if ((data = _cogl_bitmap_map (bmp, - COGL_BUFFER_ACCESS_READ, 0))) - { - GE( glTexSubImage3D (gl_target, - 0, /* level */ - 0, /* xoffset */ - 0, /* yoffset */ - i, /* zoffset */ - bmp_width, /* width */ - height, /* height */ - 1, /* depth */ - source_gl_format, - source_gl_type, - data) ); + data = _cogl_bitmap_bind (bmp, + COGL_BUFFER_ACCESS_READ, 0); - _cogl_bitmap_unmap (bmp); - } + GE( glTexSubImage3D (gl_target, + 0, /* level */ + 0, /* xoffset */ + 0, /* yoffset */ + i, /* zoffset */ + bmp_width, /* width */ + height, /* height */ + 1, /* depth */ + source_gl_format, + source_gl_type, + data) ); + + _cogl_bitmap_unbind (bmp); } cogl_object_unref (bmp); } - else if ((data = _cogl_bitmap_map (source_bmp, COGL_BUFFER_ACCESS_READ, 0))) + else { + data = _cogl_bitmap_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0); + _cogl_texture_driver_prep_gl_for_pixels_upload (rowstride, bpp); GE( glTexImage3D (gl_target, @@ -325,7 +324,7 @@ _cogl_texture_driver_upload_to_gl_3d (GLenum gl_target, source_gl_type, data) ); - _cogl_bitmap_unmap (source_bmp); + _cogl_bitmap_unbind (source_bmp); } }