[Mesa-dev] [PATCH 5/5] mesa: Fix bug about "what if we didn't find the VBO"?

[Fredrik Höglund]

On Thursday 31 October 2013, Eric Anholt wrote:
> There was some spec text, and what there isn't text for we have obvious
> intended behavior from other buffer object bindings.

The first revision of the specification didn't say anything about it,
but the intent is indeed pretty clear now.

I changed this patch slightly by adding a caller parameter to
_mesa_handle_bind_buffer_gen() so the right entry point gets reported
when there is an error.  I also moved the bufferobj.c/.h changes into a
separate commit.  I think this commit might be a candidate for the
stable branches since this was already wrong for BindBufferBase()
and BindBufferRange().

See the attachment.

> ---
>  src/mesa/main/bufferobj.c | 16 ++++++++--------
>  src/mesa/main/bufferobj.h |  8 +++++++-
>  src/mesa/main/varray.c    | 19 ++++++++++---------
>  3 files changed, 25 insertions(+), 18 deletions(-)
> 
> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> index 2d57cab..ef5fbce 100644
> --- a/src/mesa/main/bufferobj.c
> +++ b/src/mesa/main/bufferobj.c
> @@ -655,11 +655,11 @@ _mesa_free_buffer_objects( struct gl_context *ctx )
>     }
>  }
>  
> -static bool
> -handle_bind_buffer_gen(struct gl_context *ctx,
> -		       GLenum target,
> -		       GLuint buffer,
> -		       struct gl_buffer_object **buf_handle)
> +bool
> +_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
> +                             GLenum target,
> +                             GLuint buffer,
> +                             struct gl_buffer_object **buf_handle)
>  {
>     struct gl_buffer_object *buf = *buf_handle;
>  
> @@ -719,7 +719,7 @@ bind_buffer_object(struct gl_context *ctx, GLenum target, GLuint buffer)
>     else {
>        /* non-default buffer object */
>        newBufObj = _mesa_lookup_bufferobj(ctx, buffer);
> -      if (!handle_bind_buffer_gen(ctx, target, buffer, &newBufObj))
> +      if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &newBufObj))
>           return;
>     }
>     
> @@ -2181,7 +2181,7 @@ _mesa_BindBufferRange(GLenum target, GLuint index,
>     } else {
>        bufObj = _mesa_lookup_bufferobj(ctx, buffer);
>     }
> -   if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> +   if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
>        return;
>  
>     if (!bufObj) {
> @@ -2227,7 +2227,7 @@ _mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
>     } else {
>        bufObj = _mesa_lookup_bufferobj(ctx, buffer);
>     }
> -   if (!handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
> +   if (!_mesa_handle_bind_buffer_gen(ctx, target, buffer, &bufObj))
>        return;
>  
>     if (!bufObj) {
> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> index 9b582f8c..503223d 100644
> --- a/src/mesa/main/bufferobj.h
> +++ b/src/mesa/main/bufferobj.h
> @@ -28,7 +28,7 @@
>  #ifndef BUFFEROBJ_H
>  #define BUFFEROBJ_H
>  
> -
> +#include <stdbool.h>
>  #include "mtypes.h"
>  
>  
> @@ -88,6 +88,12 @@ _mesa_reference_buffer_object(struct gl_context *ctx,
>        _mesa_reference_buffer_object_(ctx, ptr, bufObj);
>  }
>  
> +bool
> +_mesa_handle_bind_buffer_gen(struct gl_context *ctx,
> +                             GLenum target,
> +                             GLuint buffer,
> +                             struct gl_buffer_object **buf_handle);
> +
>  extern GLuint
>  _mesa_total_buffer_object_memory(struct gl_context *ctx);
>  
> diff --git a/src/mesa/main/varray.c b/src/mesa/main/varray.c
> index ed3d047..71d13a7 100644
> --- a/src/mesa/main/varray.c
> +++ b/src/mesa/main/varray.c
> @@ -1400,17 +1400,18 @@ _mesa_BindVertexBuffer(GLuint bindingIndex, GLuint buffer, GLintptr offset,
>     } else if (buffer != 0) {
>        vbo = _mesa_lookup_bufferobj(ctx, buffer);
>  
> -      /* The ARB_vertex_attrib_binding spec doesn't specify that an error
> -       * should be generated when <buffer> doesn't refer to a valid buffer
> -       * object, but we assume that this is an oversight.
> +      /* From the GL_ARB_vertex_attrib_array spec:
> +       *
> +       *   "[Core profile only:]
> +       *    An INVALID_OPERATION error is generated if buffer is not zero or a
> +       *    name returned from a previous call to GenBuffers, or if such a name
> +       *    has since been deleted with DeleteBuffers.
> +       *
> +       * Otherwise, we fall back to the same compat profile behavior as other
> +       * object references (automatically gen it).
>         */
> -      if (!vbo) {
> -         _mesa_error(ctx, GL_INVALID_OPERATION,
> -                     "glBindVertexBuffer(buffer=%u is not a valid "
> -                     "buffer object)",
> -                     buffer);
> +      if (!_mesa_handle_bind_buffer_gen(ctx, GL_ARRAY_BUFFER, buffer, &vbo))
>           return;
> -      }
>     } else {
>        /* The ARB_vertex_attrib_binding spec says:
>         *
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mesa-Make-handle_bind_buffer_gen-non-static.patch
Type: text/x-patch
Size: 3970 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131101/415c27d7/attachment.bin>