<div dir="ltr">On 4 November 2013 06:57, Brian Paul <<a href="mailto:brianp@vmware.com" target="_blank">brianp@vmware.com</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <div><div>On 11/04/2013 02:09 AM, Chris Forbes wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> From: Christoph Bumiller <<a href="mailto:e0425955@student.tuwien.ac.at" target="_blank">e0425955@student.tuwien.ac.at</a>> </blockquote></div></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> diff --git a/src/mapi/glapi/gen/ARB_draw_indirect.xml b/src/mapi/glapi/gen/ARB_draw_indirect.xml new file mode 100644 index 0000000..7de03cd --- /dev/null +++ b/src/mapi/glapi/gen/ARB_draw_indirect.xml @@ -0,0 +1,45 @@ +<?xml version="1.0"?> +<!DOCTYPE OpenGLAPI SYSTEM "gl_API.dtd"> + +<OpenGLAPI> + +<category name="GL_ARB_draw_indirect" number="87"> + + <enum name="DRAW_INDIRECT_BUFFER" value="0x8F3F"/> + <enum name="DRAW_INDIRECT_BUFFER_BINDING" value="0x8F43"/> + + <function name="DrawArraysIndirect" offset="assign" exec="dynamic"> + <param name="mode" type="GLenum"/> + <param name="indirect" type="const GLvoid *"/> </blockquote> </div> The use of GLvoid has been removed in glext.h and I'd be OK with doing the same in the dispatch code.</blockquote><div> </div><div>I'd be in favor of that too, but I think it should be done as a separate patch series--at the moment all the dispatch xml consistently uses GLvoid, so IMHO we should stay consistent for now, and then later switch everything over to "void" in one fell swoop. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + </function> + + <function name="DrawElementsIndirect" offset="assign" exec="dynamic"> + <param name="mode" type="GLenum"/> + <param name="type" type="GLenum"/> + <param name="indirect" type="const GLvoid *"/> + </function> + +</category> + + +<category name="GL_ARB_multi_draw_indirect" number="133"> + + <function name="MultiDrawArraysIndirect" offset="assign" exec="dynamic"> + <param name="mode" type="GLenum"/> + <param name="indirect" type="const GLvoid *"/> + <param name="primcount" type="GLsizei"/> + <param name="stride" type="GLsizei"/> + </function> + + <function name="MultiDrawElementsIndirect" offset="assign" exec="dynamic"> + <param name="mode" type="GLenum"/> + <param name="type" type="GLenum"/> + <param name="indirect" type="const GLvoid *"/> + <param name="primcount" type="GLsizei"/> + <param name="stride" type="GLsizei"/> + </function> + +</category> + + +</OpenGLAPI> diff --git a/src/mapi/glapi/gen/Makefile.am b/src/mapi/glapi/gen/Makefile.am index cbbf659..0c67513 100644 --- a/src/mapi/glapi/gen/Makefile.am +++ b/src/mapi/glapi/gen/Makefile.am @@ -98,6 +98,7 @@ API_XML = \ ARB_draw_buffers.xml \ ARB_draw_buffers_blend.xml \ ARB_draw_elements_base_vertex.xml \ + ARB_draw_indirect.xml \ ARB_draw_instanced.xml \ ARB_ES2_compatibility.xml \ ARB_ES3_compatibility.xml \ diff --git a/src/mapi/glapi/gen/gl_API.xml b/src/mapi/glapi/gen/gl_API.xml index 69014c5..7cab5ba 100644 --- a/src/mapi/glapi/gen/gl_API.xml +++ b/src/mapi/glapi/gen/gl_API.xml @@ -8241,6 +8241,8 @@  +<xi:include href="ARB_draw_indirect.xml" xmlns:xi="<a href="http://www.w3.org/2001/XInclude" target="_blank">http://www.w3.org/2001/XInclude</a>"/> + <category name="GL_ARB_transform_feedback3" number="94"> <enum name="MAX_TRANSFORM_FEEDBACK_BUFFERS" value="0x8E70"/> <enum name="MAX_VERTEX_STREAMS" value="0x8E71"/> @@ -8466,7 +8468,7 @@ <xi:include href="ARB_invalidate_subdata.xml" xmlns:xi="<a href="http://www.w3.org/2001/XInclude" target="_blank">http://www.w3.org/2001/XInclude</a>"/> - + <xi:include href="ARB_texture_buffer_range.xml" xmlns:xi="<a href="http://www.w3.org/2001/XInclude" target="_blank">http://www.w3.org/2001/XInclude</a>"/> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c index f285c97..f3128cb 100644 --- a/src/mesa/main/api_validate.c +++ b/src/mesa/main/api_validate.c @@ -837,3 +837,156 @@ _mesa_validate_DrawTransformFeedback(struct gl_context *ctx, return GL_TRUE; } + </blockquote> </div></div> Maybe put a comment on this describing what 'size' is.<div><div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +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) +{ + FLUSH_CURRENT(ctx, 0); + + return valid_draw_indirect(ctx, mode, + indirect, 4 * sizeof(GLuint), </blockquote> </div></div> 4 here and 5 below, etc. seem like magic numbers. How about declaring and using a local var such as "const unsigned drawArraysNumParams = 4;" to clarify?<div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + "glDrawArraysIndirect"); +} + +GLboolean +_mesa_validate_DrawElementsIndirect(struct gl_context *ctx, + GLenum mode, GLenum type, + const GLvoid *indirect) +{ + FLUSH_CURRENT(ctx, 0); + + return valid_draw_indirect_elements(ctx, mode, type, + indirect, 5 * sizeof(GLuint), + "glDrawElementsIndirect"); +} + +GLboolean +_mesa_validate_MultiDrawArraysIndirect(struct gl_context *ctx, + GLenum mode, + const GLvoid *indirect, + GLsizei primcount, GLsizei stride) +{ + GLsizeiptr size = 0; + if (primcount) /* &(last command) + sizeof(DrawArraysIndirectCommand) */ </blockquote> </div> Is the commented code a debug left-over?<div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + size = (primcount - 1) * stride + 4 * 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_MultiDrawElementsIndirect(struct gl_context *ctx, + GLenum mode, GLenum type, + const GLvoid *indirect, + GLsizei primcount, GLsizei stride) +{ + GLsizeiptr size = 0; </blockquote> </div> Insert blank line here.<div><div> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + if (primcount) /* &(last command) + sizeof(DrawElementsIndirectCommand) */ + size = (primcount - 1) * stride + 5 * sizeof(GLuint); + + FLUSH_CURRENT(ctx, 0); + + if (!valid_draw_indirect_multi(ctx, primcount, stride, + "glMultiDrawElementsIndirect")) + return GL_FALSE; + + if (!valid_draw_indirect_elements(ctx, mode, type, + indirect, size, + "glMultiDrawElementsIndirect")) + return GL_FALSE; + + return GL_TRUE; +} diff --git a/src/mesa/main/api_validate.h b/src/mesa/main/api_validate.h index f2b753c..8238df1 100644 --- a/src/mesa/main/api_validate.h +++ b/src/mesa/main/api_validate.h @@ -87,5 +87,31 @@ _mesa_validate_DrawTransformFeedback(struct gl_context *ctx, GLuint stream, GLsizei numInstances); +extern GLboolean +_mesa_validate_DrawArraysIndirect(struct gl_context *ctx, + GLenum mode, + const GLvoid *indirect); + +extern GLboolean +_mesa_validate_DrawElementsIndirect(struct gl_context *ctx, + GLenum mode, + GLenum type, + const GLvoid *indirect); + +extern GLboolean +_mesa_validate_MultiDrawArraysIndirect(struct gl_context *ctx, + GLenum mode, + const GLvoid *indirect, + GLsizei primcount, + GLsizei stride); + +extern GLboolean +_mesa_validate_MultiDrawElementsIndirect(struct gl_context *ctx, + GLenum mode, + GLenum type, + const GLvoid *indirect, + GLsizei primcount, + GLsizei stride); + #endif diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 1f55061..69ea1c0 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -86,6 +86,10 @@ get_buffer_target(struct gl_context *ctx, GLenum target) return &ctx->CopyReadBuffer; case GL_COPY_WRITE_BUFFER: return &ctx->CopyWriteBuffer; + case GL_DRAW_INDIRECT_BUFFER: + if (ctx->Extensions.ARB_draw_indirect) + return &ctx->DrawIndirectBuffer; + break; case GL_TRANSFORM_FEEDBACK_BUFFER: if (ctx->Extensions.EXT_transform_feedback) { return &ctx->TransformFeedback.CurrentBuffer; @@ -626,6 +630,9 @@ _mesa_init_buffer_objects( struct gl_context *ctx ) _mesa_reference_buffer_object(ctx, &ctx->UniformBuffer, ctx->Shared->NullBufferObj); + _mesa_reference_buffer_object(ctx, &ctx->DrawIndirectBuffer, + ctx->Shared->NullBufferObj); + for (i = 0; i < MAX_COMBINED_UNIFORM_BUFFERS; i++) { _mesa_reference_buffer_object(ctx, &ctx->UniformBufferBindings[i].BufferObject, @@ -873,6 +880,11 @@ _mesa_DeleteBuffers(GLsizei n, const GLuint *ids) _mesa_BindBuffer( GL_ELEMENT_ARRAY_BUFFER_ARB, 0 ); } + /* unbind ARB_draw_indirect binding point */ + if (ctx->DrawIndirectBuffer == bufObj) { + _mesa_BindBuffer( GL_DRAW_INDIRECT_BUFFER, 0 ); + } + /* unbind ARB_copy_buffer binding points */ if (ctx->CopyReadBuffer == bufObj) { _mesa_BindBuffer( GL_COPY_READ_BUFFER, 0 ); diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c index 5956419..c953ffb 100644 --- a/src/mesa/main/dlist.c +++ b/src/mesa/main/dlist.c @@ -1356,6 +1356,42 @@ save_DrawElementsInstancedBaseVertexBaseInstance(GLenum mode, "glDrawElementsInstancedBaseVertexBaseInstance() during display list compile"); } +/* GL_ARB_draw_indirect. */ +static void GLAPIENTRY +save_DrawArraysIndirect(GLenum mode, const GLvoid *indirect) +{ + GET_CURRENT_CONTEXT(ctx); + _mesa_error(ctx, GL_INVALID_OPERATION, + "glDrawArraysIndirect() during display list compile"); +} + +static void GLAPIENTRY +save_DrawElementsIndirect(GLenum mode, GLenum type, const GLvoid *indirect) +{ + GET_CURRENT_CONTEXT(ctx); + _mesa_error(ctx, GL_INVALID_OPERATION, + "glDrawElementsIndirect() during display list compile"); +} + +/* GL_ARB_multi_draw_indirect. */ +static void GLAPIENTRY +save_MultiDrawArraysIndirect(GLenum mode, const GLvoid *indirect, + GLsizei primcount, GLsizei stride) +{ + GET_CURRENT_CONTEXT(ctx); + _mesa_error(ctx, GL_INVALID_OPERATION, + "glMultiDrawArraysIndirect() during display list compile"); +} + +static void GLAPIENTRY +save_MultiDrawElementsIndirect(GLenum mode, GLenum type, + const GLvoid *indirect, + GLsizei primcount, GLsizei stride) +{ + GET_CURRENT_CONTEXT(ctx); + _mesa_error(ctx, GL_INVALID_OPERATION, + "glMultiDrawElementsIndirect() during display list compile"); +} </blockquote> </div></div> I thought that when we build/init the display list 'save' dispatch table, the unused entries point to a generic function that generates GL_INVALID_OPERATION. In that case, I don't think we'd need these functions. I'd have to dig into the code to be sure though.</blockquote> <div> </div><div>I don't think that's the case--_mesa_initialize_save_table() starts off by making a copy of the Exec table, so stuff that is allowed in Exec mode but disallowed in display lists needs to be overridden. I think these "save_" functions are fine. </div><div>I agree with Brian's other comments, though (comment explaining "size", explain the magic numbers 4 and 5, and mysterious commented out code in _mesa_validate_MultiDrawArraysIndirect and _mesa_validate_MultiDrawElementsIndirect). </div><div>I also agree with Ian's comment that the if test in get_buffer_target() needs to include "&& ctx->API == API_OPENGL_CORE. </div><div>More comments on this patch to follow in a separate email. </div></div></div></div>