[Piglit] [PATCH] Add a new test case used to valid gl_VertexID for bug 87611

Wang, Shuo shuo.wang at intel.com
Sat Jan 3 21:01:33 PST 2015


Thank you Ian, I just modified the case, and send out another version with the subject of  " [PATCH] V2. Add a case to valid gl_VertexID for bug 87611"

Thanks,
Shuo 

-----Original Message-----
From: Ian Romanick [mailto:idr at freedesktop.org] 
Sent: Friday, January 2, 2015 6:09 PM
To: Wang, Shuo; piglit at lists.freedesktop.org
Subject: Re: [Piglit] [PATCH] Add a new test case used to valid gl_VertexID for bug 87611

On 12/31/2014 06:24 AM, Wang Shuo wrote:
> This case is added for *Bug 87611 - Exercise gl_VertexID corner cases 
> in i965 driver*, and used to watch the status of *Bug 85529 - Surfaces 
> not drawn in Unvanquished*

See below for the correct way to reference a bug.

> This case is failed by master, and passed by the patch of vcelestialragev at gmail.com(https://bugs.freedesktop.org/attachment.cgi?id=110839) which is agree by Kenneth Graunke.
> Below is the test point of this case:
> 1. Use user-allocated vertex arrays (NOT vertex buffer objects).
> 
> 2. Use a glDrawArrays with a non-zero 'first'.
> 
> 3. Use primitive restart to trigger multiple _mesa_prim without an intervening state change.
> 	For primitive restart test:
> 	In OpenGL ES, primitive restart do not support glDrawArrays.Below is the spec of OpenGLR ES Version 3.0.1 (January 10, 2013) page26
> 		 When DrawElements,DrawElementsInstanced, or DrawRangeElements transfers a set of generic attribute array elements to the GL,
> 		 if the index within the vertex arrays corresponding to that set is equal to 2N ô€€€ 1, where N is 8, 16 or 32 if the type is
> 		 UNSIGNED_-BYTE, UNSIGNED_SHORT, or UNSIGNED_INT, respectively, then the GL does not process those elements as a vertex.
> 	For OpenGL, primitive restarting to work with all rendering commands, including the non-indexed commands. However, implementations
> 	did not implement this, and the rule itself makes no sense.(https://www.opengl.org/wiki/Vertex_Rendering)
> 	So I used glDrawElements to test primitive restart
> 
> 4. Validate the value of gl_VertexID in the vertex shader.
> 	Both glDrawArrays testing and glDrawElements testing test the value 
> of gl_VertexID

Please line wrap the commit message to 72 columns.  That make it readable when people use 'git log' on an 80 column terminal.

> Signed-off-by: Wang Shuo <shuo.wang at intel.com>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=87611

> ---
>  tests/all.py                    |   1 +
>  tests/shaders/CMakeLists.gl.txt |   1 +
>  tests/shaders/glsl-bug-87611.c  | 196 
> ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 198 insertions(+)
>  create mode 100644 tests/shaders/glsl-bug-87611.c
> 
> diff --git a/tests/all.py b/tests/all.py index 0d9a971..dd1ae92 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -386,6 +386,7 @@ add_concurrent_test(shaders, 
> 'createshaderprogram-attached-shaders')
>  add_concurrent_test(shaders, 'glsl-arb-fragment-coord-conventions')
>  add_concurrent_test(shaders, 
> 'glsl-arb-fragment-coord-conventions-define')
>  add_concurrent_test(shaders, 'glsl-bug-22603')
> +add_concurrent_test(shaders, 'glsl-bug-87611')
>  add_concurrent_test(shaders, 'glsl-bindattriblocation')  
> add_concurrent_test(shaders, 'glsl-dlist-getattriblocation')  
> add_concurrent_test(shaders, 'glsl-getactiveuniform-array-size')
> diff --git a/tests/shaders/CMakeLists.gl.txt 
> b/tests/shaders/CMakeLists.gl.txt index c827256..501cad3 100644
> --- a/tests/shaders/CMakeLists.gl.txt
> +++ b/tests/shaders/CMakeLists.gl.txt
> @@ -47,6 +47,7 @@ piglit_add_executable 
> (glsl-arb-fragment-coord-conventions glsl-arb-fragment-coo  
> piglit_add_executable (glsl-arb-fragment-coord-conventions-define 
> glsl-arb-fragment-coord-conventions-define.c)
>  piglit_add_executable (glsl-bindattriblocation 
> glsl-bindattriblocation.c)  piglit_add_executable (glsl-bug-22603 
> glsl-bug-22603.c)
> +piglit_add_executable (glsl-bug-87611 glsl-bug-87611.c)
>  piglit_add_executable (glsl-dlist-getattriblocation 
> glsl-dlist-getattriblocation.c)  piglit_add_executable 
> (glsl-explicit-location-01 glsl-explicit-location-01.c)  
> piglit_add_executable (glsl-explicit-location-02 
> glsl-explicit-location-02.c) diff --git 
> a/tests/shaders/glsl-bug-87611.c b/tests/shaders/glsl-bug-87611.c new 
> file mode 100644 index 0000000..358e45c
> --- /dev/null
> +++ b/tests/shaders/glsl-bug-87611.c
> @@ -0,0 +1,196 @@
> +/*
> + * Copyright © 2009 Intel Corporation

Update the copyright year. :)

> + * Copyright © 2010 VMware, Inc.
> + *
> + * 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.
> + */
> +
> +/**
> + * \file glsl-bug-87611.c
> + *
> + * Check gl_VertexID
> + * This case is added for *Bug 87611 - Exercise gl_VertexID corner 
> +cases in i965 driver*,
> + * and used to watch the status of *Bug 85529 - Surfaces not drawn in 
> +Unvanquished*
> + * This case is failed by master, and passed by the patch of 
> +vcelestialragev at gmail.com(https://bugs.freedesktop.org/attachment.cgi
> +?id=110839)
> + * which is agree by Kenneth Graunke.
> + *
> + *
> + * \author Wang Shuo <shuo.wang at intel.com> */
> +
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +	config.supports_gl_compat_version = 10;

Change this to 20 and delete the "piglit_require_gl_version(20);" in piglit_init.

> +
> +	config.window_visual = PIGLIT_GL_VISUAL_RGB | 
> +PIGLIT_GL_VISUAL_DOUBLE;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +//	Validate the value of gl_VertexID in the vertex shader.
> +static const GLchar *vertShaderText =
> +	"#version 130\n"
> +	"attribute vec4 attrib;\n"
> +	"void main()\n"
> +	"{\n"
> +	"	gl_Position = gl_ModelViewProjectionMatrix * attrib;\n"
> +	"	if (gl_VertexID == 2 || gl_VertexID == 3 || gl_VertexID == 4 || gl_VertexID == 5 )\n"
> +	"{ \n"
> +	"	gl_FrontColor = vec4(0.0, 1.0, 0.0, 1.0);\n"
> +	"} \n"
> +	"	else"
> +	"{ \n"
> +	"	gl_FrontColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
> +	"} \n"
> +	"} \n";

Reformat this as:

	"#version 130\n"
	"in vec4 piglit_vertex;\n"
	"void main()\n"
	"{\n"
	"    gl_Position = piglit_vertex;\n"
	"    if (gl_VertexID >= 2 && gl_VertexID <= 5 ) {\n"
	"        gl_FrontColor = vec4(0.0, 1.0, 0.0, 1.0);\n"
	"    } else {\n"
	"        gl_FrontColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
	"    }\n"
	"}\n";

Notice that I changed "attrib" to "piglit_vertex".  This is the name usually used in piglit tests.

> +
> +//	Validate the value of gl_VertexID in the vertex shader.
> +static const GLchar *vertShaderTextIndex =
> +	"#version 130\n"
> +	"attribute vec4 attrib;\n"
> +	"void main()\n"
> +	"{\n"
> +	"	gl_Position = gl_ModelViewProjectionMatrix * attrib;\n"
> +	"	if (gl_VertexID == 2 || gl_VertexID == 3 || gl_VertexID == 4 || gl_VertexID == 5 )\n"
> +	"{ \n"
> +	"	gl_FrontColor = vec4(0.0, 1.0, 0.0, 1.0);\n"
> +	"} \n"
> +	"	else"
> +	"{ \n"
> +	"	gl_FrontColor = vec4(1.0, 0.0, 0.0, 1.0);\n"
> +	"} \n"
> +	"} \n";

Similar formatting changes for this one.

> +
> +
> +static const GLfloat Vcoords[6][2] = { {-1, -1}, {-1, -1}, {-1, -1}, 
> +{1, -1}, {1, 1}, {-1, 1}};
> +
> +static const unsigned short indiceTriangleFan[] = 
> +	{
> +		2, 0xFFFF, 2, 3, 0xFFFF, 0xFFFF, 2, 3, 4, 5, 0xFFFF, 0xFFFF, 0xFFFF,5
> +	};
> +
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	static const GLfloat expColor[4] = {0, 1, 0, 1};
> +	GLint vs, vsIndex;
> +	GLint prog, progIndex;
> +	GLint attrib_loc, attrib_locIndex;
> +	enum piglit_result result, resultIndex;
> +
> +	vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vertShaderText);
> +	if (vs == 0) {
> +                exit(1);
> +        }
> +
> +	prog = piglit_link_simple_program(vs, 0);
> +	if (prog == 0) {
> +                exit(1);
> +        }
> +
> +
> +	glLinkProgram(prog);
> +
> +	attrib_loc = glGetAttribLocation(prog, "attrib");
> +
> +	glUseProgram(prog);

All of the program compiling and linking should be in piglit_init.

There's also a lot of weird indentation in this function that needs to be fixed.

You should also use piglit_build_simple_program.

> +
> +	glMatrixMode(GL_PROJECTION);
> +	glLoadIdentity();
> +	glOrtho(-1.1, 1.1, -1.1, 1.1, -1, 1);

These lines are unnecessary.

> +
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +	//	Use user-allocated vertex arrays (NOT vertex buffer objects).
> +	glVertexAttribPointer(attrib_loc, 2, GL_FLOAT, GL_FALSE, 0, Vcoords);
> +	glEnableVertexAttribArray(attrib_loc);
> +
> +	//	Use a glDrawArrays with a non-zero 'first'.
> +	glDrawArrays(GL_TRIANGLE_FAN, 2, 4);
> +
> +	result = piglit_probe_pixel_rgba(20, 20, expColor)
> +		? PIGLIT_PASS : PIGLIT_FAIL;
> +
> +	glDisableVertexAttribArray(attrib_loc);
> +
> +	vsIndex = piglit_compile_shader_text(GL_VERTEX_SHADER, vertShaderTextIndex);
> +	if (vsIndex == 0) {
> +                exit(1);
> +        }
> +
> +	progIndex = piglit_link_simple_program(vsIndex, 0);
> +	if (progIndex == 0) {
> +                exit(1);
> +        }
> +
> +
> +	glLinkProgram(progIndex);
> +
> +	/* check that the bind worked */
> +	attrib_locIndex = glGetAttribLocation(progIndex, "attrib");
> +
> +	/* now draw something and check that it works */
> +	glUseProgram(progIndex);
> +
> +	glMatrixMode(GL_PROJECTION);
> +	glLoadIdentity();
> +	glOrtho(-1.1, 1.1, -1.1, 1.1, -1, 1);
> +
> +	glClear(GL_COLOR_BUFFER_BIT);
> +	//	Use primitive restart to trigger multiple _mesa_prim without an intervening state change.
> +	//	Because for OpenGL ES PRIMITIVE_RESTART only support DrawElements, DrawElementsInstanced, or DrawRangeElements
> +	//	And for OpenGL PRIMITIVE_RESTART work with all rendering commands, including the non-indexed commands. 
> +	//	However, implementations did not implement this, and the rule itself makes no sense (https://www.opengl.org/wiki/Vertex_Rendering)
> +	//	So for testing PRIMITIVE_RESTART I used glDrawElements
> +	//	OpenGL ES spec:When DrawElements,DrawElementsInstanced, or DrawRangeElements transfers a set of generic 
> +	//	attribute array elements to the GL, if the index within the vertex arrays corresponding to that set is 
> +	//	equal to 2N 􀀀 1, where N is 8, 16 or 32 if the type is UNSIGNED_-BYTE, UNSIGNED_SHORT, or UNSIGNED_INT, 
> +	//	respectively, then the GL does not process those elements as a vertex.

Please format the big comment like

    /*
     *
     *
     */

and wrap lines for 80 columns.

> +	glEnable(GL_PRIMITIVE_RESTART_FIXED_INDEX);
> +
> +	glVertexAttribPointer(attrib_locIndex, 2, GL_FLOAT, GL_FALSE, 0, Vcoords);
> +	glEnableVertexAttribArray(attrib_locIndex);
> +
> +	glDrawElements(GL_TRIANGLE_FAN, sizeof(indiceTriangleFan) / 
> +sizeof(indiceTriangleFan[0]), GL_UNSIGNED_SHORT, indiceTriangleFan);
> +
> +	resultIndex = piglit_probe_pixel_rgba(20, 20, expColor)
> +		? PIGLIT_PASS : PIGLIT_FAIL;
> +
> +	if (result == PIGLIT_PASS && resultIndex == PIGLIT_PASS)
> +		result = PIGLIT_PASS;
> +	else
> +		result = PIGLIT_FAIL;

Most piglit tests have a bool variable called pass.  The tests then do things like

    pass = piglit_probe_pixel_rgba(20, 20, expColor);

    ...


    pass = piglit_probe_pixel_rgba(20, 20, expColor) && pass;

    return pass ? PIGLIT_PASS : PIGLIT_FAIL;

> +
> +	piglit_present_results();
> +
> +	return result;
> +}
> +
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	piglit_require_vertex_shader();

Since OpenGL 2.0 is required, piglit_require_vertex_shader is unnecessary.

> +
> +	piglit_require_gl_version(20);
> +	piglit_require_GLSL_version(130);
> +}
> 



More information about the Piglit mailing list