[Piglit] [PATCH] glsl-shaders: test correct shader source is use on cache fallback

Eric Anholt eric at anholt.net
Wed Feb 15 20:27:54 UTC 2017


Timothy Arceri <tarceri at itsqueeze.com> writes:

> The scenario is:
>
> glShaderSource
> glCompileShader <-- deferred due to cache hit of shader
>
> glShaderSource <-- with new source code
>
> glAttachShader
> glLinkProgram <-- no cache hit for program
>
> At this point we need make sure we compiled the original source
> when Mesa falls back.

This is a really good test for the cache.  Thanks for writing it.  Just
a few cleanups to suggest:

> ---
>  tests/all.py                                      |   1 +
>  tests/shaders/CMakeLists.gl.txt                   |   1 +
>  tests/shaders/glsl-cache-fallback-shader-source.c | 187 ++++++++++++++++++++++
>  3 files changed, 189 insertions(+)
>  create mode 100644 tests/shaders/glsl-cache-fallback-shader-source.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 03cf0c8..4bafcc7 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -536,6 +536,7 @@ with profile.test_list.group_manager(PiglitGLTest, 'shaders') as g:
>      g(['glsl-novertexdata'])
>      g(['glsl-preprocessor-comments'])
>      g(['glsl-reload-source'])
> +    g(['glsl-cache-fallback-shader-source'])
>      g(['glsl-uniform-out-of-bounds'])
>      g(['glsl-uniform-out-of-bounds-2'])
>      g(['glsl-uniform-update'])
> diff --git a/tests/shaders/CMakeLists.gl.txt b/tests/shaders/CMakeLists.gl.txt
> index 3c50ac6..7f786ee 100644
> --- a/tests/shaders/CMakeLists.gl.txt
> +++ b/tests/shaders/CMakeLists.gl.txt
> @@ -63,6 +63,7 @@ piglit_add_executable (glsl-invalid-asm-02 glsl-invalid-asm-02.c)
>  piglit_add_executable (glsl-novertexdata glsl-novertexdata.c)
>  piglit_add_executable (glsl-orangebook-ch06-bump glsl-orangebook-ch06-bump.c)
>  piglit_add_executable (glsl-reload-source glsl-reload-source.c)
> +piglit_add_executable (glsl-cache-fallback-shader-source glsl-cache-fallback-shader-source.c)
>  piglit_add_executable (glsl-unused-varying glsl-unused-varying.c)
>  piglit_add_executable (glsl-uniform-update glsl-uniform-update.c)
>  piglit_add_executable (glsl-uniform-out-of-bounds glsl-uniform-out-of-bounds.c)
> diff --git a/tests/shaders/glsl-cache-fallback-shader-source.c b/tests/shaders/glsl-cache-fallback-shader-source.c
> new file mode 100644
> index 0000000..f764617
> --- /dev/null
> +++ b/tests/shaders/glsl-cache-fallback-shader-source.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (c) 2009 Nicolai Hähnle

I suspect your copyright should go on this, too.

> + *
> + * 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
> + * Tests setting shader source on an already compiled shader. If we don't
> + * compile the new source we need to make sure the old source being used if
> + * Mesa's on-disk shader cache is forced to fallback and recompile the shader.
> + */
> +
> +#include <stdio.h>
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> +	config.supports_gl_compat_version = 10;
> +
> +	config.window_visual = PIGLIT_GL_VISUAL_RGB;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char vs_one[] =
> +"varying vec4 color;\n"
> +"void main() {\n"
> +"   gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;\n"
> +"   color = vec4(0.0, 0.4, 0.0, 1.0);\n"
> +"}\n";

I'd love some indents before the '"'s, I find them a bit hard to read
like this.

