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