On 22 July 2013 23:04, Kenneth Graunke <kenn...@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 >> > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
