[Mesa-dev] [PATCH] program_resource: subroutine active uniforms should return NumSubroutineUniforms

Mon Aug 22 08:03:09 UTC 2016

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."

Then:

"If one of these commands is called with a program for which LinkProgram 
failed, no error is generated unless otherwise noted."

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."

> 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)
>