<div dir="ltr">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:<div> </div><blockquote style="margin:0px 0px 0px 40px;border:none;padding:0px"> <div><div>Failed to compile vertex shader: 0(3) : error C0000: syntax error, unexpected '(', expecting "::" at token "("</div></div><div><div> </div></div><div><div>source:</div></div><div><div> #version 130</div></div><div><div>#extension GL_ARB_explicit_attrib_location: require</div></div><div><div>layout(location = 0) in vec4 piglit_vertex;</div></div><div><div>void main()</div></div><div><div>{</div></div><div> <div> gl_Position = piglit_vertex;</div></div><div><div>}</div></div><div><div>PIGLIT: {"result": "fail" }</div></div></blockquote><div> </div></div><div class="gmail_extra"> <div class="gmail_quote"> On Tue, Jul 1, 2014 at 4:28 PM, Chris Forbes <<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <<a href="mailto:chrisf@ijw.co.nz">chrisf@ijw.co.nz</a>> <div class="HOEnZb"><div class="h5"> On Wed, Jul 2, 2014 at 10:08 AM, Cody Northrop <<a href="mailto:cody@lunarg.com">cody@lunarg.com</a>> 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 <<a href="mailto:chrisf@ijw.co.nz">chrisf@ijw.co.nz</a>> 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 <<a href="mailto:cody@lunarg.com">cody@lunarg.com</a>> wrote: >> > On Tue, Jul 1, 2014 at 3:05 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> >> > 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 <<a href="mailto:cody@lunarg.com">cody@lunarg.com</a>> >> >> > Reviewed-by: Courtney Goeltzenleuchter <<a href="mailto:courtney@lunarg.com">courtney@lunarg.com</a>> >> >> > --- >> >> > 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 <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> >> >> >> >> (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: <a href="mailto:cody@lunarg.com">cody@lunarg.com</a> >> > Website: <a href="http://www.lunarg.com" target="_blank">http://www.lunarg.com</a> >> > >> > _______________________________________________ >> > Piglit mailing list >> > <a href="mailto:Piglit@lists.freedesktop.org">Piglit@lists.freedesktop.org</a> >> > <a href="http://lists.freedesktop.org/mailman/listinfo/piglit" target="_blank">http://lists.freedesktop.org/mailman/listinfo/piglit</a> >> > > > > > > -- > Cody Northrop > Graphics Software Engineer > LunarG, Inc.- 3D Driver Innovations > Email: <a href="mailto:cody@lunarg.com">cody@lunarg.com</a> > Website: <a href="http://www.lunarg.com" target="_blank">http://www.lunarg.com</a> </div></div></blockquote></div> <div> </div>-- <div dir="ltr"><div> Cody Northrop Graphics Software Engineer LunarG, Inc.- 3D Driver Innovations Email: <a href="mailto:cody@lunarg.com" target="_blank">cody@lunarg.com</a> Website: <a href="http://www.lunarg.com/" target="_blank">http://www.lunarg.com</a></div></div> </div>