Jordan Justen <jordan.l.jus...@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@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/piglit
pgpGzP6lwEGWR.pgp
Description: PGP signature
_______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit