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 <ago...@igalia.com>
--
Br,
Andres
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
