Re: [Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws
>Other BO usage in the draw path doesn't check that buffers aren't >mapped. What makes this path special? Apparently nothing -- this is weirdness inherited from the original patches. I'll drop it. -- Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws
On 6 November 2013 23:06, Chris Forbes wrote:
> Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.
>
> Signed-off-by: Chris Forbes
> ---
> src/mesa/main/api_validate.c | 163
> +++
> src/mesa/main/api_validate.h | 26 +++
> 2 files changed, 189 insertions(+)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index f285c97..befd69f 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -837,3 +837,166 @@ _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;
> +
> + 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)) {
> + if (_mesa_bufferobj_mapped(ctx->DrawIndirectBuffer)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(DRAW_INDIRECT_BUFFER is mapped)", name);
> + return GL_FALSE;
> + }
> + 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) {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(primcount < 0)", name);
> + return GL_FALSE;
> + }
>
ARB_multi_draw_indirect is inconsistent about whether a primcount of 0 is
allowed. In one place it says:
must be positive, otherwise an INVALID_VALUE error will be
generated.
Which would seem to imply that 0 is not allowed. But then later it says:
INVALID_VALUE is generated by MultiDrawArraysIndirect or
MultiDrawElementsIndirect if is negative.
Which would seem to imply that 0 is allowed.
OpenGL 4.3 and 4.4 consistently say:
An INVALID_VALUE error is generated if drawcount is not positive.
Usually when there's a difference like this between the extension specs and
the OpenGL spec, the OpenGL spec is the correct one. So unless we have
experimental evidence from shipping drivers that a primcount of 0 is
allowed, I think we should make it an error.
> +
> + if (stride % 4) {
> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(stride %% 4)", name);
> + return GL_FALSE;
> + }
> +
> + return GL_TRUE;
> +}
> +
> +GLboolean
> +_mesa_validate_DrawArraysIndirect(struct gl_context *ctx,
> + GLenum mode,
> + const GLvoid *indirect)
> +{
> + const unsigned drawArraysNumParams = 4;
> +
> + FLUSH_CURRENT(ctx, 0);
> +
> + return valid_draw_indirect(ctx, mode,
> + indirect, drawArraysNumParams *
> sizeof(GLuint),
> + "glDrawArraysIndirect");
> +}
> +
> +GLboolean
> +_mesa_validate_DrawElementsIndirect(struct gl_context *ctx,
> +GLenum mode, GLenum type,
> +const GLvoid *indirect)
> +{
> + const unsigned drawElementsNumParams = 5;
> +
> + FLUSH_CURRENT(ctx, 0);
> +
> + return valid_draw_indirect_e
Re: [Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws
Chris Forbes writes:
> Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.
>
> Signed-off-by: Chris Forbes
> ---
> src/mesa/main/api_validate.c | 163
> +++
> src/mesa/main/api_validate.h | 26 +++
> 2 files changed, 189 insertions(+)
>
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index f285c97..befd69f 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -837,3 +837,166 @@ _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;
> +
> + 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)) {
> + if (_mesa_bufferobj_mapped(ctx->DrawIndirectBuffer)) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(DRAW_INDIRECT_BUFFER is mapped)", name);
> + return GL_FALSE;
> + }
Other BO usage in the draw path doesn't check that buffers aren't
mapped. What makes this path special?
> + if (ctx->DrawIndirectBuffer->Size < end) {
> + _mesa_error(ctx, GL_INVALID_OPERATION,
> + "%s(DRAW_INDIRECT_BUFFER too small)", name);
> + return GL_FALSE;
> + }
pgpTXinCnuteS.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws
Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.
Signed-off-by: Chris Forbes
---
src/mesa/main/api_validate.c | 163 +++
src/mesa/main/api_validate.h | 26 +++
2 files changed, 189 insertions(+)
diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
index f285c97..befd69f 100644
--- a/src/mesa/main/api_validate.c
+++ b/src/mesa/main/api_validate.c
@@ -837,3 +837,166 @@ _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;
+
+ 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)) {
+ if (_mesa_bufferobj_mapped(ctx->DrawIndirectBuffer)) {
+ _mesa_error(ctx, GL_INVALID_OPERATION,
+ "%s(DRAW_INDIRECT_BUFFER is mapped)", name);
+ return GL_FALSE;
+ }
+ 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) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(primcount < 0)", name);
+ return GL_FALSE;
+ }
+
+ if (stride % 4) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "%s(stride %% 4)", name);
+ return GL_FALSE;
+ }
+
+ return GL_TRUE;
+}
+
+GLboolean
+_mesa_validate_DrawArraysIndirect(struct gl_context *ctx,
+ GLenum mode,
+ const GLvoid *indirect)
+{
+ const unsigned drawArraysNumParams = 4;
+
+ FLUSH_CURRENT(ctx, 0);
+
+ return valid_draw_indirect(ctx, mode,
+ indirect, drawArraysNumParams * sizeof(GLuint),
+ "glDrawArraysIndirect");
+}
+
+GLboolean
+_mesa_validate_DrawElementsIndirect(struct gl_context *ctx,
+GLenum mode, GLenum type,
+const GLvoid *indirect)
+{
+ const unsigned drawElementsNumParams = 5;
+
+ FLUSH_CURRENT(ctx, 0);
+
+ return valid_draw_indirect_elements(ctx, mode, type,
+ indirect, drawElementsNumParams *
sizeof(GLuint),
+ "glDrawElementsIndirect");
+}
+
+GLboolean
+_mesa_validate_MultiDrawArraysIndirect(struct gl_context *ctx,
+ GLenum mode,
+ const GLvoid *indirect,
+ GLsizei primcount, GLsizei stride)
+{
+ GLsizeiptr size = 0;
+ const unsigned drawArraysNumParams = 4;
+
+ /* number of bytes of the indirect buffer which will be read */
+ if (primcount)
+ size = (primcount - 1) * stride + drawArraysNumParams * sizeof(GLuint);
+
+ FLUSH_CURRENT(ctx, 0);
+
+ if (!valid_draw_indirect_multi(ctx, primcount, stride,
+ "glMultiDrawArraysIndirect"))
+ return GL_FALSE;
+
+ if (!valid_draw_indirect(ctx, mode, indirect, size,
+"glMultiDrawArraysIndirect"))
+ return GL_FALSE;
+
+ return GL_TRUE;
+}
+
+GLboolean
+_mesa_validate_MultiDrawElementsIndir
