[Mesa-dev] [PATCH V3 0/5] Interpolation fixes for Gen4/5

[Paul Berry]

On 22 July 2013 23:04, Kenneth Graunke <kenneth at whitecape.org> wrote:

> On 07/14/2013 02:39 AM, Chris Forbes wrote:
>
>> This series adds support for GLSL 1.30 / EXT_gpu_shader4's 'flat' and
>> 'noperspective' varying interpolation qualifiers on Gen4/5.
>>
>> Based on Olivier Galibert's series from July 2012, with some
>> simplifications
>> (that series contained a number of fixes for other bugs which have been
>> addressed in master already -- two-sided lighting, vue map
>> inconsistencies)
>>
>> The interpolation qualifiers are still passed through brw->, which Eric
>> doesn't like in the original version -- which I have some alternatives
>> for:
>>
>
> A couple of thoughts:
>
> The "VUE map" is a data structure we invented to store the slot
> assignments for things between the VS -> GS and GS -> FS.  We store this as
> brw->vue_map_geom_out, and flag BRW_NEW_VUE_MAP_GEOM_OUT when it changes.
>  This is done in the brw_vs state atom currently, but probably will move to
> brw_gs when we get real geometry shaders.
>
> Patch 1 of your series seems to introduce a new concept, almost...the
> "interpolation mode map".  The order of varyings isn't enough for the
> rasterization stage - we also need to know how to interpolate each of them.
>  Perhaps interpolation_mode should be formalized as a companion to
> vue_map_geom_out.  Possibly created in its own state atom, which would
> listen to BRW_NEW_VUE_MAP_GEOM_OUT and flag BRW_NEW_INTERPOLATION_MAP or
> such.
>
> I'm also wondering if this information could be reused when emitting
> 3DSTATE_SF on Gen6 and 3DSTATE_SBE on Gen7+.  I don't know if it'd save a
> ton of code, but might help legitimize it as a data structure akin to VUE
> map, and keep me from thinking that this concept is only important on
> Gen4-5 :)
>
> Paul, do you have an opinion?
>

I like the way the series looks now, with the interpolation mode map
formalized into its own state atom, but that state atom only runs on Gen4-5
where it's needed.  I'm not yet convinced that it would save us much to try
to use it on Gen6-7.

If anyone has the energy for it, it might be interesting to try computing
the interpolation mode map in Mesa core (perhaps in
do_set_program_inouts()); that way it only has to get computed once per
compile, and all back-ends could potentially benefit from it.  The tricky
bit would be dealing with the fact that color interpolation has to get
overridden based on brw->ctx.Light.ShadeModel if it's not explicitly set in
the shader.  Anyway, this is just me spitballing ideas for the future.  I
think the series looks great as is.

Thanks again for your meticulous work on this, Chris.


>
> Regardless of how that turns out, I think the rest of your patches which
> use the new info can be reviewed now...it'll get communicated somehow. I'll
> try to do that soon.
>
>
>  1) Leave it as-is, but if anything other than the program key builders
>> go looking in brw->interpolation_mode, it's *wrong*.
>>
>> 2) Tack it onto the VUE map, and have the VS code produce it at the same
>> time
>> as the VUE map itself. (When we have a GS, the GS will have to do this
>> too).
>>
>> 3) Make the clip program key the primary source, and have the SF program
>> key
>> builder go snooping in it.
>>
>> 4) Have the clip and SF program key builders redundantly compute it.
>>
>> The other review point from V2 no longer exists -- fixes to the two-sided
>> lighting behavior are already done.
>>
>> I've dropped all the Reviewed-by: lines from V2, since this has changed a
>> fair
>> bit and could do with a fresh look.
>>
>> With this series, all of the GLSL 1.30 interpolation tests pass on Gen5
>> except
>> those which use gl_ClipDistance, since it's not supported yet.
>>
>> -- Chris
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130730/aa21df0d/attachment.html>