<div dir="ltr">On 22 July 2013 23:04, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="im">On 07/14/2013 02:39 AM, Chris Forbes wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
This series adds support for GLSL 1.30 / EXT_gpu_shader4's 'flat' and<br>
'noperspective' varying interpolation qualifiers on Gen4/5.<br>
<br>
Based on Olivier Galibert's series from July 2012, with some simplifications<br>
(that series contained a number of fixes for other bugs which have been<br>
addressed in master already -- two-sided lighting, vue map inconsistencies)<br>
<br>
The interpolation qualifiers are still passed through brw->, which Eric<br>
doesn't like in the original version -- which I have some alternatives for:<br>
</blockquote>
<br></div>
A couple of thoughts:<br>
<br>
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.<br>
<br>
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.<br>
<br>
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 :)<br>
<br>
Paul, do you have an opinion?<br></blockquote><div><br></div><div>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.<br>
<br></div><div>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.<br>
<br></div><div>Thanks again for your meticulous work on this, Chris.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
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.<div class=""><div class="h5"><br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
1) Leave it as-is, but if anything other than the program key builders<br>
go looking in brw->interpolation_mode, it's *wrong*.<br>
<br>
2) Tack it onto the VUE map, and have the VS code produce it at the same time<br>
as the VUE map itself. (When we have a GS, the GS will have to do this too).<br>
<br>
3) Make the clip program key the primary source, and have the SF program key<br>
builder go snooping in it.<br>
<br>
4) Have the clip and SF program key builders redundantly compute it.<br>
<br>
The other review point from V2 no longer exists -- fixes to the two-sided<br>
lighting behavior are already done.<br>
<br>
I've dropped all the Reviewed-by: lines from V2, since this has changed a fair<br>
bit and could do with a fresh look.<br>
<br>
With this series, all of the GLSL 1.30 interpolation tests pass on Gen5 except<br>
those which use gl_ClipDistance, since it's not supported yet.<br>
<br>
-- Chris<br>
</blockquote>
<br>
</div></div></blockquote></div><br></div></div>