On Thursday 19 November 2015, Tapani Pälli wrote:
> Patch adds additional mask for tracking which vertex buffer bindings
> are set. This array can be directly compared to which vertex arrays
> are enabled and should match when drawing.
>
> Fixes following CTS tests:
>
> ES31-CTS.draw_indirect.negative-noVBO-arrays
> ES31-CTS.draw_indirect.negative-noVBO-elements
>
> v2: update mask in vertex_array_attrib_binding
>
> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
> ---
> src/mesa/main/api_validate.c | 13 +++++++++++++
> src/mesa/main/mtypes.h | 3 +++
> src/mesa/main/varray.c | 7 +++++++
> 3 files changed, 23 insertions(+)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index a490189..e82e89a 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -710,6 +710,19 @@ valid_draw_indirect(struct gl_context *ctx,
> return GL_FALSE;
> }
>
> + /* From OpenGL ES 3.1 spec. section 10.5:
> + * "An INVALID_OPERATION error is generated if zero is bound to
> + * VERTEX_ARRAY_BINDING, DRAW_INDIRECT_BUFFER or to any enabled
> + * vertex array."
> + *
> + * Here we check that vertex buffer bindings match with enabled
> + * vertex arrays.
> + */
> + if (ctx->Array.VAO->_Enabled != ctx->Array.VAO->VertexBindingMask) {
> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(No VBO bound)", name);
> + return GL_FALSE;
> + }
> +
> if (!_mesa_valid_prim_mode(ctx, mode, name))
> return GL_FALSE;
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 4efdf1e..6c6187f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1419,6 +1419,9 @@ struct gl_vertex_array_object
> /** Vertex buffer bindings */
> struct gl_vertex_buffer_binding VertexBinding[VERT_ATTRIB_MAX];
>
> + /** Mask indicating which binding points are set. */
> + GLbitfield64 VertexBindingMask;
> +
> /** Mask of VERT_BIT_* values indicating which arrays are enabled */
> GLbitfield64 _Enabled;
>
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index 887d0c0..e24342a 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -174,6 +174,11 @@ bind_vertex_buffer(struct gl_context *ctx,
> binding->Offset = offset;
> binding->Stride = stride;
>
> + if (vbo == ctx->Shared->NullBufferObj)
> + vao->VertexBindingMask &= ~VERT_BIT(index);
> + else
> + vao->VertexBindingMask |= VERT_BIT(index);
> +
> vao->NewArrays |= binding->_BoundArrays;
> }
> }
> @@ -2126,6 +2131,8 @@ vertex_array_attrib_binding(struct gl_context *ctx,
>
> assert(VERT_ATTRIB_GENERIC(attribIndex) < ARRAY_SIZE(vao->VertexAttrib));
>
> + vao->VertexBindingMask |= VERT_BIT_GENERIC(attribIndex);
So there are still a few problems here. The _Enabled bitfield is a mask
of vertex arrays, while VertexBindingMask is a mask of vertex buffer
binding points. There is no fixed coupling between these states, so the
two bitfields can't be compared. Yet valid_draw_indirect() compares them.
bind_vertex_buffer() treats VertexBindingMask as a mask of vertex
buffer bindings, while vertex_array_attrib_binding() treats it as a mask
of vertex arrays. The latter function also incorrectly assumes that if
the association between an vertex attribute and a buffer binding point is
changed, then the new binding point has a vertex buffer set.
I suggest making VertexBindingMask a mask of vertex arrays instead,
so it can be compared with _Enabled, and renaming it to
VertexAttribBufferMask or something similar. bind_vertex_buffer()
then needs to be changed to set or clear the _BoundArrays bits in the
mask instead of the index bit. vertex_array_attrib_binding() also
needs to check if the new binding has a buffer set, and update the
attribIndex bit in the mask accordingly.
> vertex_attrib_binding(ctx, vao,
> VERT_ATTRIB_GENERIC(attribIndex),
> VERT_ATTRIB_GENERIC(bindingIndex));
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
