On 2015-01-31 11:45:59, Francisco Jerez wrote: > 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.
Okay. Let's go ahead with this for now. Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com> > > > >> - 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 _______________________________________________ Piglit mailing list Piglit@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/piglit