Re: [Mesa-dev] [PATCH] glsl: Relax requirement on Centroid matching between shader stages

2015-12-10 Thread Ian Romanick 
On 12/10/2015 01:18 AM, Lofstedt, Marta wrote:
> 
> 
>> -Original Message-
>> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
>> Behalf Of Ian Romanick
>> Sent: Friday, November 6, 2015 6:47 PM
>> To: Marta Lofstedt; mesa-dev@lists.freedesktop.org
>> Subject: Re: [Mesa-dev] [PATCH] glsl: Relax requirement on Centroid
>> matching between shader stages
>>
>> On 11/04/2015 01:04 AM, Marta Lofstedt wrote:
>>> From: Marta Lofstedt 
>>>
>>> In OpenGL 4.4, section 4.5, the requirement for interpolation
>>> qualifiers to match over shader stages was removed.
>>> In OpenGL ES 3.1, section 9.2.1 there is a table showing that centroid
>>> does not have to match between shader stages.
>>
>> I haven't checked this yet... does either spec provide guidance as to what
>> happens when only the vertex shader stage specifies centroid?  Is the
>> interpolation centroid or not?
> 
> GLSL ES 3.1 and GLSL 4.4 section 4.3.4:
> " When auxiliary qualifiers do 
> not match, those provided in the fragment shader supersede those provided in 
> previous stages. If any such 
> qualifiers are completely missing in the fragment shaders, then the default 
> is used, rather than any 
> qualifiers that may have been declared in previous stages"
> 
>> ...and are we doing the right thing? :)
>
> That is a deep question, Ian...
> I guess I could add a complicated test enforcing matching for versions prior 
> to GLSL ES 3.1 and GLSL 4.4. 
> But since the discussion in bug 92743 suggests that the matching requirement 
> is fuzzy in the prior versions of the standars, I see no point in messing 
> things up.

I don't think we need s complicated test.

It sounds like there are two possible sets of rules we could choose to
follow for centroid.  Either the VS and FS have to match or they don't
have to match so FS "wins."  In both cases we would use the centroid
setting from the FS.  Right?

I think we just need two (additional) tests:

 - VS w/centroid, FS w/o -> non-centroid
 - VS w/o centroid, FS w/ -> centroid

It should be easy to adapt an existing centroid test.

For the other qualifiers... unless there are version-specific desktop
OpenGL conformance tests for this behavior, I think we should just do
the GLSL 4.40.  It seems more likely that applications will expect the
latest behavior for something that isn't hardware related.  We should
probably emit a warning.  I'll add something to the bug.

