On Mon, 2012-05-07 at 11:37 -0700, Ian Romanick wrote: > On 04/29/2012 12:54 AM, Vadim Girlin wrote: > > On Sat, 2012-04-28 at 13:02 -0700, Kenneth Graunke wrote: > >> On 04/28/2012 12:24 PM, Vadim Girlin wrote: > >>> On Sat, 2012-04-28 at 11:42 -0700, Kenneth Graunke wrote: > >>>> On 04/28/2012 11:20 AM, Vadim Girlin wrote: > >>>>> According to GLSL 1.30 specification, initial value for all uniforms > >>>>> without initializer is 0. Some applications rely on this behaviour, > >>>>> e.g. google's MapGL doesn't set all sampler uniforms. > >>>>> > >>>>> (see https://bugs.freedesktop.org/show_bug.cgi?id=49088 ) > >>>>> > >>>>> Signed-off-by: Vadim Girlin<vadimgir...@gmail.com> > >>>>> --- > >>>>> > >>>>> Tested with r600g only - no regressions. > >>>> > >>>> Awesome find! I was at a complete loss here. :) > >>>> > >>>> It turns out this is in the 1.20 spec too; it looks like 1.10 doesn't > >>>> say (but that isn't too surprising). I might add a comment: > >>>> > >>>> /* From the GLSL 1.20 specification, page 24, section 4.3.5 "Uniform": > >>>> * "The link time initial value is either the value of the variable's > >>>> * initializer, or 0 if no initializer present. Sampler types cannot > >>>> * have initializers." > >>>> */ > >>>> > >>>> Also, do you really need the memsets in ir_to_mesa and st_glsl_to_tgsi? > >>>> Everything should go through the linker, so that seems somewhat > >>>> redundant. I tested with just the link_uniforms change and that was > >>>> enough to fix MapsGL on i965/Sandybridge. > >>> > >>> Without the memset in the st_glsl_to_tgsi firefox crashed with MapGL. > >>> Probably some code in the state tracker relies on the synchronized > >>> values of the SamplerUnits arrays in the gl_program and > >>> gl_shader_program. Also, _mesa_uniform function compares these arrays to > >>> check for update, and some piglit test failed due to the following > >>> problem: > >>> > >>> First array is initialized: 0, 0, 0, ... > >>> Second (without memset) : 0, 1, 2, ... > >>> > >>> The app calls Uniform1i to set sampler[1] to 1, so we do first[1]=1, but > >>> then it's equal with second[1] and update is not detected. > >>> > >>> Vadim > >> > >> Ah, okay...I missed that one was operating on the gl_shader_program > >> while the other was setting the parallel structure in the gl_program. > >> > >> I can believe that you'd need that, then. > >> > > > > BTW, it seems there is a patch from Ian for this issue on the list: > > > > http://lists.freedesktop.org/archives/mesa-dev/2012-April/020767.html > > Right. Without the first patch in that series, the patch you reference > probably isn't sufficient (due to ir_to_mesa and st_glsl_to_tgsi not > being modified). Eric had a couple concerns about the first patch in > the series, and I haven't had a chance to put together a test case for > those issues. > > Maybe we could combine my patch with the ir_to_mesa and st_glsl_to_tgsi > parts from yours?
Of course, feel free to use any parts of this patch if you need. Vadim _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
