cogl-buffer: Track the last used bind target in CoglBuffer

This makes CoglBuffer track the last used bind target as a private
property. This is later used when binding a buffer to map instead of
always using the PIXEL_UNPACK target.

This also adds some additional sanity checks that code doesn't try to
nest binds to the same target or bind a buffer to multiple targets at
the same time.

cogl/cogl-buffer-private.h

@@ -75,11 +75,22 @@ typedef enum {
   COGL_BUFFER_USAGE_HINT_VERTICES
 } CoglBufferUsageHint;
 
+typedef enum {
+  COGL_BUFFER_BIND_TARGET_PIXEL_PACK,
+  COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK,
+  COGL_BUFFER_BIND_TARGET_VERTEX_ARRAY,
+  COGL_BUFFER_BIND_TARGET_VERTEX_INDICES_ARRAY,
+
+  COGL_BUFFER_BIND_TARGET_COUNT
+} CoglBufferBindTarget;
+
 struct _CoglBuffer
 {
   CoglObject              _parent;
   const CoglBufferVtable *vtable;
 
+  CoglBufferBindTarget    last_target;
+
   CoglBufferFlags         flags;
 
   GLuint                  gl_handle;  /* OpenGL handle */
@@ -107,6 +118,7 @@ _cogl_buffer_register_buffer_type (GQuark type);
 void
 _cogl_buffer_initialize (CoglBuffer          *buffer,
                          unsigned int         size,
+                         CoglBufferBindTarget default_target,
                          CoglBufferUsageHint  usage_hint,
                          CoglBufferUpdateHint update_hint);
 
@@ -115,11 +127,20 @@ _cogl_buffer_fini (CoglBuffer *buffer);
 
 void
 _cogl_buffer_bind (CoglBuffer *buffer,
-                   GLenum      target);
+                   CoglBufferBindTarget target);
+
+void
+_cogl_buffer_unbind (CoglBuffer *buffer);
 
 CoglBufferUsageHint
 _cogl_buffer_get_usage_hint (CoglBuffer *buffer);
 
+GLenum
+_cogl_buffer_get_last_gl_target (CoglBuffer *buffer);
+
+CoglBufferBindTarget
+_cogl_buffer_get_last_bind_target (CoglBuffer *buffer);
+
 GLenum
 _cogl_buffer_access_to_gl_enum (CoglBufferAccess access);
 

cogl/cogl-buffer.c

@@ -59,9 +59,6 @@
 #define glDeleteBuffers ctx->drv.pf_glDeleteBuffers
 #define glMapBuffer ctx->drv.pf_glMapBuffer
 #define glUnmapBuffer ctx->drv.pf_glUnmapBuffer
-#ifndef GL_ARRAY_BUFFER
-#define GL_ARRAY_BUFFER GL_ARRAY_BUFFER_ARB
-#endif
 
 #elif defined (HAVE_COGL_GLES2)
 
@@ -69,6 +66,19 @@
 
 #endif
 
+#ifndef GL_PIXEL_PACK_BUFFER
+#define GL_PIXEL_PACK_BUFFER 0x88EB
+#endif
+#ifndef GL_PIXEL_UNPACK_BUFFER
+#define GL_PIXEL_UNPACK_BUFFER 0x88EC
+#endif
+#ifndef GL_ARRAY_BUFFER
+#define GL_ARRAY_BUFFER 0x8892
+#endif
+#ifndef GL_ELEMENT_ARRAY_BUFFER
+#define GL_ARRAY_BUFFER 0x8893
+#endif
+
 /* XXX:
  * The CoglHandle macros don't support any form of inheritance, so for
  * now we implement the CoglObject support for the CoglBuffer
@@ -105,6 +115,7 @@ cogl_is_buffer (const void *object)
 void
 _cogl_buffer_initialize (CoglBuffer           *buffer,
                          unsigned int          size,
+                         CoglBufferBindTarget  default_target,
                          CoglBufferUsageHint   usage_hint,
                          CoglBufferUpdateHint  update_hint)
 {
@@ -112,6 +123,7 @@ _cogl_buffer_initialize (CoglBuffer           *buffer,
 
   buffer->flags       = COGL_BUFFER_FLAG_NONE;
   buffer->size        = size;
+  buffer->last_target = default_target;
   buffer->usage_hint  = usage_hint;
   buffer->update_hint = update_hint;
   buffer->data        = NULL;
@@ -124,6 +136,30 @@ _cogl_buffer_fini (CoglBuffer *buffer)
     cogl_buffer_unmap (buffer);
 }
 
+GLenum
+_cogl_buffer_get_last_gl_target (CoglBuffer *buffer)
+{
+  switch (buffer->last_target)
+    {
+      case COGL_BUFFER_BIND_TARGET_PIXEL_PACK:
+        return GL_PIXEL_PACK_BUFFER;
+      case COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK:
+        return GL_PIXEL_UNPACK_BUFFER;
+      case COGL_BUFFER_BIND_TARGET_VERTEX_ARRAY:
+        return GL_ARRAY_BUFFER;
+      case COGL_BUFFER_BIND_TARGET_VERTEX_INDICES_ARRAY:
+        return GL_ELEMENT_ARRAY_BUFFER;
+      default:
+        g_return_val_if_reached (COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK);
+    }
+}
+
+CoglBufferBindTarget
+_cogl_buffer_get_last_bind_target (CoglBuffer *buffer)
+{
+  return buffer->last_target;
+}
+
 /* OpenGL ES 1.1 and 2 have a GL_OES_mapbuffer extension that is able to map
  * VBOs for write only, we don't support that in CoglBuffer */
 #if defined (COGL_HAS_GLES)
