On Thu, Oct 15, 2015 at 06:25:26PM +0100, Emil Velikov wrote: > Hi Eduardo, > > A few humble suggestions. > > On 15 October 2015 at 02:50, Eduardo Lima Mitev <[email protected]> wrote: > > This is a new test that checks valid and invalid combinations of GL_BGRA_EXT > > format and internal format in Tex(Sub)Image2D calls, as specified by the > > EXT_texture_format_BGRA8888 extension > > <https://www.khronos.org/registry/gles/extensions/EXT/EXT_texture_format_BGRA8888.txt>. > There is no need to provide a link to the spec in commit messages. > > [snip] > > +static GLboolean > Please use bool. > > > +run_test(void) > > +{ > > + GLuint tex; > > + > > + glGenTextures(1, &tex); > > + glBindTexture(GL_TEXTURE_2D, tex); > > + if (!piglit_check_gl_error(GL_NO_ERROR)) > > + return false; > > + > > + /* glTexImage2D */ > > + glTexImage2D(GL_TEXTURE_2D, 0, GL_BGRA_EXT, 2, 2, 0, GL_BGRA_EXT, > > GL_UNSIGNED_BYTE, NULL); > Wrap the last three arguments to the next line ? Here and below. > > > + if (!piglit_check_gl_error(GL_NO_ERROR)) > > + return false; > Most places in piglit continue running all (sub)tests, even if there > is a failure midway. One common pattern is > > bool pass = true;
Yes please. Not running all of the subtests (or at least returning values for them) breaks assumptions made by the framework. Dylan > > ... > > glFoo() > if (!piglit_check_gl_error...) > pass = false; > > ... > return pass; > > > Cheers, > Emil > > P.S. piglit tries to use tabs for indentation, albeit some tests use spaces. > _______________________________________________ > Piglit mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/piglit
signature.asc
Description: PGP signature
_______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
