[Piglit] [PATCH] ARB_uniform_buffer_object: Add test for discard with UBO.
Ilia Mirkin
imirkin at alum.mit.edu
Tue Jul 1 16:31:58 PDT 2014
On Tue, Jul 1, 2014 at 7:26 PM, Cody Northrop <cody at lunarg.com> wrote:
> I can lower the fragment shader to 1.3, but Nvidia closed source complains
> if I lower the vertex shader below 1.4. It's not obvious to me why:
>
> Failed to compile vertex shader: 0(3) : error C0000: syntax error,
> unexpected '(', expecting "::" at token "("
>
> source:
> #version 130
> #extension GL_ARB_explicit_attrib_location: require
They don't like that. A *ton* of piglits fail on the nvidia driver
because of it. I spent a bit of time trying diff things, but the only
thing that worked was upping to version 140. However I think the
policy is to use the lowest possible versions to support the most
drivers (some of which might support version 130 but not 140).
> layout(location = 0) in vec4 piglit_vertex;
> void main()
> {
> gl_Position = piglit_vertex;
> }
> PIGLIT: {"result": "fail" }
>
>
>
>
> On Tue, Jul 1, 2014 at 4:28 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
>>
>> Pick one of 1.40 and ARB_uniform_buffer_object -- you don't need both.
>> 1.30+ARB_ubo would allow you to drop the GL version requirement down a
>> bit too.
>>
>> Acked-by: Chris Forbes <chrisf at ijw.co.nz>
>>
>> On Wed, Jul 2, 2014 at 10:08 AM, Cody Northrop <cody at lunarg.com> wrote:
>> > Yes, the texture sample is important. It may be affecting the timing of
>> > the
>> > uniform loads via sampler.
>> >
>> > If I remove use of the texture, or replace it with vec3(1.0), the
>> > problem is
>> > very hard to detect.
>> >
>> > I'm using white to make it very clear where the incorrect results creep
>> > in.
>> >
>> > That being said, I'm sure there are other ways to get the bug to show
>> > up..
>> >
>> > -C
>> >
>> >
>> > On Tue, Jul 1, 2014 at 3:36 PM, Chris Forbes <chrisf at ijw.co.nz> wrote:
>> >>
>> >> The texture actually seems unnecessary -- are you using that to defeat
>> >> some optimization?
>> >>
>> >> On Wed, Jul 2, 2014 at 9:28 AM, Cody Northrop <cody at lunarg.com> wrote:
>> >> > 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
>> >> >
>> >> > _______________________________________________
>> >> > Piglit mailing list
>> >> > Piglit at lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/piglit
>> >> >
>> >
>> >
>> >
>> >
>> > --
>> > Cody Northrop
>> > Graphics Software Engineer
>> > LunarG, Inc.- 3D Driver Innovations
>> > Email: cody at lunarg.com
>> > Website: http://www.lunarg.com
>
>
>
>
> --
> Cody Northrop
> Graphics Software Engineer
> LunarG, Inc.- 3D Driver Innovations
> Email: cody at lunarg.com
> Website: http://www.lunarg.com
>
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
More information about the Piglit
mailing list