[Piglit] [PATCH] util/gl: Kill duplicated code using new var arg funcs

[Ian Romanick]

On 09/20/2013 06:54 PM, Chad Versace wrote:
> Replace the body of each function below with a call to its variable
> argument variant:
> 
>   piglit_link_simple_program
>   piglit_build_simple_program_unlinked
>   piglit_build_simple_program
> 
> CC: Jacob Penner <jkpenner91 at gmail.com>
> Signed-off-by: Chad Versace <chad.versace at linux.intel.com>
> ---
>  tests/util/piglit-shader.c | 74 ++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 46 deletions(-)
> 
> diff --git a/tests/util/piglit-shader.c b/tests/util/piglit-shader.c
> index b326abd..e7b5635 100644
> --- a/tests/util/piglit-shader.c
> +++ b/tests/util/piglit-shader.c
> @@ -238,31 +238,16 @@ piglit_link_check_status_quiet(GLint prog)
>  
>  GLint piglit_link_simple_program(GLint vs, GLint fs)

This function is good.  I like it.

>  {
> -	GLint prog;
> +	GLint args[2] = {0};
> +	int i = 0;
>  
> -	piglit_require_GLSL();
> -
> -	prog = glCreateProgram();
>  	if (vs)
> -		glAttachShader(prog, vs);
> +		args[i++] = vs;
>  	if (fs)
> -		glAttachShader(prog, fs);
> -
> -	/* If the shaders reference piglit_vertex or piglit_tex, bind
> -	 * them to some fixed attribute locations so they can be used
> -	 * with piglit_draw_rect_tex() in GLES.
> -	 */
> -	glBindAttribLocation(prog, PIGLIT_ATTRIB_POS, "piglit_vertex");
> -	glBindAttribLocation(prog, PIGLIT_ATTRIB_TEX, "piglit_texcoord");
> -
> -	glLinkProgram(prog);
> -
> -	if (!piglit_link_check_status(prog)) {
> -		glDeleteProgram(prog);
> -		prog = 0;
> -	}
> +		args[i++] = fs;
>  
> -	return prog;
> +	return piglit_link_simple_program_multiple_shaders(
> +		args[0], args[1], 0);
>  }
>  
>  
> @@ -274,23 +259,23 @@ GLuint
>  piglit_build_simple_program_unlinked(const char *vs_source,
>  				     const char *fs_source)

There are 7 users of this function.  Would it be better to convert them
to the new interface?

>  {
> -	GLuint prog;
> +	intptr_t args[4] = {0};
> +	int i = 0;
>  
> -	piglit_require_GLSL();
> -	prog = glCreateProgram();
>  	if (vs_source) {
> -		GLuint vs = piglit_compile_shader_text(GL_VERTEX_SHADER,
> -						       vs_source);
> -		glAttachShader(prog, vs);
> -		glDeleteShader(vs);
> +		args[i++] = GL_VERTEX_SHADER;
> +		args[i++] = (intptr_t) vs_source;
>  	}
> +
>  	if (fs_source) {
> -		GLuint fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER,
> -						       fs_source);
> -		glAttachShader(prog, fs);
> -		glDeleteShader(fs);
> +		args[i++] = GL_FRAGMENT_SHADER;
> +		args[i++] = (intptr_t) fs_source;
>  	}
> -	return prog;
> +
> +	return piglit_build_simple_program_unlinked_multiple_shaders(
> +			(GLint) args[0], (const char*) args[1],
> +			(GLint) args[2], (const char*) args[3],
> +			0);

This (and the similar code in the next function) made me feel a little
sick.  If we delete this function, is it still worth the ugliness of the
other function?  I'm not sure...

>  }
>  
>  
> @@ -301,26 +286,23 @@ piglit_build_simple_program_unlinked(const char *vs_source,
>  GLint
>  piglit_build_simple_program(const char *vs_source, const char *fs_source)
>  {
> -	GLuint vs = 0, fs = 0, prog;
> +	intptr_t args[4] = {0};
> +	int i = 0;
>  
>  	if (vs_source) {
> -		vs = piglit_compile_shader_text(GL_VERTEX_SHADER, vs_source);
> +		args[i++] = GL_VERTEX_SHADER;
> +		args[i++] = (intptr_t) vs_source;
>  	}
>  
>  	if (fs_source) {
> -		fs = piglit_compile_shader_text(GL_FRAGMENT_SHADER, fs_source);
> +		args[i++] = GL_FRAGMENT_SHADER;
> +		args[i++] = (intptr_t) fs_source;
>  	}
>  
> -	prog = piglit_link_simple_program(vs, fs);
> -	if (!prog)
> -		piglit_report_result(PIGLIT_FAIL);
> -
> -	if (fs)
> -		glDeleteShader(fs);
> -	if (vs)
> -		glDeleteShader(vs);
> -
> -	return prog;
> +	return piglit_build_simple_program_multiple_shaders(
> +			(GLint) args[0], (const char*) args[1],
> +			(GLint) args[2], (const char*) args[3],
> +			0);
>  }
>  
>  GLint piglit_link_simple_program_multiple_shaders(GLint shader1, ...)
>