On Mon, Sep 11, 2017 at 1:59 AM, Timothy Arceri <tarc...@itsqueeze.com> wrote: > On 11/09/17 14:07, Ilia Mirkin wrote: >> >> On Sun, Sep 10, 2017 at 8:03 PM, Timothy Arceri <tarc...@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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
