[Piglit] [RFC] Proposed changes to shader_runner

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brian Paul wrote:
> On Tue, Aug 24, 2010 at 7:19 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>
>> I'm considering making a couple changes to shader_runner, but I want to
>> solicit feedback before I do.
>>
>> I'm finishing up some code for Mesa's new GLSL compiler that does
>> processing on loops.  I want to create some additional "tricky" loop
>> tests to make sure some of the corner cases are handled correctly.  One
>> of the tests has a shader like the one below.  I want to set i1 to 0 and
>> i2 to 5.  If the dependency handling is correct, the shader will produce
>> green.  If it is not correct, the shader will produce red.
>>
>> uniform int i1;
>> uniform int i2;
>> uniform float f;
>>
>> void main()
>> {
>>  int c = i1;
>>  int i;
>>  float o = f;
>>
>>  i = c;
>>  c = i2;
>>  while (i < 5) {
>>    o += 0.2;
>>    i++;
>>  }
>>
>>   gl_FragColor = vec4(1.0 - (o / 2.0), o / 2.0, 0.0, 1.0);
>> }
>>
>> The problem is that the compiler (correctly) optimizes i2 away.  The
>> call to glGetUniformLocation in shader_runner fails, and the test
>> aborts.  Nvidia's implementation does the same.
> 
> So why do you need i2 at all in this example?

In this case I was trying to tease out a bug where the value used to
determine the loop count was (5-i2) instead of (5-i1).

> If you want to keep i2, you could also force/trick it into not being
> optimized away by using it in an expression for the result color
> (maybe "gl_Fragcolor.z = i2 - i2" or some other expression).

I thought about doing something like that, but I found it
unsatisfactory.  Not that in this case the optimizer will replace
(i2-i2) with 0, and i2 will still get optimized out. :)

The other option is to use an ivec2 and do 'c = iv.x' for the first
assignment and 'c = iv.y' for the second.  I think I'll just do that,
and I'll leave the glGetUniformLocation check alone.

>> I want to change the error check in shader_runner to drop uniforms that
>> return -1 from glGetUniformLocation.  The problem is that this error
>> check has helped me (and presumably others) catch errors in their tests.
>>  Should I change it to emit a warning in the non-auto case?
>>
>> The other option is to just use the GLSL 1.20 uniform initializer
>> syntax, but this won't work with sampler uniforms (when we add texture
>> support to shader runner).
> 
> You could also add another option to the shader runner config file to
> indicate that a uniform might be optimized away and skip the error
> check.

I thought about doing that too, but that felt like a lot of clutter in
shader_runner.

>> The other change that I want to make, and I know Eric has talked about
>> doing this too, is to "ignore" checks for "GL >= 2.0".  When
>> shader_runner sees "GL >= 2.0" it will interpret it as a request for
>> OpenGL 2.0 or GL_ARB_shader_objects, GL_ARB_vertex_shader, and
>> GL_ARB_fragment_shader.  Other than potentially making the code messy, I
>> don't see any ill effects of this.
>>
>> The main upshot is that those few implementations that can do GLSL but
>> lack some other OpenGL 2.0 features (i.e., i915 which lack occlusion
>> query) can play.
>>
>> Opinions?
> 
> Sounds fine.

Okay.  I'll probably make these changes later this week.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkx0v6AACgkQX1gOwKyEAw+mxgCeJrDWCam5ANT8690ojg06qQGl
/5sAn39PA7BRNZq5OGd0Jl3TySO1yUhF
=7Mo8
-----END PGP SIGNATURE-----