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

2015-12-05 Thread Kenneth Graunke
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 
> ---
>  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

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

2015-12-05 Thread Timothy Arceri
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 
---
 tests/shaders/shader_runner.c | 90 +--
 1 file changed, 86 insertions(+), 4 deletions(-)

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,
vertex_program,
tess_ctrl_shader,
tess_eval_shader,
geometry_shader,
geometry_layout,
fragment_shader,
+   fragment_sso,
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;
+   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;
 
+   cas