[Mesa-dev] [PATCH 06/23] mesa: glGetProgramResourceLocation
Tapani Pälli
tapani.palli at intel.com
Tue Mar 17 02:09:39 PDT 2015
On 03/16/2015 07:08 PM, Ilia Mirkin wrote:
> On Fri, Mar 13, 2015 at 4:37 AM, Tapani Pälli <tapani.palli at intel.com> wrote:
>> Patch adds required helper functions to shaderapi.h and
>> the actual implementation.
>>
>> corresponding Piglit test:
>> arb_program_interface_query-resource-location
>>
>> The added functionality can be tested by tests for following
>> functions that are refactored by later patches:
>>
>> GetAttribLocation
>> GetUniformLocation
>> GetFragDataLocation
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>> src/mesa/main/program_resource.c | 81 +++++++++++++++++++++++++++++++++++++++-
>> src/mesa/main/shader_query.cpp | 64 +++++++++++++++++++++++++++++++
>> src/mesa/main/shaderapi.h | 4 ++
>> 3 files changed, 148 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
>> index 4fa6ac6..87a0144 100644
>> --- a/src/mesa/main/program_resource.c
>> +++ b/src/mesa/main/program_resource.c
>> @@ -243,11 +243,90 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum programInterface,
>> {
>> }
>>
>> +/**
>> + * Function verifies syntax of given name for GetProgramResourceLocation
>> + * and GetProgramResourceLocationIndex for the following cases:
>> + *
>> + * "array element portion of a string passed to GetProgramResourceLocation
>> + * or GetProgramResourceLocationIndex must not have, a "+" sign, extra
>> + * leading zeroes, or whitespace".
>> + *
>> + * Check is written to be compatible with GL_ARB_array_of_arrays.
>> + */
>> +static bool
>> +invalid_array_element_syntax(const GLchar *name)
>> +{
>> + char *array = strrchr(name, '[');
>> +
>> + if (!array)
>> + return false;
>> +
>> + /* No '+' or ' ' allowed anywhere. */
>> + if (strchr(name, '+') || strchr(name, ' '))
>
> I guess it'd be mildly better to do a strchr('[') and use that for the
> second strchr's? You could do it like
>
> char *first = strchr(name, '[');
> char *last = strrchr(first, '[');
> if (strchr(first, '+') || ... )
>
> That way you avoid iterating over the "name" portion of it
> unnecessarily. Probably doesn't amount to too much.
Yes, it is more optimal, will change.
>> + return true;
>> +
>> + /* Check that last array index is 0. */
>> + if (array[1] == '0' && array[2] != ']')
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +static struct gl_shader_program *
>> +lookup_linked_program(GLuint program, const char *caller)
>> +{
>> + GET_CURRENT_CONTEXT(ctx);
>> + struct gl_shader_program *prog =
>> + _mesa_lookup_shader_program_err(ctx, program, caller);
>> +
>> + if (!prog)
>> + return NULL;
>> +
>> + if (prog->LinkStatus == GL_FALSE) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> + "%s(program not linked)", caller);
>> + return NULL;
>> + }
>> + return prog;
>> +}
>> +
>> GLint GLAPIENTRY
>> _mesa_GetProgramResourceLocation(GLuint program, GLenum programInterface,
>> const GLchar *name)
>> {
>> - return -1;
>> + GET_CURRENT_CONTEXT(ctx);
>> + struct gl_shader_program *shProg =
>> + lookup_linked_program(program, "glGetProgramResourceLocation");
>> +
>> + if (!shProg || invalid_array_element_syntax(name))
>> + return -1;
>> +
>> + /* Validate programInterface. */
>> + switch (programInterface) {
>> + case GL_UNIFORM:
>> + case GL_PROGRAM_INPUT:
>> + case GL_PROGRAM_OUTPUT:
>> + break;
>> +
>> + /* For reference valid cases requiring addition extension support:
>> + * GL_ARB_shader_subroutine
>> + * GL_ARB_tessellation_shader
>> + * GL_ARB_compute_shader
>> + */
>> + case GL_VERTEX_SUBROUTINE_UNIFORM:
>> + case GL_TESS_CONTROL_SUBROUTINE_UNIFORM:
>> + case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM:
>> + case GL_GEOMETRY_SUBROUTINE_UNIFORM:
>> + case GL_FRAGMENT_SUBROUTINE_UNIFORM:
>> + case GL_COMPUTE_SUBROUTINE_UNIFORM:
>> +
>> + default:
>> + _mesa_error(ctx, GL_INVALID_ENUM,
>> + "glGetProgramResourceLocation(%s %s)",
>> + _mesa_lookup_enum_by_nr(programInterface), name);
>> + }
>> +
>> + return _mesa_program_resource_location(shProg, programInterface, name);
>> }
>>
>> GLint GLAPIENTRY
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index 77a4af0..4ae00a6 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -743,3 +743,67 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
>> }
>> return true;
>> }
>> +
>> +static GLint
>> +program_resource_location(struct gl_shader_program *shProg,
>> + struct gl_program_resource *res, const char *name)
>> +{
>> + unsigned index, offset;
>> + int array_index = -1;
>
> And I suppose leaving off the initializer here makes gcc complain?
Yep,"warning: 'array_index' may be used uninitialized in this function"
>> +
>> + if (res->Type == GL_PROGRAM_INPUT ||
>> + res->Type == GL_PROGRAM_OUTPUT) {
>
> put all on one line?
will fix
>> + array_index = array_index_of_resource(res, name);
>> + if (array_index < 0)
>> + return -1;
>> + }
>> +
>> + switch (res->Type) {
>> + case GL_PROGRAM_INPUT:
>> + return RESOURCE_VAR(res)->data.location + array_index - VERT_ATTRIB_GENERIC0;
>> + case GL_PROGRAM_OUTPUT:
>> + return RESOURCE_VAR(res)->data.location + array_index - FRAG_RESULT_DATA0;
>> + case GL_UNIFORM:
>> + index = _mesa_get_uniform_location(shProg, name, &offset);
>> +
>> + if (index == GL_INVALID_INDEX)
>> + return -1;
>> +
>> + /* From the GL_ARB_uniform_buffer_object spec:
>> + *
>> + * "The value -1 will be returned if <name> does not correspond to an
>> + * active uniform variable name in <program>, if <name> is associated
>> + * with a named uniform block, or if <name> starts with the reserved
>> + * prefix "gl_"."
>> + */
>> + if (RESOURCE_UNI(res)->block_index != -1 ||
>> + RESOURCE_UNI(res)->atomic_buffer_index != -1)
>> + return -1;
>> +
>> + /* location in remap table + array element offset */
>> + return RESOURCE_UNI(res)->remap_location + offset;
>> +
>> + default:
>> + return -1;
>> + }
>> +}
>> +
>> +/**
>> + * Function implements following location queries:
>> + * glGetAttribLocation
>> + * glGetFragDataLocation
>> + * glGetUniformLocation
>> + */
>> +GLint
>> +_mesa_program_resource_location(struct gl_shader_program *shProg,
>> + GLenum interface, const char *name)
>> +{
>> + struct gl_program_resource *res =
>> + _mesa_program_resource_find_name(shProg, interface, name);
>> +
>> + /* Resource not found. */
>> + if (!res)
>> + return -1;
>> +
>> + return program_resource_location(shProg, res, name);
>> +}
>> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
>> index 7a7e3e9..73ebf60 100644
>> --- a/src/mesa/main/shaderapi.h
>> +++ b/src/mesa/main/shaderapi.h
>> @@ -244,6 +244,10 @@ _mesa_get_program_resource_name(struct gl_shader_program *shProg,
>> GLsizei bufSize, GLsizei *length,
>> GLchar *name, const char *caller);
>>
>> +extern GLint
>> +_mesa_program_resource_location(struct gl_shader_program *shProg,
>> + GLenum interface, const char *name);
>> +
>> #ifdef __cplusplus
>> }
>> #endif
>> --
>> 2.1.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list