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.jus...@intel.com> > Cc: Kenneth Graunke <kenn...@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 <kenn...@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, >
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev