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
This commit is contained in:
Neil Roberts 2010-05-20 00:16:49 +01:00
parent 99c7e51145
commit 992d5f7fb6
8 changed files with 49 additions and 49 deletions

View File

@ -97,7 +97,7 @@ cogl_create_context (void)
0, sizeof (CoglMaterialFlushOptions)); 0, sizeof (CoglMaterialFlushOptions));
_context->current_layers = g_array_new (FALSE, FALSE, _context->current_layers = g_array_new (FALSE, FALSE,
sizeof (CoglLayerInfo)); sizeof (CoglLayerInfo));
_context->n_texcoord_arrays_enabled = 0; _context->texcoord_arrays_enabled = 0;
_context->framebuffer_stack = _cogl_create_framebuffer_stack (); _context->framebuffer_stack = _cogl_create_framebuffer_stack ();

View File

@ -86,7 +86,8 @@ typedef struct
unsigned long current_material_flags; unsigned long current_material_flags;
CoglMaterialFlushOptions current_material_flush_options; CoglMaterialFlushOptions current_material_flush_options;
GArray *current_layers; GArray *current_layers;
unsigned int n_texcoord_arrays_enabled; /* Bitmask of texture coordinates arrays that are enabled */
unsigned int texcoord_arrays_enabled;
/* PBOs */ /* PBOs */
/* This can be used to check if a pbo is bound */ /* This can be used to check if a pbo is bound */

View File

