[Piglit] [PATCH] ARB_blend_func_extended: test all blending modes

[Eric Anholt]

On Sun, 25 Mar 2012 19:47:23 +0100, Dave Airlie <airlied at gmail.com> wrote:
> From: Dave Airlie <airlied at redhat.com>
> 
> This tests all the src1 blending modes along with the SRC_ALPHA_SATURATE mode for dst.
> (it also tests SRC_ALPHA_SATURATE for src just to be complete).
> 
> This test should complete the tests necessary for merging ARB_blend_func_extended.

Glad to see work on this extension.  Nitpicks inline...

> diff --git a/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
> new file mode 100644
> index 0000000..e04e935
> --- /dev/null
> +++ b/tests/spec/arb_blend_func_extended/execution/CMakeLists.gl.txt
> @@ -0,0 +1,16 @@
> +include_directories(
> +	${GLEXT_INCLUDE_DIR}
> +	${OPENGL_INCLUDE_PATH}
> +	${GLUT_INCLUDE_DIR}
> +	${piglit_SOURCE_DIR}/tests/util
> +)
> +
> +link_libraries (
> +	piglitutil
> +	${OPENGL_gl_LIBRARY}
> +	${OPENGL_glu_LIBRARY}
> +	${GLUT_glut_LIBRARY}
> +)
> +
> +add_executable (arb_blend_func_extended-fbo-extended-blend fbo-extended-blend.c)

Apparently this should be piglit_add_executable post-rebase.

> +int piglit_width = 100;
> +int piglit_height = 100;
> +int piglit_window_mode = GLUT_RGB;
> +
> +static const char *TestName = "fbo-extended-blend";
> +
> +static GLint maxDSBuffers;
> +static GLuint FBO;

iHateCamelCase.  StudlyCapsIsHardlyBetter.

> +static void
> +check_error(int line)
> +{
> +	GLenum err = glGetError();
> +	if (err) {
> +		printf("%s: Unexpected error 0x%x at line %d\n",
> +		       TestName, err, line);
> +		piglit_report_result(PIGLIT_FAIL);
> +	}
> +}

We should just wrap piglit_check_gl_error() to pass in the line number
to the util func so that we can stop rolling our own in each test.

> +static void blend(const float *rect, const float *src, const float
> *src1, const float *dst, GLenum blendsrc, GLenum blenddst, GLenum
> blendop)

Perhaps a line wrap?

If we're always just passing in (0,0,piglit_width,piglit_height) for
rect, maybe just fold that in here?

> +{
> +	glColor4fv(dst);

I find it funny that the two uniform colors are passed through (varying)
gl_Color for one and a uniform for the other.

> +	piglit_draw_rect(rect[0], rect[1], rect[2], rect[3]);
> +	glEnable(GL_BLEND);
> +	glBlendEquation(blendop);
> +	glBlendFunc(blendsrc, blenddst);
> +	glUniform4f(uniform_src1, src1[0], src1[1], src1[2], src1[3]);

glUniform4fv()?

> +	glColor4fv(src);
> +	piglit_draw_rect(rect[0], rect[1], rect[2], rect[3]);
> +	glDisable(GL_BLEND);
> +}
> +static const char *vs_text =
> +	"#version 130\n"
> +	"void main() { gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex; gl_FrontColor = gl_Color; }\n"
> +	;

More newlines in this shader.

> +static const char *fs_text =
> +	"#version 130\n"
> +	"uniform vec4 src1;\n"
> +	"out vec4 col0;\n"
> +	"out vec4 col1;\n"
> +	"void main() {\n"
> +	"        col0 = gl_Color;\n"
> +	"        col1 = src1;\n"
> +	"}\n"
> +	;

> +static enum piglit_result
> +test(void)
> +{
> +	GLenum buffers;
> +	static const GLfloat dest_color[4] = { 0.75, 0.25, 0.25, 0.5 };
> +	static const GLfloat test_color[4] = { 1.0, 0.25, 0.75, 0.25 };
> +	static const GLfloat test_color1[4] = { 0.5, 0.5, 0.5, 0.5 };
> +	GLfloat expected[4];
> +	GLuint prog;
> +	GLuint vs;
> +	GLuint fs;
> +	GLfloat rect[4];
> +	int i, j, k, o;
> +
> +	if (maxDSBuffers > 1) {
> +		printf("Test only supports 1 dual source blending color buffer\n");
> +		maxDSBuffers = 1;
> +	}
> +     

There's some wacky trailing whitespace in this function.

> +	prog = glCreateProgram();
> +	vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vs_text);
> +
> +	fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fs_text);
> +	glAttachShader(prog, vs);
> +	glAttachShader(prog, fs);
> +	piglit_check_gl_error(GL_NO_ERROR);
> +    
> +	glBindFragDataLocationIndexed(prog, 0, 0, "col0");
> +	glBindFragDataLocationIndexed(prog, 0, 1, "col1");
> +
> +	create_fbo();
> +   
> +	buffers = GL_COLOR_ATTACHMENT0_EXT;
> +	glDrawBuffersARB(1, &buffers);

glDrawBuffer(GL_COLOR_ATTACHMENT0_EXT)?

> +enum piglit_result
> +piglit_display(void)
> +{
> +	return test();
> +}

It's pretty weird to have a piglit_display() that just calls a test()
function (why have a separate test() function?), when that function
doesn't even display anything on the window.  If it's not going to
display anything, just put it in piglit_init and make the test always
exit immediately.
-------------- 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/20120326/23e6bf3f/attachment.pgp>