[Mesa-dev] [PATCH 10/10] glsl: fail when a shader's input var has not an equivalent out var in previous

[Samuel Iglesias Gonsálvez]

We have not got any feedback from this patch yet.

Can somebody review it?

Notice that there is a regression in piglit which is explained in the commit 
log.

Sam

On Monday, December 01, 2014 02:04:50 PM Eduardo Lima Mitev wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> 
> GLSL ES 3.00 spec, 4.3.10 (Linking of Vertex Outputs and Fragment Inputs),
> page 45 says the following:
> 
> "The type of vertex outputs and fragment input with the same name must
> match, otherwise the link command will fail. The precision does not need to
> match. Only those fragment inputs statically used (i.e. read) in the
> fragment shader must be declared as outputs in the vertex shader; declaring
> superfluous vertex shader outputs is permissible."
> [...]
> "The term static use means that after preprocessing the shader includes at
> least one statement that accesses the input or output, even if that
> statement is never actually executed."
> 
> And it includes a table with all the possibilities.
> 
> Similar table or content is present in other GLSL specs: GLSL 4.40, GLSL
> 1.50, etc but for more stages (vertex and geometry shaders, etc).
> 
> This patch detects that case and returns a link error. It fixes the
> following dEQP test:
> 
>   dEQP-GLES3.functional.shaders.linkage.varying.rules.illegal_usage_1
> 
> However, it adds a new regression in piglit because the test hasn't a
> vertex shader and it checks the link status.
> 
> bin/glslparsertest \
> tests/spec/glsl-1.50/compiler/gs-also-uses-smooth-flat-noperspective.geom
> pass \ 1.50 --check-link
> 
> This piglit test is wrong according to the spec wording above, so if this
> patch is merged it should be updated.
> 
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
>  src/glsl/link_varyings.cpp | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 1fe198a..036996f 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -263,6 +263,22 @@ cross_validate_outputs_to_inputs(struct
> gl_shader_program *prog, if (output != NULL) {
>              cross_validate_types_and_qualifiers(prog, input, output,
>                                                  consumer->Stage,
> producer->Stage); +         } else {
> +            /* Check for input vars with unmatched output vars in prev
> stage +             * taking into account that interface blocks could have
> a match +             * output but with different name, so we ignore them.
> +             */
> +            if (input->data.used && !input->data.assigned &&
> +                !(input->is_interface_instance() ||
> +                  input->get_interface_type() ||
> +                  input->is_in_uniform_block()) &&
> +                input->data.how_declared == ir_var_declared_normally &&
> +                input->data.location == -1)
> +               linker_error(prog,
> +                            "%s shader input `%s' "
> +                            "has no matching output in the previous
> stage\n", +                           
> _mesa_shader_stage_to_string(consumer->Stage), +                           
> input->name);
>           }
>        }
>     }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141209/b96591d7/attachment.sig>