[Mesa-dev] [PATCH 12/29] tnl: Use bitmask/ffs to iterate enabled lights

[Ian Romanick]

On 05/23/2016 11:42 PM, Mathias.Froehlich at gmx.net wrote:
> From: Mathias Fröhlich <mathias.froehlich at web.de>
> 
> Replaces loops that iterate all lights and test
> which of them is enabled by a loop only iterating over
> the bits set in the enabled bitmask.
> ---
>  src/mesa/tnl/t_vb_light.c    |  2 +-
>  src/mesa/tnl/t_vb_lighttmp.h | 27 ++++++++++++++++++++-------
>  2 files changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/tnl/t_vb_light.c b/src/mesa/tnl/t_vb_light.c
> index 029265a..0cdb925 100644
> --- a/src/mesa/tnl/t_vb_light.c
> +++ b/src/mesa/tnl/t_vb_light.c
> @@ -394,7 +394,7 @@ static void validate_lighting( struct gl_context *ctx,
>  	 tab = _tnl_light_tab;
>     }
>     else {
> -      if (ctx->Light.EnabledList.next == ctx->Light.EnabledList.prev)
> +      if (_mesa_bitcount(ctx->Light._EnabledLights) == 1)

I think using an is_pow2() predicate here would be better.  It looks
like the only one in Mesa is in src/intel/isl/isl_priv.h, but it should
be easy enough to add that to somewhere in src/util.

>  	 tab = _tnl_light_fast_single_tab;
>        else
>  	 tab = _tnl_light_fast_tab;
> diff --git a/src/mesa/tnl/t_vb_lighttmp.h b/src/mesa/tnl/t_vb_lighttmp.h
> index 3aebcd4..191f65c 100644
> --- a/src/mesa/tnl/t_vb_lighttmp.h
> +++ b/src/mesa/tnl/t_vb_lighttmp.h
> @@ -87,7 +87,7 @@ static void TAG(light_rgba_spec)( struct gl_context *ctx,
>  
>     for (j = 0; j < nr; j++,STRIDE_F(vertex,vstride),STRIDE_F(normal,nstride)) {
>        GLfloat sum[2][3], spec[2][3];
> -      struct gl_light *light;
> +      GLbitfield mask;
>  
>  #if IDX & LIGHT_MATERIAL
>        update_materials( ctx, store );
> @@ -106,7 +106,10 @@ static void TAG(light_rgba_spec)( struct gl_context *ctx,
>  #endif
>  
>        /* Add contribution from each enabled light source */
> -      foreach (light, &ctx->Light.EnabledList) {
> +      mask = ctx->Light._EnabledLights;
> +      while (mask) {
> +         int l = ffs(mask) - 1;
> +         struct gl_light *light = &ctx->Light.Light[l];

Same comments about making l const and moving the declaration of mask.

>  	 GLfloat n_dot_h;
>  	 GLfloat correction;
>  	 GLint side;
> @@ -115,6 +118,7 @@ static void TAG(light_rgba_spec)( struct gl_context *ctx,
>  	 GLfloat VP[3];          /* unit vector from vertex to light */
>  	 GLfloat n_dot_VP;       /* n dot VP */
>  	 GLfloat *h;
> +         mask ^= (1u << l);
>  
>  	 /* compute VP and attenuation */
>  	 if (!(light->_Flags & LIGHT_POSITIONAL)) {
> @@ -265,7 +269,7 @@ static void TAG(light_rgba)( struct gl_context *ctx,
>  
>     for (j = 0; j < nr; j++,STRIDE_F(vertex,vstride),STRIDE_F(normal,nstride)) {
>        GLfloat sum[2][3];
> -      struct gl_light *light;
> +      GLbitfield mask;
>  
>  #if IDX & LIGHT_MATERIAL
>        update_materials( ctx, store );
> @@ -282,7 +286,10 @@ static void TAG(light_rgba)( struct gl_context *ctx,
>  #endif
>  
>        /* Add contribution from each enabled light source */
> -      foreach (light, &ctx->Light.EnabledList) {
> +      mask = ctx->Light._EnabledLights;
> +      while (mask) {
> +         int l = ffs(mask) - 1;
> +         struct gl_light *light = &ctx->Light.Light[l];
>  	 GLfloat n_dot_h;
>  	 GLfloat correction;
>  	 GLint side;
> @@ -291,6 +298,7 @@ static void TAG(light_rgba)( struct gl_context *ctx,
>  	 GLfloat VP[3];          /* unit vector from vertex to light */
>  	 GLfloat n_dot_VP;       /* n dot VP */
>  	 GLfloat *h;
> +         mask ^= (1u << l);
>  
>  	 /* compute VP and attenuation */
>  	 if (!(light->_Flags & LIGHT_POSITIONAL)) {
> @@ -417,7 +425,8 @@ static void TAG(light_fast_rgba_single)( struct gl_context *ctx,
>  #if IDX & LIGHT_TWOSIDE
>     GLfloat (*Bcolor)[4] = (GLfloat (*)[4]) store->LitColor[1].data;
>  #endif
> -   const struct gl_light *light = ctx->Light.EnabledList.next;
> +   const struct gl_light *light =
> +      &ctx->Light.Light[ffs(ctx->Light._EnabledLights) - 1];
>     GLuint j = 0;
>     GLfloat base[2][4];
>  #if IDX & LIGHT_MATERIAL
> @@ -528,7 +537,6 @@ static void TAG(light_fast_rgba)( struct gl_context *ctx,
>  #else
>     const GLuint nr = VB->AttribPtr[_TNL_ATTRIB_NORMAL]->count;
>  #endif
> -   const struct gl_light *light;
>  
>  #ifdef TRACE
>     fprintf(stderr, "%s %d\n", __func__, nr );
> @@ -556,6 +564,7 @@ static void TAG(light_fast_rgba)( struct gl_context *ctx,
>     for (j = 0; j < nr; j++, STRIDE_F(normal,nstride)) {
>  
>        GLfloat sum[2][3];
> +      GLbitfield mask;
>  
>  #if IDX & LIGHT_MATERIAL
>        update_materials( ctx, store );
> @@ -572,8 +581,12 @@ static void TAG(light_fast_rgba)( struct gl_context *ctx,
>        COPY_3V(sum[1], ctx->Light._BaseColor[1]);
>  #endif
>  
> -      foreach (light, &ctx->Light.EnabledList) {
> +      mask = ctx->Light._EnabledLights;
> +      while (mask) {
> +         int l = ffs(mask) - 1;
> +         const struct gl_light *light = &ctx->Light.Light[l];
>  	 GLfloat n_dot_h, n_dot_VP, spec;
> +         mask ^= (1u << l);
>  
>  	 ACC_3V(sum[0], light->_MatAmbient[0]);
>  #if IDX & LIGHT_TWOSIDE
>