[Mesa-dev] [PATCH] vbo: Return INVALID_OPERATION during draw with a mapped buffer
Kenneth Graunke
kenneth at whitecape.org
Wed Apr 27 03:05:38 UTC 2016
On Monday, April 25, 2016 5:54:28 PM PDT Jordan Justen wrote:
> Fixes the OpenGLES 3.1 CTS:
> * ESEXT-CTS.draw_elements_base_vertex_tests.invalid_mapped_bos
>
> Because this is triggering the error message after the normal API
> validation phase, we don't have the API function name available, and
> therefore we generate an error message that is a little confusing:
>
> Mesa: User error: GL_INVALID_OPERATION in Vertex buffers are mapped during
draw call!
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=95142
> Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>
> Perhaps it would be better to push the vbo_bind_arrays before API
> validation, and then make this check be part of API validation...
This looks fine to me, though it'd be nice to get a review from someone
else who's familiar with this code. Cc'ing Marek and Fredrik in case
they have an opinion.
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>
> src/mesa/vbo/vbo.h | 4 +--
> src/mesa/vbo/vbo_exec_array.c | 76 ++++++++++++++++++++++
+--------------------
> 2 files changed, 42 insertions(+), 38 deletions(-)
>
> diff --git a/src/mesa/vbo/vbo.h b/src/mesa/vbo/vbo.h
> index 6494aa5..939a3a6 100644
> --- a/src/mesa/vbo/vbo.h
> +++ b/src/mesa/vbo/vbo.h
> @@ -197,9 +197,7 @@ void vbo_set_draw_func(struct gl_context *ctx,
vbo_draw_func func);
> void vbo_set_indirect_draw_func(struct gl_context *ctx,
> vbo_indirect_draw_func func);
>
> -void vbo_check_buffers_are_unmapped(struct gl_context *ctx);
> -
> -void vbo_bind_arrays(struct gl_context *ctx);
> +bool vbo_bind_arrays(struct gl_context *ctx);
>
> size_t
> vbo_count_tessellated_primitives(GLenum mode, GLuint count,
> diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c
> index 40cf3ff..79dee61 100644
> --- a/src/mesa/vbo/vbo_exec_array.c
> +++ b/src/mesa/vbo/vbo_exec_array.c
> @@ -43,22 +43,22 @@
>
> /**
> * All vertex buffers should be in an unmapped state when we're about
> - * to draw. This debug function checks that.
> + * to draw.
> */
> -static void
> -check_buffers_are_unmapped(const struct gl_client_array **inputs)
> +static bool
> +check_input_buffers_are_unmapped(const struct gl_client_array **inputs)
> {
> -#ifdef DEBUG
> GLuint i;
>
> for (i = 0; i < VERT_ATTRIB_MAX; i++) {
> if (inputs[i]) {
> struct gl_buffer_object *obj = inputs[i]->BufferObj;
> - assert(!_mesa_check_disallowed_mapping(obj));
> - (void) obj;
> + if (_mesa_check_disallowed_mapping(obj))
> + return false;
> }
> }
> -#endif
> +
> + return true;
> }
>
>
> @@ -66,15 +66,15 @@ check_buffers_are_unmapped(const struct gl_client_array
**inputs)
> * A debug function that may be called from other parts of Mesa as
> * needed during debugging.
> */
> -void
> -vbo_check_buffers_are_unmapped(struct gl_context *ctx)
> +static bool
> +check_buffers_are_unmapped(struct gl_context *ctx)
> {
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_exec_context *exec = &vbo->exec;
> +
> /* check the current vertex arrays */
> - check_buffers_are_unmapped(exec->array.inputs);
> - /* check the current glBegin/glVertex/glEnd-style VBO */
> - assert(!_mesa_check_disallowed_mapping(exec->vtx.bufferobj));
> + return !_mesa_check_disallowed_mapping(exec->vtx.bufferobj) &&
> + check_input_buffers_are_unmapped(exec->array.inputs);
> }
>
>
> @@ -395,7 +395,7 @@ recalculate_input_bindings(struct gl_context *ctx)
> * Note that this might set the _NEW_VARYING_VP_INPUTS dirty flag so state
> * validation must be done after this call.
> */
> -void
> +bool
> vbo_bind_arrays(struct gl_context *ctx)
> {
> struct vbo_context *vbo = vbo_context(ctx);
> @@ -421,6 +421,14 @@ vbo_bind_arrays(struct gl_context *ctx)
> exec->validating = GL_FALSE;
> }
> }
> +
> + if (!check_buffers_are_unmapped(ctx)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "Vertex buffers are mapped during draw call!");
> + return false;
> + } else {
> + return true;
> + }
> }
>
> /**
> @@ -437,7 +445,8 @@ vbo_draw_arrays(struct gl_context *ctx, GLenum mode,
GLint start,
> struct vbo_exec_context *exec = &vbo->exec;
> struct _mesa_prim prim[2];
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> /* init most fields to zero */
> memset(prim, 0, sizeof(prim));
> @@ -483,7 +492,6 @@ vbo_draw_arrays(struct gl_context *ctx, GLenum mode,
GLint start,
>
> if (primCount > 0) {
> /* draw one or two prims */
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, primCount, NULL,
> GL_TRUE, start, start + count - 1, NULL, 0, NULL);
> }
> @@ -493,7 +501,6 @@ vbo_draw_arrays(struct gl_context *ctx, GLenum mode,
GLint start,
> prim[0].start = start;
> prim[0].count = count;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, 1, NULL,
> GL_TRUE, start, start + count - 1,
> NULL, 0, NULL);
> @@ -789,7 +796,8 @@ vbo_validated_drawrangeelements(struct gl_context *ctx,
GLenum mode,
> struct _mesa_index_buffer ib;
> struct _mesa_prim prim[1];
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> ib.count = count;
> ib.type = type;
> @@ -840,7 +848,6 @@ vbo_validated_drawrangeelements(struct gl_context *ctx,
GLenum mode,
> * for the latter case elsewhere.
> */
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, 1, &ib,
> index_bounds_valid, start, end, NULL, 0, NULL);
>
> @@ -1134,7 +1141,8 @@ vbo_validated_multidrawelements(struct gl_context
*ctx, GLenum mode,
> return;
> }
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> min_index_ptr = (uintptr_t)indices[0];
> max_index_ptr = 0;
> @@ -1201,7 +1209,6 @@ vbo_validated_multidrawelements(struct gl_context
*ctx, GLenum mode,
> prim[i].basevertex = 0;
> }
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, primcount, &ib,
> false, ~0, ~0, NULL, 0, NULL);
> } else {
> @@ -1231,7 +1238,6 @@ vbo_validated_multidrawelements(struct gl_context
*ctx, GLenum mode,
> else
> prim[0].basevertex = 0;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, 1, &ib,
> false, ~0, ~0, NULL, 0, NULL);
> }
> @@ -1301,7 +1307,8 @@ vbo_draw_transform_feedback(struct gl_context *ctx,
GLenum mode,
> return;
> }
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> /* init most fields to zero */
> memset(prim, 0, sizeof(prim));
> @@ -1316,7 +1323,6 @@ vbo_draw_transform_feedback(struct gl_context *ctx,
GLenum mode,
> * (like in DrawArrays), but we have no way to know how many vertices
> * will be rendered. */
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_prims(ctx, prim, 1, NULL,
> GL_TRUE, 0, 0, obj, stream, NULL);
>
> @@ -1399,9 +1405,9 @@ vbo_validated_drawarraysindirect(struct gl_context
*ctx,
> struct vbo_context *vbo = vbo_context(ctx);
> struct vbo_exec_context *exec = &vbo->exec;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, (GLsizeiptr)indirect,
> 1 /* draw_count */, 16 /* stride */,
> @@ -1424,9 +1430,9 @@ vbo_validated_multidrawarraysindirect(struct
gl_context *ctx,
> if (primcount == 0)
> return;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, offset,
> primcount, stride,
> @@ -1445,14 +1451,14 @@ vbo_validated_drawelementsindirect(struct gl_context
*ctx,
> struct vbo_exec_context *exec = &vbo->exec;
> struct _mesa_index_buffer ib;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> ib.count = 0; /* unknown */
> ib.type = type;
> ib.obj = ctx->Array.VAO->IndexBufferObj;
> ib.ptr = NULL;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, (GLsizeiptr)indirect,
> 1 /* draw_count */, 20 /* stride */,
> @@ -1477,7 +1483,8 @@ vbo_validated_multidrawelementsindirect(struct
gl_context *ctx,
> if (primcount == 0)
> return;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> /* NOTE: IndexBufferObj is guaranteed to be a VBO. */
>
> @@ -1486,7 +1493,6 @@ vbo_validated_multidrawelementsindirect(struct
gl_context *ctx,
> ib.obj = ctx->Array.VAO->IndexBufferObj;
> ib.ptr = NULL;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, offset,
> primcount, stride,
> @@ -1599,9 +1605,9 @@ vbo_validated_multidrawarraysindirectcount(struct
gl_context *ctx,
> if (maxdrawcount == 0)
> return;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, offset,
> maxdrawcount, stride,
> @@ -1628,7 +1634,8 @@ vbo_validated_multidrawelementsindirectcount(struct
gl_context *ctx,
> if (maxdrawcount == 0)
> return;
>
> - vbo_bind_arrays(ctx);
> + if (!vbo_bind_arrays(ctx))
> + return;
>
> /* NOTE: IndexBufferObj is guaranteed to be a VBO. */
>
> @@ -1637,7 +1644,6 @@ vbo_validated_multidrawelementsindirectcount(struct
gl_context *ctx,
> ib.obj = ctx->Array.VAO->IndexBufferObj;
> ib.ptr = NULL;
>
> - check_buffers_are_unmapped(exec->array.inputs);
> vbo->draw_indirect_prims(ctx, mode,
> ctx->DrawIndirectBuffer, offset,
> maxdrawcount, stride,
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160426/2c6eba92/attachment.sig>
More information about the mesa-dev
mailing list