@@ -132,6 +168,7 @@ _cogl_buffer_access_to_gl_enum (CoglBufferAccess access)
 {
   return 0;
 }
+
 #else
 GLenum
 _cogl_buffer_access_to_gl_enum (CoglBufferAccess access)
@@ -174,27 +211,46 @@ _cogl_buffer_hints_to_gl_enum (CoglBufferUsageHint  usage_hint,
 #endif
 
 void
-_cogl_buffer_bind (CoglBuffer *buffer,
+_cogl_buffer_bind (CoglBuffer *buffer, CoglBufferBindTarget target)
-                   GLenum      target)
 {
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
-  /* Don't bind again an already bound pbo */
+  g_return_if_fail (buffer != NULL);
-  if (ctx->current_pbo == buffer)
-    return;
 
-  if (buffer && COGL_BUFFER_FLAG_IS_SET (buffer, BUFFER_OBJECT))
+  /* Don't allow binding the buffer to multiple targets at the same time */
+  g_return_if_fail (ctx->current_buffer[buffer->last_target] != buffer);
+
+  /* Don't allow nesting binds to the same target */
+  g_return_if_fail (ctx->current_buffer[target] == NULL);
+
+  buffer->last_target = target;
+
+  if (COGL_BUFFER_FLAG_IS_SET (buffer, BUFFER_OBJECT))
     {
-      GE( glBindBuffer (target, buffer->gl_handle) );
+      GLenum gl_target = _cogl_buffer_get_last_gl_target (buffer);
-    }
+      GE( glBindBuffer (gl_target, buffer->gl_handle) );
-  else if (buffer == NULL &&
-           ctx->current_pbo &&
-           COGL_BUFFER_FLAG_IS_SET (ctx->current_pbo, BUFFER_OBJECT))
-    {
-      GE( glBindBuffer (target, 0) );
     }
 
-  ctx->current_pbo = buffer;
+  ctx->current_buffer[target] = buffer;
+}
+
+void
+_cogl_buffer_unbind (CoglBuffer *buffer)
+{
+  _COGL_GET_CONTEXT (ctx, NO_RETVAL);
+
+  g_return_if_fail (buffer != NULL);
+
+  /* the unbind should pair up with a previous bind */
+  g_return_if_fail (ctx->current_buffer[buffer->last_target] == buffer);
+
+  if (COGL_BUFFER_FLAG_IS_SET (buffer, BUFFER_OBJECT))
+    {
+      GLenum gl_target = _cogl_buffer_get_last_gl_target (buffer);
+      GE( glBindBuffer (gl_target, 0) );
+    }
+
+  ctx->current_buffer[buffer->last_target] = NULL;
 }
 
 unsigned int

cogl/cogl-context.c

@@ -57,6 +57,7 @@ cogl_create_context (void)
   GLubyte default_texture_data[] = { 0xff, 0xff, 0xff, 0x0 };
   unsigned long  enable_flags = 0;
   CoglHandle window_buffer;
+  int i;
 
   if (_context != NULL)
     return FALSE;
@@ -148,6 +149,9 @@ cogl_create_context (void)
 
   _context->legacy_depth_test_enabled = FALSE;
 
+  for (i = 0; i < COGL_BUFFER_BIND_TARGET_COUNT; i++)
+    _context->current_buffer[i] = NULL;
+
   _context->framebuffer_stack = _cogl_create_framebuffer_stack ();
 
   window_buffer = _cogl_onscreen_new ();
@@ -204,8 +208,6 @@ cogl_create_context (void)
   _context->atlas = NULL;
   _context->atlas_texture = COGL_INVALID_HANDLE;
 
-  _context->current_pbo = NULL;
-
   return TRUE;
 }
 

cogl/cogl-context.h

@@ -118,9 +118,7 @@ typedef struct
 
   gboolean              legacy_depth_test_enabled;
 
-  /* PBOs */
+  CoglBuffer       *current_buffer[COGL_BUFFER_BIND_TARGET_COUNT];
-  /* This can be used to check if a pbo is bound */
-  CoglBuffer       *current_pbo;
 
   /* Framebuffers */
   GSList           *framebuffer_stack;

cogl/cogl-pixel-array-private.h

@@ -58,7 +58,6 @@ struct _CoglPixelArray
 
   CoglPixelArrayFlags  flags;
 
-  GLenum                gl_target;
   CoglPixelFormat       format;
   unsigned int          width;
   unsigned int          height;

cogl/cogl-pixel-array.c

