[Mesa-dev] [PATCH 1/2] glsl: Add uniform_locations_assigned parameter to do_dead_code opt pass

[Brian Paul]

On 10/21/2011 07:34 PM, Ian Romanick wrote:
> On 10/21/2011 12:53 PM, Brian Paul wrote:
>> On 10/21/2011 01:23 PM, Brian Paul wrote:
>>> On 10/21/2011 12:49 PM, Ian Romanick wrote:
>>>> From: Ian Romanick<ian.d.romanick at intel.com>
>>>>
>>>> Setting this flag prevents declarations of uniforms from being
>>>> removed
>>>> from the IR. Since the IR is directly used by several API functions
>>>> that query uniforms in shaders, uniform declarations cannot be
>>>> removed
>>>> after the locations have been set. However, it should still be safe
>>>> to reorder the declarations (this is not tested).
>>>>
>>>> Signed-off-by: Ian Romanick<ian.d.romanick at intel.com>
>>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41980
>>>> Cc: Brian Paul<brianp at vmware.com>
>>>> Cc: Bryan Cain<bryancain3 at gmail.com>
>>>> Cc: Vinson Lee<vlee at vmware.com>
>>>> Cc: José Fonseca<jfonseca at vmware.com>
>>>> Cc: Kenneth Graunke<kenneth at whitecape.org>
>>>
>>> I applied these two patches to my tree but I'm still seeing the
>>> GL_INVALID_OPERATION error in glUniform() calls that I mentioned in
>>> the bug report (param->DataType = 0).
>>>
>>> The viewperf test I'm using has a fairly complex vertex shader with an
>>> array of structures of lighting info containing a "vec3 position"
>>> field.
>>>
>>> The test calls glGetUniform(shader, "LW_LightSource1[0].position").
>>> When glUniform3f() is called to set that uniform, the
>>> GL_INVALID_OPERATION errro is raised.
>>>
>>> I'll try a bisection to see where this started failing.
>>
>> The problem started at commit 58a7461e1672935e7d30780a4dd40c00abbc28a5
>
> That's basically the same commit that Vinson bisected to. The only
> difference is this is the commit for ir_to_mesa, and his was the
> commit for st_glsl_to_tgsi.
>
>> I'm attaching a piglit shader test that exercises the bug.
>
> That's a useful test case. We should add it to piglit. I still don't
> understand what's going wrong. :( What really confuses me is there
> were no piglit regressions when I originally sent this code out for
> review.

Feel free to add the shader test to piglit.

BTW, if you add this assertion in ir_to_mesa.cpp you can catch where 
things start to go wrong:

@@ -2612,6 +2612,7 @@ add_uniform_to_shader(ir_variable *var,

     int index = _mesa_lookup_parameter_index(params, -1, var->name);
     if (index < 0) {
+      assert(type->gl_type);
        index = _mesa_add_parameter(params, file,
                                   var->name, size, type->gl_type,
                                   NULL, NULL, 0x0);


-Brian