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

[Alejandro Piñeiro]

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.

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