[Mesa-dev] [PATCH 05/23] mesa: glGetProgramResourceName
Tapani Pälli
tapani.palli at intel.com
Tue Mar 17 01:49:23 PDT 2015
On 03/16/2015 07:16 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.
>>
>> Name generation copied from '_mesa_get_uniform_name' which can
>> be removed later by refactoring functions to use resource list.
>>
>> The added functionality can be tested by tests for following
>> functions that are refactored by later patches:
>>
>> GetActiveUniformName
>> GetActiveUniformBlockName
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>> src/mesa/main/program_resource.c | 22 +++++++++
>> src/mesa/main/shader_query.cpp | 96 ++++++++++++++++++++++++++++++++++++++++
>> src/mesa/main/shaderapi.h | 10 +++++
>> 3 files changed, 128 insertions(+)
>>
>> diff --git a/src/mesa/main/program_resource.c b/src/mesa/main/program_resource.c
>> index 4190f98..4fa6ac6 100644
>> --- a/src/mesa/main/program_resource.c
>> +++ b/src/mesa/main/program_resource.c
>> @@ -211,6 +211,28 @@ _mesa_GetProgramResourceName(GLuint program, GLenum programInterface,
>> GLuint index, GLsizei bufSize, GLsizei *length,
>> GLchar *name)
>> {
>> + GET_CURRENT_CONTEXT(ctx);
>> + struct gl_shader_program *shProg =
>> + _mesa_lookup_shader_program_err(ctx, program,
>> + "glGetProgramResourceIndex");
>> + if (!shProg)
>> + return;
>> +
>> + /* Set user friendly return values in case of errors. */
>
> Should this be done above the if (!shProg) return thing then?
Sure, for the case of consistency.
>> + *name = '\0';
>> + if (length)
>> + *length = 0;
>> +
>> + if (programInterface == GL_ATOMIC_COUNTER_BUFFER) {
>> + _mesa_error(ctx, GL_INVALID_ENUM,
>> + "glGetProgramResourceName(%s)",
>> + _mesa_lookup_enum_by_nr(programInterface));
>> + return;
>> + }
>> +
>> + _mesa_get_program_resource_name(shProg, programInterface, index,
>> + bufSize, length, name,
>> + "glGetProgramResourceName");
>> }
>>
>> void GLAPIENTRY
>> diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
>> index d1974a4..77a4af0 100644
>> --- a/src/mesa/main/shader_query.cpp
>> +++ b/src/mesa/main/shader_query.cpp
>> @@ -647,3 +647,99 @@ _mesa_program_resource_index(struct gl_shader_program *shProg,
>> return calc_resource_index(shProg, res);
>> }
>> }
>> +
>> +/* Find a program resource with specific index in given interface.
>> + */
>> +struct gl_program_resource *
>> +_mesa_program_resource_find_index(struct gl_shader_program *shProg,
>> + GLenum interface, GLuint index)
>
> I feel like I've seen a very similar function before. TBH I can't
> remember which patch it was in, but if there's any unification
> possibility, please do so.
OK, will see if possible.
>> +{
>> + struct gl_program_resource *res = shProg->ProgramResourceList;
>> + int idx = -1;
>> +
>> + for (unsigned i = 0; i < shProg->NumProgramResourceList; i++, res++) {
>> + if (res->Type != interface)
>> + continue;
>> +
>> + switch (res->Type) {
>> + case GL_UNIFORM_BLOCK:
>> + case GL_ATOMIC_COUNTER_BUFFER:
>> + if (_mesa_program_resource_index(shProg, res) == index)
>> + return res;
>> +
>> + case GL_TRANSFORM_FEEDBACK_VARYING:
>> + case GL_PROGRAM_INPUT:
>> + case GL_PROGRAM_OUTPUT:
>> + case GL_UNIFORM:
>> + if (++idx == (int) index)
>> + return res;
>> + break;
>> + default:
>> + assert(!"not implemented for given interface");
>> + }
>> + }
>> + return NULL;
>> +}
>> +
>> +/* Get full name of a program resource.
>> + */
>> +bool
>> +_mesa_get_program_resource_name(struct gl_shader_program *shProg,
>> + GLenum interface, GLuint index,
>> + GLsizei bufSize, GLsizei *length,
>> + GLchar *name, const char *caller)
>> +{
>> + GET_CURRENT_CONTEXT(ctx);
>> +
>> + /* Find resource with given interface and index. */
>> + struct gl_program_resource *res =
>> + _mesa_program_resource_find_index(shProg, interface, index);
>> +
>> + /* The error INVALID_VALUE is generated if <index> is greater than
>> + * or equal to the number of entries in the active resource list for
>> + * <programInterface>.
>> + */
>> + if (!res) {
>> + _mesa_error(ctx, GL_INVALID_VALUE, "%s(index %u)", caller, index);
>> + return false;
>> + }
>> +
>> + GLsizei localLength;
>> +
>> + if (length == NULL)
>> + length = &localLength;
>> +
>> + _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]'.
>
> Only for GS inputs though, right? And same deal for TCS outputs / TES
> inputs too... [make a mention of it, no need to actually implement].
> Will this never get called for outputs?
You're right, I should special case only the input side. Will fix.
>> + */
>> + if (_mesa_program_resource_array_size(res) != 0 &&
>> + (!(res->StageReferences & (1 << MESA_SHADER_GEOMETRY)))) {
>> + int i;
>> +
>> + /* The comparison is strange because *length does *NOT* include the
>> + * terminating NUL, but maxLength does.
>> + */
>> + for (i = 0; i < 3 && (*length + i + 1) < bufSize; i++)
>> + name[*length + i] = "[0]"[i];
>> +
>> + name[*length + i] = '\0';
>> + *length += i;
>
> Consider using a memcpy (or strncpy) here. If it makes the code
> uglier, then nevermind, but there may be a good way to do it.
I had my own version of this where I checked first if there's enough
bufSize to do memcpy but I don't think it's worth the trouble. Version
above is quite minimal and generic to handle problematic cases.
>> + }
>> + return true;
>> +}
>> diff --git a/src/mesa/main/shaderapi.h b/src/mesa/main/shaderapi.h
>> index d80252b..7a7e3e9 100644
>> --- a/src/mesa/main/shaderapi.h
>> +++ b/src/mesa/main/shaderapi.h
>> @@ -234,6 +234,16 @@ extern struct gl_program_resource *
>> _mesa_program_resource_find_name(struct gl_shader_program *shProg,
>> GLenum interface, const char *name);
>>
>> +extern struct gl_program_resource *
>> +_mesa_program_resource_find_index(struct gl_shader_program *shProg,
>> + GLenum interface, GLuint index);
>> +
>> +extern bool
>> +_mesa_get_program_resource_name(struct gl_shader_program *shProg,
>> + GLenum interface, GLuint index,
>> + GLsizei bufSize, GLsizei *length,
>> + GLchar *name, const char *caller);
>> +
>> #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