Le vendredi 18 novembre 2011 à 13:40 -0700, Brian Paul a écrit :
> On 11/18/2011 01:11 PM, vlj wrote:
> > BindBuffer* functions are part of tfb extension. They are however
> > used by others extensions such as uniform buffer object.
> > This patch moves the BindBuffer* definition to to bufferobj.c
> > where it acts as a dispatcher calling original tfb function ;
> > BindBuffer* functions can be used by others extensions, even if
> > FEATURE_EXT_transform_feedback is not defined.
> > ---
> > src/mesa/main/api_exec.c | 2 +
> > src/mesa/main/bufferobj.c | 144
> > +++++++++++++++++++++++++++++++++++++
> > src/mesa/main/bufferobj.h | 12 +++
> > src/mesa/main/transformfeedback.c | 109 +---------------------------
> > src/mesa/main/transformfeedback.h | 7 --
> > 5 files changed, 159 insertions(+), 115 deletions(-)
> >
> > diff --git a/src/mesa/main/api_exec.c b/src/mesa/main/api_exec.c
> > index 93214dd..0bbfa8b 100644
> > --- a/src/mesa/main/api_exec.c
> > +++ b/src/mesa/main/api_exec.c
> > @@ -590,6 +590,8 @@ _mesa_create_exec_table(void)
> > SET_IsBufferARB(exec, _mesa_IsBufferARB);
> > SET_MapBufferARB(exec, _mesa_MapBufferARB);
> > SET_UnmapBufferARB(exec, _mesa_UnmapBufferARB);
> > + SET_BindBufferRangeEXT(exec, _mesa_BindBufferRange);
> > + SET_BindBufferBaseEXT(exec, _mesa_BindBufferBase);
> > #endif
> >
> > /* ARB 29. GL_ARB_occlusion_query */
> > diff --git a/src/mesa/main/bufferobj.c b/src/mesa/main/bufferobj.c
> > index 431eafd..0908ce6 100644
> > --- a/src/mesa/main/bufferobj.c
> > +++ b/src/mesa/main/bufferobj.c
> > @@ -703,6 +703,150 @@ _mesa_BindBufferARB(GLenum target, GLuint buffer)
> > bind_buffer_object(ctx, target, buffer);
> > }
> >
> > +/**
> > + * Helper used by BindBufferRange() and BindBufferBase().
> > + */
> > +void
> > +bind_buffer_range(struct gl_context *ctx, GLuint index,
> > + struct gl_buffer_object *bufObj,
> > + GLintptr offset, GLsizeiptr size);
>
>
> bind_buffer_range() should be moved into bufferobj.c and it should be
> static. There's no reason to keep it in transformfeedback.c
>
I wanted to avoid moving BindBufferOffset, as I'm not sure it is shared
by another extension ; it calls bind_buffer_range.
>
> > +
> > +/**
> > + * Several extensions declare a BindBufferBase API function,
> > + * this one dispatchs call according to target
> > + */
> > +void GLAPIENTRY
> > +_mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer)
> > +{
> > +
> > +/**
> > + * Declare everything here to avoid declaring inside switch statement
> > + */
> > +#if FEATURE_EXT_transform_feedback
> > + struct gl_transform_feedback_object *obj;
> > + struct gl_buffer_object *bufObj;
> > + GLsizeiptr size;
> > +#endif
>
> Move these xfb-related variables down into the switch case - the only
> place they're used.
>
>
> > +
> > + GET_CURRENT_CONTEXT(ctx);
> > + switch (target) {
> > +#if FEATURE_EXT_transform_feedback
> > + case GL_TRANSFORM_FEEDBACK_BUFFER:
> > + /**
> > + * Specify a buffer object to receive vertex shader results.
> > + * As in BindBufferRange, but start at offset = 0.
> > + */
> > + obj = ctx->TransformFeedback.CurrentObject;
> > +
> > + if (obj->Active) {
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindBufferBase(transform feedback active)");
> > + return;
> > + }
> > +
> > + if (index>= ctx->Const.MaxTransformFeedbackSeparateAttribs) {
> > + _mesa_error(ctx, GL_INVALID_VALUE,
> > "glBindBufferBase(index=%d)", index);
> > + return;
> > + }
> > +
> > + bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> > + if (!bufObj) {
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindBufferBase(invalid buffer=%u)", buffer);
> > + return;
> > + }
>
> The buffer lookup and error check should be after the switch(target)
> because it'll be needed for UBO as well.
>
>
> > + /* default size is the buffer size rounded down to nearest
> > + * multiple of four.
> > + */
> > + size = bufObj->Size& ~0x3;
> > +
> > + bind_buffer_range(ctx, index, bufObj, 0, size);
>
> This call should also be placed after the switch.
bind_buffer_range is using tfb structure, I put it in another switch.
>
>
> > + break;
> > +#endif
> > + default:
> > + _mesa_error(ctx, GL_INVALID_ENUM, "glBindBufferBase(target)");
> > + break;
> > + }
> > + return;
> > +}
> > +
> > +extern void
> > +BindBufferRange_TFB(GLenum target, GLuint index,
> > + GLuint buffer, GLintptr offset, GLsizeiptr size);
> > +
> > +/**
> > + * Several extensions declare a BindBufferRange API function,
> > + * this one dispatchs call according to target
> > + */
> > +void GLAPIENTRY
> > +_mesa_BindBufferRange(GLenum target, GLuint index,
> > + GLuint buffer, GLintptr offset, GLsizeiptr size)
> > +{
> > +/**
> > + * Declare everything here to avoid declaring inside switch statement
> > + */
> > +#if FEATURE_EXT_transform_feedback
> > + struct gl_transform_feedback_object *obj;
> > + struct gl_buffer_object *bufObj;
> > +#endif
> > +
> > + GET_CURRENT_CONTEXT(ctx);
> > + switch (target) {
> > +#if FEATURE_EXT_transform_feedback
> > + case GL_TRANSFORM_FEEDBACK_BUFFER:
> > + /**
> > + * Specify a buffer object to receive vertex shader results.
> > Plus,
> > + * specify the starting offset to place the results, and max size.
> > + */
> > + obj = ctx->TransformFeedback.CurrentObject;
> > +
> > + if (obj->Active) {
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindBufferRange(transform feedback active)");
> > + return;
> > + }
> > +
> > + if (index>= ctx->Const.MaxTransformFeedbackSeparateAttribs) {
> > + _mesa_error(ctx, GL_INVALID_VALUE,
> > "glBindBufferRange(index=%d)", index);
> > + return;
> > + }
> > +
> > + if ((size<= 0) || (size& 0x3)) {
> > + /* must be positive and multiple of four */
> > + _mesa_error(ctx, GL_INVALID_VALUE,
> > "glBindBufferRange(size%d)", (int) size);
> > + return;
> > + }
> > +
> > + if (offset& 0x3) {
> > + /* must be multiple of four */
> > + _mesa_error(ctx, GL_INVALID_VALUE,
> > + "glBindBufferRange(offset=%d)", (int) offset);
> > + return;
> > + }
> > +
> > + bufObj = _mesa_lookup_bufferobj(ctx, buffer);
> > + if (!bufObj) {
> > + _mesa_error(ctx, GL_INVALID_OPERATION,
> > + "glBindBufferRange(invalid buffer=%u)", buffer);
> > + return;
> > + }
> > +
> > + if (offset + size>= bufObj->Size) {
> > + _mesa_error(ctx, GL_INVALID_VALUE,
> > + "glBindBufferRange(offset + size %d> buffer size
> > %d)",
> > + (int) (offset + size), (int) (bufObj->Size));
> > + return;
> > + }
>
> Again, the buffer lookup and error checks should be after the switch
> since we'll need them for UBO too. I'm not sure what UBO says, if
> anything, about buffer size being a multiple of four.
>
>
> > +
> > + bind_buffer_range(ctx, index, bufObj, offset, size);
> > + break;
> > +#endif
> > + default:
> > + _mesa_error(ctx, GL_INVALID_ENUM, "glBindBufferRange(target)");
> > + }
> > + return;
> > +}
> >
> > /**
> > * Delete a set of buffer objects.
> > diff --git a/src/mesa/main/bufferobj.h b/src/mesa/main/bufferobj.h
> > index b4e70f2..1b3c70c 100644
> > --- a/src/mesa/main/bufferobj.h
> > +++ b/src/mesa/main/bufferobj.h
> > @@ -154,6 +154,18 @@ _mesa_ObjectUnpurgeableAPPLE(GLenum objectType, GLuint
> > name, GLenum option);
> >
> > extern void GLAPIENTRY
> > _mesa_GetObjectParameterivAPPLE(GLenum objectType, GLuint name, GLenum
> > pname, GLint* params);
> > +
> > +
> > +/**
> > + * These functions dont fit anywhere else, declare them here as they are
> > related to buffers
> > + */
>
> I don't think that comment really adds any value. The functions in
> question clearly are buffer object functions.
>
>
> > +extern void GLAPIENTRY
> > +_mesa_BindBufferBase(GLenum target, GLuint index, GLuint buffer);
> > +
> > +extern void GLAPIENTRY
> > +_mesa_BindBufferRange(GLenum target, GLuint index,
> > + GLuint buffer, GLintptr offset, GLsizeiptr size);
> > +
> > #endif
>
>
> -Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
