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.
In any case, the series is:
Reviewed-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
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);
> return;
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
