[Mesa-dev] [PATCH 05/16] main: Added entry point for glTransformFeedbackBufferBase
Martin Peres
martin.peres at linux.intel.com
Mon Feb 23 04:54:36 PST 2015
On 21/02/15 02:25, Laura Ekstrand wrote:
> 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."
>
> 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.
>
> 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.
Wow, this patch is a pile of crap :o I wonder what I was thinking when I
made this v4! Thanks Laura! I fixed more problems such as missing spaces
around the ternary operator usages.
>
> On Mon, Feb 16, 2015 at 6:13 AM, Martin Peres
> <martin.peres at linux.intel.com <mailto:martin.peres at linux.intel.com>>
> wrote:
>
> 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 <martin.peres at linux.intel.com
> <mailto:martin.peres at linux.intel.com>>
> ---
> 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>
> +
> <!-- Texture object functions -->
>
> <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 *
>
> Something is wrong with the spacing at GLuint xfb here.
>
> +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 *
>
> Something is wrong with the spacing at GLuint buffer here.
>
> +lookup_transform_feedback_bufferobj_err(struct gl_context *ctx,
>
> + 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");
>
> ^^^^^ Something wrong with spaces here.
>
> + 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.
> + */
>
> 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.
>
> 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);
>
> Put an empty line here (you accidentally deleted the one that was there).
>
> 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
> mesa-dev at lists.freedesktop.org <mailto:mesa-dev at lists.freedesktop.org>
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
>
More information about the mesa-dev
mailing list