Re: [Piglit] [PATCH v2] shaders: Test using a bound program after an unsuccessful relink
On 03/11/17 21:27, Neil Roberts wrote: Timothy Arceriwrites: Thanks for the test! The idea is great, the only problem I see is that the minimum GL version seems too high. This test would not run on the i915 driver which is a problem IMO. Can you lower the requirements? Good point. Here is a v2 that should work with GL2.0. However I’m not exactly sure what is the minimun requirements for i915. I read that the default version has been lowered to 1.4. Were you suggesting that it should go as low as using the GL_ARB_shader_objects extension? I guess there are two i915 drivers as well so I’m not sure which one you are referring to. Your right I forgot it was lowered, however dropping the requirements of this test is still the right thing to do so thanks for that :) One small comment further down, otherwise: Acked-by: Timothy Arceri Regards, - Neil -- >8 -- (use git am -c to cut here) If a bound program is relinked unsuccessfully it is supposed to keep the executable and the program state such as the uniforms. This adds a test for that which updates a uniform, does a failed relink and verifies that a render succeeds with the new uniform value. This is currently causing a use-after-free error in Mesa. The test has some redundant allocations to try and encourage it to fail which it does about 98% of the time. v2: Adapt to work with GL2.0. Change the order of the colours so that a successful run ends with green. Delete the program after use. --- tests/all.py| 1 + tests/shaders/CMakeLists.gl.txt | 1 + tests/shaders/unsuccessful-relink.c | 203 3 files changed, 205 insertions(+) create mode 100644 tests/shaders/unsuccessful-relink.c diff --git a/tests/all.py b/tests/all.py index 9e63fa9..ae4a995 100644 --- a/tests/all.py +++ b/tests/all.py @@ -624,6 +624,7 @@ with profile.test_list.group_manager(PiglitGLTest, 'shaders') as g: g(['point-vertex-id', 'gl_InstanceID', 'divisor']) g(['point-vertex-id', 'gl_VertexID', 'gl_InstanceID', 'divisor']) g(['glsl-vs-int-attrib']) +g(['unsuccessful-relink']) g(['glsl-link-initializer-03'], 'GLSL link two programs, global initializer') g(['glsl-getactiveuniform-count', diff --git a/tests/shaders/CMakeLists.gl.txt b/tests/shaders/CMakeLists.gl.txt index 1c06843..7090d94 100644 --- a/tests/shaders/CMakeLists.gl.txt +++ b/tests/shaders/CMakeLists.gl.txt @@ -164,5 +164,6 @@ piglit_add_executable (useshaderprogram-bad-program useshaderprogram-bad-program piglit_add_executable (useshaderprogram-flushverts-1 useshaderprogram-flushverts-1.c) piglit_add_executable (version-mixing version-mixing.c) piglit_add_executable (glsl-vs-int-attrib glsl-vs-int-attrib.c) +piglit_add_executable (unsuccessful-relink unsuccessful-relink.c) # vim: ft=cmake: diff --git a/tests/shaders/unsuccessful-relink.c b/tests/shaders/unsuccessful-relink.c new file mode 100644 index 000..b652fe9 --- /dev/null +++ b/tests/shaders/unsuccessful-relink.c @@ -0,0 +1,203 @@ +/* + * Copyright © 2017 Intel Corporation + * + * 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 unsuccessful-relink.c + * + * Render using a program with a uniform. Modify the uniform and then + * do a relink that will fail. This shouldn’t affect the original + * program and it should render with the new uniform value. + * + * GLSL 4.6 spec section 7.3: + * + * “If a program object that is active for any shader stage is + * re-linked unsuccessfully, the link status will be set to FALSE, + * but any existing executables and associated state will remain part + * of the current rendering state until a subsequent call to + * UseProgram, UseProgramStages, or BindProgramPipeline removes them + * from use.” + */ + +#include "piglit-util-gl.h" +
Re: [Piglit] [PATCH] fbo: add a test for blending with float special values
On 11/05/2017 10:46 AM, Ilia Mirkin wrote: On Sun, Nov 5, 2017 at 12:31 PM, Brian Paulwrote: On 11/04/2017 02:44 PM, Ilia Mirkin wrote: See what happens when doing 0 * Inf and NaN. Signed-off-by: Ilia Mirkin --- Not sure if this is worth checking in. I was just wondering what various hardware did, and this covers many of the permutations. This test can be run both in core profile, or with the -compat flag to force a compat profile. Perhaps different drivers do things differently for the two. On nouveau/nvc0, it assumes 0*inf = 0, on nouveau/nv50 it assumes 0*inf = nan. On i965/skl, it assumes 0*inf = nan. (I have patches to change nv50 to match the nvc0 behavior - it's configurable on both.) I'd be curious to hear what (a) radeonsi and r600 do and (b) how this works on the various blob drivers. For the blob drivers, you should run it with both -compat and without, to see if the behavior is different. If it prints lots of stuff, that means the hw does 0*inf = nan. If it prints nothing out, then it think sthat 0*inf = 0. Either way, I'd be interested in hearing what the full output is. Note that the piglit always passes, since there is no right or wrong here. tests/fbo/CMakeLists.gl.txt | 2 + tests/fbo/fbo-float-nan.c | 159 2 files changed, 161 insertions(+) create mode 100644 tests/fbo/fbo-float-nan.c diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt index 2e2cac9d9..2db8bf700 100644 --- a/tests/fbo/CMakeLists.gl.txt +++ b/tests/fbo/CMakeLists.gl.txt @@ -97,4 +97,6 @@ piglit_add_executable (fbo-cubemap fbo-cubemap.c) piglit_add_executable (fbo-scissor-bitmap fbo-scissor-bitmap.c) piglit_add_executable (fbo-viewport fbo-viewport.c) +piglit_add_executable (fbo-float-nan fbo-float-nan.c) + # vim: ft=cmake: diff --git a/tests/fbo/fbo-float-nan.c b/tests/fbo/fbo-float-nan.c new file mode 100644 index 0..ccbf53ae5 --- /dev/null +++ b/tests/fbo/fbo-float-nan.c @@ -0,0 +1,159 @@ +/* + * Copyright © 2017 Ilia Mirkin + * + * 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. + */ + +/* + * The purpose of this test is to find out what happens when the + * shader produces a NaN value with a floating point RT. This is all + * undefined as far as the GL spec goes, but it's useful to be able to + * compare implementations. + */ + +#include "piglit-util-gl.h" + +PIGLIT_GL_TEST_CONFIG_BEGIN + config.supports_gl_core_version = 31; + config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE; + config.khr_no_error_support = PIGLIT_NO_ERRORS; +PIGLIT_GL_TEST_CONFIG_END + +static const char *vs_text = + "#version 130 \n" + "in vec4 piglit_vertex; \n" + "void main() { \n" + " gl_Position = piglit_vertex; \n" + "}\n"; + +static const char *fs_text = + "#version 130 \n" + "#extension GL_ARB_shader_bit_encoding: require \n" + "out vec4 color; \n" + "uniform uint c; \n" + "uniform uint a; \n" + "void main() { \n" + " color = vec4(vec3(uintBitsToFloat(c)), uintBitsToFloat(a)); \n" + "}\n"; + +GLuint program, fb, rb; + +static const unsigned uINF = 0x7f80; +static const unsigned uNAN = 0x7fc0; + +bool is_nan(unsigned arg) { Brace on next line. + return ((arg & 0x7f80) == 0x7f80) && + (arg & 0x007f); +} + +void test_draw(unsigned u_c, unsigned u_a, unsigned r, unsigned g, unsigned b, unsigned a) +{ + unsigned pixel[4]; + + glUniform1ui(glGetUniformLocation(program, "c"), u_c); + glUniform1ui(glGetUniformLocation(program, "a"), u_a); + piglit_draw_rect(-1, -1, 2, 2); + glReadPixels(0, 0, 1, 1, GL_RGBA, GL_FLOAT, pixel); + + bool rmatch = pixel[0] == r || (is_nan(pixel[0]) && is_nan(r)); + bool gmatch = pixel[1] == g
Re: [Piglit] [PATCH] fbo: add a test for blending with float special values
On Sun, Nov 5, 2017 at 12:31 PM, Brian Paulwrote: > On 11/04/2017 02:44 PM, Ilia Mirkin wrote: >> >> See what happens when doing 0 * Inf and NaN. >> >> Signed-off-by: Ilia Mirkin >> --- >> >> Not sure if this is worth checking in. I was just wondering what various >> hardware did, and this covers many of the permutations. This test can be >> run both in core profile, or with the -compat flag to force a compat >> profile. >> Perhaps different drivers do things differently for the two. >> >> On nouveau/nvc0, it assumes 0*inf = 0, on nouveau/nv50 it assumes 0*inf = >> nan. >> On i965/skl, it assumes 0*inf = nan. (I have patches to change nv50 to >> match >> the nvc0 behavior - it's configurable on both.) >> >> I'd be curious to hear what (a) radeonsi and r600 do and (b) how this >> works >> on the various blob drivers. For the blob drivers, you should run it with >> both -compat and without, to see if the behavior is different. If it >> prints >> lots of stuff, that means the hw does 0*inf = nan. If it prints nothing >> out, >> then it think sthat 0*inf = 0. Either way, I'd be interested in hearing >> what the full output is. >> >> Note that the piglit always passes, since there is no right or wrong here. >> >> tests/fbo/CMakeLists.gl.txt | 2 + >> tests/fbo/fbo-float-nan.c | 159 >> >> 2 files changed, 161 insertions(+) >> create mode 100644 tests/fbo/fbo-float-nan.c >> >> diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt >> index 2e2cac9d9..2db8bf700 100644 >> --- a/tests/fbo/CMakeLists.gl.txt >> +++ b/tests/fbo/CMakeLists.gl.txt >> @@ -97,4 +97,6 @@ piglit_add_executable (fbo-cubemap fbo-cubemap.c) >> piglit_add_executable (fbo-scissor-bitmap fbo-scissor-bitmap.c) >> piglit_add_executable (fbo-viewport fbo-viewport.c) >> >> +piglit_add_executable (fbo-float-nan fbo-float-nan.c) >> + >> # vim: ft=cmake: >> diff --git a/tests/fbo/fbo-float-nan.c b/tests/fbo/fbo-float-nan.c >> new file mode 100644 >> index 0..ccbf53ae5 >> --- /dev/null >> +++ b/tests/fbo/fbo-float-nan.c >> @@ -0,0 +1,159 @@ >> +/* >> + * Copyright © 2017 Ilia Mirkin >> + * >> + * 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. >> + */ >> + >> +/* >> + * The purpose of this test is to find out what happens when the >> + * shader produces a NaN value with a floating point RT. This is all >> + * undefined as far as the GL spec goes, but it's useful to be able to >> + * compare implementations. >> + */ >> + >> +#include "piglit-util-gl.h" >> + >> +PIGLIT_GL_TEST_CONFIG_BEGIN >> + config.supports_gl_core_version = 31; >> + config.window_visual = PIGLIT_GL_VISUAL_RGBA | >> PIGLIT_GL_VISUAL_DOUBLE; >> + config.khr_no_error_support = PIGLIT_NO_ERRORS; >> +PIGLIT_GL_TEST_CONFIG_END >> + >> +static const char *vs_text = >> + "#version 130 \n" >> + "in vec4 piglit_vertex; \n" >> + "void main() { \n" >> + " gl_Position = piglit_vertex; \n" >> + "}\n"; >> + >> +static const char *fs_text = >> + "#version 130 \n" >> + "#extension GL_ARB_shader_bit_encoding: require \n" >> + "out vec4 color; \n" >> + "uniform uint c; \n" >> + "uniform uint a; \n" >> + "void main() { \n" >> + " color = vec4(vec3(uintBitsToFloat(c)), uintBitsToFloat(a)); \n" >> + "}\n"; >> + >> +GLuint program, fb, rb; >> + >> +static const unsigned uINF = 0x7f80; >> +static const unsigned uNAN = 0x7fc0; >> + >> +bool is_nan(unsigned arg) { > > > Brace on next line. > > >> + return ((arg & 0x7f80) == 0x7f80) && >> + (arg & 0x007f); >> +} >> + >> +void test_draw(unsigned u_c, unsigned u_a, unsigned r, unsigned g, >> unsigned b, unsigned a) >> +{ >> + unsigned
Re: [Piglit] [PATCH] fbo: add a test for blending with float special values
On 11/04/2017 02:44 PM, Ilia Mirkin wrote: See what happens when doing 0 * Inf and NaN. Signed-off-by: Ilia Mirkin--- Not sure if this is worth checking in. I was just wondering what various hardware did, and this covers many of the permutations. This test can be run both in core profile, or with the -compat flag to force a compat profile. Perhaps different drivers do things differently for the two. On nouveau/nvc0, it assumes 0*inf = 0, on nouveau/nv50 it assumes 0*inf = nan. On i965/skl, it assumes 0*inf = nan. (I have patches to change nv50 to match the nvc0 behavior - it's configurable on both.) I'd be curious to hear what (a) radeonsi and r600 do and (b) how this works on the various blob drivers. For the blob drivers, you should run it with both -compat and without, to see if the behavior is different. If it prints lots of stuff, that means the hw does 0*inf = nan. If it prints nothing out, then it think sthat 0*inf = 0. Either way, I'd be interested in hearing what the full output is. Note that the piglit always passes, since there is no right or wrong here. tests/fbo/CMakeLists.gl.txt | 2 + tests/fbo/fbo-float-nan.c | 159 2 files changed, 161 insertions(+) create mode 100644 tests/fbo/fbo-float-nan.c diff --git a/tests/fbo/CMakeLists.gl.txt b/tests/fbo/CMakeLists.gl.txt index 2e2cac9d9..2db8bf700 100644 --- a/tests/fbo/CMakeLists.gl.txt +++ b/tests/fbo/CMakeLists.gl.txt @@ -97,4 +97,6 @@ piglit_add_executable (fbo-cubemap fbo-cubemap.c) piglit_add_executable (fbo-scissor-bitmap fbo-scissor-bitmap.c) piglit_add_executable (fbo-viewport fbo-viewport.c) +piglit_add_executable (fbo-float-nan fbo-float-nan.c) + # vim: ft=cmake: diff --git a/tests/fbo/fbo-float-nan.c b/tests/fbo/fbo-float-nan.c new file mode 100644 index 0..ccbf53ae5 --- /dev/null +++ b/tests/fbo/fbo-float-nan.c @@ -0,0 +1,159 @@ +/* + * Copyright © 2017 Ilia Mirkin + * + * 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. + */ + +/* + * The purpose of this test is to find out what happens when the + * shader produces a NaN value with a floating point RT. This is all + * undefined as far as the GL spec goes, but it's useful to be able to + * compare implementations. + */ + +#include "piglit-util-gl.h" + +PIGLIT_GL_TEST_CONFIG_BEGIN + config.supports_gl_core_version = 31; + config.window_visual = PIGLIT_GL_VISUAL_RGBA | PIGLIT_GL_VISUAL_DOUBLE; + config.khr_no_error_support = PIGLIT_NO_ERRORS; +PIGLIT_GL_TEST_CONFIG_END + +static const char *vs_text = + "#version 130 \n" + "in vec4 piglit_vertex; \n" + "void main() { \n" + " gl_Position = piglit_vertex; \n" + "}\n"; + +static const char *fs_text = + "#version 130 \n" + "#extension GL_ARB_shader_bit_encoding: require \n" + "out vec4 color; \n" + "uniform uint c; \n" + "uniform uint a; \n" + "void main() { \n" + " color = vec4(vec3(uintBitsToFloat(c)), uintBitsToFloat(a)); \n" + "}\n"; + +GLuint program, fb, rb; + +static const unsigned uINF = 0x7f80; +static const unsigned uNAN = 0x7fc0; + +bool is_nan(unsigned arg) { Brace on next line. + return ((arg & 0x7f80) == 0x7f80) && + (arg & 0x007f); +} + +void test_draw(unsigned u_c, unsigned u_a, unsigned r, unsigned g, unsigned b, unsigned a) +{ + unsigned pixel[4]; + + glUniform1ui(glGetUniformLocation(program, "c"), u_c); + glUniform1ui(glGetUniformLocation(program, "a"), u_a); + piglit_draw_rect(-1, -1, 2, 2); + glReadPixels(0, 0, 1, 1, GL_RGBA, GL_FLOAT, pixel); + + bool rmatch = pixel[0] == r || (is_nan(pixel[0]) && is_nan(r)); + bool gmatch = pixel[1] == g || (is_nan(pixel[1]) && is_nan(g)); + bool bmatch = pixel[2] == b || (is_nan(pixel[2]) && is_nan(b)); + bool