[Mesa-dev] [PATCH 04/23] mesa: glGetProgramResourceIndex
Tapani Pälli
tapani.palli at intel.com
Sun Mar 15 23:40:07 PDT 2015
On 03/13/2015 10:18 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-index
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>> src/mesa/main/program_resource.c | 91 +++++++++++++++++++++++++++++++++++++++-
>> src/mesa/main/shader_query.cpp | 91 ++++++++++++++++++++++++++++++++++++++++
>> src/mesa/main/shaderapi.h | 8 ++++
>> 3 files changed, 189 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
>> index a9194d1..4190f98 100644
>> --- a/src/mesa/main/program_resource.c
>> +++ b/src/mesa/main/program_resource.c
>> @@ -110,11 +110,100 @@ _mesa_GetProgramInterfaceiv(GLuint program, GLenum programInterface,
>> }
>> }
>>
>> +static bool
>> +is_xfb_marker(const char *str)
>> +{
>> + const char *markers[] = {
>
> static const char * please.
>
>> + "gl_NextBuffer",
>> + "gl_SkipComponents1",
>> + "gl_SkipComponents2",
>> + "gl_SkipComponents3",
>> + "gl_SkipComponents4",
>> + NULL
>> + };
>> + const char **m = markers;
>
> One silly optimization -- check if str starts with gl_. If not, no
> point in checking the rest. I assume that the majority of the calls
> *won't* have this, since it's an error to use it.
True, will add this.
>> +
>> + for (; *m; m++)
>> + if (strcmp(*m, str) == 0)
>> + return true;
>> +
>> + return false;
>> +}
>> +
>> +/**
>> + * Checks if given name index is legal for GetProgramResourceIndex,
>> + * check is written to be compatible with GL_ARB_array_of_arrays.
>> + */
>> +static bool
>> +valid_program_resource_index_name(const GLchar *name)
>> +{
>> + const char *array = strstr(name, "[");
>> + const char *close = strrchr(name, ']');
>> +
>> + /* Not array, no need for the check. */
>> + if (!array)
>> + return true;
>> +
>> + /* Last array index has to be zero. */
>> + if (!close || *--close != '0')
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> GLuint GLAPIENTRY
>> _mesa_GetProgramResourceIndex(GLuint program, GLenum programInterface,
>> const GLchar *name)
>> {
>> - return 0;
>> + GET_CURRENT_CONTEXT(ctx);
>> + struct gl_program_resource *res;
>> +
>> + struct gl_shader_program *shProg =
>> + _mesa_lookup_shader_program_err(ctx, program,
>> + "glGetProgramResourceIndex");
>> + if (!shProg)
>> + return GL_INVALID_INDEX;
>> +
>> + /*
>> + * For the interface TRANSFORM_FEEDBACK_VARYING, the value INVALID_INDEX
>> + * should be returned when querying the index assigned to the special names
>> + * "gl_NextBuffer", "gl_SkipComponents1", "gl_SkipComponents2",
>> + * "gl_SkipComponents3", and "gl_SkipComponents4".
>> + */
>> + if (programInterface == GL_TRANSFORM_FEEDBACK_VARYING &&
>> + is_xfb_marker(name))
>> + return GL_INVALID_INDEX;
>> +
>> + if (programInterface == GL_ATOMIC_COUNTER_BUFFER) {
>
> Below you still have checks for GL_ATOMIC_COUNTER_BUFFER... why not
> just move the 5 lines below this if into the switch statement, and
> remove GL_ATOMIC_COUNTER_BUFFER from the switch, and also remove this
> whole if statement. That should achieve the desired effect, no?
Yes it can be removed, will do. Thanks for review!
>> + _mesa_error(ctx, GL_INVALID_ENUM,
>> + "glGetProgramResourceIndex(%s name %s)",
>> + _mesa_lookup_enum_by_nr(programInterface), name);
>> + return GL_INVALID_INDEX;
>> + }
>> +
>> + /* Validate name syntax for arrays. */
>> + if (!valid_program_resource_index_name(name))
>> + return GL_INVALID_INDEX;
>> +
>> + res = _mesa_program_resource_find_name(shProg, programInterface, name);
>> + if (!res)
>> + return GL_INVALID_INDEX;
>> +
>> + switch (programInterface) {
>> + case GL_PROGRAM_INPUT:
>> + case GL_PROGRAM_OUTPUT:
>> + case GL_UNIFORM:
>> + case GL_UNIFORM_BLOCK:
>> + case GL_TRANSFORM_FEEDBACK_VARYING:
>> + case GL_ATOMIC_COUNTER_BUFFER:
>> + return _mesa_program_resource_index(shProg, res);
>> + default:
>> + _mesa_error(ctx, GL_INVALID_ENUM,
>> + "glGetProgramResourceIndex(%s)",
>> + _mesa_lookup_enum_by_nr(programInterface));
>> + }
>> +
>> + return GL_INVALID_INDEX;
>> }
>>
>> void GLAPIENTRY
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index 9df793e..d1974a4 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -556,3 +556,94 @@ _mesa_program_resource_array_size(struct gl_program_resource *res)
>> }
>> return 0;
>> }
>> +
>> +static int
>> +array_index_of_resource(struct gl_program_resource *res,
>> + const char *name)
>> +{
>> + assert(res->Data);
>> +
>> + switch (res->Type) {
>> + case GL_PROGRAM_INPUT:
>> + case GL_PROGRAM_OUTPUT:
>> + return get_matching_index(RESOURCE_VAR(res), name);
>> + default:
>> + assert(!"support for resource type not implemented");
>> + }
>> +}
>> +
>> +/* Find a program resource with specific name in given interface.
>> + */
>> +struct gl_program_resource *
>> +_mesa_program_resource_find_name(struct gl_shader_program *shProg,
>> + GLenum interface, const char *name)
>> +{
>> + struct gl_program_resource *res = shProg->ProgramResourceList;
>> + for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++) {
>> + if (res->Type != interface)
>> + continue;
>> +
>> + /* Resource basename. */
>> + const char *rname = _mesa_program_resource_name(res);
>> + unsigned baselen = strlen(rname);
>> +
>> + switch (interface) {
>> + case GL_TRANSFORM_FEEDBACK_VARYING:
>> + case GL_UNIFORM_BLOCK:
>> + case GL_UNIFORM:
>> + if (strncmp(rname, name, baselen) == 0) {
>> + /* Basename match, check if array or struct. */
>> + if (name[baselen] == '\0' ||
>> + name[baselen] == '[' ||
>> + name[baselen] == '.') {
>> + return res;
>> + }
>> + }
>> + break;
>> + case GL_PROGRAM_INPUT:
>> + case GL_PROGRAM_OUTPUT:
>> + if (array_index_of_resource(res, name) >= 0)
>> + return res;
>> + break;
>> + default:
>> + assert(!"not implemented for given interface");
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +static GLuint
>> +calc_resource_index(struct gl_shader_program *shProg,
>> + struct gl_program_resource *res)
>> +{
>> + unsigned i;
>> + GLuint index = 0;
>> + for (i = 0; i < shProg->NumProgramResourceList; i++) {
>> + if (&shProg->ProgramResourceList[i] == res)
>> + return index;
>> + if (shProg->ProgramResourceList[i].Type == res->Type)
>> + index++;
>> + }
>> + return GL_INVALID_INDEX;
>> +}
>> +
>> +/**
>> + * Calculate index for the given resource.
>> + */
>> +GLuint
>> +_mesa_program_resource_index(struct gl_shader_program *shProg,
>> + struct gl_program_resource *res)
>> +{
>> + if (!res)
>> + return GL_INVALID_INDEX;
>> +
>> + switch (res->Type) {
>> + case GL_UNIFORM_BLOCK:
>> + return RESOURCE_UBO(res)- shProg->UniformBlocks;
>> + case GL_ATOMIC_COUNTER_BUFFER:
>> + return RESOURCE_ATC(res) - shProg->AtomicBuffers;
>> + case GL_TRANSFORM_FEEDBACK_VARYING:
>> + default:
>> + return calc_resource_index(shProg, res);
>> + }
>> +}
>> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
>> index 6db52f7..d80252b 100644
>> --- a/src/mesa/main/shaderapi.h
>> +++ b/src/mesa/main/shaderapi.h
>> @@ -226,6 +226,14 @@ _mesa_program_resource_name(struct gl_program_resource *res);
>> extern unsigned
>> _mesa_program_resource_array_size(struct gl_program_resource *res);
>>
>> +extern GLuint
>> +_mesa_program_resource_index(struct gl_shader_program *shProg,
>> + struct gl_program_resource *res);
>> +
>> +extern struct gl_program_resource *
>> +_mesa_program_resource_find_name(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