[Mesa-dev] [PATCH 1/7] mesa: implement glBindBuffersBase() and gl BindBuffersRange()

Brian Paul brianp at vmware.com
Mon Jan 6 09:11:42 PST 2014


On 01/03/2014 06:39 AM, servuswiegehtz at yahoo.de wrote:
> when you create the patches with git, you can add --cover-letter to the
> command line. then you get a PATCH 0/X file with an overview over all
> changes and a central place where you can describe what you've done in
> general/which extension etc.
>
> On 03.01.2014 01:27, Maxence Le Doré wrote:
>> ---
>>   src/mesa/main/bufferobj.c | 158 ++++++++++++++++++++++++++++++++++++++++++++++
>>   src/mesa/main/bufferobj.h |   9 ++-
>>   2 files changed, 165 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
>> index a3d8f24..bad8f90 100644
>> --- a/src/mesa/main/bufferobj.c
>> +++ b/src/mesa/main/bufferobj.c
>> @@ -2706,3 +2706,161 @@ _mesa_InvalidateBufferData(GLuint buffer)
>>       */
>>      return;
>>   }
>> +
>> +void GLAPIENTRY
>> +_mesa_BindBuffersBase(GLenum target, GLuint first, GLsizei count,
>> +                      const GLuint *buffers)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +   int i = 0;
>> +   GLboolean exceedMaxBindings = GL_FALSE;
>> +
>> +   switch(target) {
>> +     case GL_TRANSFORM_FEEDBACK_BUFFER:
>> +       first + count > ctx->Const.MaxTransformFeedbackBuffers ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>
> all of these are maybe more concise like this:
>         exceedMaxBindings = first + count >
>                             ctx->Const.MaxTransformFeedbackBuffers ?
>                             GL_TRUE : GL_FALSE;

Or, just:

          exceedMaxBindings = first + count >
                              ctx->Const.MaxTransformFeedbackBuffers;


>> +     case GL_UNIFORM_BUFFER:
>> +       first + count > ctx->Const.MaxUniformBufferBindings ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>> +     case GL_ATOMIC_COUNTER_BUFFER:
>> +       first + count > ctx->Const.MaxAtomicBufferBindings ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>> +     default:
>> +       _mesa_error(ctx, GL_INVALID_ENUM,
>> +                   "glBindBuffersBase(invalid target)");
>> +       return;

The indentation above looks wrong.  We use 3 spaces.


>> +   }
>> +
>> +   if(exceedMaxBindings) {

Please put a space between 'if' and the opening parenthesis.  Same thing 
for 'switch', 'for', etc.


>> +     _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                   "glBindBuffersBase(first+count)");
>> +       return;
>> +       }
>> +
>> +   for(i = 0 ; i < count ; i++) {
>> +      GLuint buffer;
>> +      struct gl_buffer_object *bufferObj;
>> +
>> +      if(buffers == NULL)
>> +        buffer = 0;
>> +      else
>> +        buffer = buffers[i];

buffer = buffers ? buffers[i] : 0;


>> +
>> +      if(buffer != 0) {

if (buffer) {


>> +        bufferObj = _mesa_lookup_bufferobj(ctx, buffer);
>> +        if(bufferObj) {
>> +          _mesa_BindBufferBase(target, first+i, buffer);
>> +        }
>> +        else
>> +          _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                      "glBindBufferBase(buffer[%i] is invalid)", i);
>> +      }
>> +      else
>> +        _mesa_BindBufferBase(target, first + i, 0);
>> +   }
>> +}
>> +
>> +void GLAPIENTRY
>> +_mesa_BindBuffersRange(GLenum target, GLuint first, GLsizei count,
>> +                       const GLuint *buffers, const GLintptr *offsets,
>> +                       const GLsizeiptr *sizes)
>> +{
>> +   GET_CURRENT_CONTEXT(ctx);
>> +   int i = 0;
>> +   GLboolean exceedMaxBindings = GL_FALSE;
>> +
>> +   switch(target) {
>> +     case GL_TRANSFORM_FEEDBACK_BUFFER:
>> +       first + count > ctx->Const.MaxTransformFeedbackBuffers ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>> +     case GL_UNIFORM_BUFFER:
>> +       first + count > ctx->Const.MaxUniformBufferBindings ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>> +     case GL_ATOMIC_COUNTER_BUFFER:
>> +       first + count > ctx->Const.MaxAtomicBufferBindings ?
>> +       exceedMaxBindings = GL_TRUE : exceedMaxBindings = GL_FALSE;
>> +       break;
>> +     default:
>> +       _mesa_error(ctx, GL_INVALID_ENUM,
>> +                   "glBindBuffersRange(invalid target)");
>> +       return;
>> +   }
>> +
>> +   if(exceedMaxBindings) {
>> +     _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                   "glBindBuffersRange(first+count)");
>> +       return;
>> +       }