@@ -98,6 +98,7 @@ _cogl_pixel_array_new (unsigned int size)
   /* parent's constructor */
   _cogl_buffer_initialize (buffer,
                            size,
+                           COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK,
                            COGL_BUFFER_USAGE_HINT_TEXTURE,
                            COGL_BUFFER_UPDATE_HINT_STATIC);
 
@@ -181,16 +182,16 @@ _cogl_pixel_array_map (CoglBuffer       *buffer,
                        CoglBufferMapHint hints)
 {
   CoglPixelArray *pixel_array = COGL_PIXEL_ARRAY (buffer);
-  GLenum gl_target;
   guint8 *data;
+  CoglBufferBindTarget target;
+  GLenum gl_target;
 
   _COGL_GET_CONTEXT (ctx, NULL);
 
-  /* we determine the target lazily, on the first map */
+  target = _cogl_buffer_get_last_bind_target (buffer);
-  gl_target = GL_PIXEL_UNPACK_BUFFER;
+  _cogl_buffer_bind (buffer, target);
-  pixel_array->gl_target = gl_target;
 
-  _cogl_buffer_bind (buffer, gl_target);
+  gl_target = _cogl_buffer_get_last_gl_target (buffer);
 
   /* create an empty store if we don't have one yet. creating the store
    * lazily allows the user of the CoglBuffer to set a hint before the
@@ -211,7 +212,7 @@ _cogl_pixel_array_map (CoglBuffer       *buffer,
   if (data)
     COGL_BUFFER_SET_FLAG (buffer, MAPPED);
 
-  _cogl_buffer_bind (NULL, gl_target);
+  _cogl_buffer_unbind (buffer);
 
   return data;
 }
@@ -219,16 +220,17 @@ _cogl_pixel_array_map (CoglBuffer       *buffer,
 static void
 _cogl_pixel_array_unmap (CoglBuffer *buffer)
 {
-  CoglPixelArray *pixel_array = COGL_PIXEL_ARRAY (buffer);
+  CoglBufferBindTarget target;
 
   _COGL_GET_CONTEXT (ctx, NO_RETVAL);
 
-  _cogl_buffer_bind (buffer, pixel_array->gl_target);
+  target = _cogl_buffer_get_last_bind_target (buffer);
+  _cogl_buffer_bind (buffer, target);
 
-  GE( glUnmapBuffer (pixel_array->gl_target) );
+  GE( glUnmapBuffer (_cogl_buffer_get_last_gl_target (buffer)) );
   COGL_BUFFER_CLEAR_FLAG (buffer, MAPPED);
 
-  _cogl_buffer_bind (NULL, pixel_array->gl_target);
+  _cogl_buffer_unbind (buffer);
 }
 
 static gboolean
@@ -238,19 +240,22 @@ _cogl_pixel_array_set_data (CoglBuffer   *buffer,
                             unsigned int  size)
 {
   CoglPixelArray *pixel_array = COGL_PIXEL_ARRAY (buffer);
+  CoglBufferBindTarget target;
+  GLenum gl_target;
 
   _COGL_GET_CONTEXT (ctx, FALSE);
 
-  pixel_array->gl_target = GL_PIXEL_UNPACK_BUFFER;
+  target = _cogl_buffer_get_last_bind_target (buffer);
+  _cogl_buffer_bind (buffer, target);
 
-  _cogl_buffer_bind (buffer, pixel_array->gl_target);
+  gl_target = _cogl_buffer_get_last_gl_target (buffer);
 
   /* create an empty store if we don't have one yet. creating the store
    * lazily allows the user of the CoglBuffer to set a hint before the
    * store is created. */
   if (!COGL_PIXEL_ARRAY_FLAG_IS_SET (pixel_array, STORE_CREATED))
     {
-      GE( glBufferData (pixel_array->gl_target,
+      GE( glBufferData (gl_target,
                         buffer->size,
                         NULL,
                         _cogl_buffer_hints_to_gl_enum (buffer->usage_hint,
@@ -258,9 +263,9 @@ _cogl_pixel_array_set_data (CoglBuffer   *buffer,
       COGL_PIXEL_ARRAY_SET_FLAG (pixel_array, STORE_CREATED);
     }
 
-  GE( glBufferSubData (pixel_array->gl_target, offset, size, data) );
+  GE( glBufferSubData (gl_target, offset, size, data) );
 
-  _cogl_buffer_bind (NULL, pixel_array->gl_target);
+  _cogl_buffer_unbind (buffer);
 
   return TRUE;
 }

cogl/cogl-texture.c

@@ -581,9 +581,10 @@ cogl_texture_new_from_buffer_EXP (CoglHandle          buffer,
       bitmap.format = format;
       bitmap.rowstride = rowstride;
 
-      _cogl_buffer_bind (cogl_buffer, GL_PIXEL_UNPACK_BUFFER);
+      _cogl_buffer_bind (cogl_buffer,
+                         COGL_BUFFER_BIND_TARGET_PIXEL_UNPACK);
       texture =  cogl_texture_new_from_bitmap (&bitmap, flags, internal_format);
-      _cogl_buffer_bind (NULL, GL_PIXEL_UNPACK_BUFFER);
+      _cogl_buffer_unbind (cogl_buffer);
     }
   else
 #endif