[Piglit] [PATCH V3 1/4] shader_runner: Add basic SSO support to shader runner

Kenneth Graunke kenneth at whitecape.org
Sat Dec 5 23:38:47 PST 2015


On Sunday, December 06, 2015 01:58:31 PM Timothy Arceri wrote:
> This sets up the basics for using SSO with shader runner. This will
> only support vertex and fragment shaders but is easily extended.
> 
> V2: delete pipeline in cleanup code rather than calling gen again,
> output error message when SSO fails to link
> 
> V3: add new option to [require] to allow separate shader objects to be
> enabled for the entire test.
> 
> Example use:
> [require]
> SSO ENABLED
> 
> Adding the ENABLED field rather than just using SSO will allow us to use
> DISABLED in future should we ever add the ability to automatically run
> all tests as SSO.
> 
> Example shader:
> 
> [require]
> GLSL >= 1.50
> SSO ENABLED
> 
> [vertex shader]
> 
> layout(location = 0) in vec4 piglit_vertex;
> 
> layout(location = 2) out vec3 a;
> layout(location = 3) out vec3 b;
> 
> void main()
> {
>     gl_Position = piglit_vertex;
>     a = vec3(0, 0, 1);
>     b = vec3(1, 0, 0);
> }
> 
> [fragment shader]
> 
> layout(location = 0) out vec4 out_color;
> 
> layout(location = 2) in vec3 b; /* should get vec3(0, 0, 1) */
> layout(location = 3) in vec3 a; /* should get vec3(1, 0, 0) */
> 
> void main()
> {
>     out_color = vec4(cross(b, a), 1);
> }
> 
> [test]
> draw rect -1 -1 2 2
> probe all rgb 0 1 0
> 
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  tests/shaders/shader_runner.c | 90 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 86 insertions(+), 4 deletions(-)

I think you could simplify the code substantially by adding this to
the top of compile_glsl():

	if (sso_in_use) {
		create_sso(target, shader_string, shader_string_size);
		return;
	}

Then you could...

> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index eeb1aac..a5f456f 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -123,10 +123,12 @@ GLint shader_string_size;
>  const char *vertex_data_start = NULL;
>  const char *vertex_data_end = NULL;
>  GLuint prog;
> +GLuint pipeline;
>  size_t num_vbo_rows = 0;
>  bool vbo_present = false;
>  bool link_ok = false;
>  bool prog_in_use = false;
> +bool sso_in_use = false;
>  GLchar *prog_err_info = NULL;
>  GLuint vao = 0;
>  GLuint fbo = 0;
> @@ -137,12 +139,14 @@ enum states {
>  	requirements,
>  	vertex_shader,
>  	vertex_shader_passthrough,
> +	vertex_sso,

drop this

>  	vertex_program,
>  	tess_ctrl_shader,
>  	tess_eval_shader,
>  	geometry_shader,
>  	geometry_layout,
>  	fragment_shader,
> +	fragment_sso,

and this

>  	fragment_program,
>  	compute_shader,
>  	vertex_data,
> @@ -480,6 +484,55 @@ compile_and_bind_program(GLenum target, const char *start, int len)
>  	prog_in_use = true;
>  }
>  
> +void
> +create_sso(GLenum target, const char *start, int len)
> +{
> +	GLuint prog;
> +	GLint ok;
> +	char *source;
> +
> +	piglit_require_extension("GL_ARB_separate_shader_objects");
> +
> +	source = malloc(len + 1);
> +	memcpy(source, start, len);
> +	source[len] = 0;
> +	prog = glCreateShaderProgramv(target, 1,
> +					(const GLchar *const *) &source);
> +
> +	glGetProgramiv(prog, GL_LINK_STATUS, &ok);
> +	if (ok) {
> +		link_ok = true;
> +	} else {
> +		GLint size;
> +
> +		glGetProgramiv(prog, GL_INFO_LOG_LENGTH, &size);
> +		prog_err_info = malloc(size);
> +
> +		glGetProgramInfoLog(prog, size, NULL, prog_err_info);
> +
> +		fprintf(stderr, "glCreateShaderProgramv(%s) failed: %s\n",
> +			target_to_short_name(target),
> +			prog_err_info);
> +
> +		free(prog_err_info);
> +		piglit_report_result(PIGLIT_FAIL);
> +
> +		return;
> +	}
> +
> +	switch (target) {
> +	case GL_VERTEX_SHADER:
> +		glUseProgramStages(pipeline, GL_VERTEX_SHADER_BIT, prog);
> +		break;
> +	case GL_FRAGMENT_SHADER:
> +		glUseProgramStages(pipeline, GL_FRAGMENT_SHADER_BIT, prog);
> +		break;
> +	}
> +
> +	sso_in_use = true;

This doesn't seem useful now that we have a global option

> +	prog_in_use = true;
> +}
> +
>  /**
>   * Compare two values given a specified comparison operator
>   */
> @@ -705,13 +758,14 @@ process_requirement(const char *line)
>  		return;
>  	}
>  
> -	/* There are four types of requirements that a test can currently
> +	/* There are five types of requirements that a test can currently
>  	 * have:
>  	 *
>  	 *    * Require that some GL extension be supported
>  	 *    * Require some particular versions of GL
>  	 *    * Require some particular versions of GLSL
>  	 *    * Require some particular number of uniform components
> +	 *    * Require shaders be built as separate shader objects
>  	 *
>  	 * The tests for GL and GLSL versions can be equal, not equal,
>  	 * less, less-or-equal, greater, or greater-or-equal.  Extension tests
> @@ -797,6 +851,10 @@ process_requirement(const char *line)
>  		}
>  
>  		piglit_set_rlimit(lim);
> +	}  else if (string_match("SSO", line)) {
> +		line = eat_whitespace(line + 3);
> +		if (string_match("ENABLED", line))
> +			sso_in_use = true;
>  	}
>  }
>  
> @@ -846,6 +904,13 @@ leave_state(enum states state, const char *line)
>  		compile_glsl(GL_VERTEX_SHADER);
>  		break;
>  
> +	case vertex_sso:
> +		shader_string_size = line - shader_string;
> +		create_sso(GL_VERTEX_SHADER,
> +			   shader_string,
> +			   line - shader_string);
> +		break;
> +

and you could drop this

>  	case vertex_program:
>  		compile_and_bind_program(GL_VERTEX_PROGRAM_ARB,
>  					 shader_string,
> @@ -875,6 +940,12 @@ leave_state(enum states state, const char *line)
>  		compile_glsl(GL_FRAGMENT_SHADER);
>  		break;
>  
> +	case fragment_sso:
> +		create_sso(GL_FRAGMENT_SHADER,
> +			   shader_string,
> +			   line - shader_string);
> +		break;
> +

and this

>  	case fragment_program:
>  		compile_and_bind_program(GL_FRAGMENT_PROGRAM_ARB,
>  					 shader_string,
> @@ -1038,7 +1109,7 @@ process_test_script(const char *script_name)
>  			if (string_match("[require]", line)) {
>  				state = requirements;
>  			} else if (string_match("[vertex shader]", line)) {
> -				state = vertex_shader;
> +				state = sso_in_use ? vertex_sso : vertex_shader;

this too

>  				shader_string = NULL;
>  			} else if (string_match("[vertex program]", line)) {
>  				state = vertex_program;
> @@ -1061,7 +1132,7 @@ process_test_script(const char *script_name)
>  				state = geometry_layout;
>  				shader_string = NULL;
>  			} else if (string_match("[fragment shader]", line)) {
> -				state = fragment_shader;
> +				state = sso_in_use ? fragment_sso : fragment_shader;

and this

>  				shader_string = NULL;
>  			} else if (string_match("[fragment program]", line)) {
>  				state = fragment_program;
> @@ -1098,11 +1169,13 @@ process_test_script(const char *script_name)
>  				break;
>  
>  			case vertex_shader:
> +			case vertex_sso:

and this

>  			case vertex_program:
>  			case tess_ctrl_shader:
>  			case tess_eval_shader:
>  			case geometry_shader:
>  			case fragment_shader:
> +			case fragment_sso:

and this

>  			case fragment_program:
>  			case compute_shader:
>  				if (shader_string == NULL)
> @@ -3094,8 +3167,10 @@ piglit_display(void)
>  			glDeleteProgram(prog);
>  			glUseProgram(0);
>  		} else {
> -			glDeleteProgramsARB(1, &prog);
> +			if (!sso_in_use)
> +				glDeleteProgramsARB(1, &prog);
>  		}
> +		glDeleteProgramPipelines(1, &pipeline);
>  	}
>  
>  	return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> @@ -3138,6 +3213,9 @@ piglit_init(int argc, char **argv)
>  		glGetIntegerv(GL_MAX_VARYING_COMPONENTS,
>  			      &gl_max_varying_components);
>  	glGetIntegerv(GL_MAX_CLIP_PLANES, &gl_max_clip_planes);
> +
> +	if (piglit_is_extension_supported("GL_ARB_separate_shader_objects"))
> +		glGenProgramPipelines(1, &pipeline);
>  #else
>  	glGetIntegerv(GL_MAX_FRAGMENT_UNIFORM_VECTORS,
>  		      &gl_max_fragment_uniform_components);
> @@ -3157,6 +3235,10 @@ piglit_init(int argc, char **argv)
>  
>  	process_test_script(argv[1]);
>  	link_and_use_shaders();
> +
> +	if (sso_in_use)
> +		glBindProgramPipeline(pipeline);
> +
>  	if (link_ok && vertex_data_start != NULL) {
>  		program_must_be_in_use();
>  		bind_vao_if_supported();
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20151205/8bab076a/attachment.sig>


More information about the Piglit mailing list