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

[Ilia Mirkin]

On Mon, Sep 11, 2017 at 1:59 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
> 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.

Sounds like a pretty substantial change. I definitely won't have time
to do it. I guess this patch is withdrawn for now until someone with
more time can pick it back up.

  -ilia