> +
> +static const char vs_two[] =
> +"varying vec4 color;\n"
> +"void main() {\n"
> +"   gl_Position = gl_ModelViewProjectionMatrix * gl_Vertex;\n"
> +"   color = vec4(0.4, 0.4, 0.0, 1.0);\n"
> +"}\n";
> +
> +static const char fs_one[] =
> +"varying vec4 color;\n"
> +"void main() {\n"
> +"   gl_FragColor = color;\n"
> +"}\n";
> +
> +static const char fs_two[] =
> +"varying vec4 color;\n"
> +"void main() {\n"
> +"   gl_FragColor = color + vec4(0.4, 0.0, 0.4, 0.0);\n"
> +"}\n";
> +
> +static const GLfloat expect_one_one[3] = { 0.0, 0.4, 0.0 };
> +static const GLfloat expect_one_two[3] = { 0.4, 0.4, 0.4 };
> +static const GLfloat expect_two_one[3] = { 0.4, 0.4, 0.0 };
> +static const GLfloat expect_two_two[3] = { 0.8, 0.4, 0.4 };
> +
> +static GLuint vs;
> +static GLuint fs;
> +static GLuint program;
> +
> +
> +static void compile_shader(GLuint shader)
> +{
> +	GLint status;
> +
> +	glCompileShaderARB(shader);
> +
> +	glGetObjectParameterivARB(shader, GL_OBJECT_COMPILE_STATUS_ARB, &status);

The piglit-dispatch stuff will let you write GL 2.0 code and it'll
automatically use the old-school function aliases as necessary.

> +	if (!status) {
> +		GLchar log[1000];
> +		GLsizei len;
> +		glGetInfoLogARB(shader, 1000, &len, log);
> +		fprintf(stderr, "Error: problem compiling shader: %s\n", log);
> +		piglit_report_result(PIGLIT_FAIL);
> +	}
> +}
> +
> +static void link_and_use_program()
> +{
> +	GLint status;
> +
> +	glLinkProgramARB(program);
> +	glGetObjectParameterivARB(program, GL_OBJECT_LINK_STATUS_ARB, &status);
> +	if (!status) {
> +		GLchar log[1000];
> +		GLsizei len;
> +		glGetInfoLogARB(program, 1000, &len, log);
> +		fprintf(stderr, "Error: problem linking program: %s\n", log);
> +		piglit_report_result(PIGLIT_FAIL);
> +	}
> +
> +	glUseProgramObjectARB(program);
> +}
> +
> +static void compile_shaders()
> +{
> +	compile_shader(vs);
> +	compile_shader(fs);
> +

stray whitespace

> +}
> +
> +static void setup_shaders(const char * vstext, const char * fstext)
> +{
> +	glShaderSourceARB(vs, 1, (const GLchar **)&vstext, NULL);
> +	glShaderSourceARB(fs, 1, (const GLchar **)&fstext, NULL);
> +}
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> +	enum piglit_result result = PIGLIT_PASS;
> +
> +	piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +
> +	glClear(GL_COLOR_BUFFER_BIT);
> +
> +	setup_shaders(vs_one, fs_one);
> +	compile_shaders();
> +	link_and_use_program();
> +	piglit_draw_rect(0, 0, piglit_width/2, piglit_height/2);

general piglit style is to have whitespace around binary operators, like
in Mesa.

> +	if (!piglit_probe_pixel_rgb(piglit_width/4, piglit_height/4, expect_one_one))
> +		result = PIGLIT_FAIL;
> +
> +	setup_shaders(vs_two, fs_two);
> +	compile_shaders();
> +	link_and_use_program();
> +	piglit_draw_rect(piglit_width/2, 0, piglit_width/2, piglit_height/2);
> +	if (!piglit_probe_pixel_rgb(3*piglit_width/4, piglit_height/4, expect_two_two))
> +		result = PIGLIT_FAIL;
> +
> +	/* We have now seen all the shaders so Mesa will skip compiling them
> +	 * in future. If we link with a combination it hasn't seen before it
> +	 * will be forced to fallback and compile them, which is what will
> +	 * happen in the following two tests.
> +	 */
> +
> +	setup_shaders(vs_two, fs_one);
> +	compile_shaders();
> +	setup_shaders(vs_one, fs_two);
> +	link_and_use_program();
> +	piglit_draw_rect(0, piglit_height/2, piglit_width/2, piglit_height/2);
> +	if (!piglit_probe_pixel_rgb(piglit_width/4, 3*piglit_height/4, expect_two_one))
> +		result = PIGLIT_FAIL;
> +
> +	compile_shaders();
> +	setup_shaders(vs_two, fs_two);
> +	setup_shaders(vs_two, fs_one);
> +	link_and_use_program();
> +	piglit_draw_rect(piglit_width/2, piglit_height/2, piglit_width/2, piglit_height/2);
> +	if (!piglit_probe_pixel_rgb(3*piglit_width/4, 3*piglit_height/4, expect_one_two))
> +		result = PIGLIT_FAIL;
> +
> +	return result;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> +	if (!piglit_is_extension_supported("GL_ARB_shader_objects") || !piglit_is_extension_supported("GL_ARB_vertex_shader") || !piglit_is_extension_supported("GL_ARB_fragment_shader")) {
> +		printf("Requires ARB_shader_objects and ARB_{vertex,fragment}_shader\n");
> +		piglit_report_result(PIGLIT_SKIP);
> +	}

We've got the piglit_require_GLSL() helper for this check.  With at
least this and the copyright fixed, you can have a:

Reviewed-by: Eric Anholt <eric at anholt.net> (the rest is just style
preferences)

> +
> +	vs = glCreateShaderObjectARB(GL_VERTEX_SHADER_ARB);
> +	fs = glCreateShaderObjectARB(GL_FRAGMENT_SHADER_ARB);
> +	program = glCreateProgramObjectARB();
> +	glAttachObjectARB(program, vs);
> +	glAttachObjectARB(program, fs);
> +}
> -- 
> 2.9.3
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20170215/dd9d7f27/attachment.sig>


More information about the Piglit mailing list