[Mesa-dev] [PATCH] glsl: disallow mixed varying types within a location

[Timothy Arceri]

On 06/09/17 11:59, Ilia Mirkin wrote:
> On Tue, Sep 5, 2017 at 9:54 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>>
>> On 06/09/17 11:23, Ilia Mirkin wrote:
>>>
>>> The enhanced layouts spec has all kinds of restrictions about what can
>>> and cannot be mixed in a location. Integer/float(/presumably double)
>>> can't occupy a single location, interpolation has to be the same, as
>>> well as auxiliary storage (including patch!).
>>>
>>> The implication of this is ... don't specify explicit locations/components
>>> if you want better packing, since the auto-packer doesn't care at all
>>> about most of these restrictions. Sad.
>>
>>
>> There are still use cases such as SSO, tessellation shaders and varyings
>> used by interpolateAt (although we just enable the enhanced layout packing
>> rules by default for those anyway) were we cannot use the auto-packer.
>>
>> As far as the patch goes this should really be in link_varyings.cpp rather
>> than linker.cpp, also there is already related validation code in
>> cross_validate_outputs_to_inputs() any reason for not just modifying the
>> code there?
> 
> This applies to whole shader stages. So e.g. in SSO, you still
> validate the inputs and the outputs. Similarly, you do this for vertex
> shader inputs.

I'm not following what you are trying to say here. 
cross_validate_outputs_to_inputs() does almost the same thing you are 
doing here, but also validates the outputs from one stage with the input 
from another. You just need to adjust the offsets for patches like you 
have done here and add the missing interpolation checks etc, the base 
type is already validated there.

> 
> Ideally it'd be done earlier on, but we need to wait for the interface
> types to go away, or else it'd be a disaster.
> 
> Most of link_varyings is concerned with inter-stage logic. It could be
> moved there, of course, just didn't really seem to belong.

link_varyings should be used for all things varyings unless it really 
doesn't make sense. There is no reason to dump everything in linker.cpp. 
The only reason there are still bits in linker.cpp is because Paul left 
the project in the middle of re-factoring things and beside a single 
patch from me that moved more varying related code here a while ago, 
there has been pretty much zero effort in re-factoring any of the GLSL 
IR compiler into more sensible pieces.