[Piglit] [PATCH 3/9] sso: new test IsProgramPipeline

Ian Romanick idr at freedesktop.org
Fri Sep 6 16:21:43 PDT 2013


On 09/06/2013 03:00 PM, Paul Berry wrote:
> On 4 September 2013 12:57, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     From: Gregory Hainaut <gregory.hainaut at gmail.com
>     <mailto:gregory.hainaut at gmail.com>>
> 
>     Check the pipeline object state creation.  glGenProgramPipelines only
>     reserves the names.  The object does not "exist" until it is bound.
> 
>     Also take the opportunity to test negative value on
>     glGenProgramPipelines and glDeleteProgramPipelines.
> 
>     This test fails on AMD's closed-source driver (Catalyst 13.4 on
>     HD5770).  The test crashes when -1 is passed to glGenProgramPipelines.
>     They are presumably treating the GLsizei as unsigned and generating a
>     lot of names.  They also return GL_TRUE from glIsProgramPipeline
>     before the object has been bound.
> 
>     This test fails on NVIDIA's closed-source driver (unknown version on
>     GTX 650).  The test crashes in glGetProgramPipelineiv for a pipeline
>     that has never been bound.  They are presumably dereferencing a NULL
>     pointer in some internal data structure.  They also return GL_TRUE
>     from glIsProgramPipeline before the object has been bound.
> 
>     V4: split the new test in a separate commit
> 
>     V5 (idr):
>     * Use 'pass = foo() && pass;' idiom instead of 'pass &= foo();'.  C's
>       short-circuit evaulation rules cause these to be different.
>     * s/GLboolean/bool/g
>     * Produce no output with -auto.
>     * Add test to CMakeLists.gl.txt.
>     * Trivial reformatting.
>     ---
>      tests/all.tests                                    |   1 +
>      .../arb_separate_shader_objects/CMakeLists.gl.txt  |   1 +
>      .../IsProgramPipeline.c                            | 138
>     +++++++++++++++++++++
>      3 files changed, 140 insertions(+)
>      create mode 100644
>     tests/spec/arb_separate_shader_objects/IsProgramPipeline.c
> 
>     diff --git a/tests/all.tests b/tests/all.tests
>     index 9464244..fe24224 100644
>     --- a/tests/all.tests
>     +++ b/tests/all.tests
>     @@ -1238,6 +1238,7 @@ add_concurrent_test(arb_occlusion_query,
>     'occlusion_query_order')
>      arb_separate_shader_objects = Group()
>      spec['ARB_separate_shader_objects'] = arb_separate_shader_objects
>      arb_separate_shader_objects['GetProgramPipelineiv'] =
>     concurrent_test('arb_separate_shader_object-GetProgramPipelineiv')
>     +arb_separate_shader_objects['IsProgramPipeline'] =
>     concurrent_test('arb_separate_shader_object-IsProgramPipeline')
> 
>      # Group ARB_sampler_objects
>      arb_sampler_objects = Group()
>     diff --git
>     a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
>     b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
>     index 2ea2c80..fa955c6 100644
>     --- a/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
>     +++ b/tests/spec/arb_separate_shader_objects/CMakeLists.gl.txt
>     @@ -10,3 +10,4 @@ link_libraries (
>      )
> 
>      piglit_add_executable
>     (arb_separate_shader_object-GetProgramPipelineiv GetProgramPipelineiv.c)
>     +piglit_add_executable (arb_separate_shader_object-IsProgramPipeline
>     IsProgramPipeline.c)
>     diff --git
>     a/tests/spec/arb_separate_shader_objects/IsProgramPipeline.c
>     b/tests/spec/arb_separate_shader_objects/IsProgramPipeline.c
>     new file mode 100644
>     index 0000000..a0f561a
>     --- /dev/null
>     +++ b/tests/spec/arb_separate_shader_objects/IsProgramPipeline.c
>     @@ -0,0 +1,138 @@
>     +/*
>     + * Copyright © 2013 Gregory Hainaut <gregory.hainaut at gmail.com
>     <mailto:gregory.hainaut at gmail.com>>
>     + *
>     + * 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.
>     + */
>     +
>     +/**
>     + * \name IsProgramPipeline.c
>     + * Verify correct behavior of glIsProgramPipeline relative to when
>     a pipeline
>     + * actually starts to exist.
>     + *
>     + * Also verify that glGenProgramPipelines and
>     glDeleteProgramPipelines with
>     + * negative counts function correctly.
>     + *
>     + * Section 2.11.4 (Program Pipeline Objects) of the OpenGL 4.1 spec
>     says:
>     + *
>     + *     "The command
>     + *
>     + *         void GenProgramPipelines( sizei n, uint *pipelines );
>     + *
>     + *     returns n previously unused program pipeline object names in
>     + *     pipelines. These names are marked as used, for the purposes of
>     + *     GenProgramPipelines only, but they acquire state only when
>     they are
>     + *     first bound."
>     + */
>     +
>     +#include "piglit-util-gl-common.h"
>     +
>     +PIGLIT_GL_TEST_CONFIG_BEGIN
>     +
>     +       config.supports_gl_compat_version = 10;
>     +
>     +       config.window_width = 32;
>     +       config.window_height = 32;
>     +
>     +PIGLIT_GL_TEST_CONFIG_END
>     +
>     +static bool pass;
>     +
>     +enum piglit_result
>     +piglit_display(void)
>     +{
>     +       /* UNREACHED */
>     +       return PIGLIT_FAIL;
>     +}
>     +
>     +static void
>     +IsProgramPipeline(GLuint pipe, GLboolean expected)
>     +{
>     +       GLboolean status = glIsProgramPipeline(pipe);
>     +
>     +       if (status != expected) {
>     +               pass = GL_FALSE;
>     +
>     +               fprintf(stderr,
>     +                       "Pipeline %d has wrong IsProgramPipeline. "
>     +                       "Expected %d, got %d\n",
>     +                       pipe, expected, status);
>     +       }
>     +}
>     +
>     +void
>     +piglit_init(int argc, char **argv)
>     +{
>     +       GLuint id[4];
>     +       int i;
>     +       GLint dummy;
>     +
>     +       piglit_require_gl_version(20);

And it looks like this can use the config.supports_gl_compat_version
change too.

>     +       piglit_require_extension("GL_ARB_separate_shader_objects");
>     +
>     +       pass = true;
>     +
>     +       assert(glGetError() == GL_NO_ERROR);
>     +
>     +       if (!piglit_automatic)
>     +               printf("glGenProgramPipelines with negative n value\n");
> 
> 
> As with the previous patch, I think summary messages like these should
> be printed regardless of whether we're runing in "automatic" mode.

I'm working on a bunch of stuff for the previous patch, but I wanted to
reply to this comment sooner rather than later.  We /used to/ try to
have tests produce as little output as possible with -auto, but generate
information helpful to debugging without -auto.

That said, I actually think this test (and the other ones) should use
piglit_report_subtest_result instead.  Would that be better?

> With that fixed, this patch is:
> 
> Reviewed-by: Paul Berry <stereotype441 at gmail.com
> <mailto:stereotype441 at gmail.com>>
>  
> 
>     +
>     +       glGenProgramPipelines(-1, id);
>     +       pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
>     +
>     +       if (!piglit_automatic)
>     +               printf("glGenProgramPipelines with correct n value\n");
>     +
>     +       glGenProgramPipelines(4, id);
>     +       pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>     +
>     +       for (i = 0; i < 4 ; i++) {
>     +               IsProgramPipeline(id[i], GL_FALSE);
>     +       }
>     +
>     +       glBindProgramPipeline(id[0]);
>     +       glUseProgramStages(id[1], GL_ALL_SHADER_BITS, 0);
>     +       glActiveShaderProgram(id[2], 0);
>     +       glGetProgramPipelineiv(id[3], GL_VERTEX_SHADER, &dummy);
>     +
>     +       /* Flush any errors.  The goal is to check that object acquire a
>     +        * state.
>     +        */
>     +       while (glGetError() != GL_NO_ERROR)
>     +               ;
>     +
>     +       for (i = 0; i < 4 ; i++) {
>     +               IsProgramPipeline(id[i], GL_TRUE);
>     +       }
>     +
>     +       if (!piglit_automatic)
>     +               printf("glDeleteProgramPipelines with negative n
>     value\n");
>     +
>     +       glDeleteProgramPipelines(-1, id);
>     +       pass = piglit_check_gl_error(GL_INVALID_VALUE) && pass;
>     +
>     +       if (!piglit_automatic)
>     +               printf("glDeleteProgramPipelines with correct n
>     value\n");
>     +
>     +       glDeleteProgramPipelines(4, id);
>     +       pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
>     +
>     +       piglit_present_results();
>     +       piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
>     +}
>     --
>     1.8.1.4
> 
>     _______________________________________________
>     Piglit mailing list
>     Piglit at lists.freedesktop.org <mailto:Piglit at lists.freedesktop.org>
>     http://lists.freedesktop.org/mailman/listinfo/piglit



More information about the Piglit mailing list