Re: [Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws

2013-11-09 Thread Chris Forbes
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

2013-11-07 Thread Eric Anholt
Chris Forbes chr...@ijw.co.nz writes:

 Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.

 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  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


Re: [Mesa-dev] [PATCH V2 05/15] mesa: Add validation helpers for new indirect draws

2013-11-07 Thread Paul Berry
On 6 November 2013 23:06, Chris Forbes chr...@ijw.co.nz wrote:

 Based on part of Patch 2 of Christoph Bumiller's ARB_draw_indirect series.

 Signed-off-by: Chris Forbes chr...@ijw.co.nz
 ---
  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:

primcount 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 primcount 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_elements(ctx, mode, type,
 +   indirect, drawElementsNumParams *