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

[Ian Romanick]

On 7/8/19 8:08 PM, Abel Briggs wrote:
> Maybe a command line argument could work?


Yeah... if there's no need to have tests with unused uniforms in piglit,
that's probably the easiest way.

> Thanks,
> 
> Abel
> 
> ------------------------------------------------------------------------
> *From:* Ian Romanick <idr at freedesktop.org>
> *Sent:* Monday, July 8, 2019 2:27:16 PM
> *To:* Abel Briggs; piglit at lists.freedesktop.org
> *Subject:* Re: [Piglit] shader_runner: ignoring uniforms in [test]
> header when location cannot be found instead of returning PIGLIT_FAIL
>  
> 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
>> 
>