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

[Ian Romanick]

On 11/01/2013 12:01 PM, Fredrik Höglund wrote:
> 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.

These patches squashed are

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>> ---
>>  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:
>>         *
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev