[Piglit] shader_runner: ignoring uniforms in [test] header when location cannot be found instead of returning PIGLIT_FAIL

[Ian Romanick]

On 7/8/19 11:57 AM, Abel Briggs wrote:
> Hi,
> 
> Recently, I've been working on scripts for a GLSL fuzz testing framework
> that use piglit's shader_runner to render shaders. As part of the test
> generation process, shaders can be generated that declare uniforms
> without using them (which is valid GLSL according to glslangvalidator).
> One of my scripts turns the shader and uniform data from the framework's
> test format into a shader_test file and runs it via shader_runner_gles3.
> 
> However, it seems that some of these unused uniforms are optimized away
> when the shader is compiled, and when shader_runner tries to use the
> uniform data in the [test] header, it can't find the location of the
> given uniform and throws an error.

Yes.  We did that intentionally because it almost always means that
there is an error in the test case.  Over the years, this has helped me
catch errors in several tests that I have developed.

> Relevant code (there are similar pieces of code in the file for vertex
> and subuniforms):
> 
> piglit/tests/shaders/shader_runner.c, line 2289:
>> loc = glGetUniformLocation(prog, name);
>> if (loc < 0) {
>>  printf("cannot get location of uniform \"%s\"\n",
>>         name);
>>  piglit_report_result(PIGLIT_FAIL);
>> }
> 
> I'd like to discuss whether it'd be possible to instead just ignore a
> piece of uniform data if the corresponding uniform can't be found in the
> compiled shader (I'd be willing to submit a patch for this).

Rather than always ignoring the error, I'd prefer a mechanism where
specific tests can request that the error be ignored.  I'm not sure what
that would look like.  Maybe a directive in the [test] section that
disables this specific error?  Maybe a new uniform setter directive that
silently fails if the location for that uniform cannot be found?
Something else?

> Something like:
> 
>> loc = glGetUniformLocation(prog, name);
>> if (loc < 0) {
>>  printf("cannot get location of uniform \"%s\"\n",
>>         name);
>>  return;
>> }
> 
> From my limited grepping of the code, there doesn't seem to be anything
> that would fail horribly if this were to be changed (as the setting of a
> uniform only occurs at the end of set_uniform, after various checks like
> this), but please correct me if I'm wrong about any of this.
> 
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
>