program_resource: subroutine active uniforms should return NumSubroutineUniforms
authorAlejandro Piñeiro <apinheiro@igalia.com>
Thu, 18 Aug 2016 17:44:55 +0000 (19:44 +0200)
committerAlejandro Piñeiro <apinheiro@igalia.com>
Wed, 24 Aug 2016 09:33:04 +0000 (11:33 +0200)
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 num of 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 a 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. If the program is not linked we
return 0.

v2: don't generate an error if the program is not linked, returning 0
    active uniforms instead, plus extra spec references (Tapani Palli)

Fixes GL44-CTS.program_interface_query.subroutines-compute

Reviewed-by: Tapani Pälli <tapani.palli@intel.com>
src/mesa/main/program_resource.c

index f2a9f006dd8b9bef4fc212fd3e962f3082bcc7e4..6ddbdad997f8da58399860ba7048636b651d924f 100644 (file)
@@ -66,6 +66,79 @@ supported_interface_enum(struct gl_context *ctx, GLenum iface)
    }
 }
 
+static struct gl_shader_program *
+lookup_linked_program(GLuint program,
+                      const char *caller,
+                      bool raise_link_error)
+{
+   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) {
+      if (raise_link_error)
+         _mesa_error(ctx, GL_INVALID_OPERATION, "%s(program not linked)",
+                     caller);
+      return NULL;
+   }
+   return prog;
+}
+
+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");
+   }
+}
+
+static struct gl_linked_shader *
+lookup_linked_shader(GLuint program,
+                     GLenum programInterface,
+                     const char *caller)
+{
+   struct gl_shader_program *shLinkedProg =
+      lookup_linked_program(program, caller, false);
+   gl_shader_stage stage = stage_from_program_interface(programInterface);
+
+   if (!shLinkedProg)
+      return NULL;
+
+   return shLinkedProg->_LinkedShaders[stage];
+}
+
+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;
+   }
+}
+
 void GLAPIENTRY
 _mesa_GetProgramInterfaceiv(GLuint program, GLenum programInterface,
                             GLenum pname, GLint *params)
@@ -101,9 +174,49 @@ _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 real number of active subroutine uniforms
+          * we would need a linked shader .
+          *
+          * At the same time, Section 7.3 (Program Objects) of the OpenGL 4.4
+          * Core Profile says:
+          *
+          *    "The GL provides various commands allowing applications to
+          *     enumerate and query properties of active variables and in-
+          *     terface blocks for a specified program. If one of these
+          *     commands is called with a program for which LinkProgram
+          *     succeeded, the information recorded when the program was
+          *     linked is returned. If one of these commands is called with a
+          *     program for which LinkProgram failed, no error is generated
+          *     unless otherwise noted."
+          *     <skip>
+          *    "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."
+          *
+          * So if the program is not linked we will return 0.
+          */
+         struct gl_linked_shader *sh =
+            lookup_linked_shader(program, programInterface, "glGetProgramInterfaceiv");
+
+         *params = sh ? sh->NumSubroutineUniforms : 0;
+      } 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 +488,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)
@@ -405,7 +500,7 @@ _mesa_GetProgramResourceLocation(GLuint program, GLenum programInterface,
    }
 
    struct gl_shader_program *shProg =
-      lookup_linked_program(program, "glGetProgramResourceLocation");
+      lookup_linked_program(program, "glGetProgramResourceLocation", true);
 
    if (!shProg || !name)
       return -1;
@@ -461,7 +556,7 @@ _mesa_GetProgramResourceLocationIndex(GLuint program, GLenum programInterface,
    }
 
    struct gl_shader_program *shProg =
-      lookup_linked_program(program, "glGetProgramResourceLocationIndex");
+      lookup_linked_program(program, "glGetProgramResourceLocationIndex", true);
 
    if (!shProg || !name)
       return -1;