ARB_uniform_buffer_object: Add test for discard with UBO.

Submitted by Cody Northrop on July 1, 2014, 6:53 p.m.

Details

Message ID 1404240798-29320-1-git-send-email-cody@lunarg.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Cody Northrop July 1, 2014, 6:53 p.m.
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@lunarg.com>
Reviewed-by:  Courtney Goeltzenleuchter <courtney@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

Patch hide | download patch | download mbox

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"
+	"    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");
+
+	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;
+}

Comments

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@lunarg.com>
> Reviewed-by:  Courtney Goeltzenleuchter <courtney@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?

> +	"    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.

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@whitecape.org>

(perhaps other people will want to review it, though)

Thanks for doing this.

> +
> +	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;
> +}
On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <kenneth@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@lunarg.com>
> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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;
> > +}
>
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@lunarg.com> wrote:
> On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <kenneth@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@lunarg.com>
>> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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@lunarg.com
>  Website: http://www.lunarg.com
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>
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@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@lunarg.com> wrote:
> > On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <kenneth@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@lunarg.com>
> >> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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@lunarg.com
> >  Website: http://www.lunarg.com
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
> >
>
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@ijw.co.nz>

On Wed, Jul 2, 2014 at 10:08 AM, Cody Northrop <cody@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@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@lunarg.com> wrote:
>> > On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <kenneth@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@lunarg.com>
>> >> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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@lunarg.com
>> >  Website: http://www.lunarg.com
>> >
>> > _______________________________________________
>> > Piglit mailing list
>> > Piglit@lists.freedesktop.org
>> > http://lists.freedesktop.org/mailman/listinfo/piglit
>> >
>
>
>
>
> --
>  Cody Northrop
>  Graphics Software Engineer
>  LunarG, Inc.- 3D Driver Innovations
>  Email: cody@lunarg.com
>  Website: http://www.lunarg.com
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
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@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@ijw.co.nz>
>
> On Wed, Jul 2, 2014 at 10:08 AM, Cody Northrop <cody@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@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@lunarg.com> wrote:
> >> > On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <
> kenneth@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@lunarg.com>
> >> >> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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@lunarg.com
> >> >  Website: http://www.lunarg.com
> >> >
> >> > _______________________________________________
> >> > Piglit mailing list
> >> > Piglit@lists.freedesktop.org
> >> > http://lists.freedesktop.org/mailman/listinfo/piglit
> >> >
> >
> >
> >
> >
> > --
> >  Cody Northrop
> >  Graphics Software Engineer
> >  LunarG, Inc.- 3D Driver Innovations
> >  Email: cody@lunarg.com
> >  Website: http://www.lunarg.com
>
On Tue, Jul 1, 2014 at 7:26 PM, Cody Northrop <cody@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@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@ijw.co.nz>
>>
>> On Wed, Jul 2, 2014 at 10:08 AM, Cody Northrop <cody@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@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@lunarg.com> wrote:
>> >> > On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke
>> >> > <kenneth@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@lunarg.com>
>> >> >> > Reviewed-by:  Courtney Goeltzenleuchter <courtney@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@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@lunarg.com
>> >> >  Website: http://www.lunarg.com
>> >> >
>> >> > _______________________________________________
>> >> > Piglit mailing list
>> >> > Piglit@lists.freedesktop.org
>> >> > http://lists.freedesktop.org/mailman/listinfo/piglit
>> >> >
>> >
>> >
>> >
>> >
>> > --
>> >  Cody Northrop
>> >  Graphics Software Engineer
>> >  LunarG, Inc.- 3D Driver Innovations
>> >  Email: cody@lunarg.com
>> >  Website: http://www.lunarg.com
>
>
>
>
> --
>  Cody Northrop
>  Graphics Software Engineer
>  LunarG, Inc.- 3D Driver Innovations
>  Email: cody@lunarg.com
>  Website: http://www.lunarg.com
>
> _______________________________________________
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
>