[Piglit] [PATCH] ARB_uniform_buffer_object: Add test for discard with UBO.

Cody Northrop cody at lunarg.com
Tue Jul 1 14:28:36 PDT 2014


On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> On Tuesday, July 01, 2014 12:53:18 PM Cody Northrop wrote:
> > Add a test that ensures fragment discard while loading from
> > uniform buffer objects works correctly.  At time of submission,
> > this test fails on i965.
> >
> > Test will work with the following patch, sent to mesa-dev:
> >
> > i965/fs: Update discard jump to preserve uniform loads via sampler.
> >
> > Signed-off-by: Cody Northrop <cody at lunarg.com>
> > Reviewed-by:  Courtney Goeltzenleuchter <courtney at lunarg.com>
> > ---
> >  tests/all.py                                       |   1 +
> >  .../arb_uniform_buffer_object/CMakeLists.gl.txt    |   1 +
> >  .../arb_uniform_buffer_object/rendering-discard.c  | 195
> +++++++++++++++++++++
> >  3 files changed, 197 insertions(+)
> >  create mode 100644
> tests/spec/arb_uniform_buffer_object/rendering-discard.c
> >
> > diff --git a/tests/all.py b/tests/all.py
> > index 17d5d9b..04ae9e5 100644
> > --- a/tests/all.py
> > +++ b/tests/all.py
> > @@ -2965,6 +2965,7 @@ arb_uniform_buffer_object['negative-
> getactiveuniformblockiv'] = concurrent_test(
> >  arb_uniform_buffer_object['negative-getactiveuniformsiv'] =
> concurrent_test('arb_uniform_buffer_object-negative-getactiveuniformsiv')
> >  arb_uniform_buffer_object['referenced-by-shader'] =
> concurrent_test('arb_uniform_buffer_object-referenced-by-shader')
> >  arb_uniform_buffer_object['rendering'] =
> concurrent_test('arb_uniform_buffer_object-rendering')
> > +arb_uniform_buffer_object['rendering-discard'] =
> concurrent_test('arb_uniform_buffer_object-rendering-discard')
> >  arb_uniform_buffer_object['row-major'] =
> concurrent_test('arb_uniform_buffer_object-row-major')
> >  arb_uniform_buffer_object['uniformblockbinding'] =
> concurrent_test('arb_uniform_buffer_object-uniformblockbinding')
> >
> > diff --git a/tests/spec/arb_uniform_buffer_object/CMakeLists.gl.txt
> b/tests/spec/arb_uniform_buffer_object/CMakeLists.gl.txt
> > index 7d65e2d..6bc3976 100644
> > --- a/tests/spec/arb_uniform_buffer_object/CMakeLists.gl.txt
> > +++ b/tests/spec/arb_uniform_buffer_object/CMakeLists.gl.txt
> > @@ -39,6 +39,7 @@ piglit_add_executable
> (arb_uniform_buffer_object-negative-
> getactiveuniformblocki
> >  piglit_add_executable (arb_uniform_buffer_object-negative-
> getactiveuniformsiv negative-getactiveuniformsiv.c)
> >  piglit_add_executable (arb_uniform_buffer_object-referenced-by-shader
> referenced-by-shader.c)
> >  piglit_add_executable (arb_uniform_buffer_object-rendering rendering.c)
> > +piglit_add_executable (arb_uniform_buffer_object-rendering-discard
> rendering-discard.c)
> >  piglit_add_executable (arb_uniform_buffer_object-row-major row-major.c)
> >  piglit_add_executable (arb_uniform_buffer_object-uniformblockbinding
> uniformblockbinding.c)
> >
> > diff --git a/tests/spec/arb_uniform_buffer_object/rendering-discard.c
> b/tests/spec/arb_uniform_buffer_object/rendering-discard.c
> > new file mode 100644
> > index 0000000..cf3624d
> > --- /dev/null
> > +++ b/tests/spec/arb_uniform_buffer_object/rendering-discard.c
> > @@ -0,0 +1,195 @@
> > +/*
> > + * Copyright (c) 2014 LunarG, Inc.
> > + *
> > + * 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 rendering-discard.c
> > + *
> > + * Test rendering with UBOs in the presence of discard.  Draw a single
> square
> > + * with a fragment shader that conditionally discards along a boundary
> that
> can
> > + * cause problems when rendering multiple fragment quads at once.  Test
> should
> > + * render a single color, no texture should bleed through.
> > + */
> > +
> > +#include "piglit-util-gl-common.h"
> > +
> > +PIGLIT_GL_TEST_CONFIG_BEGIN
> > +
> > +     config.supports_gl_core_version = 32;
> > +     config.supports_gl_compat_version = 32;
> > +     config.window_visual = PIGLIT_GL_VISUAL_DOUBLE |
> PIGLIT_GL_VISUAL_RGBA;
> > +
> > +PIGLIT_GL_TEST_CONFIG_END
> > +
> > +static const char vertex_shader_text[] =
> > +     "#version 140\n"
> > +     "#extension GL_ARB_explicit_attrib_location: require\n"
> > +     "layout(location = 0) in vec4 piglit_vertex;\n"
> > +     "void main()\n"
> > +     "{\n"
> > +     "    gl_Position = piglit_vertex;\n"
> > +     "}\n"
> > +     ;
> > +
> > +static const char fragment_shader_text[] =
> > +     "#version 140\n"
> > +     "#extension GL_ARB_shading_language_420pack : enable\n"
> > +     "#extension GL_ARB_uniform_buffer_object : enable\n"
> > +     "\n"
> > +     "layout(std140, binding = 0) uniform globals \n"
> > +     "{\n"
> > +     "    vec3   global0; \n"
> > +     "    vec3   global1; \n"
>
> These two UBO fields appear to be unused - are they necessary?
>

Hmmm, they were in the original test case, but the test appears to still
fail if I shrink the buffer back down to size.  I'll reduce it to just one
vec4.


>
> > +     "    vec3   global2; \n"
> > +     "    float  global3; \n"
> > +     "}\n;"
> > +     "\n"
> > +     "layout(binding = 0) uniform sampler2D tex2;\n"
> > +     "\n"
> > +     "void main()\n"
> > +     "{\n"
> > +     "    vec3 foo = texture(tex2, vec2(0.5)).xyz ;\n"
> > +     ""
> > +     "    // This condition will discard fragments to the left \n"
> > +     "    // of a diagonal line.  It is designed to partially  \n"
> > +     "    // discard the contents of SIMD registers.           \n"
> > +     "    if ((gl_FragCoord.x - gl_FragCoord.y) < 0.0)         \n"
> > +     "        discard;                                         \n"
> > +     ""
> > +     "    gl_FragColor = vec4(foo * global2 * global3, 1.0 );\n"
> > +     "}\n"
> > +     ;
> > +
> > +/* Note that the expected color is set up to both match the clear color,
> > + * but also be multiplied in the fragment shader to generate the same
> > + * value.  The multiplication is critical to testing behavior on some
> > + * backends, so it should remain in the shader.
> > + */
> > +const float expected_color[4] = { 0.2, 0.2, 0.2, 1.0 };
> > +
> > +/*
> > + * Create a single-color image.
> > + */
> > +static GLubyte *
> > +create_image(GLint w, GLint h, const GLubyte color[4])
> > +{
> > +        GLubyte *buf = (GLubyte *) malloc(w * h * 4);
> > +        int i;
> > +        for (i = 0; i < w * h; i++) {
> > +                buf[i*4+0] = color[0];
> > +                buf[i*4+1] = color[1];
> > +                buf[i*4+2] = color[2];
> > +                buf[i*4+3] = color[3];
> > +        }
> > +        return buf;
> > +}
> > +
> > +static void
> > +setup_texture(void)
> > +{
> > +        GLubyte colors[4] = {255,   255,  255,  255};
> > +        GLuint tex1;
> > +        GLint width = 128, height = 64, levels = 1;
> > +        GLint level = 0;
> > +        GLubyte *colorBuf = create_image(width, height, colors);
> > +
> > +        /* Set up a texture to pull from, just fill it with solid white
> */
> > +        glGenTextures(1, &tex1);
> > +        glBindTexture(GL_TEXTURE_2D, tex1);
> > +        glTexStorage2D(GL_TEXTURE_2D, levels, GL_RGBA8, width, height);
> > +        piglit_check_gl_error(GL_NO_ERROR);
> > +        glTexSubImage2D(GL_TEXTURE_2D, level, 0, 0, width, height,
> GL_RGBA,
> GL_UNSIGNED_BYTE, colorBuf);
> > +        glActiveTexture(GL_TEXTURE0);
> > +        glBindTexture(GL_TEXTURE_2D, tex1);
> > +}
> > +
> > +static void
> > +setup_uniform_buffer(void)
> > +{
> > +        GLuint uBuffer;
> > +        GLfloat bufData[12] = {
> > +           expected_color[0], expected_color[1], expected_color[2],
> expected_color[3],
> > +           expected_color[0], expected_color[1], expected_color[2],
> expected_color[3],
> > +           expected_color[0], expected_color[1], expected_color[2],
> expected_color[3],
> > +        };
> > +
> > +        /* Set up a uniform buffer with data that, when processed by the
> > +         * fragment shader, should draw the same color as the clear
> color.
> > +         * This normally "bad" test behavior is designed to make it
> easier
> > +         * to see that any unwanted color has entered along the boundary
> > +         * condition.  In this case, it will usually be the contents of
> the
> > +         * texture, but it can also be random data.
> > +         * Note that the size of the buffer and the offsets of the
> values
> > +         * being loaded are important to trigger specific behavior in
> some
> > +         * backends that optimize loading of uniform values.
> > +         */
> > +        glGenBuffers(1, &uBuffer);
> > +        glBindBuffer(GL_UNIFORM_BUFFER, uBuffer);
> > +        glBufferData(GL_UNIFORM_BUFFER, sizeof(bufData), NULL,
> GL_STATIC_DRAW);
> > +        glBindBufferBase(GL_UNIFORM_BUFFER, 0, uBuffer);
> > +        glBufferSubData(GL_UNIFORM_BUFFER, 0, sizeof(bufData), bufData);
> > +}
> > +
> > +void
> > +piglit_init(int argc, char **argv)
> > +{
> > +     GLint prog = 0;
> > +
> > +     piglit_require_extension("GL_ARB_uniform_buffer_object");
> > +     piglit_require_extension("GL_ARB_shading_language_420pack");
> > +     piglit_require_extension("GL_ARB_explicit_attrib_location");
>
> As written, this test also requires GL_ARB_texture_storage.
>

Thanks, I'll add that.  Did I miss some debug spew that could have caught
this?


> I didn't review the test too thoroughly, but it looks reasonable enough.
>  With
> the extra extension check added, this would get:
>
> Acked-by: Kenneth Graunke <kenneth at whitecape.org>
>
> (perhaps other people will want to review it, though)
>
> Thanks for doing this.
>

I'll wait a bit before resubmitting, see if any other comments are made.


>
> > +
> > +     prog = piglit_build_simple_program(vertex_shader_text,
> fragment_shader_text);
> > +     assert(prog);
> > +     glUseProgram(prog);
> > +
> > +     setup_texture();
> > +     setup_uniform_buffer();
> > +
> > +     glClearColor(expected_color[0], expected_color[1],
> expected_color[2],
> expected_color[3]);
> > +}
> > +
> > +
> > +enum piglit_result
> > +piglit_display(void)
> > +{
> > +     int probe = 0;
> > +
> > +     glViewport(0, 0, piglit_width, piglit_height);
> > +
> > +     glClear(GL_COLOR_BUFFER_BIT);
> > +
> > +     if (!piglit_check_gl_error(GL_NO_ERROR))
> > +             return PIGLIT_FAIL;
> > +
> > +     piglit_draw_rect(-1, -1, 2, 2);
> > +
> > +     /* Probe a rect so we don't have to guess at which pixel is
> > +      * incorrect.  Just a small area in the corner should be
> sufficient,
> as
> > +      * long as the boundary condition exists within it.  The expected
> value
> > +      * should match both fragment shader output and the clear color.
> > +      */
> > +     probe = piglit_probe_rect_rgba(0, 0, piglit_width/4,
> piglit_height/4,
> expected_color);
> > +
> > +     piglit_present_results();
> > +
> > +     return probe == 1 ? PIGLIT_PASS : PIGLIT_FAIL;
> > +}
>



-- 
 Cody Northrop
 Graphics Software Engineer
 LunarG, Inc.- 3D Driver Innovations
 Email: cody at lunarg.com
 Website: http://www.lunarg.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140701/7b83b645/attachment-0001.html>


More information about the Piglit mailing list