[Mesa-dev] [PATCH v2] mesa: validate sampler uniforms during gluniform calls

[Tapani Pälli]

Patch fixes 'glsl-2types-of-textures-on-same-unit' in WebGL conformance
test suite. No Piglit regressions, fixes gl-2.0-active-sampler-conflict.

To avoid adding potentially heavy check during draw (valid_to_render),
check is done during uniform updates by inspecting TexturesUsed mask.

A new boolean variable is introduced to cache validation state.

v2: take into account case where 2 uniforms use same unit (curro)
    also do the check only when SSO is not in use, SSO has own
    path for sampler validation.

Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
---
 src/mesa/main/context.c         | 12 +++++++++++
 src/mesa/main/mtypes.h          |  1 +
 src/mesa/main/uniform_query.cpp | 44 +++++++++--------------------------------
 src/mesa/main/uniforms.c        | 14 +++++++++++++
 4 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index 5a8f718..25b9bfc 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -133,6 +133,7 @@
 #include "program/prog_print.h"
 #include "math/m_matrix.h"
 #include "main/dispatch.h" /* for _gloffset_COUNT */
+#include "uniforms.h"
 
 #ifdef USE_SPARC_ASM
 #include "sparc/sparc.h"
@@ -1949,6 +1950,17 @@ _mesa_valid_to_render(struct gl_context *ctx, const char *where)
       }
    }
 
+   /* If a program is active and SSO not in use, check if validation of
+    * samplers succeeded for the active program. */
+   if (ctx->_Shader->ActiveProgram && ctx->_Shader != ctx->Pipeline.Current) {
+      char errMsg[100];
+      if (!_mesa_sampler_uniforms_are_valid(ctx->_Shader->ActiveProgram,
+                                            errMsg, 100)) {
+         _mesa_error(ctx, GL_INVALID_OPERATION, "%s", errMsg);
+         return GL_FALSE;
+      }
+   }
+
    if (ctx->DrawBuffer->_Status != GL_FRAMEBUFFER_COMPLETE_EXT) {
       _mesa_error(ctx, GL_INVALID_FRAMEBUFFER_OPERATION_EXT,
                   "%s(incomplete framebuffer)", where);
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5e9453b..27670bd 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2873,6 +2873,7 @@ struct gl_shader_program
    GLboolean LinkStatus;   /**< GL_LINK_STATUS */
    GLboolean Validated;
    GLboolean _Used;        /**< Ever used for drawing? */
+   GLboolean SamplersValidated; /**< Samplers validated against texture units? */
    GLchar *InfoLog;
 
    unsigned Version;       /**< GLSL version used for linking */
diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
index 1592c9b..c2776c0 100644
--- a/src/mesa/main/uniform_query.cpp
+++ b/src/mesa/main/uniform_query.cpp
@@ -1064,42 +1064,16 @@ extern "C" bool
 _mesa_sampler_uniforms_are_valid(const struct gl_shader_program *shProg,
 				 char *errMsg, size_t errMsgLength)
 {
-   const glsl_type *unit_types[MAX_COMBINED_TEXTURE_IMAGE_UNITS];
-
-   memset(unit_types, 0, sizeof(unit_types));
-
-   for (unsigned i = 0; i < shProg->NumUserUniformStorage; i++) {
-      const struct gl_uniform_storage *const storage =
-	 &shProg->UniformStorage[i];
-      const glsl_type *const t = (storage->type->is_array())
-	 ? storage->type->fields.array : storage->type;
-
-      if (!t->is_sampler())
-	 continue;
-
-      const unsigned count = MAX2(1, storage->type->array_size());
-      for (unsigned j = 0; j < count; j++) {
-	 const unsigned unit = storage->storage[j].i;
-
-	 /* The types of the samplers associated with a particular texture
-	  * unit must be an exact match.  Page 74 (page 89 of the PDF) of the
-	  * OpenGL 3.3 core spec says:
-	  *
-	  *     "It is not allowed to have variables of different sampler
-	  *     types pointing to the same texture image unit within a program
-	  *     object."
-	  */
-	 if (unit_types[unit] == NULL) {
-	    unit_types[unit] = t;
-	 } else if (unit_types[unit] != t) {
-	    _mesa_snprintf(errMsg, errMsgLength,
-			   "Texture unit %d is accessed both as %s and %s",
-			   unit, unit_types[unit]->name, t->name);
-	    return false;
-	 }
-      }
+   /* Shader does not have samplers. */
+   if (shProg->NumUserUniformStorage == 0)
+      return true;
+
+   if (!shProg->SamplersValidated) {
+      _mesa_snprintf(errMsg, errMsgLength,
+                     "active samplers with a different type "
+                     "refer to the same texture image unit");
+      return false;
    }
-
    return true;
 }
 
diff --git a/src/mesa/main/uniforms.c b/src/mesa/main/uniforms.c
index 0d0cbf5..598b4d4 100644
--- a/src/mesa/main/uniforms.c
+++ b/src/mesa/main/uniforms.c
@@ -75,12 +75,26 @@ _mesa_update_shader_textures_used(struct gl_shader_program *shProg,
    memcpy(prog->SamplerUnits, shader->SamplerUnits, sizeof(prog->SamplerUnits));
    memset(prog->TexturesUsed, 0, sizeof(prog->TexturesUsed));
 
+   shProg->SamplersValidated = GL_TRUE;
+
    for (s = 0; s < MAX_SAMPLERS; s++) {
       if (prog->SamplersUsed & (1 << s)) {
          GLuint unit = shader->SamplerUnits[s];
          GLuint tgt = shader->SamplerTargets[s];
          assert(unit < Elements(prog->TexturesUsed));
          assert(tgt < NUM_TEXTURE_TARGETS);
+
+         /* The types of the samplers associated with a particular texture
+          * unit must be an exact match.  Page 74 (page 89 of the PDF) of the
+          * OpenGL 3.3 core spec says:
+          *
+          *     "It is not allowed to have variables of different sampler
+          *     types pointing to the same texture image unit within a program
+          *     object."
+          */
+         if (prog->TexturesUsed[unit] & ~(1 << tgt))
+            shProg->SamplersValidated = GL_FALSE;
+
          prog->TexturesUsed[unit] |= (1 << tgt);
       }
    }
-- 
1.9.3