>>> Also see bug 92743 for more discussions.
>>
>> Please note this, after your s-o-b, as
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
>>
>> I think this is also a candidate for stable:
>>
>> Cc: mesa-sta...@lists.freedesktop.org
>>
>>> Signed-off-by: Marta Lofstedt 
>>> ---
>>>  src/glsl/link_varyings.cpp | 14 --
>>>  1 file changed, 14 deletions(-)
>>>
>>> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
>>> index 7e77a67..d0edc71 100644
>>> --- a/src/glsl/link_varyings.cpp
>>> +++ b/src/glsl/link_varyings.cpp
>>> @@ -96,20 +96,6 @@ cross_validate_types_and_qualifiers(struct
>> gl_shader_program *prog,
>>>}
>>> }
>>>
>>> -   /* Check that all of the qualifiers match between stages.
>>> -*/
>>> -   if (input->data.centroid != output->data.centroid) {
>>> -  linker_error(prog,
>>> -   "%s shader output `%s' %s centroid qualifier, "
>>> -   "but %s shader input %s centroid qualifier\n",
>>> -   _mesa_shader_stage_to_string(producer_stage),
>>> -   output->name,
>>> -   (output->data.centroid) ? "has" : "lacks",
>>> -   _mesa_shader_stage_to_string(consumer_stage),
>>> -   (input->data.centroid) ? "has" : "lacks");
>>> -  return;
>>> -   }
>>> -
>>> if (input->data.sample != output->data.sample) {
>>>linker_error(prog,
>>> "%s shader output `%s' %s sample qualifier, "
>>>
>>
>> ___
>> 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

Re: [Mesa-dev] [PATCH] glsl: Relax requirement on Centroid matching between shader stages

2015-12-10 Thread Lofstedt, Marta 


> -Original Message-
> From: mesa-dev [mailto:mesa-dev-boun...@lists.freedesktop.org] On
> Behalf Of Ian Romanick
> Sent: Friday, November 6, 2015 6:47 PM
> To: Marta Lofstedt; mesa-dev@lists.freedesktop.org
> Subject: Re: [Mesa-dev] [PATCH] glsl: Relax requirement on Centroid
> matching between shader stages
> 
> On 11/04/2015 01:04 AM, Marta Lofstedt wrote:
> > From: Marta Lofstedt 
> >
> > In OpenGL 4.4, section 4.5, the requirement for interpolation
> > qualifiers to match over shader stages was removed.
> > In OpenGL ES 3.1, section 9.2.1 there is a table showing that centroid
> > does not have to match between shader stages.
> 
> I haven't checked this yet... does either spec provide guidance as to what
> happens when only the vertex shader stage specifies centroid?  Is the
> interpolation centroid or not?

GLSL ES 3.1 and GLSL 4.4 section 4.3.4:
" When auxiliary qualifiers do 
not match, those provided in the fragment shader supersede those provided in 
previous stages. If any such 
qualifiers are completely missing in the fragment shaders, then the default is 
used, rather than any 
qualifiers that may have been declared in previous stages"

> 
> ...and are we doing the right thing? :)
>
That is a deep question, Ian...
I guess I could add a complicated test enforcing matching for versions prior to 
GLSL ES 3.1 and GLSL 4.4. 
But since the discussion in bug 92743 suggests that the matching requirement is 
fuzzy in the prior versions of the standars, I see no point in messing things 
up.

>
> > Also see bug 92743 for more discussions.
> 
> Please note this, after your s-o-b, as
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743
> 
> I think this is also a candidate for stable:
> 
> Cc: mesa-sta...@lists.freedesktop.org
> 
> > Signed-off-by: Marta Lofstedt 
> > ---
> >  src/glsl/link_varyings.cpp | 14 --
> >  1 file changed, 14 deletions(-)
> >
> > diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> > index 7e77a67..d0edc71 100644
> > --- a/src/glsl/link_varyings.cpp
> > +++ b/src/glsl/link_varyings.cpp
> > @@ -96,20 +96,6 @@ cross_validate_types_and_qualifiers(struct
> gl_shader_program *prog,
> >}
> > }
> >
> > -   /* Check that all of the qualifiers match between stages.
> > -*/
> > -   if (input->data.centroid != output->data.centroid) {
> > -  linker_error(prog,
> > -   "%s shader output `%s' %s centroid qualifier, "
> > -   "but %s shader input %s centroid qualifier\n",
> > -   _mesa_shader_stage_to_string(producer_stage),
> > -   output->name,
> > -   (output->data.centroid) ? "has" : "lacks",
> > -   _mesa_shader_stage_to_string(consumer_stage),
> > -   (input->data.centroid) ? "has" : "lacks");
> > -  return;
> > -   }
> > -
> > if (input->data.sample != output->data.sample) {
> >linker_error(prog,
> > "%s shader output `%s' %s sample qualifier, "
> >
> 
> ___
> 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

Re: [Mesa-dev] [PATCH] glsl: Relax requirement on Centroid matching between shader stages

2015-11-06 Thread Ian Romanick 
On 11/04/2015 01:04 AM, Marta Lofstedt wrote:
> From: Marta Lofstedt 
> 
> In OpenGL 4.4, section 4.5, the requirement for interpolation
> qualifiers to match over shader stages was removed.
> In OpenGL ES 3.1, section 9.2.1 there is a table showing that
> centroid does not have to match between shader stages.

I haven't checked this yet... does either spec provide guidance as to
what happens when only the vertex shader stage specifies centroid?  Is
the interpolation centroid or not?

...and are we doing the right thing? :)

> Also see bug 92743 for more discussions.

Please note this, after your s-o-b, as

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92743

I think this is also a candidate for stable:

Cc: mesa-sta...@lists.freedesktop.org

> Signed-off-by: Marta Lofstedt 
> ---
>  src/glsl/link_varyings.cpp | 14 --
>  1 file changed, 14 deletions(-)
> 
> diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
> index 7e77a67..d0edc71 100644
> --- a/src/glsl/link_varyings.cpp
> +++ b/src/glsl/link_varyings.cpp
> @@ -96,20 +96,6 @@ cross_validate_types_and_qualifiers(struct 
> gl_shader_program *prog,
>}
> }
>  
> -   /* Check that all of the qualifiers match between stages.
> -*/
> -   if (input->data.centroid != output->data.centroid) {
> -  linker_error(prog,
> -   "%s shader output `%s' %s centroid qualifier, "
> -   "but %s shader input %s centroid qualifier\n",
> -   _mesa_shader_stage_to_string(producer_stage),
> -   output->name,
> -   (output->data.centroid) ? "has" : "lacks",
> -   _mesa_shader_stage_to_string(consumer_stage),
> -   (input->data.centroid) ? "has" : "lacks");
> -  return;
> -   }
> -
> if (input->data.sample != output->data.sample) {
>linker_error(prog,
> "%s shader output `%s' %s sample qualifier, "
> 

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] glsl: Relax requirement on Centroid matching between shader stages

2015-11-04 Thread Marta Lofstedt 
From: Marta Lofstedt 

In OpenGL 4.4, section 4.5, the requirement for interpolation
qualifiers to match over shader stages was removed.
In OpenGL ES 3.1, section 9.2.1 there is a table showing that
centroid does not have to match between shader stages.

Also see bug 92743 for more discussions.

Signed-off-by: Marta Lofstedt 
---
 src/glsl/link_varyings.cpp | 14 --
 1 file changed, 14 deletions(-)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 7e77a67..d0edc71 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -96,20 +96,6 @@ cross_validate_types_and_qualifiers(struct gl_shader_program 
*prog,
   }
}
 
-   /* Check that all of the qualifiers match between stages.
-*/
-   if (input->data.centroid != output->data.centroid) {
-  linker_error(prog,
-   "%s shader output `%s' %s centroid qualifier, "
-   "but %s shader input %s centroid qualifier\n",
-   _mesa_shader_stage_to_string(producer_stage),
-   output->name,
-   (output->data.centroid) ? "has" : "lacks",
-   _mesa_shader_stage_to_string(consumer_stage),
-   (input->data.centroid) ? "has" : "lacks");
-  return;
-   }
-
if (input->data.sample != output->data.sample) {
   linker_error(prog,
"%s shader output `%s' %s sample qualifier, "
-- 
2.1.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev