<div dir="ltr">These comments have been addressed. </div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Dec 30, 2014 at 3:46 PM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Dec 16, 2014 at 7:46 AM, Brian Paul <<a href="mailto:brianp@vmware.com">brianp@vmware.com</a>> 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. </div></div>This extra space is used at many other places in this patch. <div><div class="h5"> > > >> { >> 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; >> } >> >> >> > </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>