On 11/09/2013 01:02 AM, Chris Forbes wrote:
> Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.
>
> V3: - Disallow primcount==0 for DrawMulti*Indirect. The extension spec
> contradicts itself on this, but the GL4.3 spec disallows it.
>
> - Make it clear that the caller has dealt with stride==0
>
> Signed-off-by: Chris Forbes <chr...@ijw.co.nz>
> Reviewed-by: Paul Berry <stereotype...@gmail.com>
> ---
> src/mesa/main/api_validate.c | 167
> +++++++++++++++++++++++++++++++++++++++++++
> src/mesa/main/api_validate.h | 26 +++++++
> 2 files changed, 193 insertions(+)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index f285c97..f462b68 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -837,3 +837,170 @@ _mesa_validate_DrawTransformFeedback(struct gl_context
> *ctx,
>
> return GL_TRUE;
> }
> +
> +static GLboolean
> +valid_draw_indirect(struct gl_context *ctx,
> + GLenum mode, const GLvoid *indirect,
> + GLsizei size, const char *name)
> +{
> + const GLsizeiptr end = (GLsizeiptr)indirect + size;
> +
> + if (!_mesa_valid_prim_mode(ctx, mode, name))
> + return GL_FALSE;
/* From the ARB_draw_indirect specification:
* "An INVALID_OPERATION error is generated [...] if <indirect> is no
* word aligned."
*/
> + if ((GLsizeiptr)indirect & (sizeof(GLuint) - 1)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(indirect is not aligned)", name);
> + return GL_FALSE;
> + }
> +
> + if (_mesa_is_bufferobj(ctx->DrawIndirectBuffer)) {
I think there's some "most commands will detect attempts to read from a
mapped buffer object" text which can justify this. I'm fine with
leaving it as is.
> + if (_mesa_bufferobj_mapped(ctx->DrawIndirectBuffer)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(DRAW_INDIRECT_BUFFER is mapped)", name);
> + return GL_FALSE;
> + }
/* From the ARB_draw_indirect specification:
* "An INVALID_OPERATION error is generated if the commands source data
* beyond the end of the buffer object [...]"
*/
> + if (ctx->DrawIndirectBuffer->Size < end) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(DRAW_INDIRECT_BUFFER too small)", name);
> + return GL_FALSE;
> + }
> + } else {
> + if (ctx->API != API_OPENGL_COMPAT) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s: no buffer bound to DRAW_INDIRECT_BUFFER", name);
> + return GL_FALSE;
> + }
> + }
> +
> + if (!check_valid_to_render(ctx, name))
> + return GL_FALSE;
> +
> + return GL_TRUE;
> +}
> +
> +static inline GLboolean
> +valid_draw_indirect_elements(struct gl_context *ctx,
> + GLenum mode, GLenum type, const GLvoid
> *indirect,
> + GLsizeiptr size, const char *name)
> +{
> + if (!valid_elements_type(ctx, type, name))
> + return GL_FALSE;
> +
> + /*
> + * Unlike regular DrawElementsInstancedBaseVertex commands, the indices
> + * may not come from a client array and must come from an index buffer.
> + * If no element array buffer is bound, an INVALID_OPERATION error is
> + * generated.
> + */
> + if (!_mesa_is_bufferobj(ctx->Array.ArrayObj->ElementArrayBufferObj)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(no buffer bound to GL_ELEMENT_ARRAY_BUFFER)", name);
> + return GL_FALSE;
> + }
> +
> + return valid_draw_indirect(ctx, mode, indirect, size, name);
> +}
> +
> +static inline GLboolean
> +valid_draw_indirect_multi(struct gl_context *ctx,
> + GLsizei primcount, GLsizei stride,
> + const char *name)
> +{
> + if (primcount <= 0) {
It would be great to put a citation for this:
/* From the ARB_multi_draw_indirect specification:
* "INVALID_VALUE is generated by MultiDrawArraysIndirect or
* MultiDrawElementsIndirect if <primcount> is negative."
*
* "<primcount> must be positive, otherwise an INVALID_VALUE error will
* be generated."
*/
These beg the question of whether 0 is allowed. Usually I interpret
"negative" as < 0, "positive" as >= 0, and "strictly positive" as > 0.
So I think zero should be allowed, and I don't see a contradiction.
The only text I can find in 4.3 and 4.4 just reiterate that it needs to
positive, and I don't see any text defining "positive."
Our existing implementation of MultiDrawElements appears to allow 0,
simply turning it into a noop. This seems to be supported by the
pseudocode; the loop will simply execute zero times.
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(primcount <= 0)", name);
> + return GL_FALSE;
> + }
> +
/* From the ARB_multi_draw_indirect specification:
* "<stride> must be a multiple of four, otherwise an INVALID_VALUE
* error is generated."
*/
> + if (stride % 4) {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(stride %% 4)", name);
> + return GL_FALSE;
> + }
> +
> + return GL_TRUE;
> +}
Assuming you add spec citations, and either allow primcount == 0 or
refute my claim that it should be valid, this would get a:
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
