On Fri, 2015-03-27 at 18:07 +1100, Timothy Arceri wrote:
> On Thu, 2015-03-26 at 23:10 -0700, Ben Widawsky wrote:
> > On Mon, Dec 01, 2014 at 02:04:50PM +0100, Eduardo Lima Mitev wrote:
> > > From: Samuel Iglesias Gonsalvez <sigles...@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 <sigles...@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);
> >
> > The current code isn't wrong, but it made things somewhat confusing for me
> > when
> > reviewing. I don't think there is ever a case where input->data.assigned is
> > valid. Inputs are supposed to be read only, and so they would never be
> > assigned.
> > How about instead: assert(!input->data.assigned);
>
> Currently the compile check for read only inputs is missing. I wrote a
> patch for it in December [1], it needs to be rebased. I will do that and
> also track down and create a second patch for interface support if your
> interested in reviewing them.
Ignore this patch it doesn't seem to be needed any more, but I have a
patch for the interface check on the way.
>
> [1]
> http://lists.freedesktop.org/archives/mesa-dev/2014-December/073358.html
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
