[Piglit] [PATCH] Add test case clearbuffer mixed format

[Eric Anholt]

On Mon, 12 Dec 2011 11:29:06 -0800, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> Adding a new testcase to verify clearing mixed format color buffers with 
> glClearBufferfv/glClearBufferiv/glClearBufferuiv
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>

> +/**
> + * \file clearbuffer-mixed-format.c
> + * Verify clearing mixed format color buffers with glClearBufferfv/glClearBufferiv/glClearBufferuiv
> + *
> + * This test works by generating several mixed foarmt color framebuffer objects and attempting to
> + * clear the color buffer of those FBOs by calling glClearBufferfv/glClearBufferiv/glClearBufferuiv.

We generally wrap our code at 80 lines, unless there's some clear
formatting reason to do so.

> + *     - FBOs with a float color buffer attachment. glClearBufferfv should clear the color buffer to
> + *     	 desired float value. glClearBufferuiv/glClearBufferiv should not modify the color data in 
> + *       float color buffer.

Instead of paraphrasing the spec, is there a short section that could be
directly quoted?

> +#define FBO_COUNT 10

It looks like this should just be ARRAY_SIZE(test_vectors)

> +	if (status == GL_FRAMEBUFFER_UNSUPPORTED) {
> +		glDeleteRenderbuffers(1,&rb);

s/,/, /

> +		glDeleteFramebuffers(1, &fb);
> +		return 0;
> +	}
> +
> +	/* Color buffer is cleared to default RGBA (0, 0, 0, 0) color */
> +	glClear(GL_COLOR_BUFFER_BIT);
> +	glFinish();

Why is there a glFinish() here?

> +int
> +probe_rect_color(int x, int y, int w, int h, GLenum format, GLenum type, GLvoid *refcolor)
> +{

> +return 1;

For a function returning a success value, please use "bool" and
true/false instead of ints.  Also, indent the return statement.

> +}

> +
> +void piglit_init(int argc, char **argv)
> +{
> +	static const float fcolor[4][4] = { { 0.5,  0.3,  0.7,  0.0 },
> +	                                    { 0.8,  0.0,  0.2,  1.0 }, 
> +	                                    { 1.2, -2.9,  0.2,  5.8 },
> +	                                    { 0.5,  2.5, -5.2,  1.0 } };
> +
> +	static const unsigned int  uicolor[6][4] = { {  10,   90, 100, 150 },
> +	                                             { 100,  190, 200,  15 }, 
> +	                                             { 100,  190, 200,  15 }, 
> +	                                             {  10,   80,  0,   25 }, 
> +	                                             { 110,   85,  10,  75 }, 
> +	                                             {  90,  120,  60,  30 } };
> +
> +	static const unsigned int  icolor[6][4] = { {  -10,  -90, 100,  15 },
> +	                                             { 100,  190, 200, -15 }, 
> +	                                             { -50,  -50, -50,  50 }, 
> +	                                             { -30,  -10,  20,  50 }, 
> +	                                             { -70,   20, -40, 100 }, 
> +	                                             { -80,  170,   0,   0 } };

trim trailing whitespace throughout this function.  You can run the
following command once to always catch this before committing:

chmod +x .git/hooks/pre-commit

> +        GLuint fb[FBO_COUNT];

weird indentation

> +		/* Test the color buffer values of all existing FBOs against their expected color values.
> +		 * Previously issued glClearBuffer[uif]v should not modify color buffers of other existing FBOs
> +		 * */

/* Please line up your comments like this, notably the closing slash
 * should be right next to the line of asterisks.
 */

> +		for (j =0; j< i; j++)

Please consistently put a space before and after binary operators,
unless there's a clear formatting need otherwise.

> +       	/* Delete all generated framebuffer objects */

weird indentation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20111213/863eb97f/attachment.pgp>