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

[Timothy Arceri]

On 11/09/17 14:07, Ilia Mirkin wrote:
> On Sun, Sep 10, 2017 at 8:03 PM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> At the risk of sounding like a broken record. Why not just update/re-factor
>> cross_validate_outputs_to_inputs()?
>>
>> If there is a good reason not to have these checks in a single location
>> that's fine but you haven't answered the question yet.
> 
> The checks are just unrelated to one another. I thought that was
> clear, but I guess not.
> 
> cross_validate_outputs_to_inputs deals in inter-stage varyings within
> a program. At no point does it, e.g., look at vertex shader inputs. It
> won't apply to separable shaders at all (assuming a single stage per
> separate program). It looks to ensure that the interface between pairs
> of shaders matches up. Here we need to look at each interface
> separately.
> 
> I realize that there's some slight similarity in how the code looks,
> but that doesn't mean the functionality needs to be jammed in
> together.

I was just asking you to explain the difference to me since so I could 
review the code.

I obviously overlooked that we need a way to do this for SSO it wasn't 
mentioned in you commit message as a reason for requiring this change.

My questions was to establish 1. if your code was really needed and 2. 
if so we could tidy up existing code if it is. As you know I've had to 
deal with this code quite a bit.

Vertex inputs and frag shader outputs for example already have a bunch 
of code to deal with aliased variables in 
assign_attribute_or_color_locations() and we do cross stage validation 
in cross_validate_outputs_to_inputs().

Yes cross_validate_outputs_to_inputs() does interstage validation but 
there is no reason you can't make check_location_mixing() more generic 
and remove the duplicate checks from cross_validate_outputs_to_inputs(). 
Currently we check one side first and just keep the equivalent of your 
data_types struct around so we can check the other side. With your patch 
we will now validate aliased types for the separate interfaces twice for 
non SSO shaders, and the code is 90% logically equivalent it isn't just 
a slight similarity.

As you point out SSO is not currently validated a link time which means 
it could benefit from the merging of the two passes as the additional 
checks in cross_validate_outputs_to_inputs() check for overlapping 
components which we should be catching at link time.