[Mesa-dev] [PATCH 10/18] mesa: Add helper functions for looking up multiple buffers

[Fredrik Höglund]

On Wednesday 22 January 2014, Brian Paul wrote:
> On 01/21/2014 03:35 PM, Fredrik Höglund wrote:
> > ---
> >   src/mesa/main/bufferobj.c |   66 +++++++++++++++++++++++++++++++++++++++++++++
> >   src/mesa/main/bufferobj.h |   14 ++++++++++
> >   2 files changed, 80 insertions(+)
> >
> > diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> > index 4094e31..81344ac 100644
> > --- a/src/mesa/main/bufferobj.c
> > +++ b/src/mesa/main/bufferobj.c
> > @@ -993,6 +993,72 @@ _mesa_lookup_bufferobj(struct gl_context *ctx, GLuint buffer)
> >   }
> >
> >
> > +struct gl_buffer_object *
> > +_mesa_lookup_bufferobj_without_locking(struct gl_context *ctx, GLuint buffer)
> > +{
> > +   return (struct gl_buffer_object *)
> > +      _mesa_HashLookupWithoutLocking(ctx->Shared->BufferObjects, buffer);
> > +}
> > +
> > +
> > +void
> > +_mesa_begin_bufferobj_lookups(struct gl_context *ctx)
> > +{
> > +   _mesa_HashLockMutex(ctx->Shared->BufferObjects);
> > +}
> > +
> > +
> > +void
> > +_mesa_end_bufferobj_lookups(struct gl_context *ctx)
> > +{
> > +   _mesa_HashUnlockMutex(ctx->Shared->BufferObjects);
> > +}
> > +
> > +
> > +struct gl_buffer_object *
> > +_mesa_bind_buffers_lookup_bufferobj(struct gl_context *ctx, GLenum target,
> > +                                    const GLuint *buffers,
> > +                                    GLuint index, const char *caller)
> 
> Let's put a comment on this function explaining what it does and how 
> it's different from _mesa_lookup_bufferobj().
> 
> It looks like you pass both 'buffers' and 'index' just for the error 
> message.  I think you could just pass a 'buffer' and simplify the error 
> message.

I agree that passing both those parameters is less than ideal.  But I do
think it's important to tell the user which buffer the error refers to.
As I've pointed out before, the error messages are passed to the client
when it uses KHR_debug, so I think the quality of these messages has
become more important.

The proprietary drivers tend to provide more verbose error messages
than Mesa.

> > +{
> > +   struct gl_buffer_object *bufObj;
> > +
> > +   if (buffers[index] != 0)
> > +      bufObj = _mesa_lookup_bufferobj_without_locking(ctx, buffers[index]);
> > +   else
> > +      bufObj = ctx->Shared->NullBufferObj;
> > +
> > +   if (!bufObj) {
> > +      /* The ARB_multi_bind spec says:
> > +       *
> > +       *    "An INVALID_OPERATION error is generated if any value
> > +       *     in <buffers> is not zero or the name of an existing
> > +       *     buffer object (per binding)."
> > +       */
> > +      _mesa_error(ctx, GL_INVALID_OPERATION,
> > +                 "%s(buffers[%u]=%u is not zero or the name "
> > +                 "of an existing buffer object)",
> > +                 caller, index, buffers[index]);
> > +   }
> > +
> > +   if (unlikely(bufObj == &DummyBufferObject)) {
> > +      /* If this is a new buffer object id, or one which was generated but
> > +       * never used before, allocate a buffer object now.
> > +       */
> > +      ASSERT(ctx->Driver.NewBufferObject);
> > +      bufObj = ctx->Driver.NewBufferObject(ctx, buffers[index], target);
> > +      if (unlikely(!bufObj)) {
> > +         _mesa_error(ctx, GL_OUT_OF_MEMORY, "%s", caller);
> > +         return NULL;
> > +      }
> > +
> > +      _mesa_HashInsertWithoutLocking(ctx->Shared->BufferObjects,
> > +                                     buffers[index], bufObj);
> > +   }
> > +
> > +   return bufObj;
> > +}
> > +
> > +
> >   /**
> >    * If *ptr points to obj, set ptr = the Null/default buffer object.
> >    * This is a helper for buffer object deletion.
> > diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> > index 2239b83..9d21dab 100644
> > --- a/src/mesa/main/bufferobj.h
> > +++ b/src/mesa/main/bufferobj.h
> > @@ -76,6 +76,20 @@ _mesa_update_default_objects_buffer_objects(struct gl_context *ctx);
> >   extern struct gl_buffer_object *
> >   _mesa_lookup_bufferobj(struct gl_context *ctx, GLuint buffer);
> >
> > +extern struct gl_buffer_object *
> > +_mesa_lookup_bufferobj_without_locking(struct gl_context *ctx, GLuint buffer);
> > +
> > +extern void
> > +_mesa_begin_bufferobj_lookups(struct gl_context *ctx);
> > +
> > +extern void
> > +_mesa_end_bufferobj_lookups(struct gl_context *ctx);
> > +
> > +extern struct gl_buffer_object *
> > +_mesa_bind_buffers_lookup_bufferobj(struct gl_context *ctx, GLenum target,
> > +                                    const GLuint *buffers,
> > +                                    GLuint index, const char *caller);
> > +
> >   extern void
> >   _mesa_initialize_buffer_object(struct gl_context *ctx,
> >                                  struct gl_buffer_object *obj,
> >
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>