[Mesa-dev] [PATCH 1/5] mesa/subroutines: start adding per-context subroutine index support

[Andres Gomez]

On Tue, 2016-06-07 at 15:25 +1000, Dave Airlie wrote:

[snip]

> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 471d41d..a7ffbf7 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -4307,6 +4307,15 @@ struct gl_atomic_buffer_binding
>  };
>  
>  /**
> + * Shader subroutines storage
> + */
> +struct gl_subroutine_index_binding
> +{
> +   int NumIndex;
> +   int *IndexPtr;
> +};

Nitpick: maybe GLuint, instead of int ?

[snip]

> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 9d440a0..818a88d 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c

[snip]

> @@ -2803,29 +2787,60 @@ find_compat_subroutine(struct gl_shader *sh, const struct glsl_type *type)
>  }
>  
>  static void
> -_mesa_shader_init_subroutine_defaults(struct gl_shader *sh)
> +_mesa_shader_write_subroutine_index(struct gl_context *ctx,
> +                                    struct gl_shader *sh)
>  {
>     int i, j;
>  
> -   for (i = 0; i < sh->NumSubroutineUniformRemapTable; i++) {
> +   if (sh->NumSubroutineUniformRemapTable == 0)
> +      return;
> +
> +   i = 0;
> +   do {
>        struct gl_uniform_storage *uni = sh->SubroutineUniformRemapTable[i];
>        int uni_count;
>        int val;
>  
> -      if (!uni)
> +      if (!uni) {
> +         i++;
>           continue;
> +      }

Nitpick: we could add a new empty line here.

>        uni_count = uni->array_elements ? uni->array_elements : 1;
> -      val = find_compat_subroutine(sh, uni->type);
> -
> -      for (j = 0; j < uni_count; j++)
> +      for (j = 0; j < uni_count; j++) {
> +         val = ctx->SubroutineIndex[sh->Stage].IndexPtr[i + j];
>           memcpy(&uni->storage[j], &val, sizeof(int));
> +      }
>  
>        _mesa_propagate_uniforms_to_driver_storage(uni, 0, uni_count);
> +      i += uni_count;
> +   } while(i < sh->NumSubroutineUniformRemapTable);
> +}
> +
> +static void
> +_mesa_shader_init_subroutine_defaults(struct gl_context *ctx,
> +                                      struct gl_shader *sh)
> +{
> +   int i;
> +
> +   if (ctx->SubroutineIndex[sh->Stage].NumIndex != sh->NumSubroutineUniformRemapTable) {
> +      ctx->SubroutineIndex[sh->Stage].IndexPtr = realloc(ctx->SubroutineIndex[sh->Stage].IndexPtr, sh->NumSubroutineUniformRemapTable * (sizeof(int)));

Nitpick: maybe (sizeof(GLuint)) instead ? (Depending on the comment
above).

> +      ctx->SubroutineIndex[sh->Stage].NumIndex = sh->NumSubroutineUniformRemapTable;
> +   }
> +
> +   for (i = 0; i < sh->NumSubroutineUniformRemapTable; i++) {
> +      struct gl_uniform_storage *uni = sh->SubroutineUniformRemapTable[i];
> +
> +      if (!uni)
> +         continue;

Nitpick: we could add a new empty line here.

[snip]

As a final nitpick, there are a lot of new really long lines. Maybe we
can give some love to the length limit ☺

Other than the nitpicks, this is:

Reviewed-by: Andres Gomez <agomez at igalia.com>

-- 

Br,

Andres