Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
On Tue, 2015-12-15 at 12:51 +0200, Tapani Pälli wrote:
> On 12/15/2015 10:56 AM, Timothy Arceri wrote:
> > On Tue, 2015-12-15 at 07:58 +0200, Tapani Pälli wrote:
> > > On 12/15/2015 03:31 AM, Timothy Arceri wrote:
> > > > On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
> > > > > Patch makes following changes for interface matching:
> > > > >
> > > > > - do not try to match builtin variables
> > > > > - handle swizzle in input name, as example 'a.z' should
> > > > >match with 'a'
> > > > > - check that amount of inputs and outputs matches
> > > > >
> > > > > These changes make interface matching tests to work in:
> > > > > ES31-CTS.sepshaderobjs.StateInteraction
> > > > >
> > > > > Test does not still pass completely due to errors in
> > > > > rendering
> > > > > output. IMO this is unrelated to interface matching.
> > > > >
> > > > > v2: add spec reference, return true on desktop since we do
> > > > > not
> > > > > have failing cases for it, inputs and outputs amount do
> > > > > not
> > > > > need to match on desktop.
> > > > >
> > > > > Signed-off-by: Tapani Pälli
> > > > Hi Tapani,
> > > >
> > > > Just a general comment first.
> > > >
> > > > I think we should first move _mesa_validate_pipeline_io() and
> > > >validate_io() to src/mesa/main/pipelineobj.c I don't think
> > > > it
> > > > belongs
> > > > here right?
> > > Sure, it can be done now. Original intention was to use program
> > > resources and that is why it ended up being here.
>
> Ah but it uses ir_variable so it may be painful to move. Would it be
> OK
> to still have it in shader_query.cpp?
If its not straight forword to move then thats fine for now. I'd rather
get this fixed then worry about where the code it located :)
>
> > > > > ---
> > > > >src/mesa/main/shader_query.cpp | 54
> > > > > ++
> > > > >1 file changed, 50 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/src/mesa/main/shader_query.cpp
> > > > > b/src/mesa/main/shader_query.cpp
> > > > > index ced10a9..bc01b97 100644
> > > > > --- a/src/mesa/main/shader_query.cpp
> > > > > +++ b/src/mesa/main/shader_query.cpp
> > > > > @@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
> > > > > *input_stage,
> > > > >const struct gl_shader *output_stage, bool
> > > > > isES)
> > > > >{
> > > > > assert(input_stage && output_stage);
> > > > > + unsigned inputs = 0, outputs = 0;
> > > > > +
> > > > > + /* Currently no matching done for desktop. */
> > > > I think the spec quote should be moved here as it applies to
> > > > all
> > > > the
> > > > rules in the function then you can also have the comment
> > > > explaining
> > > > why
> > > > validation for desktop it not done.
> > > OK
> > >
> > > > I've also filed a spec bug for desktop for the reasons I
> > > > outlined
> > > > in
> > > > irc previously. It would be great if you could quote the bug
> > > > here
> > > > also.
> > > > Something like:
> > > >
> > > > /* FIXME: Update once Khronos spec bug #15331 is resolved. */
> > > Sure, will add.
> > >
> > > > > + if (!isES)
> > > > > + return true;
> > > > >
> > > > > /* For each output in a, find input in b and do any
> > > > > required
> > > > > checks. */
> > > > > foreach_in_list(ir_instruction, out, input_stage->ir) {
> > > > > ir_variable *out_var = out->as_variable();
> > > > It's existing code but it would also be nice to have a patch
> > > > that
> > > > renames input_stage/output_stage to
> > > > producer_stage/consumer_stage
> > > > this
> > > > it what they are called in the linker code. Maybe its just me
> > > > but
> > > > getting the outputs from input_stage just looks wrong.
> > > OK, can change this.
> > >
> > > > > - if (!out_var || out_var->data.mode !=
> > > > > ir_var_shader_out)
> > > > > + if (!out_var || out_var->data.mode !=
> > > > > ir_var_shader_out ||
> > > > > + is_gl_identifier(out_var->name))
> > > > > continue;
> > > > >
> > > > > + outputs++;
> > > > > +
> > > > > + inputs = 0;
> > > > > foreach_in_list(ir_instruction, in, output_stage
> > > > > ->ir) {
> > > > Two comments here:
> > > >
> > > > 1. Take a look at cross_validate_outputs_to_inputs() in
> > > > link_varyings.cpp for a way to avoid the nested loop? Although
> > > > it
> > > > may
> > > > cause even more overhaed using the symbol table not sure.
> > > I don't know if symbol table can be trusted as variables that get
> > > optimized away or changed in some way are still there. Only way
> > > to be
> > > sure is to iterate IR or use resource list. Also, symbol table
> > > gets
> > > destroyed after linking. My first implementation was using a hash
> > > but
> > > that was also bad idea because variables names do not necessarily
> > > match
> > > exactly.
> >
> > The code in in cross_validate_outputs_to_inputs() doesn't use *the*
> > symbol table i
Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
On 12/15/2015 10:56 AM, Timothy Arceri wrote:
On Tue, 2015-12-15 at 07:58 +0200, Tapani Pälli wrote:
On 12/15/2015 03:31 AM, Timothy Arceri wrote:
On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
Patch makes following changes for interface matching:
- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
match with 'a'
- check that amount of inputs and outputs matches
These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction
Test does not still pass completely due to errors in rendering
output. IMO this is unrelated to interface matching.
v2: add spec reference, return true on desktop since we do not
have failing cases for it, inputs and outputs amount do not
need to match on desktop.
Signed-off-by: Tapani Pälli
Hi Tapani,
Just a general comment first.
I think we should first move _mesa_validate_pipeline_io() and
validate_io() to src/mesa/main/pipelineobj.c I don't think it
belongs
here right?
Sure, it can be done now. Original intention was to use program
resources and that is why it ended up being here.
Ah but it uses ir_variable so it may be painful to move. Would it be OK
to still have it in shader_query.cpp?
---
src/mesa/main/shader_query.cpp | 54
++
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index ced10a9..bc01b97 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
*input_stage,
const struct gl_shader *output_stage, bool isES)
{
assert(input_stage && output_stage);
+ unsigned inputs = 0, outputs = 0;
+
+ /* Currently no matching done for desktop. */
I think the spec quote should be moved here as it applies to all
the
rules in the function then you can also have the comment explaining
why
validation for desktop it not done.
OK
I've also filed a spec bug for desktop for the reasons I outlined
in
irc previously. It would be great if you could quote the bug here
also.
Something like:
/* FIXME: Update once Khronos spec bug #15331 is resolved. */
Sure, will add.
+ if (!isES)
+ return true;
/* For each output in a, find input in b and do any required
checks. */
foreach_in_list(ir_instruction, out, input_stage->ir) {
ir_variable *out_var = out->as_variable();
It's existing code but it would also be nice to have a patch that
renames input_stage/output_stage to producer_stage/consumer_stage
this
it what they are called in the linker code. Maybe its just me but
getting the outputs from input_stage just looks wrong.
OK, can change this.
- if (!out_var || out_var->data.mode != ir_var_shader_out)
+ if (!out_var || out_var->data.mode != ir_var_shader_out ||
+ is_gl_identifier(out_var->name))
continue;
+ outputs++;
+
+ inputs = 0;
foreach_in_list(ir_instruction, in, output_stage->ir) {
Two comments here:
1. Take a look at cross_validate_outputs_to_inputs() in
link_varyings.cpp for a way to avoid the nested loop? Although it
may
cause even more overhaed using the symbol table not sure.
I don't know if symbol table can be trusted as variables that get
optimized away or changed in some way are still there. Only way to be
sure is to iterate IR or use resource list. Also, symbol table gets
destroyed after linking. My first implementation was using a hash but
that was also bad idea because variables names do not necessarily
match
exactly.
The code in in cross_validate_outputs_to_inputs() doesn't use *the*
symbol table it use *a* which it builds from iterating over the
producers IR. But I guess it will have the same problem as the hash
table. I wonder why the linking code uses it rather than a plain hash
table.
2. Take a look at the same function for matching via explicit
location.
Does the CTS not test for mismatched explicit locations? Maybe we
should create a piglit test for this as your existing code doesn't
take
into account explicit locations.
No, I haven't seen this test using explicit locations. This patch
also
makes the interface matching pass.
Right but it would break any varyings with explicit locations that
don't have a matching names which is legal.
"An output variable is considered to match an input variable in the
subsequent shader if:
–the two variables match in name, type, and qualification; or
–the two variables are declared with the same location qualifier and
match in type and qualification."
I was going to suggest sharing the code between here and the linker
however I'm about to add a bunch of rules for matching the
component
qualifier for enhanced layouts so not entirely sure if we should do
this what do you think?
Linker will need to do much more so maybe do separately, at least for
now?
Yeah I
Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
On Tue, 2015-12-15 at 07:58 +0200, Tapani Pälli wrote:
> On 12/15/2015 03:31 AM, Timothy Arceri wrote:
> > On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
> > > Patch makes following changes for interface matching:
> > >
> > > - do not try to match builtin variables
> > > - handle swizzle in input name, as example 'a.z' should
> > > match with 'a'
> > > - check that amount of inputs and outputs matches
> > >
> > > These changes make interface matching tests to work in:
> > > ES31-CTS.sepshaderobjs.StateInteraction
> > >
> > > Test does not still pass completely due to errors in rendering
> > > output. IMO this is unrelated to interface matching.
> > >
> > > v2: add spec reference, return true on desktop since we do not
> > > have failing cases for it, inputs and outputs amount do not
> > > need to match on desktop.
> > >
> > > Signed-off-by: Tapani Pälli
> > Hi Tapani,
> >
> > Just a general comment first.
> >
> > I think we should first move _mesa_validate_pipeline_io() and
> > validate_io() to src/mesa/main/pipelineobj.c I don't think it
> > belongs
> > here right?
>
> Sure, it can be done now. Original intention was to use program
> resources and that is why it ended up being here.
>
> >
> > > ---
> > > src/mesa/main/shader_query.cpp | 54
> > > ++
> > > 1 file changed, 50 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/src/mesa/main/shader_query.cpp
> > > b/src/mesa/main/shader_query.cpp
> > > index ced10a9..bc01b97 100644
> > > --- a/src/mesa/main/shader_query.cpp
> > > +++ b/src/mesa/main/shader_query.cpp
> > > @@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
> > > *input_stage,
> > > const struct gl_shader *output_stage, bool isES)
> > > {
> > > assert(input_stage && output_stage);
> > > + unsigned inputs = 0, outputs = 0;
> > > +
> > > + /* Currently no matching done for desktop. */
> > I think the spec quote should be moved here as it applies to all
> > the
> > rules in the function then you can also have the comment explaining
> > why
> > validation for desktop it not done.
>
> OK
>
> > I've also filed a spec bug for desktop for the reasons I outlined
> > in
> > irc previously. It would be great if you could quote the bug here
> > also.
> > Something like:
> >
> > /* FIXME: Update once Khronos spec bug #15331 is resolved. */
>
> Sure, will add.
>
> > > + if (!isES)
> > > + return true;
> > >
> > > /* For each output in a, find input in b and do any required
> > > checks. */
> > > foreach_in_list(ir_instruction, out, input_stage->ir) {
> > > ir_variable *out_var = out->as_variable();
> >
> > It's existing code but it would also be nice to have a patch that
> > renames input_stage/output_stage to producer_stage/consumer_stage
> > this
> > it what they are called in the linker code. Maybe its just me but
> > getting the outputs from input_stage just looks wrong.
>
> OK, can change this.
>
> >
> > > - if (!out_var || out_var->data.mode != ir_var_shader_out)
> > > + if (!out_var || out_var->data.mode != ir_var_shader_out ||
> > > + is_gl_identifier(out_var->name))
> > >continue;
> > >
> > > + outputs++;
> > > +
> > > + inputs = 0;
> > > foreach_in_list(ir_instruction, in, output_stage->ir) {
> > Two comments here:
> >
> > 1. Take a look at cross_validate_outputs_to_inputs() in
> > link_varyings.cpp for a way to avoid the nested loop? Although it
> > may
> > cause even more overhaed using the symbol table not sure.
>
> I don't know if symbol table can be trusted as variables that get
> optimized away or changed in some way are still there. Only way to be
> sure is to iterate IR or use resource list. Also, symbol table gets
> destroyed after linking. My first implementation was using a hash but
> that was also bad idea because variables names do not necessarily
> match
> exactly.
The code in in cross_validate_outputs_to_inputs() doesn't use *the*
symbol table it use *a* which it builds from iterating over the
producers IR. But I guess it will have the same problem as the hash
table. I wonder why the linking code uses it rather than a plain hash
table.
>
> > 2. Take a look at the same function for matching via explicit
> > location.
> > Does the CTS not test for mismatched explicit locations? Maybe we
> > should create a piglit test for this as your existing code doesn't
> > take
> > into account explicit locations.
>
> No, I haven't seen this test using explicit locations. This patch
> also
> makes the interface matching pass.
Right but it would break any varyings with explicit locations that
don't have a matching names which is legal.
"An output variable is considered to match an input variable in the
subsequent shader if:
–the two variables match in name, type, and qualification; or
–the two variables are declared with the same location qualifier and
match in
Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
On 12/15/2015 03:31 AM, Timothy Arceri wrote:
On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
Patch makes following changes for interface matching:
- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
match with 'a'
- check that amount of inputs and outputs matches
These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction
Test does not still pass completely due to errors in rendering
output. IMO this is unrelated to interface matching.
v2: add spec reference, return true on desktop since we do not
have failing cases for it, inputs and outputs amount do not
need to match on desktop.
Signed-off-by: Tapani Pälli
Hi Tapani,
Just a general comment first.
I think we should first move _mesa_validate_pipeline_io() and
validate_io() to src/mesa/main/pipelineobj.c I don't think it belongs
here right?
Sure, it can be done now. Original intention was to use program
resources and that is why it ended up being here.
---
src/mesa/main/shader_query.cpp | 54
++
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/shader_query.cpp
b/src/mesa/main/shader_query.cpp
index ced10a9..bc01b97 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
*input_stage,
const struct gl_shader *output_stage, bool isES)
{
assert(input_stage && output_stage);
+ unsigned inputs = 0, outputs = 0;
+
+ /* Currently no matching done for desktop. */
I think the spec quote should be moved here as it applies to all the
rules in the function then you can also have the comment explaining why
validation for desktop it not done.
OK
I've also filed a spec bug for desktop for the reasons I outlined in
irc previously. It would be great if you could quote the bug here also.
Something like:
/* FIXME: Update once Khronos spec bug #15331 is resolved. */
Sure, will add.
+ if (!isES)
+ return true;
/* For each output in a, find input in b and do any required
checks. */
foreach_in_list(ir_instruction, out, input_stage->ir) {
ir_variable *out_var = out->as_variable();
It's existing code but it would also be nice to have a patch that
renames input_stage/output_stage to producer_stage/consumer_stage this
it what they are called in the linker code. Maybe its just me but
getting the outputs from input_stage just looks wrong.
OK, can change this.
- if (!out_var || out_var->data.mode != ir_var_shader_out)
+ if (!out_var || out_var->data.mode != ir_var_shader_out ||
+ is_gl_identifier(out_var->name))
continue;
+ outputs++;
+
+ inputs = 0;
foreach_in_list(ir_instruction, in, output_stage->ir) {
Two comments here:
1. Take a look at cross_validate_outputs_to_inputs() in
link_varyings.cpp for a way to avoid the nested loop? Although it may
cause even more overhaed using the symbol table not sure.
I don't know if symbol table can be trusted as variables that get
optimized away or changed in some way are still there. Only way to be
sure is to iterate IR or use resource list. Also, symbol table gets
destroyed after linking. My first implementation was using a hash but
that was also bad idea because variables names do not necessarily match
exactly.
2. Take a look at the same function for matching via explicit location.
Does the CTS not test for mismatched explicit locations? Maybe we
should create a piglit test for this as your existing code doesn't take
into account explicit locations.
No, I haven't seen this test using explicit locations. This patch also
makes the interface matching pass.
I was going to suggest sharing the code between here and the linker
however I'm about to add a bunch of rules for matching the component
qualifier for enhanced layouts so not entirely sure if we should do
this what do you think?
Linker will need to do much more so maybe do separately, at least for now?
ir_variable *in_var = in->as_variable();
- if (!in_var || in_var->data.mode != ir_var_shader_in)
+ if (!in_var || in_var->data.mode != ir_var_shader_in ||
+ is_gl_identifier(in_var->name))
continue;
- if (strcmp(in_var->name, out_var->name) == 0) {
+ inputs++;
+
+ unsigned len = strlen(in_var->name);
+
+ /* Handle input swizzle in variable name. */
+ const char *dot = strchr(in_var->name, '.');
+ if (dot)
+len = dot - in_var->name;
Hmm ... I wonder if this is also missing from the linker or maybe the
symbol table stuff handles this.
Variable names get mangled during optimizations so symbol table should
have the correct names left during linking.
+
+ if (strncmp(in_var->name, out_var->name, len) == 0) {
/* Since we now on
Re: [Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
On Mon, 2015-12-14 at 10:29 +0200, Tapani Pälli wrote:
> Patch makes following changes for interface matching:
>
>- do not try to match builtin variables
>- handle swizzle in input name, as example 'a.z' should
> match with 'a'
>- check that amount of inputs and outputs matches
>
> These changes make interface matching tests to work in:
>ES31-CTS.sepshaderobjs.StateInteraction
>
> Test does not still pass completely due to errors in rendering
> output. IMO this is unrelated to interface matching.
>
> v2: add spec reference, return true on desktop since we do not
> have failing cases for it, inputs and outputs amount do not
> need to match on desktop.
>
> Signed-off-by: Tapani Pälli
Hi Tapani,
Just a general comment first.
I think we should first move _mesa_validate_pipeline_io() and
validate_io() to src/mesa/main/pipelineobj.c I don't think it belongs
here right?
> ---
> src/mesa/main/shader_query.cpp | 54
> ++
> 1 file changed, 50 insertions(+), 4 deletions(-)
>
> diff --git a/src/mesa/main/shader_query.cpp
> b/src/mesa/main/shader_query.cpp
> index ced10a9..bc01b97 100644
> --- a/src/mesa/main/shader_query.cpp
> +++ b/src/mesa/main/shader_query.cpp
> @@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader
> *input_stage,
> const struct gl_shader *output_stage, bool isES)
> {
> assert(input_stage && output_stage);
> + unsigned inputs = 0, outputs = 0;
> +
> + /* Currently no matching done for desktop. */
I think the spec quote should be moved here as it applies to all the
rules in the function then you can also have the comment explaining why
validation for desktop it not done.
I've also filed a spec bug for desktop for the reasons I outlined in
irc previously. It would be great if you could quote the bug here also.
Something like:
/* FIXME: Update once Khronos spec bug #15331 is resolved. */
> + if (!isES)
> + return true;
>
> /* For each output in a, find input in b and do any required
> checks. */
> foreach_in_list(ir_instruction, out, input_stage->ir) {
>ir_variable *out_var = out->as_variable();
It's existing code but it would also be nice to have a patch that
renames input_stage/output_stage to producer_stage/consumer_stage this
it what they are called in the linker code. Maybe its just me but
getting the outputs from input_stage just looks wrong.
> - if (!out_var || out_var->data.mode != ir_var_shader_out)
> + if (!out_var || out_var->data.mode != ir_var_shader_out ||
> + is_gl_identifier(out_var->name))
> continue;
>
> + outputs++;
> +
> + inputs = 0;
>foreach_in_list(ir_instruction, in, output_stage->ir) {
Two comments here:
1. Take a look at cross_validate_outputs_to_inputs() in
link_varyings.cpp for a way to avoid the nested loop? Although it may
cause even more overhaed using the symbol table not sure.
2. Take a look at the same function for matching via explicit location.
Does the CTS not test for mismatched explicit locations? Maybe we
should create a piglit test for this as your existing code doesn't take
into account explicit locations.
I was going to suggest sharing the code between here and the linker
however I'm about to add a bunch of rules for matching the component
qualifier for enhanced layouts so not entirely sure if we should do
this what do you think?
> ir_variable *in_var = in->as_variable();
> - if (!in_var || in_var->data.mode != ir_var_shader_in)
> + if (!in_var || in_var->data.mode != ir_var_shader_in ||
> + is_gl_identifier(in_var->name))
> continue;
>
> - if (strcmp(in_var->name, out_var->name) == 0) {
> + inputs++;
> +
> + unsigned len = strlen(in_var->name);
> +
> + /* Handle input swizzle in variable name. */
> + const char *dot = strchr(in_var->name, '.');
> + if (dot)
> +len = dot - in_var->name;
Hmm ... I wonder if this is also missing from the linker or maybe the
symbol table stuff handles this.
> +
> + if (strncmp(in_var->name, out_var->name, len) == 0) {
> /* Since we now only validate precision, we can skip
> this step for
> * desktop GLSL shaders, there precision qualifier is
> ignored.
> *
> @@ -1412,7 +1431,34 @@ validate_io(const struct gl_shader
> *input_stage,
> }
>}
> }
> - return true;
> +
> + /* Amount of inputs vs outputs should match when using OpenGL ES.
> +*
> +* From OpenGL ES 3.1 spec (Interface matching):
> +*
> +*"At an interface between program objects, the set of inputs
> and outputs
> +*are considered to match exactly if and only if:
> +*
> +*- Every declared input variable has a matching output, as
> described
> +*above.
> +*
> +*- There are no user-defined output variables declared
> without a
> +*
[Mesa-dev] [PATCH v2] mesa: fix interface matching done in validate_io
Patch makes following changes for interface matching:
- do not try to match builtin variables
- handle swizzle in input name, as example 'a.z' should
match with 'a'
- check that amount of inputs and outputs matches
These changes make interface matching tests to work in:
ES31-CTS.sepshaderobjs.StateInteraction
Test does not still pass completely due to errors in rendering
output. IMO this is unrelated to interface matching.
v2: add spec reference, return true on desktop since we do not
have failing cases for it, inputs and outputs amount do not
need to match on desktop.
Signed-off-by: Tapani Pälli
---
src/mesa/main/shader_query.cpp | 54 ++
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index ced10a9..bc01b97 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -1377,19 +1377,38 @@ validate_io(const struct gl_shader *input_stage,
const struct gl_shader *output_stage, bool isES)
{
assert(input_stage && output_stage);
+ unsigned inputs = 0, outputs = 0;
+
+ /* Currently no matching done for desktop. */
+ if (!isES)
+ return true;
/* For each output in a, find input in b and do any required checks. */
foreach_in_list(ir_instruction, out, input_stage->ir) {
ir_variable *out_var = out->as_variable();
- if (!out_var || out_var->data.mode != ir_var_shader_out)
+ if (!out_var || out_var->data.mode != ir_var_shader_out ||
+ is_gl_identifier(out_var->name))
continue;
+ outputs++;
+
+ inputs = 0;
foreach_in_list(ir_instruction, in, output_stage->ir) {
ir_variable *in_var = in->as_variable();
- if (!in_var || in_var->data.mode != ir_var_shader_in)
+ if (!in_var || in_var->data.mode != ir_var_shader_in ||
+ is_gl_identifier(in_var->name))
continue;
- if (strcmp(in_var->name, out_var->name) == 0) {
+ inputs++;
+
+ unsigned len = strlen(in_var->name);
+
+ /* Handle input swizzle in variable name. */
+ const char *dot = strchr(in_var->name, '.');
+ if (dot)
+len = dot - in_var->name;
+
+ if (strncmp(in_var->name, out_var->name, len) == 0) {
/* Since we now only validate precision, we can skip this step for
* desktop GLSL shaders, there precision qualifier is ignored.
*
@@ -1412,7 +1431,34 @@ validate_io(const struct gl_shader *input_stage,
}
}
}
- return true;
+
+ /* Amount of inputs vs outputs should match when using OpenGL ES.
+*
+* From OpenGL ES 3.1 spec (Interface matching):
+*
+*"At an interface between program objects, the set of inputs and
outputs
+*are considered to match exactly if and only if:
+*
+*- Every declared input variable has a matching output, as described
+*above.
+*
+*- There are no user-defined output variables declared without a
+*matching input variable declaration.
+*
+*- All matched input and output variables have identical precision
+*qualification.
+*
+*When the set of inputs and outputs on an interface between programs
+*matches exactly, all inputs are well-defined except when the
+*corresponding outputs were not written in the previous shader.
However,
+*any mismatch between inputs and outputs will result in a validation
+*failure."
+*
+* OpenGL Core 4.5 spec includes same paragraph as above but without last
+* precision or the last 'validation failure' clause. Therefore behaviour is
+* more relaxed, input and output amount does not need to match on desktop.
+*/
+ return isES ? inputs == outputs : true;
}
/**
--
2.5.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
