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

[Ian Romanick]

On 12/10/2015 01:18 AM, Lofstedt, Marta wrote:
> 
> 
>> -----Original Message-----
>> From: mesa-dev [mailto:mesa-dev-bounces at lists.freedesktop.org] On
>> Behalf Of Ian Romanick
>> Sent: Friday, November 6, 2015 6:47 PM
>> To: Marta Lofstedt; mesa-dev at 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 <marta.lofstedt at intel.com>
>>>
>>> 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-stable at lists.freedesktop.org
>>
>>> Signed-off-by: Marta Lofstedt <marta.lofstedt at linux.intel.com>
>>> ---
>>>  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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev