On 11/10/2015 01:15 PM, Iago Toral wrote:
On Tue, 2015-11-10 at 12:10 +0100, Iago Toral wrote:
On Thu, 2015-11-05 at 13:33 +0200, Tapani Pälli wrote:
Fixes following failing ES3.1 CTS tests:

    ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingFloat
    ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingInt
    ES31-CTS.sepshaderobjs.InterfacePrecisionMatchingUInt

Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
  src/mesa/main/pipelineobj.c    | 15 ++++++++++++
  src/mesa/main/shader_query.cpp | 55 ++++++++++++++++++++++++++++++++++++++++++
  src/mesa/main/shaderobj.h      |  3 +++
  3 files changed, 73 insertions(+)

diff --git a/src/mesa/main/pipelineobj.c b/src/mesa/main/pipelineobj.c
index 699a2ae..90dff13 100644
--- a/src/mesa/main/pipelineobj.c
+++ b/src/mesa/main/pipelineobj.c
@@ -907,6 +907,21 @@ _mesa_ValidateProgramPipeline(GLuint pipeline)

     _mesa_validate_program_pipeline(ctx, pipe,
                                     (ctx->_Shader->Name == pipe->Name));
+
+   /* Validate inputs against outputs, this cannot be done during linking
+    * since programs have been linked separately from each other.
+    *
+    * From OpenGL 4.5 Core spec:
+    *     "Separable program objects may have validation failures that cannot 
be
+    *     detected without the complete program pipeline. Mismatched 
interfaces,
+    *     improper usage of program objects together, and the same
+    *     state-dependent failures can result in validation errors for such
+    *     program objects."
+    *
+    * OpenGL ES 3.1 specification has the same text.
+    */
+   if (!_mesa_validate_pipeline_io(pipe))
+      pipe->Validated = GL_FALSE;
  }

  void GLAPIENTRY
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 5cb877b..595bdea 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1359,3 +1359,58 @@ _mesa_get_program_resourceiv(struct gl_shader_program 
*shProg,
     if (length)
        *length = amount;
  }
+
+static bool
+validate_io(const struct gl_shader *input_stage,
+            const struct gl_shader *output_stage)
+{
+   assert(input_stage && output_stage);

Maybe add the relevant spec quote for this:

"When both shaders are in separate programs, mismatched precision
  qualifiers will result in a program interface mismatch that will
  result in program pipeline  validation failures"

Sorry, this was cut, the complete text reads:

"When both shaders are in separate programs, mismatched precision
qualifiers will result in a program interface mismatch that will result
in program pipeline validation failures, as described in section 7.4.1
(“Shader Interface Matching”) of the OpenGL ES 3.1 Specification."

This text is from chapter "4.7.3. Precision Qualifiers" of the OpenGL ES
3.1 spec.

OK, I'll add spec quote.

+   /* For each output in a, find input in b and do any required checks. */

s/a/input_stage
s/b/output_stage

+   foreach_in_list(ir_instruction, out, input_stage->ir) {
+      ir_variable *out_var = out->as_variable();
+      if (!out_var || out_var->data.mode != ir_var_shader_out)
+         continue;
+
+      foreach_in_list(ir_instruction, in, output_stage->ir) {
+         ir_variable *in_var = in->as_variable();
+         if (!in_var || in_var->data.mode != ir_var_shader_in)
+            continue;
+
+         if (strcmp(in_var->name, out_var->name) == 0) {
+            if (in_var->data.precision != out_var->data.precision)
+               return false;
+         }
+      }

This is O(n²) but we can easily make it O(n) by moving the inner loop
outside and putting the input variables we find there in a hash table.
Then the loop over input_stage just needs to do hash table lookups
instead of looping for each output variable it has. What do you think?

I'm not sure if it's worth the trouble but I can take a try. We would need to also destroy hash which will take some time.

I guess in long run this should be implemented by iterating program resource list and maybe those should be also split to resource pools instead of one single list. I chose single list originally because typically that list is quite small (we are talking about ~10 to 20, not hundreds) and there are api calls that iterate through all resources at once. However I would like to do that change only when quest for GLES 3.1 is over.


Iago

+   }
+   return true;
+}
+
+/**
+ * Validate inputs against outputs in a program pipeline.
+ */
+extern "C" bool
+_mesa_validate_pipeline_io(struct gl_pipeline_object *pipeline)
+{
+   struct gl_shader_program **shProg =
+      (struct gl_shader_program **) pipeline->CurrentProgram;
+
+   /* Find first active stage in pipeline. */
+   unsigned idx, prev = 0;
+   for (idx = 0; idx < ARRAY_SIZE(pipeline->CurrentProgram); idx++) {
+      if (shProg[idx]) {
+         prev = idx;
+         break;
+      }
+   }
+
+   for (idx = prev + 1; idx < ARRAY_SIZE(pipeline->CurrentProgram); idx++) {
+      if (shProg[idx]) {
+         if (!validate_io(shProg[prev]->_LinkedShaders[prev],
+                          shProg[idx]->_LinkedShaders[idx]))
+            return false;
+         prev = idx;
+      }
+   }
+   return true;
+}
diff --git a/src/mesa/main/shaderobj.h b/src/mesa/main/shaderobj.h
index 796de47..be80752 100644
--- a/src/mesa/main/shaderobj.h
+++ b/src/mesa/main/shaderobj.h
@@ -234,6 +234,9 @@ _mesa_shader_stage_to_subroutine_uniform(gl_shader_stage 
stage)
     }
  }

+extern bool
+_mesa_validate_pipeline_io(struct gl_pipeline_object *);
+
  #ifdef __cplusplus
  }
  #endif



_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to