@ -132,6 +132,9 @@ _cogl_get_max_texture_image_units (void);
void void
_cogl_flush_face_winding (void); _cogl_flush_face_winding (void);
void
_cogl_disable_texcoord_arrays (unsigned int mask);
#ifdef COGL_HAS_XLIB_SUPPORT #ifdef COGL_HAS_XLIB_SUPPORT
/* /*

View File

@ -372,8 +372,8 @@ _cogl_journal_flush_texcoord_vbo_offsets_and_entries (
void *data) void *data)
{ {
CoglJournalFlushState *state = data; CoglJournalFlushState *state = data;
int prev_n_texcoord_arrays_enabled;
int i; int i;
unsigned int layers_mask;
_COGL_GET_CONTEXT (ctx, NO_RETVAL); _COGL_GET_CONTEXT (ctx, NO_RETVAL);
@ -395,14 +395,10 @@ _cogl_journal_flush_texcoord_vbo_offsets_and_entries (
(POS_STRIDE + COLOR_STRIDE) * 4 + (POS_STRIDE + COLOR_STRIDE) * 4 +
TEX_STRIDE * 4 * i))); TEX_STRIDE * 4 * i)));
} }
prev_n_texcoord_arrays_enabled =
ctx->n_texcoord_arrays_enabled; layers_mask = ~((~(unsigned int) 0) << batch_start->n_layers);
ctx->n_texcoord_arrays_enabled = batch_start->n_layers; ctx->texcoord_arrays_enabled |= layers_mask;
for (; i < prev_n_texcoord_arrays_enabled; i++) _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled & ~layers_mask);
{
GE (glClientActiveTexture (GL_TEXTURE0 + i));
GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY));
}
batch_and_call (batch_start, batch_and_call (batch_start,
batch_len, batch_len,

View File

@ -190,7 +190,6 @@ _cogl_add_path_to_stencil_buffer (CoglHandle path_handle,
float bounds_h; float bounds_h;
unsigned long enable_flags = COGL_ENABLE_VERTEX_ARRAY; unsigned long enable_flags = COGL_ENABLE_VERTEX_ARRAY;
CoglHandle prev_source; CoglHandle prev_source;
int i;
CoglHandle framebuffer = _cogl_get_framebuffer (); CoglHandle framebuffer = _cogl_get_framebuffer ();
CoglMatrixStack *modelview_stack = CoglMatrixStack *modelview_stack =
_cogl_framebuffer_get_modelview_stack (framebuffer); _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)); GE (glStencilOp (GL_INVERT, GL_INVERT, GL_INVERT));
for (i = 0; i < ctx->n_texcoord_arrays_enabled; i++) /* Disable all client texture coordinate arrays */
{ _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled);
GE (glClientActiveTexture (GL_TEXTURE0 + i));
GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY));
}
ctx->n_texcoord_arrays_enabled = 0;
while (path_start < path->data->path_nodes->len) while (path_start < path->data->path_nodes->len)
{ {

View File

@ -1012,7 +1012,7 @@ cogl_polygon (const CoglTextureVertex *vertices,
unsigned int stride; unsigned int stride;
gsize stride_bytes; gsize stride_bytes;
GLfloat *v; GLfloat *v;
int prev_n_texcoord_arrays_enabled; unsigned int layers_mask;
CoglMaterialWrapModeOverrides wrap_mode_overrides; CoglMaterialWrapModeOverrides wrap_mode_overrides;
CoglMaterialWrapModeOverrides *wrap_mode_overrides_p = NULL; CoglMaterialWrapModeOverrides *wrap_mode_overrides_p = NULL;
@ -1165,14 +1165,9 @@ cogl_polygon (const CoglTextureVertex *vertices,
v + 3 + 2 * i)); v + 3 + 2 * i));
} }
prev_n_texcoord_arrays_enabled = ctx->n_texcoord_arrays_enabled; layers_mask = ~((~(unsigned int) 0) << n_layers);
ctx->n_texcoord_arrays_enabled = n_layers; ctx->texcoord_arrays_enabled |= layers_mask;
_cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled & ~layers_mask);
for (; i < prev_n_texcoord_arrays_enabled; i++)
{
GE (glClientActiveTexture (GL_TEXTURE0 + i));
GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY));
}
if (use_sliced_polygon_fallback) if (use_sliced_polygon_fallback)
_cogl_texture_polygon_multiple_primitives (vertices, _cogl_texture_polygon_multiple_primitives (vertices,

View File

@ -1518,10 +1518,9 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer)
GLuint generic_index = 0; GLuint generic_index = 0;
#endif #endif
unsigned long enable_flags = 0; unsigned long enable_flags = 0;
int max_texcoord_attrib_unit = -1; unsigned int texcoord_arrays_used = 0;
const GList *layers; const GList *layers;
guint32 fallback_layers = 0; guint32 fallback_layers = 0;
guint32 disable_layers = ~0;
int i; int i;
CoglMaterialFlushOptions options; CoglMaterialFlushOptions options;
@ -1531,8 +1530,7 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer)
cogl_vertex_buffer_submit_real (buffer); cogl_vertex_buffer_submit_real (buffer);
options.flags = options.flags =
COGL_MATERIAL_FLUSH_FALLBACK_MASK | COGL_MATERIAL_FLUSH_FALLBACK_MASK;
COGL_MATERIAL_FLUSH_DISABLE_MASK;
memset (&options.wrap_mode_overrides, 0, memset (&options.wrap_mode_overrides, 0,
sizeof (options.wrap_mode_overrides)); sizeof (options.wrap_mode_overrides));
@ -1598,9 +1596,7 @@ enable_state_for_drawing_buffer (CoglVertexBuffer *buffer)
gl_type, gl_type,
attribute->stride, attribute->stride,
pointer)); pointer));
if (attribute->texture_unit > max_texcoord_attrib_unit) texcoord_arrays_used |= (1 << attribute->texture_unit);
max_texcoord_attrib_unit = attribute->texture_unit;
disable_layers &= ~(1 << attribute->texture_unit);
break; break;
case COGL_VERTEX_BUFFER_ATTRIB_FLAG_VERTEX_ARRAY: case COGL_VERTEX_BUFFER_ATTRIB_FLAG_VERTEX_ARRAY:
enable_flags |= COGL_ENABLE_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); layers = cogl_material_get_layers (ctx->source_material);
for (tmp = (GList *)layers, i = 0; for (tmp = (GList *)layers, i = 0;
tmp != NULL && i <= max_texcoord_attrib_unit; tmp != NULL;
tmp = tmp->next, i++) tmp = tmp->next, i++)
{ {
CoglHandle layer = (CoglHandle)tmp->data; 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++) _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled &
{ ~texcoord_arrays_used);
GE (glClientActiveTexture (GL_TEXTURE0 + i));
GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY));
}
ctx->n_texcoord_arrays_enabled = max_texcoord_attrib_unit + 1;
/* NB: _cogl_framebuffer_flush_state may disrupt various state (such /* NB: _cogl_framebuffer_flush_state may disrupt various state (such
* as the material state) when flushing the clip stack, so should * 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); _cogl_framebuffer_flush_state (_cogl_get_framebuffer (), 0);
options.fallback_layers = fallback_layers; options.fallback_layers = fallback_layers;
options.disable_layers = disable_layers;
_cogl_material_flush_gl_state (ctx->source_material, &options); _cogl_material_flush_gl_state (ctx->source_material, &options);
enable_flags |= _cogl_material_get_cogl_enable_flags (ctx->source_material); enable_flags |= _cogl_material_get_cogl_enable_flags (ctx->source_material);

View File

@ -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 void
cogl_begin_gl (void) cogl_begin_gl (void)
{ {
CoglMaterialFlushOptions options; CoglMaterialFlushOptions options;
unsigned long enable_flags = 0; unsigned long enable_flags = 0;
int i;
_COGL_GET_CONTEXT (ctx, NO_RETVAL); _COGL_GET_CONTEXT (ctx, NO_RETVAL);
@ -816,12 +840,7 @@ cogl_begin_gl (void)
_cogl_flush_face_winding (); _cogl_flush_face_winding ();
/* Disable all client texture coordinate arrays */ /* Disable all client texture coordinate arrays */
for (i = 0; i < ctx->n_texcoord_arrays_enabled; i++) _cogl_disable_texcoord_arrays (ctx->texcoord_arrays_enabled);
{
GE (glClientActiveTexture (GL_TEXTURE0 + i));
GE (glDisableClientState (GL_TEXTURE_COORD_ARRAY));
}
ctx->n_texcoord_arrays_enabled = 0;
} }
void void