[Mesa-dev] [PATCH 7/7] mesa: validate precision of varyings during ValidateProgramPipeline

[Iago Toral]

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

> +   /* 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?

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