On Wed, 2018-01-31 at 01:38 +0100, Matteo Bruni wrote:
> 2018-01-30 16:11 GMT+01:00 Juan A. Suarez Romero <jasua...@igalia.com>:
> > According with OpenGL GLSL 3.20 spec, section 4.3.9:
> >
> > "It is a link-time error if any particular shader interface
> > contains:
> > - two different blocks, each having no instance name, and each
> > having a member of the same name, or
> > - a variable outside a block, and a block with no instance name,
> > where the variable has the same name as a member in the block."
> >
> > This fixes a previous commit 9b894c8 ("glsl/linker: link-error using the
> > same name in unnamed block and outside") that covered this case, but
> > did not take in account that precision qualifiers are ignored when
> > comparing blocks with no instance name.
>
> Thanks! This also fixes Wine, although the reason is slightly
> different: Wine might end up linking a program with a vertex shader
> containing "#extension GL_ARB_cull_distance : enable" together with a
> vertex shader without it. I guess this makes the gl_PerVertex builtin
> defined in the two shader objects differ and that triggered the old
> check but not the new one from this patch.
>
> I don't know if you want to change the commit message to account for
> that (maybe make it more general?) but in any case, and FWIW:
>
> Tested-by: Matteo Bruni <matteo.myst...@gmail.com>
>
> I also have a nitpick which you can entirely ignore, below.
>
> > + if (var->get_interface_type() != existing->get_interface_type()) {
> > + if (!var->get_interface_type() ||
> > !existing->get_interface_type()) {
> > + linker_error(prog, "declarations for %s `%s` are inside
> > block "
> > + "`%s` and outside a block",
> > + mode_string(var), var->name,
> > + var->get_interface_type()
> > + ? var->get_interface_type()->name
> > + : existing->get_interface_type()->name);
> > + return;
> > + } else if (strcmp(var->get_interface_type()->name,
> > + existing->get_interface_type()->name) != 0) {
> > + linker_error(prog, "declarations for %s `%s` are inside
> > blocks "
> > + "`%s` and `%s`",
> > + mode_string(var), var->name,
> > + existing->get_interface_type()->name,
> > + var->get_interface_type()->name);
>
> What about declaring a couple of variables for the interface types and
> using them through? Like:
>
> const glsl_type var_itype = var->get_interface_type();
> const glsl_type existing_itype = existing->get_interface_type();
>
Yes, I'll use a couple of variables. Thanks.
J.A.
> (maybe with different names, I don't know what's Mesa's preferred style)
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
