Re: [Piglit] [PATCH 06/25] arb_direct_state_access: Use piglit_reset_gl_error in texunits

2015-05-20 Thread Pohjolainen, Topi 
On Mon, May 18, 2015 at 01:49:59PM -0700, Ian Romanick wrote:
 From: Ian Romanick ian.d.roman...@intel.com
 
 Signed-off-by: Ian Romanick ian.d.roman...@intel.com

This is also:

Reviewed-by: Topi Pohjolainen topi.pohjolai...@intel.com

Unfortunately I don't have much input for the rest of the patches
in the series.

 ---
  tests/spec/arb_direct_state_access/texunits.c | 16 
  1 file changed, 4 insertions(+), 12 deletions(-)
 
 diff --git a/tests/spec/arb_direct_state_access/texunits.c 
 b/tests/spec/arb_direct_state_access/texunits.c
 index 54bd886..d4710e3 100644
 --- a/tests/spec/arb_direct_state_access/texunits.c
 +++ b/tests/spec/arb_direct_state_access/texunits.c
 @@ -91,20 +91,12 @@ report4v(const GLfloat exp[4], const GLfloat act[4])
  }
  
  
 -static void
 -clear_errors()
 -{
 -   while (glGetError())
 -  ;
 -}
 -
 -
  static bool
  test_rasterpos(void)
  {
 int i;
  
 -   clear_errors();
 +   piglit_reset_gl_error();
  
 /* set current texcoords */
 for (i = 0; i  MaxTextureCoordUnits; i++) {
 @@ -169,7 +161,7 @@ test_texture_matrix(void)
  {
 int i;
  
 -   clear_errors();
 +   piglit_reset_gl_error();
  
 /* set tex matrices */
 for (i = 0; i  MaxTextureCoordUnits; i++) {
 @@ -221,7 +213,7 @@ test_texture_params(void)
 int i;
 int maxUnit;
  
 -   clear_errors();
 +   piglit_reset_gl_error();
  
 glCreateTextures(GL_TEXTURE_2D, MaxTextureCombinedUnits, tex);
  
 @@ -269,7 +261,7 @@ test_texture_env(void)
 /* Texture Environment state is fixed-function; not used by shaders */
 int i;
  
 -   clear_errors();
 +   piglit_reset_gl_error();
  
 /* set per-unit state */
 for (i = 0; i  MaxTextureCombinedUnits; i++) {
 -- 
 2.1.0
 
 ___
 Piglit mailing list
 Piglit@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v2] program_interface_query: add new subtest for getprogramresourceiv

2015-05-20 Thread Samuel Iglesias Gonsalvez 
Test that uniforms inside arrays of uniform blocks with instance name are
queried properly.

Tested on NVIDIA's proprietary driver version 340.65

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90397

Signed-off-by: Samuel Iglesias Gonsalvez sigles...@igalia.com
---

V2:

  * Change instance name for the array of uniform blocks
  * Delete modification of an unrelated test.

 tests/spec/arb_program_interface_query/common.h   | 12 +---
 .../getprogramresourceiv.c| 19 +++
 .../spec/arb_program_interface_query/resource-query.c | 16 
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/tests/spec/arb_program_interface_query/common.h 
b/tests/spec/arb_program_interface_query/common.h
index 55f0358..e083a6e 100755
--- a/tests/spec/arb_program_interface_query/common.h
+++ b/tests/spec/arb_program_interface_query/common.h
@@ -102,13 +102,19 @@ static const char fs_std[] =
uniform fs_uniform_block {
   vec4 fs_color;\n
   float fs_array[4];\n
-   };
+   };\n
+   uniform fs_array_uniform_block {\n
+  vec4 fs_color;\n
+  float fs_array[4];\n
+   } faub[4];\n
in vec4 fs_input1;\n
out vec4 fs_output0;\n
out vec4 fs_output1;\n
void main() {\n
-   fs_output0 = fs_color * fs_input1 * fs_array[2];\n
-   fs_output1 = fs_color * fs_input1 * fs_array[3];\n
+   fs_output0 = fs_color * fs_input1 * fs_array[2] * \n
+faub[0].fs_array[2] * faub[2].fs_array[2];\n
+   fs_output1 = fs_color * fs_input1 * fs_array[3] * \n
+faub[1].fs_array[3] * faub[3].fs_array[3];\n
};
 
 static const char vs_stor[] =
diff --git a/tests/spec/arb_program_interface_query/getprogramresourceiv.c 
b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
index da9751a..03f2fc6 100755
--- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
+++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
@@ -454,6 +454,25 @@ static const struct subtest_t subtests[] = {
{ GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
{ 0, 0, { 0 } }}
  },
+ { prog_std, GL_UNIFORM, fs_array_uniform_block.fs_array, 
fs_array_uniform_block[0], {
+   { GL_NAME_LENGTH, 1, { 35 } },
+   { GL_TYPE, 1, { GL_FLOAT } },
+   { GL_ARRAY_SIZE, 1, { 4 } },
+   { GL_OFFSET, 1, { 0 } }, /* valid index == anything but -1 */
+   { GL_BLOCK_INDEX, 1, { 2 } }, /* compared to 
fs_array_uniform_block[0]'s idx */
+   { GL_ARRAY_STRIDE, 1, { 0 } }, /* valid index == anything but -1 */
+   { GL_MATRIX_STRIDE, 1, { 0 } },
+   { GL_IS_ROW_MAJOR, 1, { 0 } },
+   { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* valid index == 
anything but -1 */
+   { GL_REFERENCED_BY_VERTEX_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_TESS_CONTROL_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_TESS_EVALUATION_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 1 } },
+   { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
+   { GL_LOCATION, 1, { -1 } }, /* valid index == anything but -1 */
+   { 0, 0, { 0 } }}
+ },
  { prog_stor, GL_BUFFER_VARIABLE, gs_buf_var, gs_buffer_block, {
{ GL_NAME_LENGTH, 1, { 11 } },
{ GL_TYPE, 1, { GL_FLOAT_VEC4 } },
diff --git a/tests/spec/arb_program_interface_query/resource-query.c 
b/tests/spec/arb_program_interface_query/resource-query.c
index 92b8cd8..1db5585 100755
--- a/tests/spec/arb_program_interface_query/resource-query.c
+++ b/tests/spec/arb_program_interface_query/resource-query.c
@@ -189,13 +189,21 @@ PIGLIT_GL_TEST_CONFIG_END
  * white space anywhere in the string.
  */
 static const char *st_r_uniform[] = {vs_test, gs_test, fs_color,
-fs_array[0], sa[0].a[0], sa[1].a[0],
+fs_array[0],
+fs_array_uniform_block.fs_color,
+fs_array_uniform_block.fs_array[0],
+sa[0].a[0], sa[1].a[0],
 NULL};
 static const char *st_r_tess_uniform[] = {tcs_test, tes_test, NULL};
 static const char *st_r_cs_uniform[] = {cs_test, tex, NULL};
 static const char *st_r_uniform_block[] = {vs_uniform_block,
   gs_uniform_block,
-  fs_uniform_block, NULL};
+  fs_uniform_block,
+  fs_array_uniform_block[0],
+  fs_array_uniform_block[1],
+  fs_array_uniform_block[2],
+  fs_array_uniform_block[3],
+  NULL};

Re: [Piglit] [PATCH] program_interface_query: add new subtest for getprogramresourceiv

2015-05-20 Thread Martin Peres 

On 20/05/15 11:39, Samuel Iglesias Gonsálvez wrote:

On 19/05/15 12:02, Martin Peres wrote:

On 19/05/15 08:46, Samuel Iglesias Gonsalvez wrote:

Test that uniforms inside arrays of uniform blocks with instance name are
queried properly.

Tested on NVIDIA's proprietary driver version 340.65

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90397

Signed-off-by: Samuel Iglesias Gonsalvez sigles...@igalia.com
---
   tests/spec/arb_program_interface_query/common.h   | 12 +---
   .../getprogramresourceiv.c| 19
+++
   .../spec/arb_program_interface_query/resource-query.c | 16

   tests/spec/arb_shader_storage_buffer_object/minmax.c  |  4 ++--
   4 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/tests/spec/arb_program_interface_query/common.h
b/tests/spec/arb_program_interface_query/common.h
index 55f0358..4079ac8 100755
--- a/tests/spec/arb_program_interface_query/common.h
+++ b/tests/spec/arb_program_interface_query/common.h
@@ -102,13 +102,19 @@ static const char fs_std[] =
   uniform fs_uniform_block {
   vec4 fs_color;\n
   float fs_array[4];\n
-};
+};\n
+uniform fs_array_uniform_block {\n
+vec4 fs_color;\n
+float fs_array[4];\n
+} ubo[4];\n

Hmm, aside from the obvious problem of having ubo != uniform_block,

I am going to rename that name uniform block in the second version of
the patch.


sounds nicer indeed!




I
wonder what would happen if you instead had:

+} faub1[2], faub2[2];\n


That kind of definition is not allowed in the spec. I think it is only
one instance name per interface block.

I tried defining faub2[2] by repeating the fs_array_uniform_block
definition but i965 doesn't allowed it, although NVIDIA does.


I hope that no games rely on this...




I am worried that you would get the following resources listed:

+   fs_array_uniform_block[0],
+   fs_array_uniform_block[1],
+   fs_array_uniform_block[0],
+   fs_array_uniform_block[1],


In my tests on NVIDIA, I get:

+   fs_array_uniform_block[0],
+   fs_array_uniform_block[1],

But they are not repeated. I guess NVIDIA thinks they are the same
uniform block but I did not check it further.


Seems like a lovely bug!




which would be a clear indication that the driver is broken. My view on
this is that named uniform blocks should use their named version when
being referenced. What do you think?

I have been checking what the specs say about it.

* In ARB_program_interface_query, GetProgramResourceName() is equivalent
to GetActiveUniformBlockName().

* Looking at ARB_uniform_buffer_object, GetActiveUniformBlockName() is
talking about uniform block name.

* Looking at GLSL spec (for example 4.4), when talking about interface
block (4.3.9 Interface Blocks), its definition is:

layout-qualifieropt interface-qualifier block-name { member-list }
instance-nameopt ;

But also, the same sections talks about the name:

For blocks declared as arrays, the array index must also be included
  when accessing members, as in this example

   uniform Transform { // API uses “Transform[2]” to refer to instance 2
 mat4 ModelViewMatrix;
 mat4 ModelViewProjectionMatrix;
 vec4 a[]; // array will get implicitly sized
 float Deformation;
   } transforms[4];

  [...]
  When using OpenGL API entry points to identify the name of an
  individual block in an array of blocks, the name string must include
  an array index (e.g., Transform[2]).

Notice that it is talking about the block name, not the instance name.
So the current behaviour is right.


Wow, I am baffled by those crazy rules... Thanks for tracking this down 
the spec!





   in vec4 fs_input1;\n
   out vec4 fs_output0;\n
   out vec4 fs_output1;\n
   void main() {\n
-fs_output0 = fs_color * fs_input1 * fs_array[2];\n
-fs_output1 = fs_color * fs_input1 * fs_array[3];\n
+fs_output0 = fs_color * fs_input1 * fs_array[2] * \n
+  ubo[0].fs_array[2] * ubo[2].fs_array[2];\n
+fs_output1 = fs_color * fs_input1 * fs_array[3] * \n
+ ubo[1].fs_array[3] * ubo[3].fs_array[3];\n
   };
 static const char vs_stor[] =
diff --git
a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
index da9751a..03f2fc6 100755
--- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
+++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
@@ -454,6 +454,25 @@ static const struct subtest_t subtests[] = {
   { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
   { 0, 0, { 0 } }}
},
+ { prog_std, GL_UNIFORM, fs_array_uniform_block.fs_array,
fs_array_uniform_block[0], {
+{ GL_NAME_LENGTH, 1, { 35 } },
+{ GL_TYPE, 1, { GL_FLOAT } },
+{ GL_ARRAY_SIZE, 1, { 4 } },

Re: [Piglit] [PATCH] program_interface_query: add new subtest for getprogramresourceiv

2015-05-20 Thread Samuel Iglesias Gonsálvez 
On 19/05/15 12:02, Martin Peres wrote:
 On 19/05/15 08:46, Samuel Iglesias Gonsalvez wrote:
 Test that uniforms inside arrays of uniform blocks with instance name are
 queried properly.

 Tested on NVIDIA's proprietary driver version 340.65

 Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90397

 Signed-off-by: Samuel Iglesias Gonsalvez sigles...@igalia.com
 ---
   tests/spec/arb_program_interface_query/common.h   | 12 +---
   .../getprogramresourceiv.c| 19
 +++
   .../spec/arb_program_interface_query/resource-query.c | 16
 
   tests/spec/arb_shader_storage_buffer_object/minmax.c  |  4 ++--
   4 files changed, 42 insertions(+), 9 deletions(-)

 diff --git a/tests/spec/arb_program_interface_query/common.h
 b/tests/spec/arb_program_interface_query/common.h
 index 55f0358..4079ac8 100755
 --- a/tests/spec/arb_program_interface_query/common.h
 +++ b/tests/spec/arb_program_interface_query/common.h
 @@ -102,13 +102,19 @@ static const char fs_std[] =
   uniform fs_uniform_block {
   vec4 fs_color;\n
   float fs_array[4];\n
 -};
 +};\n
 +uniform fs_array_uniform_block {\n
 +vec4 fs_color;\n
 +float fs_array[4];\n
 +} ubo[4];\n
 
 Hmm, aside from the obvious problem of having ubo != uniform_block,

I am going to rename that name uniform block in the second version of
the patch.

 I
 wonder what would happen if you instead had:
 
 +} faub1[2], faub2[2];\n
 

That kind of definition is not allowed in the spec. I think it is only
one instance name per interface block.

I tried defining faub2[2] by repeating the fs_array_uniform_block
definition but i965 doesn't allowed it, although NVIDIA does.

 
 I am worried that you would get the following resources listed:
 
 +   fs_array_uniform_block[0],
 +   fs_array_uniform_block[1],
 +   fs_array_uniform_block[0],
 +   fs_array_uniform_block[1],
 

In my tests on NVIDIA, I get:

+   fs_array_uniform_block[0],
+   fs_array_uniform_block[1],

But they are not repeated. I guess NVIDIA thinks they are the same
uniform block but I did not check it further.

 
 which would be a clear indication that the driver is broken. My view on
 this is that named uniform blocks should use their named version when
 being referenced. What do you think?

I have been checking what the specs say about it.

* In ARB_program_interface_query, GetProgramResourceName() is equivalent
to GetActiveUniformBlockName().

* Looking at ARB_uniform_buffer_object, GetActiveUniformBlockName() is
talking about uniform block name.

* Looking at GLSL spec (for example 4.4), when talking about interface
block (4.3.9 Interface Blocks), its definition is:

layout-qualifieropt interface-qualifier block-name { member-list }
instance-nameopt ;

But also, the same sections talks about the name:

For blocks declared as arrays, the array index must also be included
 when accessing members, as in this example

  uniform Transform { // API uses “Transform[2]” to refer to instance 2
mat4 ModelViewMatrix;
mat4 ModelViewProjectionMatrix;
vec4 a[]; // array will get implicitly sized
float Deformation;
  } transforms[4];

 [...]
 When using OpenGL API entry points to identify the name of an
 individual block in an array of blocks, the name string must include
 an array index (e.g., Transform[2]).

Notice that it is talking about the block name, not the instance name.
So the current behaviour is right.


   in vec4 fs_input1;\n
   out vec4 fs_output0;\n
   out vec4 fs_output1;\n
   void main() {\n
 -fs_output0 = fs_color * fs_input1 * fs_array[2];\n
 -fs_output1 = fs_color * fs_input1 * fs_array[3];\n
 +fs_output0 = fs_color * fs_input1 * fs_array[2] * \n
 +  ubo[0].fs_array[2] * ubo[2].fs_array[2];\n
 +fs_output1 = fs_color * fs_input1 * fs_array[3] * \n
 + ubo[1].fs_array[3] * ubo[3].fs_array[3];\n
   };
 static const char vs_stor[] =
 diff --git
 a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
 b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
 index da9751a..03f2fc6 100755
 --- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
 +++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
 @@ -454,6 +454,25 @@ static const struct subtest_t subtests[] = {
   { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
   { 0, 0, { 0 } }}
},
 + { prog_std, GL_UNIFORM, fs_array_uniform_block.fs_array,
 fs_array_uniform_block[0], {
 +{ GL_NAME_LENGTH, 1, { 35 } },
 +{ GL_TYPE, 1, { GL_FLOAT } },
 +{ GL_ARRAY_SIZE, 1, { 4 } },
 +{ GL_OFFSET, 1, { 0 } }, /* valid index == anything but -1 */
 +{ GL_BLOCK_INDEX, 1, { 2 } }, /* compared to
 fs_array_uniform_block[0]'s idx */
 +{ GL_ARRAY_STRIDE, 1, { 0 } },

Re: [Piglit] [PATCH v2] program_interface_query: add new subtest for getprogramresourceiv

2015-05-20 Thread Martin Peres 

On 20/05/15 15:10, Samuel Iglesias Gonsalvez wrote:

Test that uniforms inside arrays of uniform blocks with instance name are
queried properly.

Tested on NVIDIA's proprietary driver version 340.65

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90397

Signed-off-by: Samuel Iglesias Gonsalvez sigles...@igalia.com
---

V2:

   * Change instance name for the array of uniform blocks
   * Delete modification of an unrelated test.

  tests/spec/arb_program_interface_query/common.h   | 12 +---
  .../getprogramresourceiv.c| 19 +++
  .../spec/arb_program_interface_query/resource-query.c | 16 
  3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/tests/spec/arb_program_interface_query/common.h 
b/tests/spec/arb_program_interface_query/common.h
index 55f0358..e083a6e 100755
--- a/tests/spec/arb_program_interface_query/common.h
+++ b/tests/spec/arb_program_interface_query/common.h
@@ -102,13 +102,19 @@ static const char fs_std[] =
uniform fs_uniform_block {
  vec4 fs_color;\n
  float fs_array[4];\n
-   };
+   };\n
+   uniform fs_array_uniform_block {\n
+ vec4 fs_color;\n
+ float fs_array[4];\n
+   } faub[4];\n
in vec4 fs_input1;\n
out vec4 fs_output0;\n
out vec4 fs_output1;\n
void main() {\n
-   fs_output0 = fs_color * fs_input1 * fs_array[2];\n
-   fs_output1 = fs_color * fs_input1 * fs_array[3];\n
+   fs_output0 = fs_color * fs_input1 * fs_array[2] * \n
+   faub[0].fs_array[2] * faub[2].fs_array[2];\n
+   fs_output1 = fs_color * fs_input1 * fs_array[3] * \n
+faub[1].fs_array[3] * faub[3].fs_array[3];\n
};
  
  static const char vs_stor[] =

diff --git a/tests/spec/arb_program_interface_query/getprogramresourceiv.c 
b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
index da9751a..03f2fc6 100755
--- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
+++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
@@ -454,6 +454,25 @@ static const struct subtest_t subtests[] = {
{ GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
{ 0, 0, { 0 } }}
   },
+ { prog_std, GL_UNIFORM, fs_array_uniform_block.fs_array, 
fs_array_uniform_block[0], {
+   { GL_NAME_LENGTH, 1, { 35 } },
+   { GL_TYPE, 1, { GL_FLOAT } },
+   { GL_ARRAY_SIZE, 1, { 4 } },
+   { GL_OFFSET, 1, { 0 } }, /* valid index == anything but -1 */
+   { GL_BLOCK_INDEX, 1, { 2 } }, /* compared to 
fs_array_uniform_block[0]'s idx */
+   { GL_ARRAY_STRIDE, 1, { 0 } }, /* valid index == anything but -1 */
+   { GL_MATRIX_STRIDE, 1, { 0 } },
+   { GL_IS_ROW_MAJOR, 1, { 0 } },
+   { GL_ATOMIC_COUNTER_BUFFER_INDEX, 1, { -1 } }, /* valid index == 
anything but -1 */
+   { GL_REFERENCED_BY_VERTEX_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_TESS_CONTROL_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_TESS_EVALUATION_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_GEOMETRY_SHADER, 1, { 0 } },
+   { GL_REFERENCED_BY_FRAGMENT_SHADER, 1, { 1 } },
+   { GL_REFERENCED_BY_COMPUTE_SHADER, 1, { 0 } },
+   { GL_LOCATION, 1, { -1 } }, /* valid index == anything but -1 */
+   { 0, 0, { 0 } }}
+ },
   { prog_stor, GL_BUFFER_VARIABLE, gs_buf_var, gs_buffer_block, {
{ GL_NAME_LENGTH, 1, { 11 } },
{ GL_TYPE, 1, { GL_FLOAT_VEC4 } },
diff --git a/tests/spec/arb_program_interface_query/resource-query.c 
b/tests/spec/arb_program_interface_query/resource-query.c
index 92b8cd8..1db5585 100755
--- a/tests/spec/arb_program_interface_query/resource-query.c
+++ b/tests/spec/arb_program_interface_query/resource-query.c
@@ -189,13 +189,21 @@ PIGLIT_GL_TEST_CONFIG_END
   * white space anywhere in the string.
   */
  static const char *st_r_uniform[] = {vs_test, gs_test, fs_color,
-fs_array[0], sa[0].a[0], sa[1].a[0],
+fs_array[0],
+fs_array_uniform_block.fs_color,
+fs_array_uniform_block.fs_array[0],
+sa[0].a[0], sa[1].a[0],
 NULL};
  static const char *st_r_tess_uniform[] = {tcs_test, tes_test, NULL};
  static const char *st_r_cs_uniform[] = {cs_test, tex, NULL};
  static const char *st_r_uniform_block[] = {vs_uniform_block,
   gs_uniform_block,
-  fs_uniform_block, NULL};
+  fs_uniform_block,
+  fs_array_uniform_block[0],
+  fs_array_uniform_block[1],
+  fs_array_uniform_block[2],
+  fs_array_uniform_block[3],
+

Re: [Piglit] [PATCH] framework/run.py: allow additional excluded tests on resume

2015-05-20 Thread Mason, Michael W 
OK, I'm abandoning this patch in favor of this one: 

[Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on 
resume

Mike

 -Original Message-
 From: Dylan Baker [mailto:baker.dyla...@gmail.com]
 Sent: Tuesday, May 19, 2015 3:05 PM
 To: Mason, Michael W
 Cc: Daniel Vetter; piglit@lists.freedesktop.org
 Subject: Re: [Piglit] [PATCH] framework/run.py: allow additional excluded 
 tests on resume
 
 On Tue, May 19, 2015 at 04:45:59PM +, Mason, Michael W wrote:
  How about something like this instead of my original patch? I think we
  should preserve the fact that some tests didn't complete. I'll submit
  this in a separate email if you all agree.
 
  --- a/framework/programs/run.py
  +++ b/framework/programs/run.py
  @@ -313,6 +313,9 @@ def resume(input_):
   type=argparse.FileType(r),
   help=Optionally specify a piglit config file to 
  use. 
Default is piglit.conf)
  +parser.add_argument(-n, --no-retry, dest='no_retry',
 
 Drop dest down to a new line like the others please.
 
  +action='store_true',
  +help=Do not retry incomplete tests)
   args = parser.parse_args(input_)
   _disable_windows_exception_messages()
 
  @@ -342,7 +345,7 @@ def resume(input_):
   # Don't re-run tests that have already completed, incomplete status 
  tests
   # have obviously not completed.
   for name, result in results.tests.iteritems():
  -if result['result'] != 'incomplete':
  +if result['result'] != 'incomplete' or args.no_retry:
 
 It really doesn't matter but, should we check args.no_retry first? Since 
 python is lazy I think it will be optimal in most cases.
 
   opts.exclude_tests.add(name)
 
 This looks reasonable to me. I left a couple of nits above.
 
 Dylan
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on resume

2015-05-20 Thread Mike Mason 
This patch adds an option to not retry incomplete tests when resuming
a test run. This is especially useful when a failing test causes
a crash or reboot. Currently, that same test runs again when
attempting to resume the test run, resulting in the same crash or
reboot.

Signed-off-by: Mike Mason michael.w.ma...@intel.com
---
 framework/programs/run.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index 6053074..c6d7afe 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -313,6 +313,10 @@ def resume(input_):
 type=argparse.FileType(r),
 help=Optionally specify a piglit config file to use. 
  Default is piglit.conf)
+parser.add_argument(-n, --no-retry,
+dest=no_retry,
+action=store_true,
+help=Do not retry incomplete tests)
 args = parser.parse_args(input_)
 _disable_windows_exception_messages()
 
@@ -342,7 +346,7 @@ def resume(input_):
 # Don't re-run tests that have already completed, incomplete status tests
 # have obviously not completed.
 for name, result in results.tests.iteritems():
-if result['result'] != 'incomplete':
+if args.no_retry or result['result'] != 'incomplete':
 opts.exclude_tests.add(name)
 
 profile = framework.profile.merge_test_profiles(results.options['profile'])
-- 
1.9.1

___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on resume

2015-05-20 Thread Mason, Michael W 
 -Original Message-
 From: Dylan Baker [mailto:baker.dyla...@gmail.com]
 Sent: Wednesday, May 20, 2015 5:27 PM
 To: Mason, Michael W
 Cc: piglit@lists.freedesktop.org
 Subject: Re: [Piglit] [PATCH] framework/run.py: add option to not retry 
 incomplete tests on resume
 
 Looks good to me.
 
 Reviewed-by: Dylan Baker baker.dyla...@gmail.com
 
 btw, do you have push access?

I don't think so. I've never tried.

 
 On Wed, May 20, 2015 at 02:17:04PM -0700, Mike Mason wrote:
  This patch adds an option to not retry incomplete tests when resuming
  a test run. This is especially useful when a failing test causes a
  crash or reboot. Currently, that same test runs again when attempting
  to resume the test run, resulting in the same crash or reboot.
 
  Signed-off-by: Mike Mason michael.w.ma...@intel.com
  ---
   framework/programs/run.py | 6 +-
   1 file changed, 5 insertions(+), 1 deletion(-)
 
  diff --git a/framework/programs/run.py b/framework/programs/run.py
  index 6053074..c6d7afe 100644
  --- a/framework/programs/run.py
  +++ b/framework/programs/run.py
  @@ -313,6 +313,10 @@ def resume(input_):
   type=argparse.FileType(r),
   help=Optionally specify a piglit config file to 
  use. 
Default is piglit.conf)
  +parser.add_argument(-n, --no-retry,
  +dest=no_retry,
  +action=store_true,
  +help=Do not retry incomplete tests)
   args = parser.parse_args(input_)
   _disable_windows_exception_messages()
 
  @@ -342,7 +346,7 @@ def resume(input_):
   # Don't re-run tests that have already completed, incomplete status 
  tests
   # have obviously not completed.
   for name, result in results.tests.iteritems():
  -if result['result'] != 'incomplete':
  +if args.no_retry or result['result'] != 'incomplete':
   opts.exclude_tests.add(name)
 
   profile =
  framework.profile.merge_test_profiles(results.options['profile'])
  --
  1.9.1
 
  ___
  Piglit mailing list
  Piglit@lists.freedesktop.org
  http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on resume

2015-05-20 Thread Dylan Baker 
Looks good to me.

Reviewed-by: Dylan Baker baker.dyla...@gmail.com

btw, do you have push access?

On Wed, May 20, 2015 at 02:17:04PM -0700, Mike Mason wrote:
 This patch adds an option to not retry incomplete tests when resuming
 a test run. This is especially useful when a failing test causes
 a crash or reboot. Currently, that same test runs again when
 attempting to resume the test run, resulting in the same crash or
 reboot.
 
 Signed-off-by: Mike Mason michael.w.ma...@intel.com
 ---
  framework/programs/run.py | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/framework/programs/run.py b/framework/programs/run.py
 index 6053074..c6d7afe 100644
 --- a/framework/programs/run.py
 +++ b/framework/programs/run.py
 @@ -313,6 +313,10 @@ def resume(input_):
  type=argparse.FileType(r),
  help=Optionally specify a piglit config file to 
 use. 
   Default is piglit.conf)
 +parser.add_argument(-n, --no-retry,
 +dest=no_retry,
 +action=store_true,
 +help=Do not retry incomplete tests)
  args = parser.parse_args(input_)
  _disable_windows_exception_messages()
  
 @@ -342,7 +346,7 @@ def resume(input_):
  # Don't re-run tests that have already completed, incomplete status tests
  # have obviously not completed.
  for name, result in results.tests.iteritems():
 -if result['result'] != 'incomplete':
 +if args.no_retry or result['result'] != 'incomplete':
  opts.exclude_tests.add(name)
  
  profile = 
 framework.profile.merge_test_profiles(results.options['profile'])
 -- 
 1.9.1
 
 ___
 Piglit mailing list
 Piglit@lists.freedesktop.org
 http://lists.freedesktop.org/mailman/listinfo/piglit


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH v2] arb_direct_state_access: Add a FramebufferTextureLayer cube-map test

2015-05-20 Thread Ian Romanick 
On 04/29/2015 12:21 PM, Fredrik Höglund wrote:
 This test verifies that faces of a cube-map texture can be attached
 to a framebuffer object using glNamedFramebufferTextureLayer and
 glFramebufferTextureLayer.
 ---
 
 v2: - Remove the check for ARB_vertex_array_object (core in 3.0)
 - Fix the executable name in all.py
 
  tests/all.py   |   4 +
  .../spec/arb_direct_state_access/CMakeLists.gl.txt |   1 +
  .../fbo-texturelayer-cubemap.c | 321 
 +
  3 files changed, 326 insertions(+)
  create mode 100644 
 tests/spec/arb_direct_state_access/fbo-texturelayer-cubemap.c
 
 diff --git a/tests/all.py b/tests/all.py
 index 6197c18..efecc09 100755
 --- a/tests/all.py
 +++ b/tests/all.py
 @@ -4305,6 +4305,10 @@ with profile.group_manager(
  g(['arb_direct_state_access-create-programpipelines'],
'create-programpipelines')
  g(['arb_direct_state_access-create-queries'], 'create-queries')
 +g(['arb_direct_state_access-fbo-texturelayer-cubemap', 
 'glNamedFramebufferTextureLayer'],
 +  'fbo-texturelayer-cubemap glNamedFramebufferTextureLayer')
 +g(['arb_direct_state_access-fbo-texturelayer-cubemap', 
 'glFramebufferTextureLayer'],
 +  'fbo-texturelayer-cubemap glFramebufferTextureLayer')
  
  with profile.group_manager(
  PiglitGLTest,
 diff --git a/tests/spec/arb_direct_state_access/CMakeLists.gl.txt 
 b/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
 index a5ae423..1b837f9 100644
 --- a/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
 +++ b/tests/spec/arb_direct_state_access/CMakeLists.gl.txt
 @@ -36,4 +36,5 @@ piglit_add_executable 
 (arb_direct_state_access-texture-buffer texture-buffer.c)
  piglit_add_executable (arb_direct_state_access-create-samplers 
 create-samplers.c)
  piglit_add_executable (arb_direct_state_access-create-programpipelines 
 create-programpipelines.c)
  piglit_add_executable (arb_direct_state_access-create-queries 
 create-queries.c)
 +piglit_add_executable (arb_direct_state_access-fbo-texturelayer-cubemap 
 fbo-texturelayer-cubemap.c)
  # vim: ft=cmake:
 diff --git a/tests/spec/arb_direct_state_access/fbo-texturelayer-cubemap.c 
 b/tests/spec/arb_direct_state_access/fbo-texturelayer-cubemap.c
 new file mode 100644
 index 000..e3e9ac3
 --- /dev/null
 +++ b/tests/spec/arb_direct_state_access/fbo-texturelayer-cubemap.c
 @@ -0,0 +1,321 @@
 +/*
 + * Copyright (C) 2015 Fredrik Höglund
 + *
 + * 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
 + * on the rights to use, copy, modify, merge, publish, distribute, sub
 + * license, 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 NON-INFRINGEMENT.  IN NO EVENT SHALL
 + * THE AUTHORS AND/OR THEIR SUPPLIERS 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 fbo-texturelayer-cubemap.c
 + *
 + * Verifies that faces of a cube-map texture can be attached to a
 + * framebuffer object using NamedFramebufferTextureLayer and
 + * FramebufferTextureLayer.
 + */
 +
 +#include piglit-util-gl.h
 +
 +
 +PIGLIT_GL_TEST_CONFIG_BEGIN
 +
 + config.supports_gl_core_version = 31;
 +
 + config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
 +
 +PIGLIT_GL_TEST_CONFIG_END
 +
 +
 +const char *vs =
 +#version 140\n
 +#extension GL_ARB_draw_instanced : require\n
 +in vec4 piglit_vertex;\n
 +in vec3 piglit_texcoord;\n
 +out vec3 texcoord;\n
 +void main(void)\n{\n
 +gl_Position = piglit_vertex;\n
 +gl_Position.xy +=\n
 +vec2(gl_InstanceID) * vec2(1.0 / 3.0, -1.0 / 3.0);\n
 +texcoord = piglit_texcoord;\n
 +};
 +
 +const char *fs =
 +#version 140\n
 +in vec3 texcoord;\n
 +out vec4 fragColor;\n
 +uniform samplerCube sampler;\n
 +void main(void)\n{\n
 +fragColor = texture(sampler, texcoord);\n
 +};
 +
 +static bool test_named_fbo = false;
 +
 +
 +static bool
 +check_attachment_parameter(GLuint fbo, GLenum attachment,
 +   GLenum pname, GLint expected, int line)
 +{
 + GLint value;
 + glGetNamedFramebufferAttachmentParameteriv(fbo, attachment,
 +

Re: [Piglit] [PATCH] framework/run.py: add option to not retry incomplete tests on resume

2015-05-20 Thread Dylan Baker 
On Thu, May 21, 2015 at 12:31:40AM +, Mason, Michael W wrote:
  -Original Message-
  From: Dylan Baker [mailto:baker.dyla...@gmail.com]
  Sent: Wednesday, May 20, 2015 5:27 PM
  To: Mason, Michael W
  Cc: piglit@lists.freedesktop.org
  Subject: Re: [Piglit] [PATCH] framework/run.py: add option to not retry 
  incomplete tests on resume
  
  Looks good to me.
  
  Reviewed-by: Dylan Baker baker.dyla...@gmail.com
  
  btw, do you have push access?
 
 I don't think so. I've never tried.

If you don't know the answer is probably no. I'll push this tomorrow
unless there are objections.

Dylan


signature.asc
Description: Digital signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit