From 429d7cf696a19181cfd7408344803b757b331b69 Mon Sep 17 00:00:00 2001 From: Robert Bragg Date: Mon, 5 Jul 2010 23:24:34 +0100 Subject: [PATCH] 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. --- clutter/cogl/cogl/cogl-buffer-private.h | 23 ++++- clutter/cogl/cogl/cogl-buffer.c | 90 ++++++++++++++++---- clutter/cogl/cogl/cogl-context.c | 6 +- clutter/cogl/cogl/cogl-context.h | 4 +- clutter/cogl/cogl/cogl-pixel-array-private.h | 1 - clutter/cogl/cogl/cogl-pixel-array.c | 35 ++++---- clutter/cogl/cogl/cogl-texture.c | 5 +- 7 files changed, 123 insertions(+), 41 deletions(-) diff --git a/clutter/cogl/cogl/cogl-buffer-private.h b/clutter/cogl/cogl/cogl-buffer-private.h index fb56631b7..acf2c63d4 100644 --- a/clutter/cogl/cogl/cogl-buffer-private.h +++ b/clutter/cogl/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); diff --git a/clutter/cogl/cogl/cogl-buffer.c b/clutter/cogl/cogl/cogl-buffer.c index 600eefb8c..1000d74dc 100644 --- a/clutter/cogl/cogl/cogl-buffer.c +++ b/clutter/cogl/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, - GLenum target) +_cogl_buffer_bind (CoglBuffer *buffer, CoglBufferBindTarget target) { _COGL_GET_CONTEXT (ctx, NO_RETVAL); - /* Don't bind again an already bound pbo */ - if (ctx->current_pbo == buffer) - return; + g_return_if_fail (buffer != NULL); - 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) ); - } - else if (buffer == NULL && - ctx->current_pbo && - COGL_BUFFER_FLAG_IS_SET (ctx->current_pbo, BUFFER_OBJECT)) - { - GE( glBindBuffer (target, 0) ); + GLenum gl_target = _cogl_buffer_get_last_gl_target (buffer); + GE( glBindBuffer (gl_target, buffer->gl_handle) ); } - 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 diff --git a/clutter/cogl/cogl/cogl-context.c b/clutter/cogl/cogl/cogl-context.c index 6a0af95c4..b9436f703 100644 --- a/clutter/cogl/cogl/cogl-context.c +++ b/clutter/cogl/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; } diff --git a/clutter/cogl/cogl/cogl-context.h b/clutter/cogl/cogl/cogl-context.h index e0ad904f5..645bb4151 100644 --- a/clutter/cogl/cogl/cogl-context.h +++ b/clutter/cogl/cogl/cogl-context.h @@ -118,9 +118,7 @@ typedef struct gboolean legacy_depth_test_enabled; - /* PBOs */ - /* This can be used to check if a pbo is bound */ - CoglBuffer *current_pbo; + CoglBuffer *current_buffer[COGL_BUFFER_BIND_TARGET_COUNT]; /* Framebuffers */ GSList *framebuffer_stack; diff --git a/clutter/cogl/cogl/cogl-pixel-array-private.h b/clutter/cogl/cogl/cogl-pixel-array-private.h index 7293d2e7c..9b362927e 100644 --- a/clutter/cogl/cogl/cogl-pixel-array-private.h +++ b/clutter/cogl/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; diff --git a/clutter/cogl/cogl/cogl-pixel-array.c b/clutter/cogl/cogl/cogl-pixel-array.c index f158c727f..e98468110 100644 --- a/clutter/cogl/cogl/cogl-pixel-array.c +++ b/clutter/cogl/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 */ - gl_target = GL_PIXEL_UNPACK_BUFFER; - pixel_array->gl_target = gl_target; + target = _cogl_buffer_get_last_bind_target (buffer); + _cogl_buffer_bind (buffer, 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; } diff --git a/clutter/cogl/cogl/cogl-texture.c b/clutter/cogl/cogl/cogl-texture.c index d85ab906c..2a70ee5d5 100644 --- a/clutter/cogl/cogl/cogl-texture.c +++ b/clutter/cogl/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