[Mesa-dev] [PATCH] mesa: fix ARRAY_SIZE query for GetProgramResourceiv
Martin Peres
martin.peres at linux.intel.com
Tue Sep 29 02:30:35 PDT 2015
On 28/09/15 13:51, Tapani Pälli wrote:
> Patch also refactors name length queries which were using array size
> in computation, this has to be done in same time to avoid regression in
> arb_program_interface_query-resource-query Piglit test.
>
> Fixes rest of the failures with
> ES31-CTS.program_interface_query.no-locations
>
> v2: make additional check only for GS inputs
> v3: create helper function for resource name length
> so that it gets calculated only in one place
>
> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com> [v2]
> ---
> src/mesa/main/program_resource.c | 8 ++--
> src/mesa/main/shader_query.cpp | 94 ++++++++++++++++++++++++----------------
> src/mesa/main/shaderapi.h | 3 ++
> 3 files changed, 62 insertions(+), 43 deletions(-)
>
> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
> index c609abe..eb71fdd 100644
> --- a/src/mesa/main/program_resource.c
> +++ b/src/mesa/main/program_resource.c
> @@ -111,11 +111,9 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum programInterface,
> for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++) {
> if (shProg->ProgramResourceList[i].Type != programInterface)
> continue;
> - const char *name =
> - _mesa_program_resource_name(&shProg->ProgramResourceList[i]);
> - unsigned array_size =
> - _mesa_program_resource_array_size(&shProg->ProgramResourceList[i]);
> - *params = MAX2(*params, strlen(name) + (array_size ? 3 : 0) + 1);
> + unsigned len =
> + _mesa_program_resource_name_len(&shProg->ProgramResourceList[i]);
> + *params = MAX2(*params, len + 1);
> }
> break;
> case GL_MAX_NUM_ACTIVE_VARIABLES:
> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
> index 99d9e10..a5572ff 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -478,7 +478,7 @@ _mesa_program_resource_array_size(struct gl_program_resource *res)
> RESOURCE_XFB(res)->Size : 0;
> case GL_PROGRAM_INPUT:
> case GL_PROGRAM_OUTPUT:
> - return RESOURCE_VAR(res)->data.max_array_access;
> + return RESOURCE_VAR(res)->type->length;
> case GL_UNIFORM:
> case GL_VERTEX_SUBROUTINE_UNIFORM:
> case GL_GEOMETRY_SUBROUTINE_UNIFORM:
> @@ -670,6 +670,57 @@ _mesa_program_resource_find_index(struct gl_shader_program *shProg,
> return NULL;
> }
>
> +/* Function returns if resource name is expected to have index
> + * appended into it.
> + *
> + *
> + * Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
> + * spec says:
> + *
> + * "If the active uniform is an array, the uniform name returned in
> + * name will always be the name of the uniform array appended with
> + * "[0]"."
> + *
> + * The same text also appears in the OpenGL 4.2 spec. It does not,
> + * however, appear in any previous spec. Previous specifications are
> + * ambiguous in this regard. However, either name can later be passed
> + * to glGetUniformLocation (and related APIs), so there shouldn't be any
> + * harm in always appending "[0]" to uniform array names.
> + *
> + * Geometry shader stage has different naming convention where the 'normal'
> + * condition is an array, therefore for variables referenced in geometry
> + * stage we do not add '[0]'.
> + *
> + * Note, that TCS outputs and TES inputs should not have index appended
> + * either.
> + */
> +bool
> +add_index_to_name(struct gl_program_resource *res)
static please!
With this, you get:
Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
Thanks for the cleanup!
> +{
> + bool add_index = !(((res->Type == GL_PROGRAM_INPUT) &&
> + res->StageReferences & (1 << MESA_SHADER_GEOMETRY)));
> +
> + /* Transform feedback varyings have array index already appended
> + * in their names.
> + */
> + if (res->Type == GL_TRANSFORM_FEEDBACK_VARYING)
> + add_index = false;
> +
> + return add_index;
> +}
> +
> +/* Get name length of a program resource. This consists of
> + * base name + 3 for '[0]' if resource is an array.
> + */
> +extern unsigned
> +_mesa_program_resource_name_len(struct gl_program_resource *res)
> +{
> + unsigned length = strlen(_mesa_program_resource_name(res));
> + if (_mesa_program_resource_array_size(res) && add_index_to_name(res))
> + length += 3;
> + return length;
> +}
> +
> /* Get full name of a program resource.
> */
> bool
> @@ -705,36 +756,7 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
>
> _mesa_copy_string(name, bufSize, length, _mesa_program_resource_name(res));
>
> - /* Page 61 (page 73 of the PDF) in section 2.11 of the OpenGL ES 3.0
> - * spec says:
> - *
> - * "If the active uniform is an array, the uniform name returned in
> - * name will always be the name of the uniform array appended with
> - * "[0]"."
> - *
> - * The same text also appears in the OpenGL 4.2 spec. It does not,
> - * however, appear in any previous spec. Previous specifications are
> - * ambiguous in this regard. However, either name can later be passed
> - * to glGetUniformLocation (and related APIs), so there shouldn't be any
> - * harm in always appending "[0]" to uniform array names.
> - *
> - * Geometry shader stage has different naming convention where the 'normal'
> - * condition is an array, therefore for variables referenced in geometry
> - * stage we do not add '[0]'.
> - *
> - * Note, that TCS outputs and TES inputs should not have index appended
> - * either.
> - */
> - bool add_index = !(((programInterface == GL_PROGRAM_INPUT) &&
> - res->StageReferences & (1 << MESA_SHADER_GEOMETRY)));
> -
> - /* Transform feedback varyings have array index already appended
> - * in their names.
> - */
> - if (programInterface == GL_TRANSFORM_FEEDBACK_VARYING)
> - add_index = false;
> -
> - if (add_index && _mesa_program_resource_array_size(res)) {
> + if (_mesa_program_resource_array_size(res) && add_index_to_name(res)) {
> int i;
>
> /* The comparison is strange because *length does *NOT* include the
> @@ -1205,13 +1227,9 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
> switch (res->Type) {
> case GL_ATOMIC_COUNTER_BUFFER:
> goto invalid_operation;
> - case GL_TRANSFORM_FEEDBACK_VARYING:
> - *val = strlen(_mesa_program_resource_name(res)) + 1;
> - break;
> default:
> - /* Base name +3 if array '[0]' + terminator. */
> - *val = strlen(_mesa_program_resource_name(res)) +
> - (_mesa_program_resource_array_size(res) > 0 ? 3 : 0) + 1;
> + /* Resource name length + terminator. */
> + *val = _mesa_program_resource_name_len(res) + 1;
> }
> return 1;
> case GL_TYPE:
> @@ -1238,7 +1256,7 @@ _mesa_program_resource_prop(struct gl_shader_program *shProg,
> return 1;
> case GL_PROGRAM_INPUT:
> case GL_PROGRAM_OUTPUT:
> - *val = MAX2(RESOURCE_VAR(res)->type->length, 1);
> + *val = MAX2(_mesa_program_resource_array_size(res), 1);
> return 1;
> case GL_TRANSFORM_FEEDBACK_VARYING:
> *val = MAX2(RESOURCE_XFB(res)->Size, 1);
> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
> index 0a10191..fba767b 100644
> --- a/src/mesa/main/shaderapi.h
> +++ b/src/mesa/main/shaderapi.h
> @@ -245,6 +245,9 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
> GLsizei bufSize, GLsizei *length,
> GLchar *name, const char *caller);
>
> +extern unsigned
> +_mesa_program_resource_name_len(struct gl_program_resource *res);
> +
> extern GLint
> _mesa_program_resource_location(struct gl_shader_program *shProg,
> GLenum programInterface, const char *name);
More information about the mesa-dev
mailing list