[Mesa-dev] [PATCH 1/2] mesa: Avoid computing fixed function texture state setup if possible.

[Brian Paul]

On 02/15/2012 07:01 PM, Eric Anholt wrote:
> None of the consumers of this state will be called while the fs or vs
> is in place, and the update_texture_state() call will get re-called on
> fs/vs change, so it should be safe to skip the computation.  Improves
> the performance of a VS state change microbenchmark by 1.60186% +/-
> 0.443318% (n=20).
> ---
>   src/mesa/main/texstate.c |  117 +++++++++++++++++++++++++++------------------
>   1 files changed, 70 insertions(+), 47 deletions(-)

This looks good.  Minor comment below.


> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> index 187ec9c..2dbf530 100644
> --- a/src/mesa/main/texstate.c
> +++ b/src/mesa/main/texstate.c
> @@ -476,6 +476,68 @@ update_tex_combine(struct gl_context *ctx, struct gl_texture_unit *texUnit)
>      }
>   }
>

It would be good if the following two functions had comments 
indicating that the _ReallyEnabled and _EnabledCoordUnits field must 
have already been computed first (as well as any other dependencies).

> +static void
> +update_fffs_texture_state(struct gl_context *ctx)
> +{
> +   GLuint unit;
> +
> +   ctx->Texture._EnabledUnits = 0;
> +   ctx->Texture._EnabledCoordUnits = 0;
> +
> +   for (unit = 0; unit<  ctx->Const.MaxCombinedTextureImageUnits; unit++) {
> +      struct gl_texture_unit *texUnit =&ctx->Texture.Unit[unit];
> +
> +      if (!texUnit->_ReallyEnabled)
> +	 continue;
> +
> +      ctx->Texture._EnabledUnits |= (1<<  unit);
> +      ctx->Texture._EnabledCoordUnits |= (1<<  unit);
> +
> +      update_tex_combine(ctx, texUnit);
> +   }
> +}
> +
> +static void
> +update_ffvs_texture_state(struct gl_context *ctx)
> +{
> +   GLuint unit;
> +
> +   ctx->Texture._GenFlags = 0x0;
> +   ctx->Texture._TexMatEnabled = 0x0;
> +   ctx->Texture._TexGenEnabled = 0x0;
> +
> +   /* Setup texgen for those texture coordinate sets that are in use */
> +   for (unit = 0; unit<  ctx->Const.MaxTextureCoordUnits; unit++) {
> +      struct gl_texture_unit *texUnit =&ctx->Texture.Unit[unit];
> +
> +      texUnit->_GenFlags = 0x0;
> +
> +      if (!(ctx->Texture._EnabledCoordUnits&  (1<<  unit)))
> +	 continue;
> +
> +      if (texUnit->TexGenEnabled) {
> +	 if (texUnit->TexGenEnabled&  S_BIT) {
> +	    texUnit->_GenFlags |= texUnit->GenS._ModeBit;
> +	 }
> +	 if (texUnit->TexGenEnabled&  T_BIT) {
> +	    texUnit->_GenFlags |= texUnit->GenT._ModeBit;
> +	 }
> +	 if (texUnit->TexGenEnabled&  R_BIT) {
> +	    texUnit->_GenFlags |= texUnit->GenR._ModeBit;
> +	 }
> +	 if (texUnit->TexGenEnabled&  Q_BIT) {
> +	    texUnit->_GenFlags |= texUnit->GenQ._ModeBit;
> +	 }
> +
> +	 ctx->Texture._TexGenEnabled |= ENABLE_TEXGEN(unit);
> +	 ctx->Texture._GenFlags |= texUnit->_GenFlags;
> +      }
> +
> +      ASSERT(unit<  Elements(ctx->TextureMatrixStack));
> +      if (ctx->TextureMatrixStack[unit].Top->type != MATRIX_IDENTITY)
> +	 ctx->Texture._TexMatEnabled |= ENABLE_TEXMAT(unit);
> +   }
> +}
>
>   /**
>    * \note This routine refers to derived texture matrix values to
> @@ -491,7 +553,6 @@ update_texture_state( struct gl_context *ctx )
>      GLuint unit;
>      struct gl_program *fprog = NULL;
>      struct gl_program *vprog = NULL;
> -   GLbitfield enabledFragUnits = 0x0;
>
>      if (ctx->Shader.CurrentVertexProgram&&
>          ctx->Shader.CurrentVertexProgram->LinkStatus) {
> @@ -518,11 +579,6 @@ update_texture_state( struct gl_context *ctx )
>      /* TODO: only set this if there are actual changes */
>      ctx->NewState |= _NEW_TEXTURE;
>
> -   ctx->Texture._EnabledUnits = 0x0;
> -   ctx->Texture._GenFlags = 0x0;
> -   ctx->Texture._TexMatEnabled = 0x0;
> -   ctx->Texture._TexGenEnabled = 0x0;
> -
>      /*
>       * Update texture unit state.
>       */
> @@ -601,59 +657,26 @@ update_texture_state( struct gl_context *ctx )
>               continue;
>            }
>         }
> -
> -      /* if we get here, we know this texture unit is enabled */
> -
> -      ctx->Texture._EnabledUnits |= (1<<  unit);
> -
> -      if (enabledFragTargets)
> -         enabledFragUnits |= (1<<  unit);
> -
> -      update_tex_combine(ctx, texUnit);
>      }
>
>
>      /* Determine which texture coordinate sets are actually needed */
>      if (fprog) {
>         const GLuint coordMask = (1<<  MAX_TEXTURE_COORD_UNITS) - 1;
> +      /* Note that this gets consumed by update_ffvs_texture_state(). */
>         ctx->Texture._EnabledCoordUnits
>            = (fprog->InputsRead>>  FRAG_ATTRIB_TEX0)&  coordMask;
>      }
>      else {
> -      ctx->Texture._EnabledCoordUnits = enabledFragUnits;
> +      update_fffs_texture_state(ctx);
>      }
>
> -   /* Setup texgen for those texture coordinate sets that are in use */
> -   for (unit = 0; unit<  ctx->Const.MaxTextureCoordUnits; unit++) {
> -      struct gl_texture_unit *texUnit =&ctx->Texture.Unit[unit];
> -
> -      texUnit->_GenFlags = 0x0;
> -
> -      if (!(ctx->Texture._EnabledCoordUnits&  (1<<  unit)))
> -	 continue;
> -
> -      if (texUnit->TexGenEnabled) {
> -	 if (texUnit->TexGenEnabled&  S_BIT) {
> -	    texUnit->_GenFlags |= texUnit->GenS._ModeBit;
> -	 }
> -	 if (texUnit->TexGenEnabled&  T_BIT) {
> -	    texUnit->_GenFlags |= texUnit->GenT._ModeBit;
> -	 }
> -	 if (texUnit->TexGenEnabled&  R_BIT) {
> -	    texUnit->_GenFlags |= texUnit->GenR._ModeBit;
> -	 }
> -	 if (texUnit->TexGenEnabled&  Q_BIT) {
> -	    texUnit->_GenFlags |= texUnit->GenQ._ModeBit;
> -	 }
> -
> -	 ctx->Texture._TexGenEnabled |= ENABLE_TEXGEN(unit);
> -	 ctx->Texture._GenFlags |= texUnit->_GenFlags;
> -      }
> -
> -      ASSERT(unit<  Elements(ctx->TextureMatrixStack));
> -      if (ctx->TextureMatrixStack[unit].Top->type != MATRIX_IDENTITY)
> -	 ctx->Texture._TexMatEnabled |= ENABLE_TEXMAT(unit);
> -   }
> +   /* Skip texgen and texmat computed state setup in the presence of a vertex
> +    * program.  Those state flags are only used in the case of fixed function
> +    * vertex shading, in the tnl pipeline or ff_vertex_shader.
> +    */
> +   if (!vprog)
> +      update_ffvs_texture_state(ctx);
>   }
>
>