[Piglit] [PATCH 01/23] util/shader: Define "nothrow" variant of piglit_compile_shader_text().

[Francisco Jerez]

Jordan Justen <jordan.l.justen at intel.com> writes:

> On 2015-01-31 07:41:23, Francisco Jerez wrote:
>> Define a variant of piglit_compile_shader_text() that doesn't call
>> piglit_report_result() on failure killing the program, which is quite
>> annoying for tests that expect a compilation to fail and for tests
>> that are structured in a number of subtests, because a single sub-test
>> failing to compile a shader will prevent the remaining tests from
>> running.
>> 
>> I guess this would ideally be the default behavior of
>> piglit_compile_shader_text(), but with >300 callers in tree it seems
>> rather difficult to change at this stage.
>
> sed?
>
> Maybe piglit_compile_shader_text => piglit_require_compile_shader_text?
>
I personally don't care enough to make such a change affecting hundreds
of other tests that wouldn't have the slightest chance of being reviewed
before it no longer applies cleanly.  I would support the change though
if you feel like doing it.

>> ---
>>  tests/util/piglit-shader.c | 20 ++++++++++++++++++--
>>  tests/util/piglit-shader.h |  1 +
>>  2 files changed, 19 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tests/util/piglit-shader.c b/tests/util/piglit-shader.c
>> index e8fe9c4..37cc7cc 100644
>> --- a/tests/util/piglit-shader.c
>> +++ b/tests/util/piglit-shader.c
>> @@ -122,7 +122,7 @@ shader_name(GLenum target)
>>   * Convenience function to compile a GLSL shader.
>>   */
>>  GLuint
>> -piglit_compile_shader_text(GLenum target, const char *text)
>> +piglit_compile_shader_text_nothrow(GLenum target, const char *text)
>>  {
>>         GLuint prog;
>>         GLint ok;
>> @@ -149,7 +149,8 @@ piglit_compile_shader_text(GLenum target, const char *text)
>>                                 info);
>>  
>>                         fprintf(stderr, "source:\n%s", text);
>
> Do you think piglit_compile_shader_text_nothrow should be silent if
> the shader fails to compile?
>
> Maybe move the fprintf's as well?
>

As the main motivation for this function is to avoid killing the program
when a shader compilation fails, I think the fprintf() is fine and
useful to find out what is going on when something fails.  But we could
make it dependent on a parameter or factor it out to a separate function
if you like.

> -Jordan
>
>> -                       piglit_report_result(PIGLIT_FAIL);
>> +                       glDeleteShader(prog);
>> +                       prog = 0;
>>                 }
>>                 else if (0) {
>>                         /* Enable this to get extra compilation info.
>> @@ -164,6 +165,21 @@ piglit_compile_shader_text(GLenum target, const char *text)
>>         return prog;
>>  }
>>  
>> +/**
>> + * Convenience function to compile a GLSL shader.  Throws PIGLIT_FAIL
>> + * on error terminating the program.
>> + */
>> +GLuint
>> +piglit_compile_shader_text(GLenum target, const char *text)
>> +{
>> +        GLuint shader = piglit_compile_shader_text_nothrow(target, text);
>> +
>> +        if (!shader)
>> +                piglit_report_result(PIGLIT_FAIL);
>> +
>> +        return shader;
>> +}
>> +
>>  static GLboolean
>>  link_check_status(GLint prog, FILE *output)
>>  {
>> diff --git a/tests/util/piglit-shader.h b/tests/util/piglit-shader.h
>> index e2eef03..9208451 100644
>> --- a/tests/util/piglit-shader.h
>> +++ b/tests/util/piglit-shader.h
>> @@ -31,6 +31,7 @@
>>  void piglit_get_glsl_version(bool *es, int* major, int* minor);
>>  
>>  GLuint piglit_compile_shader(GLenum target, const char *filename);
>> +GLuint piglit_compile_shader_text_nothrow(GLenum target, const char *text);
>>  GLuint piglit_compile_shader_text(GLenum target, const char *text);
>>  GLboolean piglit_link_check_status(GLint prog);
>>  GLboolean piglit_link_check_status_quiet(GLint prog);
>> -- 
>> 2.1.3
>> 
>> _______________________________________________
>> Piglit mailing list
>> Piglit at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20150131/067718fd/attachment.sig>