On Tue, 2016-03-08 at 16:24 +0100, Samuel Iglesias Gonsálvez wrote:
> On Wed, 2016-03-09 at 00:47 +1100, Timothy Arceri wrote:
> > Since we store some member qualifiers in the interface type
> > we need to be more careful about rejecting shaders just because
> > the pointer doesn't match. Its perfectly valid for some qualifiers
> > such as precision to not match across shader interfaces.
> > ---
> > src/compiler/glsl/link_interface_blocks.cpp | 78
> > ++++++++++++++++++++++++++---
> > 1 file changed, 72 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/compiler/glsl/link_interface_blocks.cpp
> > b/src/compiler/glsl/link_interface_blocks.cpp
> > index 9d36836..f868c99 100644
> > --- a/src/compiler/glsl/link_interface_blocks.cpp
> > +++ b/src/compiler/glsl/link_interface_blocks.cpp
> > @@ -81,6 +81,66 @@ intrastage_match(ir_variable *a,
> > return true;
> > }
> >
> > +/**
> > + * Return true if interface members mismatch and its not allowed
> > by
> > GLSL.
> > + */
> > +static bool
> > +interstage_member_mismatch(struct gl_shader_program *prog,
> > + const glsl_type *c, const glsl_type *p)
> > {
> > +
> > + if (c->length != p->length)
> > + return true;
> > +
> > + for (unsigned i = 0; i < c->length; i++) {
> > + if (c->fields.structure[i].type != p-
> > > fields.structure[i].type)
> > + return true;
> > + if (strcmp(c->fields.structure[i].name,
> > + p->fields.structure[i].name) != 0)
> > + return true;
> > + if (c->fields.structure[i].location
> > + != p->fields.structure[i].location)
>
> Just one comment, I usually see the relational operator at the end of
> the previous line, although there are several examples in Mesa like
> this. I checked the coding style and I have not found any reference
> of
> the preferred way.
Right that is the suggested way for new code, this was copy and pasted
so forgot about that. Will fix before pushing.
>
> In any case, the series is:
>
> Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
>
Thanks.
> Sam
>
> > + return true;
> > + if (c->fields.structure[i].patch
> > + != p->fields.structure[i].patch)
> > + return true;
> > +
> > + /* From Section 4.5 (Interpolation Qualifiers) of the GLSL
> > 4.40 spec:
> > + *
> > + * "It is a link-time error if, within the same stage,
> > the
> > + * interpolation qualifiers of variables of the same name
> > do not
> > + * match."
> > + */
> > + if (prog->IsES || prog->Version < 440)
> > + if (c->fields.structure[i].interpolation
> > + != p->fields.structure[i].interpolation)
> > + return true;
> > +
> > + /* From Section 4.3.4 (Input Variables) of the GLSL ES 3.0
> > spec:
> > + *
> > + * "The output of the vertex shader and the input of the
> > fragment
> > + * shader form an interface. For this interface, vertex
> > shader
> > + * output variables and fragment shader input variables
> > of
> > the same
> > + * name must match in type and qualification (other than
> > precision
> > + * and out matching to in).
> > + *
> > + * The table in Section 9.2.1 Linked Shaders of the GLSL ES
> > 3.1 spec
> > + * says that centroid no longer needs to match for varyings.
> > + *
> > + * The table in Section 9.2.1 Linked Shaders of the GLSL ES
> > 3.2 spec
> > + * says that sample need not match for varyings.
> > + */
> > + if (!prog->IsES || prog->Version < 310)
> > + if (c->fields.structure[i].centroid
> > + != p->fields.structure[i].centroid)
> > + return true;
> > + if (!prog->IsES || prog->Version < 320)
> > + if (c->fields.structure[i].sample
> > + != p->fields.structure[i].sample)
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> >
> > /**
> > * Check if two interfaces match, according to interstage (in/out)
> > interface
> > @@ -91,9 +151,8 @@ intrastage_match(ir_variable *a,
> > * This is used for tessellation control and geometry shader
> > consumers.
> > */
> > static bool
> > -interstage_match(ir_variable *producer,
> > - ir_variable *consumer,
> > - bool extra_array_level)
> > +interstage_match(struct gl_shader_program *prog, ir_variable
> > *producer,
> > + ir_variable *consumer, bool extra_array_level)
> > {
> > /* Unsized arrays should not occur during interstage
> > linking. They
> > * should have all been assigned a size by
> > link_intrastage_shaders.
> > @@ -106,9 +165,16 @@ interstage_match(ir_variable *producer,
> > /* Exception: if both the interface blocks are implicitly
> > declared,
> > * don't force their types to match. They might mismatch
> > due
> > to the two
> > * shaders using different GLSL versions, and that's ok.
> > + *
> > + * Also we store some member information in glsl_type that
> > doesn't
> > + * always have to match across shaders such as interpolation
> > so we need
> > + * to make a pass over members glsl_struct_field to make
> > sure
> > we don't
> > + * reject shaders where fields don't need to match.
> > */
> > - if (consumer->data.how_declared !=
> > ir_var_declared_implicitly
> > > >
> > - producer->data.how_declared !=
> > ir_var_declared_implicitly)
> > + if ((consumer->data.how_declared !=
> > ir_var_declared_implicitly
> > > >
> > + producer->data.how_declared !=
> > ir_var_declared_implicitly) &&
> > + interstage_member_mismatch(prog, consumer-
> > > get_interface_type(),
> > + producer-
> > > get_interface_type()))
> > return false;
> > }
> >
> > @@ -311,7 +377,7 @@ validate_interstage_inout_blocks(struct
> > gl_shader_program *prog,
> > if (consumer_def == NULL)
> > continue;
> >
> > - if (!interstage_match(var, consumer_def, extra_array_level))
> > {
> > + if (!interstage_match(prog, var, consumer_def,
> > extra_array_level)) {
> > linker_error(prog, "definitions of interface block `%s'
> > do
> > not "
> > "match\n", var->get_interface_type()->name);
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
