[Piglit] [PATCH] ext_texture_format_bgra8888: Add test for GL_BGRA format in Tex(Sub)Image calls

[Dylan Baker]

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 <elima at igalia.com> 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
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151015/89e95027/attachment.sig>