[Mesa-dev] [PATCH] program_resource: subroutine active uniforms should return NumSubroutineUniforms
Alejandro Piñeiro
apinheiro at igalia.com
Tue Aug 23 07:12:33 UTC 2016
On 22/08/16 10:03, Tapani Pälli wrote:
>
> On 08/19/2016 09:31 PM, Alejandro Piñeiro wrote:
>> Before this commit, GetProgramInterfaceiv for pname ACTIVE_RESOURCES
>> and all the <shader>_SUBROUTINE_UNIFORM programInterface were
>> returning the count of resources on the shader program using that
>> interface, instead of the number of active uniform resources. This
>> would get a
>> wrong value (for example) if the shader has an array of subroutine
>> uniforms.
>>
>> Note that this means that in order to get the proper value, the shader
>> needs to be linked, something that is not explicitly mentioned on
>> ARB_program_interface_query spec, but comes from the general
>> definition of active uniform.
>
> I checked also and did not find evidence of linking requirement but I
> found text that says we should be able to query properties without
> errors for programs that either failed to link or are not linked at all:
>
> From 7.3 "Program Objects" in OpenGL ES 3.1 spec:
>
> First:
>
> "The GL provides various commands allowing applications to enumerate and
> query properties of active variables and interface blocks for a
> specified program."
I see, thanks for the reference.
> Then:
>
> "If one of these commands is called with a program for which LinkProgram
> failed, no error is generated unless otherwise noted."
Ok, so then I would need to soft the patch a little, doing the same but
avoiding the error if it is not linked and ...
> And also:
>
> "If one of these commands is called with a program for which LinkProgram
> had never been called, no error is generated unless otherwise noted, and
> the program object is considered to have no active variables or
> interface blocks."
... return 0 if it is not linked. FWIW, that would also means that when
asking for the same the same info, GetProgramStageiv would fail, but
GetProgramInterfaceiv would return 0. I guess that it is okish, as that
info is only really correct if the program is linked.
Thanks for the feedback.
>
>
>> Fixes GL44-CTS.program_interface_query.subroutines-compute
>> ---
>>
>> It is worth to mention that the implementation of glGetProgramStage
>> also cames to the conclusion that the shader needs to be linked in
>> order to provide the equivalent GL_ACTIVE_SUBROUTINES for each stage,
>> and throws and INVALID_OPERATION if that is not the case, even if the
>> spec of ARB_shader_subroutine or core GL 4.4 doesn't says so.
>>
>> See see shaderapi.c:_mesa_GetProgramStageiv for more info.
>>
>> It is worth to note that although this commit doesn't cause any
>> regression on piglit it causes a CTS regression:
>>
>> Regresses GL44-CTS.program_interface_query.empty-shaders
>>
>> And the reason is that this tests tries to get ACTIVE_RESOURCES for a
>> <shader>_SUBROUTINE_UNIFORM with a program not linked. Taking into
>> account all what I said, I think that that test is wrong. At least one
>> of the tests is wrong, as it is not possible to change mesa to fix one
>> without breaking the other.
>>
>> Final: I have just sent a new piglit tests to the list, in order to
>> ensure that the equivalent queries from ARB_shader_subroutine and
>> ARB_program_interface_query returns the same values:
>>
>> https://lists.freedesktop.org/archives/piglit/2016-August/020488.html
>>
>>
>> src/mesa/main/program_resource.c | 108
>> +++++++++++++++++++++++++++++++--------
>> 1 file changed, 87 insertions(+), 21 deletions(-)
>>
>> diff --git a/src/mesa/main/program_resource.c
>> b/src/mesa/main/program_resource.c
>> index f2a9f00..a7088cd 100644
>> --- a/src/mesa/main/program_resource.c
>> +++ b/src/mesa/main/program_resource.c
>> @@ -66,6 +66,61 @@ supported_interface_enum(struct gl_context *ctx,
>> GLenum iface)
>> }
>> }
>>
>> +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;
>> +}
>> +
>> +static bool
>> +is_subroutine_uniform_program_interface(GLenum programInterface)
>> +{
>> + switch(programInterface) {
>> + 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:
>> + return true;
>> + default:
>> + return false;
>> + }
>> +}
>> +
>> +static GLenum
>> +stage_from_program_interface(GLenum programInterface)
>> +{
>> + switch(programInterface) {
>> + case GL_VERTEX_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_VERTEX;
>> + case GL_TESS_CONTROL_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_TESS_CTRL;
>> + case GL_TESS_EVALUATION_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_TESS_EVAL;
>> + case GL_GEOMETRY_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_GEOMETRY;
>> + case GL_FRAGMENT_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_FRAGMENT;
>> + case GL_COMPUTE_SUBROUTINE_UNIFORM:
>> + return MESA_SHADER_COMPUTE;
>> + default:
>> + assert(!"unexpected programInterface value");
>> + }
>> +}
>> +
>> void GLAPIENTRY
>> _mesa_GetProgramInterfaceiv(GLuint program, GLenum programInterface,
>> GLenum pname, GLint *params)
>> @@ -101,9 +156,38 @@ _mesa_GetProgramInterfaceiv(GLuint program,
>> GLenum programInterface,
>> /* Validate pname against interface. */
>> switch(pname) {
>> case GL_ACTIVE_RESOURCES:
>> - for (i = 0, *params = 0; i < shProg->NumProgramResourceList; i++)
>> - if (shProg->ProgramResourceList[i].Type == programInterface)
>> - (*params)++;
>> + if (is_subroutine_uniform_program_interface(programInterface)) {
>> + /* ARB_program_interface_query doesn't explicitly says that
>> those
>> + * uniforms would need a linked shader, or that should fail
>> if it is
>> + * not the case, but Section 7.6 (Uniform Variables) of the
>> OpenGL
>> + * 4.4 Core Profile says:
>> + *
>> + * "A uniform is considered an active uniform if the
>> compiler and
>> + * linker determine that the uniform will actually be
>> accessed
>> + * when the executable code is executed. In cases where the
>> + * compiler and linker cannot make a conclusive
>> determination,
>> + * the uniform will be considered active."
>> + *
>> + * So in order to know the active subroutine uniforms we
>> need the
>> + * linked shader.
>> + */
>> + struct gl_shader_program *shLinkedProg =
>> + lookup_linked_program(program, "glGetProgramInterfaceiv");
>> + if (!shLinkedProg)
>> + return;
>> + gl_shader_stage stage =
>> stage_from_program_interface(programInterface);
>> + struct gl_linked_shader *sh =
>> shLinkedProg->_LinkedShaders[stage];
>> + if (!sh) {
>> + _mesa_error(ctx, GL_INVALID_OPERATION,
>> "glGetProgramInterfaceiv");
>> + return;
>> + }
>> +
>> + *params = sh->NumSubroutineUniforms;
>> + } else {
>> + for (i = 0, *params = 0; i < shProg->NumProgramResourceList;
>> i++)
>> + if (shProg->ProgramResourceList[i].Type == programInterface)
>> + (*params)++;
>> + }
>> break;
>> case GL_MAX_NAME_LENGTH:
>> if (programInterface == GL_ATOMIC_COUNTER_BUFFER ||
>> @@ -375,24 +459,6 @@ _mesa_GetProgramResourceiv(GLuint program, GLenum
>> programInterface,
>> propCount, props, bufSize, length,
>> params);
>> }
>>
>> -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)
>>
>
--
Alejandro Piñeiro <apinheiro at igalia.com>
More information about the mesa-dev
mailing list