[Mesa-dev] [PATCH 3/3] mesa: Only flag _NEW_TEXTURE on texture updates when a texture changes.

[Brian Paul]

On 02/16/2012 04:40 PM, Eric Anholt wrote:
> There are three reasons left for this function flagging it:
> Re-computing completeness (and therefore the number of levels),
> binding a new current texture object, or you're doing some sort of
> fixed function (and nobody cares).  For other cases, like rebinding a
> shader, we no longer trigger the driver recomputing every piece of
> texture state.
>
> Improves a VS state change microbenchmark by 14.8284% +/- 1.02% (n=10)
> on gen7.  No statistically significant performance difference on
> 640x480 nexuiz (n=78).
>
> v2: Remove obsoleted comment, fix state flagging on transition to
>      !_ReallyEnabled by unreffing the object (which seems like a good
>      idea anyway).
> ---
>
> The unreffing appears to have cleared up my hang.  I'm thinking about
> how to write a testcase.
>
>   src/mesa/main/texstate.c |   36 ++++++++++++++++++++++++++----------
>   1 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c
> index 641e1f8..8c811d7 100644
> --- a/src/mesa/main/texstate.c
> +++ b/src/mesa/main/texstate.c
> @@ -592,9 +592,6 @@ update_texture_state( struct gl_context *ctx )
>       * FINISHME: here.
>       */
>
> -   /* TODO: only set this if there are actual changes */
> -   ctx->NewState |= _NEW_TEXTURE;
> -
>      /*
>       * Update texture unit state.
>       */
> @@ -604,6 +601,7 @@ update_texture_state( struct gl_context *ctx )
>         GLbitfield enabledFragTargets = 0x0;
>         GLbitfield enabledTargets = 0x0;
>         GLuint texIndex;
> +      struct gl_texture_object *oldObj = texUnit->_Current;
>
>         /* Get the bitmask of texture target enables.
>          * enableBits will be a mask of the TEXTURE_*_BIT flags indicating
> @@ -625,10 +623,18 @@ update_texture_state( struct gl_context *ctx )
>
>         enabledTargets = enabledVertTargets | enabledFragTargets;
>
> +      /* This is not flagged with _NEW_TEXTURE, because the test for changing of
> +       * the _Current texture object will cover every case where _ReallyEnabled
> +       * could have changed.
> +       */
>         texUnit->_ReallyEnabled = 0x0;
>
>         if (enabledTargets == 0x0) {
>            /* neither vertex nor fragment processing uses this unit */
> +	 if (texUnit->_Current) {
> +	    _mesa_reference_texobj(&texUnit->_Current, NULL);
> +	    ctx->NewState |= _NEW_TEXTURE;
> +	 }
>            continue;
>         }
>
> @@ -642,6 +648,10 @@ update_texture_state( struct gl_context *ctx )
>            if (enabledTargets&  (1<<  texIndex)) {
>               struct gl_texture_object *texObj = texUnit->CurrentTex[texIndex];
>               if (!texObj->_Complete) {
> +	       /* While _Complete is only consumed by us, this call also updates
> +		  _MaxLevel, which drivers do need to see.
> +		*/
> +	       ctx->NewState |= _NEW_TEXTURE;
>                  _mesa_test_texobj_completeness(ctx, texObj);
>               }
>               if (texObj->_Complete) {
> @@ -668,22 +678,26 @@ update_texture_state( struct gl_context *ctx )
>               _mesa_reference_texobj(&texUnit->_Current, texObj);
>               texUnit->_ReallyEnabled = 1<<  texTarget;
>            }
> -         else {
> -            /* fixed-function: texture unit is really disabled */
> -            continue;
> -         }
>         }
> +
> +      if (oldObj != texUnit->_Current)
> +	 ctx->NewState |= _NEW_TEXTURE;
>      }
>
>
>      /* Determine which texture coordinate sets are actually needed */
>      if (fprog) {
>         const GLuint coordMask = (1<<  MAX_TEXTURE_COORD_UNITS) - 1;
> +      GLuint new_coord_units = ((fprog->InputsRead>>  FRAG_ATTRIB_TEX0)&
> +				coordMask);

I have a feeling that MSVC is going to generate a warning here since 
InpusRead is a 64-bit field but new_coord_units is 32 bits.  If so, I 
can follow-up with a fix.


>         /* Note that this gets consumed by update_ffvs_texture_state(). */
> -      ctx->Texture._EnabledCoordUnits
> -         = (fprog->InputsRead>>  FRAG_ATTRIB_TEX0)&  coordMask;
> +      if (ctx->Texture._EnabledCoordUnits != new_coord_units) {
> +	 ctx->NewState |= _NEW_TEXTURE;
> +	 ctx->Texture._EnabledCoordUnits = new_coord_units;
> +      }
>      }
>      else {
> +      ctx->NewState |= _NEW_TEXTURE;
>         update_fffs_texture_state(ctx);
>      }
>
> @@ -691,8 +705,10 @@ update_texture_state( struct gl_context *ctx )
>       * 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)
> +   if (!vprog) {
> +      ctx->NewState |= _NEW_TEXTURE;
>         update_ffvs_texture_state(ctx);
> +   }
>   }
>
>

Reviewed-by: Brian Paul <brianp at vmware.com>