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

@@ -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 */

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 <string.h>
 
@@ -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,12 +369,33 @@ _cogl_bitmap_map (CoglBitmap *bitmap,
     return _cogl_bitmap_map (bitmap->shared_bmp, access, hints);
 
   g_assert (!bitmap->mapped);
+
+  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;
 
-  /* Currently the bitmap is always in regular memory so we can just
-     directly return the pointer */
       return bitmap->data;
     }
+}
 
 void
 _cogl_bitmap_unmap (CoglBitmap *bitmap)
@@ -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
+  if (bitmap->buffer)
-     to do anything */
+    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);
 }

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)
+  /* Wrap the buffer into a bitmap */
-  if (cogl_features_available (COGL_FEATURE_PBOS))
+  bmp = _cogl_bitmap_new_from_buffer (cogl_buffer,
-    {
-      CoglBitmap *bmp;
-
-      /* Wrap the data into a bitmap */
-      bmp = _cogl_bitmap_new_from_data (GUINT_TO_POINTER (offset),
                                       format,
                                       width, height,
                                       rowstride,
-                                        NULL, NULL);
+                                      offset);
 
-      _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);
-    }
 
   return texture;
 }

clutter/cogl/cogl/driver/gl/cogl-texture-driver.c

@@ -154,11 +154,10 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum       gl_target,
 				             GLuint       source_gl_type)
 {
   guint8 *data;
-
-  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,
@@ -175,8 +174,7 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum       gl_target,
                        source_gl_type,
                        data) );
 
-      _cogl_bitmap_unmap (source_bmp);
+  _cogl_bitmap_unbind (source_bmp);
-    }
 }
 
 void
@@ -189,11 +187,10 @@ _cogl_texture_driver_upload_to_gl (GLenum       gl_target,
                                    GLuint       source_gl_type)
 {
   guint8 *data;
-
-  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);
@@ -209,8 +206,7 @@ _cogl_texture_driver_upload_to_gl (GLenum       gl_target,
                     source_gl_type,
                     data) );
 
-      _cogl_bitmap_unmap (source_bmp);
+  _cogl_bitmap_unbind (source_bmp);
-    }
 }
 
 void
@@ -225,12 +221,11 @@ _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)))
+  data = _cogl_bitmap_bind (source_bmp, COGL_BUFFER_ACCESS_READ, 0);
-    {
-      int bpp = _cogl_get_format_bpp (_cogl_bitmap_get_format (source_bmp));
 
   /* Setup gl alignment to match rowstride and top-left corner */
   prep_gl_for_pixels_upload_full (_cogl_bitmap_get_rowstride (source_bmp),
@@ -251,8 +246,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);
-    }
 }
 
 gboolean

clutter/cogl/cogl/driver/gles/cogl-texture-driver.c

@@ -150,8 +150,8 @@ _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)))
+  data = _cogl_bitmap_bind (slice_bmp, COGL_BUFFER_ACCESS_READ, 0);
-    {
+
   _cogl_bind_gl_texture_transient (gl_target, gl_handle, is_foreign);
 
   GE( glTexSubImage2D (gl_target, 0,
@@ -161,8 +161,7 @@ _cogl_texture_driver_upload_subregion_to_gl (GLenum       gl_target,
                        source_gl_type,
                        data) );
 
-      _cogl_bitmap_unmap (slice_bmp);
+  _cogl_bitmap_unbind (slice_bmp);
-    }
 
   cogl_object_unref (slice_bmp);
 }
@@ -209,8 +208,8 @@ _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)))
+  data = _cogl_bitmap_bind (bmp, COGL_BUFFER_ACCESS_READ, 0);
-    {
+
   GE( glTexImage2D (gl_target, 0,
                     internal_gl_format,
                     bmp_width, bmp_height,
@@ -219,8 +218,7 @@ _cogl_texture_driver_upload_to_gl (GLenum       gl_target,
                     source_gl_type,
                     data) );
 
-      _cogl_bitmap_unmap (bmp);
+  _cogl_bitmap_unbind (bmp);
-    }
 
   cogl_object_unref (bmp);
 }
@@ -289,9 +287,9 @@ _cogl_texture_driver_upload_to_gl_3d (GLenum       gl_target,
                                        bmp_width,
                                        bmp_height);
 
-          if ((data = _cogl_bitmap_map (bmp,
+          data = _cogl_bitmap_bind (bmp,
-                                        COGL_BUFFER_ACCESS_READ, 0)))
+                                    COGL_BUFFER_ACCESS_READ, 0);
-            {
+
           GE( glTexSubImage3D (gl_target,
                                0, /* level */
                                0, /* xoffset */
@@ -304,14 +302,15 @@ _cogl_texture_driver_upload_to_gl_3d (GLenum       gl_target,
                                source_gl_type,
                                data) );
 
-              _cogl_bitmap_unmap (bmp);
+          _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);
     }
 }