[Piglit] [PATCH] [v2] Add test case of render FBO with point coordinate.

[Ian Romanick]

On 01/18/2012 03:24 AM, Jian Zhao wrote:
> Add test case of rendering with point coordinate, it should work with
> direct rendering, rendering with FBO.
>
> As the es spec 2.0.25 says:
> "the gl-PointCoord fragment shader input defines a per-fragment coordinate
> space (s; t) where s varies from 0 to 1 across the point horizontally
> left-to-right, and t ranges from 0 to 1 across the point vertically
> top-to-bottom."
>
> v2: To simplify the case, set the point size in the program instead of the
> shader. (per Yuanhan's suggestion)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44613
> Signed-off-by: Jian Zhao<jian.j.zhao at intel.com>
> ---
>   tests/all.tests              |    1 +
>   tests/bugs/CMakeLists.gl.txt |    1 +
>   tests/bugs/pointcoord.c      |  115 ++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 117 insertions(+), 0 deletions(-)
>   create mode 100644 tests/bugs/pointcoord.c
>
> diff --git a/tests/all.tests b/tests/all.tests
> index 2e23634..a0babe2 100644
> --- a/tests/all.tests
> +++ b/tests/all.tests
> @@ -566,6 +566,7 @@ add_plain_test(bugs, 'fdo25614-genmipmap')
>   add_plain_test(bugs, 'fdo28551')
>   add_plain_test(bugs, 'fdo31934')
>   add_plain_test(bugs, 'point-sprite')
> +add_plain_test(bugs, 'pointcoord')

Put this in tests/fbo and call it fbo-gl_PointCoord.

>   add_plain_test(bugs, 'r300-readcache')
>   add_plain_test(bugs, 'tex1d-2dborder')
>   add_plain_test(bugs, 'tri-tex-crash')
> diff --git a/tests/bugs/CMakeLists.gl.txt b/tests/bugs/CMakeLists.gl.txt
> index 5c1864e..ed4b528 100644
> --- a/tests/bugs/CMakeLists.gl.txt
> +++ b/tests/bugs/CMakeLists.gl.txt
> @@ -24,6 +24,7 @@ add_executable (r300-readcache r300-readcache.c)
>   add_executable (tex1d-2dborder tex1d-2dborder.c)
>   add_executable (fdo20701 fdo20701.c)
>   add_executable (point-sprite point-sprite.c)
> +add_executable (pointcoord pointcoord.c)
>   add_executable (fdo22540 fdo22540.c)
>   add_executable (fdo23489 fdo23489.c)
>   add_executable (fdo23670-depth_test fdo23670-depth_test.c)
> diff --git a/tests/bugs/pointcoord.c b/tests/bugs/pointcoord.c
> new file mode 100644
> index 0000000..6b03fb3
> --- /dev/null
> +++ b/tests/bugs/pointcoord.c
> @@ -0,0 +1,115 @@
> +/*
> + * Copyright © 2012 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + *
> + * Authors:
> + *  *    Jian Zhao<jian.j.zhao at intel.com>

This is unnecessary.  git-log already captures authorship information.

> + *
> + */
> +
> +/**
> + * \file pointcoord.c
> + * Verify that applications can use point coordinate correctly,
> + * especailly with FBO.
> + */
> +
> +#include "piglit-util.h"
> +
> +int piglit_width = 100, piglit_height = 100;
> +int piglit_window_mode = GLUT_RGB | GLUT_SINGLE;
> +
> +static const char vs_text[] =
> +	"#version 120 \n"
> +	"attribute vec4 vPosition;\n"
> +	"void main()\n"
> +	"{\n"
> +	"gl_Position = vPosition;\n"
> +	"}\n"
> +	;

Since your code never calls glBindAttribLocation or glGetAttribLocation, 
you should just use gl_Vertex.

> +
> +static const char fs_text[] =
> +	"#version 120 \n"
> +	"void main() { gl_FragColor = vec4(gl_PointCoord.x, gl_PointCoord.y, 0.0, 1.0); }\n"
> +	;
> +static GLuint prog;
> +
> +static const float green[] = { 0.0, 1.0, 0.0, 1.0 };
> +static const float black[] = { 0.0, 0.0, 0.0, 1.0 };
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	GLboolean pass = GL_TRUE;
> +	int PointSize = 64;
> +	GLuint fb, rb;
> +
> +	piglit_require_extension("GL_ARB_point_sprite");

This belongs in piglit_init.  This test also uses 
GL_ARB_framebuffer_object, but it doesn't check for it.

> +	glEnable(GL_POINT_SPRITE_ARB);
> +
> +	glPointSize(PointSize);
> +
> +	glGenRenderbuffers(1,&rb);
> +	glBindRenderbuffer(GL_RENDERBUFFER, rb);
> +	glRenderbufferStorage(GL_RENDERBUFFER, GL_RGBA, 100, 100);
> +
> +	glGenFramebuffers(1,&fb);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, fb);
> +	glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
> +				  GL_RENDERBUFFER, rb);

All of this FBO setup belongs in piglit_init.

> +
> +	glClearColor(0.0, 0.0, 0.0, 1.0);
> +	glClear(GL_COLOR_BUFFER_BIT);
> +	glBegin(GL_POINTS);
> +	glVertex2f(0.0, 0.0);
> +	glEnd();
> +
> +	glBindFramebuffer(GL_READ_FRAMEBUFFER, fb);
> +	glBindFramebuffer(GL_DRAW_FRAMEBUFFER, 0);
> +	glBlitFramebuffer(0, 0, 100, 100, 0, 0, 100, 100, GL_COLOR_BUFFER_BIT, GL_NEAREST);

Is this necessary?  Is the failure present if probing directly from the 
FBO?  Probing directly from the FBO would save a step and allow making 
this a concurrent test.  That would make everyone happier.  If that 
works, surround this block with 'if (!piglit_automatic)'.

> +	glFlush();

This is unnecessary.

> +
> +	pass&= piglit_probe_pixel_rgb(0, 0, black);
> +	pass&= piglit_probe_pixel_rgb(18, 18, green);
> +	pass&= piglit_probe_pixel_rgb(18, 81, black);

It is better to do

     pass = piglit_probe_pixel_rgb(...) && pass;

because we want all of the pixels to be probed when there is a failure. 
  C's evaluation rules will cause later calls to piglit_probe_pixel_rgb 
to be skipped once pass is false.  Also, this should probe more than one 
pixel that is covered by the sprite.

> +
> +	assert(!glGetError());

Use piglit_check_gl_error.

> +
> +	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	GLuint vs, fs;
> +
> +	piglit_require_vertex_shader();
> +	piglit_require_fragment_shader();
> +
> +	fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fs_text);
> +	vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vs_text);
> +	prog = piglit_link_simple_program(vs, fs);
> +
> +	piglit_LinkProgram(prog);
> +	piglit_link_check_status(prog);

piglit_link_simple_program already does both of these things.

> +
> +	piglit_UseProgram(prog);
> +}