Fix indentation.


>> +
>> +   for(i = 0 ; i < count ; i++) {

for (i = 0; i < count; i++) {


>> +      GLuint buffer;
>> +      GLintptr offset;
>> +      GLsizeiptr size;
>> +      struct gl_buffer_object *bufferObj;
>> +
>> +      if(buffers == NULL)
>> +        buffer = 0;
>> +      else {
>> +        buffer = buffers[i];
>> +        offset = offsets[i];
>> +        size = sizes[i];
>> +      }
>> +
>> +      if(buffer != 0) {
>> +        bufferObj = _mesa_lookup_bufferobj(ctx, buffer);
>> +        if(bufferObj) {
>> +          GLboolean validOffet, validSize;

"validOffset"


>> +
>> +          switch(target) {
>> +            case GL_TRANSFORM_FEEDBACK_BUFFER:
>> +              (offset >= 0) ?
>> +              validOffset = GL_TRUE : validOffet = GL_FALSE;

validOffset = offset >= 0;

>> +              (size >= 0) ?
>> +              validSize = GL_TRUE : validSize = GL_FALSE;
>> +              /* TODO : add target specific checks */
>> +              break;
>> +            case GL_UNIFORM_BUFFER:
>> +              (offset >= 0) ?
>> +              validOffset = GL_TRUE : validOffet = GL_FALSE;
>> +              (size >= 0) ?
>> +              validSize = GL_TRUE : validSize = GL_FALSE;
>> +              /* TODO : add target specific checks */
>> +              break;
>> +            case GL_ATOMIC_COUNTER_BUFFER:
>> +              (offset >= 0) ?
>> +              validOffset = GL_TRUE : validOffet = GL_FALSE;
>> +              (size >= 0) ?
>> +              validSize = GL_TRUE : validSize = GL_FALSE;
>> +              /* TODO : add target specific checks */
>> +              break;
>> +            default:
>> +              /* should not get there at this point */
>> +              return;

assert(!"Unexpected buffer target");
return;

Actually, why do you have a switch here at all?  All the cases look the 
same.



>> +          }
>> +
>> +          if(!validOffet || !validSize) {
>> +            _mesa_error(ctx, GL_INVALID_VALUE,
>> +                      "glBindBuffersRange(offsets and/or sizes invalid)");
>> +            return;
>> +          }
>> +
>> +          if(offset+size > bufferObj->Size) {
>> +            _mesa_error(ctx, GL_INVALID_VALUE,
>> +                      "glBindBuffersRange(offset+size > GL_BUFFER_SIZE)");
>> +            return;
>> +          }
>
> maybe you can separate the buffer_object_subdata_range_good function
> into a range check and a map check and then just call the range check
> here instead of doing it yourself (repeating code)
>
>
>> +          _mesa_BindBufferRange(target, first+i, buffer, offset, size);
>> +        }
>> +        else
>> +          _mesa_error(ctx, GL_INVALID_OPERATION,
>> +                      "glBindBuffersRange(buffers)");
>> +      }
>> +      else
>> +        _mesa_BindBufferRange(target, first + i, 0, 0, 0);
>> +   }
>> +}
>> diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
>> index 71988b0..5bc3734 100644
>> --- a/src/mesa/main/bufferobj.h
>> +++ b/src/mesa/main/bufferobj.h
>> @@ -191,6 +191,11 @@ _mesa_InvalidateBufferSubData(GLuint buffer, GLintptr offset,
>>
>>   void GLAPIENTRY
>>   _mesa_InvalidateBufferData(GLuint buffer);
>> -
>> -
>> +void GLAPIENTRY
>> +_mesa_BindBuffersBase(GLenum target, GLuint first, GLsizei count,
>> +                           const GLuint *buffers);
>> +void GLAPIENTRY
>> +_mesa_BindBuffersRange(GLenum target, GLuint first, GLsizei count,
>> +                       const GLuint *buffers, const GLintptr *offsets,
>> +                       const GLsizeiptr *sizes);
>>   #endif
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://urldefense.proofpoint.com/v1/url?u=http://lists.freedesktop.org/mailman/listinfo/mesa-dev&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=lGQMzzTgII0I7jefp2FHq7WtZ%2BTLs8wadB%2BiIj9xpBY%3D%0A&m=RYCY8pFUQFO0sGR8WLh%2BcXgF4cpQtTj5uspvSFoLA0Y%3D%0A&s=f37db178eacd38b430b12f4facee6d93b647c2e514652a1f806f9f242721c6bb
>



More information about the mesa-dev mailing list