<div dir="ltr"><div><div>You still seem to be confused about the names of internal functions. If a function exists outside of its "class," (ie. is extern-ed from a *.h file), it should have the "_mesa" in front. If a function exists purely inside of a *.c file, it should be static and not have "_mesa." </div>Since _mesa_lookup_transform_feedback_object in transformfeedback.h is used outside of transformfeedback.[c|h], it should have the "_mesa" in front and not be static. In this patch, you renamed it to lookup_transform_feedback_object, which is incorrect. </div>On the other hand, the names that you gave to lookup_transform_feedback_object_err and lookup_transform_feedback_bufferobj_err are just fine because these functions are static and never called outside transformfeedback.c. <div><div><div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Feb 16, 2015 at 6:13 AM, Martin Peres <<a href="mailto:martin.peres@linux.intel.com" target="_blank">martin.peres@linux.intel.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">v2: Review from Laura Ekstrand - give more helpful error messages - factor the lookup code for the xfb and objBuf - replace some already-existing tabs with spaces - add comments to explain the cases where xfb == 0 or buffer == 0 - fix the condition for binding the transform buffer or not v3: Review from Laura Ekstrand - rename _mesa_lookup_bufferobj_err to _mesa_lookup_transform_feedback_bufferobj_err and make it static to avoid a future conflict - make _mesa_lookup_transform_feedback_object_err static v4: Review from Laura Ekstrand - add the pdf page number when quoting the spec - rename some of the symbols to follow the public/private conventions Signed-off-by: Martin Peres <<a href="mailto:martin.peres@linux.intel.com" target="_blank">martin.peres@linux.intel.com</a>> --- src/mapi/glapi/gen/ARB_direct_state_access.xml | 6 + src/mesa/main/bufferobj.c | 9 +- src/mesa/main/objectlabel.c | 2 +- src/mesa/main/tests/dispatch_sanity.cpp | 1 + src/mesa/main/transformfeedback.c | 146 +++++++++++++++++++------ src/mesa/main/transformfeedback.h | 13 ++- src/mesa/vbo/vbo_exec_array.c | 8 +- 7 files changed, 139 insertions(+), 46 deletions(-) diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml b/src/mapi/glapi/gen/ARB_direct_state_access.xml index 15b00c2..35d6906 100644 --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml @@ -14,6 +14,12 @@ <param name="ids" type="GLuint *" /> </function> + <function name="TransformFeedbackBufferBase" offset="assign"> + <param name="xfb" type="GLuint" /> + <param name="index" type="GLuint" /> + <param name="buffer" type="GLuint" /> + </function> +  <function name="CreateTextures" offset="assign"> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c index 0c1ce98..86532ea 100644 --- a/src/mesa/main/bufferobj.c +++ b/src/mesa/main/bufferobj.c @@ -3546,8 +3546,9 @@ _mesa_BindBufferRange(GLenum target, GLuint index, switch (target) { case GL_TRANSFORM_FEEDBACK_BUFFER: - _mesa_bind_buffer_range_transform_feedback(ctx, index, bufObj, - offset, size); + _mesa_bind_buffer_range_transform_feedback(ctx, + ctx->TransformFeedback.CurrentObject, + index, bufObj, offset, size); return; case GL_UNIFORM_BUFFER: bind_buffer_range_uniform_buffer(ctx, index, bufObj, offset, size); @@ -3611,7 +3612,9 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer) switch (target) { case GL_TRANSFORM_FEEDBACK_BUFFER: - _mesa_bind_buffer_base_transform_feedback(ctx, index, bufObj); + _mesa_bind_buffer_base_transform_feedback(ctx, + ctx->TransformFeedback.CurrentObject, + index, bufObj, false); return; case GL_UNIFORM_BUFFER: bind_buffer_base_uniform_buffer(ctx, index, bufObj); diff --git a/src/mesa/main/objectlabel.c b/src/mesa/main/objectlabel.c index 78df96b..19a7e59 100644 --- a/src/mesa/main/objectlabel.c +++ b/src/mesa/main/objectlabel.c @@ -170,7 +170,7 @@ get_label_pointer(struct gl_context *ctx, GLenum identifier, GLuint name, case GL_TRANSFORM_FEEDBACK: { struct gl_transform_feedback_object *tfo = - _mesa_lookup_transform_feedback_object(ctx, name); + lookup_transform_feedback_object(ctx, name); if (tfo) labelPtr = &tfo->Label; } diff --git a/src/mesa/main/tests/dispatch_sanity.cpp b/src/mesa/main/tests/dispatch_sanity.cpp index bf320bf..183755f 100644 --- a/src/mesa/main/tests/dispatch_sanity.cpp +++ b/src/mesa/main/tests/dispatch_sanity.cpp @@ -956,6 +956,7 @@ const struct function gl_core_functions_possible[] = { /* GL_ARB_direct_state_access */ { "glCreateTransformFeedbacks", 45, -1 }, + { "glTransformFeedbackBufferBase", 45, -1 }, { "glCreateTextures", 45, -1 }, { "glTextureStorage1D", 45, -1 }, { "glTextureStorage2D", 45, -1 }, diff --git a/src/mesa/main/transformfeedback.c b/src/mesa/main/transformfeedback.c index 10bb6a1..d932943 100644 --- a/src/mesa/main/transformfeedback.c +++ b/src/mesa/main/transformfeedback.c @@ -514,22 +514,24 @@ _mesa_EndTransformFeedback(void) * Helper used by BindBufferRange() and BindBufferBase(). */ static void -bind_buffer_range(struct gl_context *ctx, GLuint index, +bind_buffer_range(struct gl_context *ctx, + struct gl_transform_feedback_object *obj, + GLuint index, struct gl_buffer_object *bufObj, - GLintptr offset, GLsizeiptr size) + GLintptr offset, GLsizeiptr size, + bool dsa) { - struct gl_transform_feedback_object *obj = - ctx->TransformFeedback.CurrentObject; - /* Note: no need to FLUSH_VERTICES or flag NewTransformFeedback, because * transform feedback buffers can't be changed while transform feedback is * active. */ - /* The general binding point */ - _mesa_reference_buffer_object(ctx, - &ctx->TransformFeedback.CurrentBuffer, - bufObj); + if (!dsa) { + /* The general binding point */ + _mesa_reference_buffer_object(ctx, + &ctx->TransformFeedback.CurrentBuffer, + bufObj); + } /* The per-attribute binding point */ _mesa_set_transform_feedback_binding(ctx, obj, index, bufObj, offset, size); @@ -543,15 +545,12 @@ bind_buffer_range(struct gl_context *ctx, GLuint index, */ void _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, - GLuint index, - struct gl_buffer_object *bufObj, - GLintptr offset, - GLsizeiptr size) + struct gl_transform_feedback_object *obj, + GLuint index, + struct gl_buffer_object *bufObj, + GLintptr offset, + GLsizeiptr size) { - struct gl_transform_feedback_object *obj; - - obj = ctx->TransformFeedback.CurrentObject; - if (obj->Active) { _mesa_error(ctx, GL_INVALID_OPERATION, "glBindBufferRange(transform feedback active)"); @@ -559,13 +558,15 @@ _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, } if (index >= ctx->Const.MaxTransformFeedbackBuffers) { - _mesa_error(ctx, GL_INVALID_VALUE, "glBindBufferRange(index=%d)", index); + _mesa_error(ctx, GL_INVALID_VALUE, "glBindBufferRange(index=%d " + "out of bounds)", index); return; } if (size & 0x3) { /* must a multiple of four */ - _mesa_error(ctx, GL_INVALID_VALUE, "glBindBufferRange(size=%d)", (int) size); + _mesa_error(ctx, GL_INVALID_VALUE, "glBindBufferRange(size=%d)", + (int) size); return; } @@ -576,38 +577,109 @@ _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, return; } - bind_buffer_range(ctx, index, bufObj, offset, size); + bind_buffer_range(ctx, obj, index, bufObj, offset, size, false); } /** * Specify a buffer object to receive transform feedback results. * As above, but start at offset = 0. - * Called from the glBindBufferBase() function. + * Called from the glBindBufferBase() and glTransformFeedbackBufferBase() + * functions. */ void _mesa_bind_buffer_base_transform_feedback(struct gl_context *ctx, - GLuint index, - struct gl_buffer_object *bufObj) + struct gl_transform_feedback_object *obj, + GLuint index, + struct gl_buffer_object *bufObj, + bool dsa) { - struct gl_transform_feedback_object *obj; - - obj = ctx->TransformFeedback.CurrentObject; - if (obj->Active) { _mesa_error(ctx, GL_INVALID_OPERATION, - "glBindBufferBase(transform feedback active)"); + "%s(transform feedback active)", + dsa?"glTransformFeedbackBufferBase":"glBindBufferBase"); return; } if (index >= ctx->Const.MaxTransformFeedbackBuffers) { - _mesa_error(ctx, GL_INVALID_VALUE, "glBindBufferBase(index=%d)", index); + _mesa_error(ctx, GL_INVALID_VALUE, "%s(index=%d out of bounds)", + dsa?"glTransformFeedbackBufferBase":"glBindBufferBase", + index); return; } - bind_buffer_range(ctx, index, bufObj, 0, 0); + bind_buffer_range(ctx, obj, index, bufObj, 0, 0, dsa); } +/** + * Wrapper around _mesa_lookup_transform_feedback_object that throws + * GL_INVALID_OPERATION if id is not in the hash table. After calling + * _mesa_error, it returns NULL. + */ +static struct gl_transform_feedback_object * </blockquote><div>Something is wrong with the spacing at GLuint xfb here. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +lookup_transform_feedback_object_err(struct gl_context *ctx, + GLuint xfb, const char* func) +{ + struct gl_transform_feedback_object *obj; + + obj = lookup_transform_feedback_object(ctx, xfb); + if (!obj) { + _mesa_error(ctx, GL_INVALID_OPERATION, + "%s(xfb=%u: non-generated object name)", func, xfb); + } + + return obj; +} + +/** + * Wrapper around _mesa_lookup_bufferobj that throws GL_INVALID_VALUE if id + * is not in the hash table. Specialised version for the + * transform-feedback-related functions. After calling _mesa_error, it + * returns NULL. + */ +static struct gl_buffer_object * </blockquote><div>Something is wrong with the spacing at GLuint buffer here. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> +lookup_transform_feedback_bufferobj_err(struct gl_context *ctx, </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + GLuint buffer, const char* func) +{ + struct gl_buffer_object *bufObj; + + /* OpenGL 4.5 core profile, 13.2, pdf page 444: buffer must be zero or the + * name of an existing buffer object. + */ + if (buffer == 0) { + bufObj = ctx->Shared->NullBufferObj; + } else { + bufObj = _mesa_lookup_bufferobj(ctx, buffer); + if (!bufObj) { + _mesa_error(ctx, GL_INVALID_VALUE, "%s(invalid buffer=%u)", func, + buffer); + } + } + + return bufObj; +} + +void GLAPIENTRY +_mesa_TransformFeedbackBufferBase(GLuint xfb, GLuint index, GLuint buffer) +{ + GET_CURRENT_CONTEXT(ctx); + struct gl_transform_feedback_object *obj; + struct gl_buffer_object *bufObj; + + obj = lookup_transform_feedback_object_err(ctx, xfb, + "glTransformFeedbackBufferBase"); </blockquote><div> ^^^^^ Something wrong with spaces here. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> + if(!obj) { + return; + } + + bufObj = lookup_transform_feedback_bufferobj_err(ctx, buffer, + "glTransformFeedbackBufferBase"); + if(!bufObj) { + return; + } + + _mesa_bind_buffer_base_transform_feedback(ctx, obj, index, bufObj, true); +} /** * Specify a buffer object to receive transform feedback results, plus the @@ -660,7 +732,7 @@ _mesa_BindBufferOffsetEXT(GLenum target, GLuint index, GLuint buffer, return; } - bind_buffer_range(ctx, index, bufObj, offset, 0); + bind_buffer_range(ctx, obj, index, bufObj, offset, 0, false); } @@ -815,8 +887,12 @@ _mesa_GetTransformFeedbackVarying(GLuint program, GLuint index, struct gl_transform_feedback_object * -_mesa_lookup_transform_feedback_object(struct gl_context *ctx, GLuint name) +lookup_transform_feedback_object(struct gl_context *ctx, GLuint name) { + /* OpenGL 4.5 core profile, 13.2 pdf page 444: xfb must be zero, indicating + * the default transform feedback object, or the name of an existing + * transform feedback object. + */ </blockquote><div>This isn't a Mesa thing, but it makes me uncomfortable when verbatim text doesn't have quotes around it. Too many years in academia, I suppose. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> if (name == 0) { return ctx->TransformFeedback.DefaultObject; } @@ -919,7 +995,7 @@ _mesa_IsTransformFeedback(GLuint name) if (name == 0) return GL_FALSE; - obj = _mesa_lookup_transform_feedback_object(ctx, name); + obj = lookup_transform_feedback_object(ctx, name); if (obj == NULL) return GL_FALSE; @@ -948,7 +1024,7 @@ _mesa_BindTransformFeedback(GLenum target, GLuint name) return; } - obj = _mesa_lookup_transform_feedback_object(ctx, name); + obj = lookup_transform_feedback_object(ctx, name); if (!obj) { _mesa_error(ctx, GL_INVALID_OPERATION, "glBindTransformFeedback(name=%u)", name); @@ -981,7 +1057,7 @@ _mesa_DeleteTransformFeedbacks(GLsizei n, const GLuint *names) for (i = 0; i < n; i++) { if (names[i] > 0) { struct gl_transform_feedback_object *obj - = _mesa_lookup_transform_feedback_object(ctx, names[i]); + = lookup_transform_feedback_object(ctx, names[i]); if (obj) { if (obj->Active) { _mesa_error(ctx, GL_INVALID_OPERATION, diff --git a/src/mesa/main/transformfeedback.h b/src/mesa/main/transformfeedback.h index 9de1fef..89c0155 100644 --- a/src/mesa/main/transformfeedback.h +++ b/src/mesa/main/transformfeedback.h @@ -65,6 +65,7 @@ _mesa_EndTransformFeedback(void); extern void _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, + struct gl_transform_feedback_object *obj, GLuint index, struct gl_buffer_object *bufObj, GLintptr offset, @@ -72,9 +73,10 @@ _mesa_bind_buffer_range_transform_feedback(struct gl_context *ctx, extern void _mesa_bind_buffer_base_transform_feedback(struct gl_context *ctx, + struct gl_transform_feedback_object *obj, GLuint index, - struct gl_buffer_object *bufObj); - + struct gl_buffer_object *bufObj, + bool dsa); </blockquote><div>Put an empty line here (you accidentally deleted the one that was there). </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> extern void GLAPIENTRY _mesa_BindBufferOffsetEXT(GLenum target, GLuint index, GLuint buffer, GLintptr offset); @@ -97,7 +99,7 @@ _mesa_init_transform_feedback_object(struct gl_transform_feedback_object *obj, GLuint name); struct gl_transform_feedback_object * -_mesa_lookup_transform_feedback_object(struct gl_context *ctx, GLuint name); +lookup_transform_feedback_object(struct gl_context *ctx, GLuint name); extern void GLAPIENTRY _mesa_GenTransformFeedbacks(GLsizei n, GLuint *names); @@ -144,4 +146,9 @@ _mesa_set_transform_feedback_binding(struct gl_context *ctx, tfObj->RequestedSize[index] = size; } +/*** GL_ARB_direct_state_access ***/ + +extern void GLAPIENTRY +_mesa_TransformFeedbackBufferBase(GLuint xfb, GLuint index, GLuint buffer); + #endif /* TRANSFORM_FEEDBACK_H */ diff --git a/src/mesa/vbo/vbo_exec_array.c b/src/mesa/vbo/vbo_exec_array.c index c16fe77..354cb04 100644 --- a/src/mesa/vbo/vbo_exec_array.c +++ b/src/mesa/vbo/vbo_exec_array.c @@ -1484,7 +1484,7 @@ vbo_exec_DrawTransformFeedback(GLenum mode, GLuint name) { GET_CURRENT_CONTEXT(ctx); struct gl_transform_feedback_object *obj = - _mesa_lookup_transform_feedback_object(ctx, name); + lookup_transform_feedback_object(ctx, name); if (MESA_VERBOSE & VERBOSE_DRAW) _mesa_debug(ctx, "glDrawTransformFeedback(%s, %d)\n", @@ -1498,7 +1498,7 @@ vbo_exec_DrawTransformFeedbackStream(GLenum mode, GLuint name, GLuint stream) { GET_CURRENT_CONTEXT(ctx); struct gl_transform_feedback_object *obj = - _mesa_lookup_transform_feedback_object(ctx, name); + lookup_transform_feedback_object(ctx, name); if (MESA_VERBOSE & VERBOSE_DRAW) _mesa_debug(ctx, "glDrawTransformFeedbackStream(%s, %u, %u)\n", @@ -1513,7 +1513,7 @@ vbo_exec_DrawTransformFeedbackInstanced(GLenum mode, GLuint name, { GET_CURRENT_CONTEXT(ctx); struct gl_transform_feedback_object *obj = - _mesa_lookup_transform_feedback_object(ctx, name); + lookup_transform_feedback_object(ctx, name); if (MESA_VERBOSE & VERBOSE_DRAW) _mesa_debug(ctx, "glDrawTransformFeedbackInstanced(%s, %d)\n", @@ -1528,7 +1528,7 @@ vbo_exec_DrawTransformFeedbackStreamInstanced(GLenum mode, GLuint name, { GET_CURRENT_CONTEXT(ctx); struct gl_transform_feedback_object *obj = - _mesa_lookup_transform_feedback_object(ctx, name); + lookup_transform_feedback_object(ctx, name); if (MESA_VERBOSE & VERBOSE_DRAW) _mesa_debug(ctx, "glDrawTransformFeedbackStreamInstanced" -- 2.3.0 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div></div></div></div>