[Piglit] [PATCH] glsl-shaders: test correct shader source is use on cache fallback
Nicolai Hähnle
nhaehnle at gmail.com
Thu Feb 16 14:34:20 UTC 2017
On 16.02.2017 01:21, Timothy Arceri wrote:
> On 16/02/17 07:27, Eric Anholt wrote:
>> 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)
>
> Thanks. This is mostly a copy of glsl-reload-source which is why I left
> the copyright as Nicolai Hähnle, this is what I've seen most people do
> in the past not sure if that is correct.
I'm perfectly fine with you adding your name.
> I thought about extending that test but I think it would just case
> confusion. I guess we could share more but that is likely more trouble
> than its worth.
I agree.
Nicolai
>
>
>>
>>> +
>>> + 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
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
More information about the Piglit
mailing list