Re: [Piglit] [PATCH v2] shaders: Test using a bound program after an unsuccessful relink

2017-11-05 Thread Timothy Arceri



On 03/11/17 21:27, Neil Roberts wrote:

Timothy Arceri  writes:


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

2017-11-05 Thread Brian Paul

On 11/05/2017 10:46 AM, Ilia Mirkin wrote:

On Sun, Nov 5, 2017 at 12:31 PM, Brian Paul  wrote:

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

2017-11-05 Thread Ilia Mirkin
On Sun, Nov 5, 2017 at 12:31 PM, Brian Paul  wrote:
> 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

2017-11-05 Thread Brian Paul

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