From ae0354e9ba6e59a681e27e46c5a2cf98d33687f0 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 20 May 2010 00:16:49 +0100 Subject: [PATCH] cogl-vertex-buffer: Don't disable layers with no texture coords It should be quite acceptable to use a texture without defining any texture coords. For example a shader may be in use that is doing texture lookups without referencing the texture coordinates. Also it should be possible to replace the vertex colors using a texture layer without a texture but with a constant layer color. enable_state_for_drawing_buffer no longer sets any disabled layers in the overrides. Instead of counting the number of units with texture coordinates it now keeps them in a mask. This means there can now be gaps in the list of enabled texture coordinate arrays. To cope with this, the Cogl context now also stores a mask to track the enabled arrays. Instead of code manually iterating each enabled array to disable them, there is now an internal function called _cogl_disable_texcoord_arrays which disables a given mask. I think this could also fix potential bugs when a vertex buffer has gaps in the texture coordinate attributes that it provides. For example if the vertex buffer only had texture coordinates for layer 2 then the disabling code would not disable the coordinates for layers 0 and 1 even though they are not used. This could cause a crash if the previous data for those arrays is no longer valid. http://bugzilla.openedhand.com/show_bug.cgi?id=2132 --- clutter/cogl/cogl/cogl-context.c | 2 +- clutter/cogl/cogl/cogl-context.h | 3 ++- clutter/cogl/cogl/cogl-internal.h | 3 +++ clutter/cogl/cogl/cogl-journal.c | 14 ++++------- clutter/cogl/cogl/cogl-path.c | 9 ++----- clutter/cogl/cogl/cogl-primitives.c | 13 ++++------ clutter/cogl/cogl/cogl-vertex-buffer.c | 21 +++++----------- clutter/cogl/cogl/cogl.c | 33 ++++++++++++++++++++------ 8 files changed, 49 insertions(+), 49 deletions(-) diff --git a/clutter/cogl/cogl/cogl-context.c b/clutter/cogl/cogl/cogl-context.c index 956ea0aa0..c8ff3dd51 100644 --- a/clutter/cogl/cogl/cogl-context.c +++ b/clutter/cogl/cogl/cogl-context.c @@ -97,7 +97,7 @@ cogl_create_context (void) 0, sizeof (CoglMaterialFlushOptions)); _context->current_layers = g_array_new (FALSE, FALSE, sizeof (CoglLayerInfo)); - _context->n_texcoord_arrays_enabled = 0; + _context->texcoord_arrays_enabled = 0; _context->framebuffer_stack = _cogl_create_framebuffer_stack (); diff --git a/clutter/cogl/cogl/cogl-context.h b/clutter/cogl/cogl/cogl-context.h index a8b3e5ea9..b406446a8 100644 --- a/clutter/cogl/cogl/cogl-context.h +++ b/clutter/cogl/cogl/cogl-context.h @@ -86,7 +86,8 @@ typedef struct unsigned long current_material_flags; CoglMaterialFlushOptions current_material_flush_options; GArray *current_layers; - unsigned int n_texcoord_arrays_enabled; + /* Bitmask of texture coordinates arrays that are enabled */ + unsigned int texcoord_arrays_enabled; /* PBOs */ /* This can be used to check if a pbo is bound */ diff --git a/clutter/cogl/cogl/cogl-internal.h b/clutter/cogl/cogl/cogl-internal.h index 8987401ef..50036ab64 100644 --- a/clutter/cogl/cogl/cogl-internal.h +++ b/clutter/cogl/cogl/cogl-internal.h @@ -132,6 +132,9 @@ _cogl_get_max_texture_image_units (void); void _cogl_flush_face_winding (void); +void +_cogl_disable_texcoord_arrays (unsigned int mask); + #ifdef COGL_HAS_XLIB_SUPPORT /* diff --git a/clutter/cogl/cogl/cogl-journal.c b/clutter/cogl/cogl/cogl-journal.c index 3b3a1cce8..ae412b034 100644 --- a/clutter/cogl/cogl/cogl-journal.c +++ b/clutter/cogl/cogl/cogl-journal.c @@ -372,8 +372,8 @@ _cogl_journal_flush_texcoord_vbo_offsets_and_entries ( void *data) { CoglJournalFlushState *state = data; - int prev_n_texcoord_arrays_enabled; int i; + unsigned int layers_mask; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -395,14 +395,10 @@ _cogl_journal_flush_texcoord_vbo_offsets_and_entries ( (POS_STRIDE + COLOR_STRIDE) * 4 + TEX_STRIDE * 4 * i))); } - prev_n_texcoord_arrays_enabled = - ctx->n_texcoord_arrays_enabled; - ctx->n_texcoord_arrays_enabled = batch_start->n_layers; - for (; i < prev_n_texcoord_arrays_enabled; i++) - { - GE (glClientActiveTexture (GL_TEXTURE0 + i)); - GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); - } + + layers_mask = ~((~(unsigned int) 0) << batch_start->n_layers); + ctx->texcoord_arrays_enabled |= layers_mask; + _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled & ~layers_mask); batch_and_call (batch_start, batch_len, diff --git a/clutter/cogl/cogl/cogl-path.c b/clutter/cogl/cogl/cogl-path.c index c24eda088..553189b1f 100644 --- a/clutter/cogl/cogl/cogl-path.c +++ b/clutter/cogl/cogl/cogl-path.c @@ -190,7 +190,6 @@ _cogl_add_path_to_stencil_buffer (CoglHandle path_handle, float bounds_h; unsigned long enable_flags = COGL_ENABLE_VERTEX_ARRAY; CoglHandle prev_source; - int i; CoglHandle framebuffer = _cogl_get_framebuffer (); CoglMatrixStack *modelview_stack = _cogl_framebuffer_get_modelview_stack (framebuffer); @@ -263,12 +262,8 @@ _cogl_add_path_to_stencil_buffer (CoglHandle path_handle, GE (glStencilOp (GL_INVERT, GL_INVERT, GL_INVERT)); - for (i = 0; i < ctx->n_texcoord_arrays_enabled; i++) - { - GE (glClientActiveTexture (GL_TEXTURE0 + i)); - GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); - } - ctx->n_texcoord_arrays_enabled = 0; + /* Disable all client texture coordinate arrays */ + _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled); while (path_start < path->data->path_nodes->len) { diff --git a/clutter/cogl/cogl/cogl-primitives.c b/clutter/cogl/cogl/cogl-primitives.c index 123db1692..be7bcb4f8 100644 --- a/clutter/cogl/cogl/cogl-primitives.c +++ b/clutter/cogl/cogl/cogl-primitives.c @@ -1012,7 +1012,7 @@ cogl_polygon (const CoglTextureVertex *vertices, unsigned int stride; gsize stride_bytes; GLfloat *v; - int prev_n_texcoord_arrays_enabled; + unsigned int layers_mask; CoglMaterialWrapModeOverrides wrap_mode_overrides; CoglMaterialWrapModeOverrides *wrap_mode_overrides_p = NULL; @@ -1165,14 +1165,9 @@ cogl_polygon (const CoglTextureVertex *vertices, v + 3 + 2 * i)); } - prev_n_texcoord_arrays_enabled = ctx->n_texcoord_arrays_enabled; - ctx->n_texcoord_arrays_enabled = n_layers; - - for (; i < prev_n_texcoord_arrays_enabled; i++) - { - GE (glClientActiveTexture (GL_TEXTURE0 + i)); - GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); - } + layers_mask = ~((~(unsigned int) 0) << n_layers); + ctx->texcoord_arrays_enabled |= layers_mask; + _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled & ~layers_mask); if (use_sliced_polygon_fallback) _cogl_texture_polygon_multiple_primitives (vertices, diff --git a/clutter/cogl/cogl/cogl-vertex-buffer.c b/clutter/cogl/cogl/cogl-vertex-buffer.c index 34b289001..c71d39e80 100644 --- a/clutter/cogl/cogl/cogl-vertex-buffer.c +++ b/clutter/cogl/cogl/cogl-vertex-buffer.c @@ -1518,10 +1518,9 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) GLuint generic_index = 0; #endif unsigned long enable_flags = 0; - int max_texcoord_attrib_unit = -1; + unsigned int texcoord_arrays_used = 0; const GList *layers; guint32 fallback_layers = 0; - guint32 disable_layers = ~0; int i; CoglMaterialFlushOptions options; @@ -1531,8 +1530,7 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) cogl_vertex_buffer_submit_real (buffer); options.flags = - COGL_MATERIAL_FLUSH_FALLBACK_MASK | - COGL_MATERIAL_FLUSH_DISABLE_MASK; + COGL_MATERIAL_FLUSH_FALLBACK_MASK; memset (&options.wrap_mode_overrides, 0, sizeof (options.wrap_mode_overrides)); @@ -1598,9 +1596,7 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) gl_type, attribute->stride, pointer)); - if (attribute->texture_unit > max_texcoord_attrib_unit) - max_texcoord_attrib_unit = attribute->texture_unit; - disable_layers &= ~(1 << attribute->texture_unit); + texcoord_arrays_used |= (1 << attribute->texture_unit); break; case COGL_VERTEX_BUFFER_ATTRIB_FLAG_VERTEX_ARRAY: enable_flags |= COGL_ENABLE_VERTEX_ARRAY; @@ -1638,7 +1634,7 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) layers = cogl_material_get_layers (ctx->source_material); for (tmp = (GList *)layers, i = 0; - tmp != NULL && i <= max_texcoord_attrib_unit; + tmp != NULL; tmp = tmp->next, i++) { CoglHandle layer = (CoglHandle)tmp->data; @@ -1702,12 +1698,8 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) } } - for (i = max_texcoord_attrib_unit + 1; i < ctx->n_texcoord_arrays_enabled; i++) - { - GE (glClientActiveTexture (GL_TEXTURE0 + i)); - GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); - } - ctx->n_texcoord_arrays_enabled = max_texcoord_attrib_unit + 1; + _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled & + ~texcoord_arrays_used); /* NB: _cogl_framebuffer_flush_state may disrupt various state (such * as the material state) when flushing the clip stack, so should @@ -1715,7 +1707,6 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer) _cogl_framebuffer_flush_state (_cogl_get_framebuffer (), 0); options.fallback_layers = fallback_layers; - options.disable_layers = disable_layers; _cogl_material_flush_gl_state (ctx->source_material, &options); enable_flags |= _cogl_material_get_cogl_enable_flags (ctx->source_material); diff --git a/clutter/cogl/cogl/cogl.c b/clutter/cogl/cogl/cogl.c index 2386b3ea5..0d5f438de 100644 --- a/clutter/cogl/cogl/cogl.c +++ b/clutter/cogl/cogl/cogl.c @@ -763,12 +763,36 @@ cogl_read_pixels (int x, } } +void +_cogl_disable_texcoord_arrays (unsigned int mask) +{ + int i = 0; + + _COGL_GET_CONTEXT (ctx, NO_RETVAL); + + /* Update the mask of arrays that are enabled */ + ctx->texcoord_arrays_enabled &= ~mask; + + /* Disable all of the texture coord arrays that have a 1 in the bit + position in mask */ + while (mask) + { + if ((mask & 1)) + { + GE (glClientActiveTexture (GL_TEXTURE0 + i)); + GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); + } + + i++; + mask >>= 1; + } +} + void cogl_begin_gl (void) { CoglMaterialFlushOptions options; unsigned long enable_flags = 0; - int i; _COGL_GET_CONTEXT (ctx, NO_RETVAL); @@ -816,12 +840,7 @@ cogl_begin_gl (void) _cogl_flush_face_winding (); /* Disable all client texture coordinate arrays */ - for (i = 0; i < ctx->n_texcoord_arrays_enabled; i++) - { - GE (glClientActiveTexture (GL_TEXTURE0 + i)); - GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY)); - } - ctx->n_texcoord_arrays_enabled = 0; + _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled); } void