These comments have been addressed.
On Tue, Dec 30, 2014 at 3:46 PM, Anuj Phogat <anuj.pho...@gmail.com> wrote:
> On Tue, Dec 16, 2014 at 7:46 AM, Brian Paul <bri...@vmware.com> wrote:
> > On 12/15/2014 06:22 PM, Laura Ekstrand wrote:
> >>
> >> The following preparations were made in texstate.c and texstate.h to
> >> better facilitate the BindTextureUnit function:
> >>
> >> Dylan Noblesmith:
> >> mesa: add _mesa_get_tex_unit()
> >> mesa: factor out _mesa_max_tex_unit()
> >> This is about to appear in a lot more places, so
> >> reduce boilerplate copy paste.
> >> add _mesa_get_tex_unit_err() checking getter function
> >> Reduce boilerplate across files.
> >>
> >> Laura Ekstrand:
> >> Made note of why BindTextureUnit should throw GL_INVALID_OPERATION if
> the
> >> unit is out of range.
> >> Added assert(unit > 0) to _mesa_get_tex_unit.
> >> ---
> >> src/mapi/glapi/gen/ARB_direct_state_access.xml | 5 ++
> >> src/mesa/main/texobj.c | 108
> >> ++++++++++++++++++++++++-
> >> src/mesa/main/texobj.h | 15 +++-
> >> src/mesa/main/texstate.c | 4 +-
> >> src/mesa/main/texstate.h | 39 ++++++++-
> >> 5 files changed, 160 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >> b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >> index 4c5005f..f54c3f8 100644
> >> --- a/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >> +++ b/src/mapi/glapi/gen/ARB_direct_state_access.xml
> >> @@ -75,5 +75,10 @@
> >> <param name="pixels" type="const GLvoid *" />
> >> </function>
> >>
> >> + <function name="BindTextureUnit" offset="assign">
> >> + <param name="unit" type="GLuint" />
> >> + <param name="texture" type="GLuint" />
> >> + </function>
> >> +
> >> </category>
> >> </OpenGLAPI>
> >> diff --git a/src/mesa/main/texobj.c b/src/mesa/main/texobj.c
> >> index 26d07ee..eb16f47 100644
> >> --- a/src/mesa/main/texobj.c
> >> +++ b/src/mesa/main/texobj.c
> >> @@ -482,8 +482,8 @@ valid_texture_object(const struct gl_texture_object
> >> *tex)
> >> * when there's a real pointer change.
> >> */
> >> void
> >> -_mesa_reference_texobj_(struct gl_texture_object **ptr,
> >> - struct gl_texture_object *tex)
> >> +_mesa_reference_texobj_( struct gl_texture_object **ptr,
> >> + struct gl_texture_object *tex )
> >
> >
> > I prefer the lack of whitespace as it was.
> This extra space is used at many other places in this patch.
>
> >
> >
> >> {
> >> assert(ptr);
> >>
> >> @@ -503,6 +503,8 @@ _mesa_reference_texobj_(struct gl_texture_object
> >> **ptr,
> >> mtx_unlock(&oldTex->Mutex);
> >>
> >> if (deleteFlag) {
> >> + /* Passing in the context drastically changes the driver code
> >> for
> >> + * framebuffer deletion. */
> >
> >
> > Closing */ on next line.
> >
> >
> >> GET_CURRENT_CONTEXT(ctx);
> >> if (ctx)
> >> ctx->Driver.DeleteTexture(ctx, oldTex);
> >> @@ -1571,6 +1573,108 @@ _mesa_BindTexture( GLenum target, GLuint
> texName )
> >> ctx->Driver.BindTexture(ctx, ctx->Texture.CurrentUnit, target,
> >> newTexObj);
> >> }
> >>
> >> +/** Do the actual binding to a numbered texture unit.
> >
> >
> > /**
> > * Do the ...
> >
> >
> >> + * The refcount on the previously bound
> >> + * texture object will be decremented. It'll be deleted if the
> >> + * count hits zero.
> >> + */
> >> +void
> >> +_mesa_bind_texture_unit( struct gl_context *ctx,
> >> + GLuint unit,
> >> + struct gl_texture_object *texObj )
> >> +{
> >> + struct gl_texture_unit *texUnit;
> >> +
> >> + /* Get the texture unit (this is an array look-up) */
> >> + texUnit = _mesa_get_tex_unit_err(ctx, unit, "glBindTextureUnit");
> >> + if (!texUnit)
> >> + return;
> >> +
> >> + /* Check if this texture is only used by this context and is already
> >> bound.
> >> + * If so, just return.
> >> + */
> >> + {
> >> + GLboolean early_out;
> >
> >
> > bool
> >
> >
> >> + mtx_lock(&ctx->Shared->Mutex);
> >> + early_out = ((ctx->Shared->RefCount == 1)
> >> + && (texObj ==
> >> texUnit->CurrentTex[texObj->TargetIndex]));
> >> + mtx_unlock(&ctx->Shared->Mutex);
> >> + if (early_out) {
> >> + return;
> >> + }
> >> + }
> >> +
> >> + /* flush before changing binding */
> >> + FLUSH_VERTICES(ctx, _NEW_TEXTURE);
> >> +
> >> + _mesa_reference_texobj(&texUnit->CurrentTex[texObj->TargetIndex],
> >> + texObj);
> >> + ASSERT(texUnit->CurrentTex[texObj->TargetIndex]);
> >> + ctx->Texture.NumCurrentTexUsed =
> MAX2(ctx->Texture.NumCurrentTexUsed,
> >> + unit + 1);
> >> + texUnit->_BoundTextures |= (1 << texObj->TargetIndex);
> >> +
> >> +
> >> + /* Pass BindTexture call to device driver */
> >> + if (ctx->Driver.BindTexture)
> >> + {
> >
> >
> > Opening { on the if line.
> >
> >
> >
> >> + ctx->Driver.BindTexture(ctx, unit, texObj->Target, texObj);
> >> + }
> >> +}
> >> +
> >> +/**
> >> + * Bind a named texture to the specified texture unit.
> >> + *
> >> + * \param unit texture unit.
> >> + * \param texture texture name.
> >> + *
> >> + * \sa glBindTexture().
> >> + *
> >> + * If the named texture is 0, this will reset each target for the
> >> specified
> >> + * texture unit to its default texture.
> >> + * If the named texture is not 0 or a recognized texture name, this
> >> throws
> >> + * GL_INVALID_OPERATION.
> >> + */
> >> +void GLAPIENTRY
> >> +_mesa_BindTextureUnit( GLuint unit, GLuint texture )
> >> +{
> >> + GET_CURRENT_CONTEXT(ctx);
> >> + struct gl_texture_object *texObj;
> >> +
> >> + if (MESA_VERBOSE & (VERBOSE_API|VERBOSE_TEXTURE))
> >> + _mesa_debug(ctx, "glBindTextureUnit %s %d\n",
> >> + _mesa_lookup_enum_by_nr(GL_TEXTURE0+unit), (GLint)
> >> texture);
> >> +
> >> + /* Section 8.1 (Texture Objects) of the OpenGL 4.5 core profile spec
> >> + * (20141030) says:
> >> + * "When texture is zero, each of the targets enumerated at the
> >> + * beginning of this section is reset to its default texture for
> >> the
> >> + * corresponding texture image unit."
> >> + */
> >> + if (texture == 0) {
> >> + unbind_textures_from_unit(ctx, unit);
> >> + return;
> >> + }
> >> +
> >> + /* Get the non-default texture object */
> >> + texObj = _mesa_lookup_texture(ctx, texture);
> >> +
> >> + /* Error checking */
> >> + if (!texObj) {
> >> + _mesa_error(ctx, GL_INVALID_OPERATION,
> >> + "glBindTextureUnit(non-gen name)");
> >> + return;
> >> + }
> >> + if (texObj->Target == 0) {
> >> + _mesa_error(ctx, GL_INVALID_ENUM, "glBindTextureUnit(target)");
> >> + return;
> >> + }
> >> + assert(valid_texture_object(texObj));
> >> +
> >> + _mesa_bind_texture_unit(ctx, unit, texObj);
> >> +}
> >> +
> >> +
> >> void GLAPIENTRY
> >> _mesa_BindTextures(GLuint first, GLsizei count, const GLuint
> *textures)
> >> {
> >> diff --git a/src/mesa/main/texobj.h b/src/mesa/main/texobj.h
> >> index b957ac5..34daebb 100644
> >> --- a/src/mesa/main/texobj.h
> >> +++ b/src/mesa/main/texobj.h
> >> @@ -85,12 +85,12 @@ _mesa_clear_texture_object(struct gl_context *ctx,
> >> struct gl_texture_object *obj);
> >>
> >> extern void
> >> -_mesa_reference_texobj_(struct gl_texture_object **ptr,
> >> - struct gl_texture_object *tex);
> >> +_mesa_reference_texobj_( struct gl_texture_object **ptr,
> >> + struct gl_texture_object *tex );
> >>
> >> static inline void
> >> -_mesa_reference_texobj(struct gl_texture_object **ptr,
> >> - struct gl_texture_object *tex)
> >> +_mesa_reference_texobj( struct gl_texture_object **ptr,
> >> + struct gl_texture_object *tex )
> >> {
> >> if (*ptr != tex)
> >> _mesa_reference_texobj_(ptr, tex);
> >> @@ -190,6 +190,11 @@ _mesa_unlock_context_textures( struct gl_context
> *ctx
> >> );
> >> extern void
> >> _mesa_lock_context_textures( struct gl_context *ctx );
> >>
> >> +extern void
> >> +_mesa_bind_texture_unit( struct gl_context *ctx,
> >> + GLuint unit,
> >> + struct gl_texture_object *texObj );
> >> +
> >> /*@}*/
> >>
> >> /**
> >> @@ -210,6 +215,8 @@ _mesa_DeleteTextures( GLsizei n, const GLuint
> >> *textures );
> >> extern void GLAPIENTRY
> >> _mesa_BindTexture( GLenum target, GLuint texture );
> >>
> >> +extern void GLAPIENTRY
> >> +_mesa_BindTextureUnit( GLuint unit, GLuint texture );
> >>
> >> extern void GLAPIENTRY
> >> _mesa_BindTextures( GLuint first, GLsizei count, const GLuint
> *textures
> >> );
> >> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> >> index 36eefa6..66fd718 100644
> >> --- a/src/mesa/main/texstate.c
> >> +++ b/src/mesa/main/texstate.c
> >> @@ -290,9 +290,7 @@ _mesa_ActiveTexture(GLenum texture)
> >> GLuint k;
> >> GET_CURRENT_CONTEXT(ctx);
> >>
> >> - /* See OpenGL spec for glActiveTexture: */
> >> - k = MAX2(ctx->Const.MaxCombinedTextureImageUnits,
> >> - ctx->Const.MaxTextureCoordUnits);
> >> + k = _mesa_max_tex_unit(ctx);
> >>
> >> ASSERT(k <= Elements(ctx->Texture.Unit));
> >>
> >> diff --git a/src/mesa/main/texstate.h b/src/mesa/main/texstate.h
> >> index 5cd1684..2514d10 100644
> >> --- a/src/mesa/main/texstate.h
> >> +++ b/src/mesa/main/texstate.h
> >> @@ -33,9 +33,19 @@
> >>
> >>
> >> #include "compiler.h"
> >> +#include "enums.h"
> >> +#include "macros.h"
> >> #include "mtypes.h"
> >>
> >>
> >> +static inline struct gl_texture_unit *
> >> +_mesa_get_tex_unit(struct gl_context *ctx, GLuint unit)
> >> +{
> >> + ASSERT(unit >= 0);
> >> + ASSERT(unit < Elements(ctx->Texture.Unit));
> >> + return &(ctx->Texture.Unit[unit]);
> >> +}
> >> +
> >> /**
> >> * Return pointer to current texture unit.
> >> * This the texture unit set by glActiveTexture(), not
> >> glClientActiveTexture().
> >> @@ -43,8 +53,33 @@
> >> static inline struct gl_texture_unit *
> >> _mesa_get_current_tex_unit(struct gl_context *ctx)
> >> {
> >> - ASSERT(ctx->Texture.CurrentUnit < Elements(ctx->Texture.Unit));
> >> - return &(ctx->Texture.Unit[ctx->Texture.CurrentUnit]);
> >> + return _mesa_get_tex_unit(ctx, ctx->Texture.CurrentUnit);
> >> +}
> >> +
> >> +static inline GLuint
> >> +_mesa_max_tex_unit(struct gl_context *ctx)
> >> +{
> >> + /* See OpenGL spec for glActiveTexture: */
> >> + return MAX2(ctx->Const.MaxCombinedTextureImageUnits,
> >> + ctx->Const.MaxTextureCoordUnits);
> >> +}
> >> +
> >> +static inline struct gl_texture_unit *
> >> +_mesa_get_tex_unit_err(struct gl_context *ctx, GLuint unit, const char
> >> *func)
> >> +{
> >> + if (unit < _mesa_max_tex_unit(ctx))
> >> + return _mesa_get_tex_unit(ctx, unit);
> >> +
> >> + /* Note: This error is a precedent set by glBindTextures. From the
> GL
> >> 4.5
> >> + * specification (30.10.2014) Section 8.1 ("Texture Objects"):
> >> + *
> >> + * "An INVALID_OPERATION error is generated if first + count is
> >> greater
> >> + * than the number of texture image units supported by the
> >> + * implementation."
> >> + */
> >> + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(unit=%s)", func,
> >> + _mesa_lookup_enum_by_nr(GL_TEXTURE0+unit));
> >> + return NULL;
> >> }
> >>
> >>
> >>
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
