Re: [Piglit] [PATCH 1/5] arb_post_depth_coverage-basic: Add a basic initial test.

2016-11-09 Thread Ilia Mirkin 
On Wed, Nov 9, 2016 at 6:39 PM, Plamena Manolova
 wrote:
> A basic test to check whether the values written to gl_SampleMaskIn
> are still correct after enabling the ARB_post_depth_coverage
> extension.
>
> Signed-off-by: Plamena Manolova 
> ---
>  registry/gl.xml|   2 +
>  tests/all.py   |   5 +
>  tests/spec/CMakeLists.txt  |   1 +
>  .../spec/arb_post_depth_coverage/CMakeLists.gl.txt |  11 ++
>  tests/spec/arb_post_depth_coverage/CMakeLists.txt  |   1 +
>  tests/spec/arb_post_depth_coverage/basic.c | 165 
> +
>  6 files changed, 185 insertions(+)
>  create mode 100644 tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
>  create mode 100644 tests/spec/arb_post_depth_coverage/CMakeLists.txt
>  create mode 100644 tests/spec/arb_post_depth_coverage/basic.c
>
> diff --git a/registry/gl.xml b/registry/gl.xml
> index 3fc8863..59894e8 100644
> --- a/registry/gl.xml
> +++ b/registry/gl.xml

This file comes from Khronos and should be synchronized accordingly.
It appears that there's already a line



in there. Not sure why you needed your change in the first place, this
only matters for dispatch, of which there is none for this ext.

> @@ -38345,6 +38345,8 @@ typedef unsigned int GLhandleARB;
>  
>  
>  
> +
> +
>  
>  
>  
> diff --git a/tests/all.py b/tests/all.py
> index e56cae8..d47187e 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -4766,6 +4766,11 @@ with profile.group_manager(
>  g(['arb_shader_image_load_store-unused'], 'unused')
>
>  with profile.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', 'arb_post_depth_coverage')) as g:
> +g(['arb_post_depth_coverage-basic'])
> +
> +with profile.group_manager(
>  PiglitGLTest,
>  grouptools.join('spec', 'arb_shader_image_size')) as g:
>  g(['arb_shader_image_size-builtin'], 'builtin')
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index a057c02..4377196 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -152,3 +152,4 @@ add_subdirectory (arb_query_buffer_object)
>  add_subdirectory (ext_window_rectangles)
>  add_subdirectory (arb_shader_texture_image_samples)
>  add_subdirectory (arb_texture_barrier)
> +add_subdirectory (arb_post_depth_coverage)
> diff --git a/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt 
> b/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
> new file mode 100644
> index 000..1a71774
> --- /dev/null
> +++ b/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
> @@ -0,0 +1,11 @@
> +include_directories(
> +   ${GLEXT_INCLUDE_DIR}
> +   ${OPENGL_INCLUDE_PATH}
> +)
> +
> +link_libraries (
> +   piglitutil_${piglit_target_api}
> +   ${OPENGL_gl_LIBRARY}
> +)
> +
> +piglit_add_executable (arb_post_depth_coverage-basic basic.c)
> diff --git a/tests/spec/arb_post_depth_coverage/CMakeLists.txt 
> b/tests/spec/arb_post_depth_coverage/CMakeLists.txt
> new file mode 100644
> index 000..144a306
> --- /dev/null
> +++ b/tests/spec/arb_post_depth_coverage/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/arb_post_depth_coverage/basic.c 
> b/tests/spec/arb_post_depth_coverage/basic.c
> new file mode 100644
> index 000..e1bd71f
> --- /dev/null
> +++ b/tests/spec/arb_post_depth_coverage/basic.c
> @@ -0,0 +1,165 @@
> +/*
> + * Copyright (c) 2015 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.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +   config.supports_gl_compat_version = 45;
> +   config.supports_gl_core_version = 45;
> +   config.window_width = 160;
> +   config.window_height = 160;
> +   conf

[Piglit] [PATCH 1/5] arb_post_depth_coverage-basic: Add a basic initial test.

2016-11-09 Thread Plamena Manolova 
A basic test to check whether the values written to gl_SampleMaskIn
are still correct after enabling the ARB_post_depth_coverage
extension.

Signed-off-by: Plamena Manolova 
---
 registry/gl.xml|   2 +
 tests/all.py   |   5 +
 tests/spec/CMakeLists.txt  |   1 +
 .../spec/arb_post_depth_coverage/CMakeLists.gl.txt |  11 ++
 tests/spec/arb_post_depth_coverage/CMakeLists.txt  |   1 +
 tests/spec/arb_post_depth_coverage/basic.c | 165 +
 6 files changed, 185 insertions(+)
 create mode 100644 tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
 create mode 100644 tests/spec/arb_post_depth_coverage/CMakeLists.txt
 create mode 100644 tests/spec/arb_post_depth_coverage/basic.c

diff --git a/registry/gl.xml b/registry/gl.xml
index 3fc8863..59894e8 100644
--- a/registry/gl.xml
+++ b/registry/gl.xml
@@ -38345,6 +38345,8 @@ typedef unsigned int GLhandleARB;
 
 
 
+
+
 
 
 
diff --git a/tests/all.py b/tests/all.py
index e56cae8..d47187e 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4766,6 +4766,11 @@ with profile.group_manager(
 g(['arb_shader_image_load_store-unused'], 'unused')
 
 with profile.group_manager(
+PiglitGLTest,
+grouptools.join('spec', 'arb_post_depth_coverage')) as g:
+g(['arb_post_depth_coverage-basic'])
+
+with profile.group_manager(
 PiglitGLTest,
 grouptools.join('spec', 'arb_shader_image_size')) as g:
 g(['arb_shader_image_size-builtin'], 'builtin')
diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
index a057c02..4377196 100644
--- a/tests/spec/CMakeLists.txt
+++ b/tests/spec/CMakeLists.txt
@@ -152,3 +152,4 @@ add_subdirectory (arb_query_buffer_object)
 add_subdirectory (ext_window_rectangles)
 add_subdirectory (arb_shader_texture_image_samples)
 add_subdirectory (arb_texture_barrier)
+add_subdirectory (arb_post_depth_coverage)
diff --git a/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt 
b/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
new file mode 100644
index 000..1a71774
--- /dev/null
+++ b/tests/spec/arb_post_depth_coverage/CMakeLists.gl.txt
@@ -0,0 +1,11 @@
+include_directories(
+   ${GLEXT_INCLUDE_DIR}
+   ${OPENGL_INCLUDE_PATH}
+)
+
+link_libraries (
+   piglitutil_${piglit_target_api}
+   ${OPENGL_gl_LIBRARY}
+)
+
+piglit_add_executable (arb_post_depth_coverage-basic basic.c)
diff --git a/tests/spec/arb_post_depth_coverage/CMakeLists.txt 
b/tests/spec/arb_post_depth_coverage/CMakeLists.txt
new file mode 100644
index 000..144a306
--- /dev/null
+++ b/tests/spec/arb_post_depth_coverage/CMakeLists.txt
@@ -0,0 +1 @@
+piglit_include_target_api()
diff --git a/tests/spec/arb_post_depth_coverage/basic.c 
b/tests/spec/arb_post_depth_coverage/basic.c
new file mode 100644
index 000..e1bd71f
--- /dev/null
+++ b/tests/spec/arb_post_depth_coverage/basic.c
@@ -0,0 +1,165 @@
+/*
+ * Copyright (c) 2015 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.
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+   config.supports_gl_compat_version = 45;
+   config.supports_gl_core_version = 45;
+   config.window_width = 160;
+   config.window_height = 160;
+   config.window_visual = PIGLIT_GL_VISUAL_DEPTH | PIGLIT_GL_VISUAL_RGBA;
+PIGLIT_GL_TEST_CONFIG_END
+
+static GLuint prog, vao, ssbo;
+static GLint *sample_mask;
+
+static GLuint
+make_shader_program(void)
+{
+   static const char *vs_text =
+   "#version 430\n"
+   "in vec4 pos_in;\n"
+   "void main()\n"
+   "{\n"
+   "   gl_Position = pos_in;\n"
+   "}\n";
+
+   static const char *fs_text =
+   "#version 430\n"
+   "out vec4 color;\n"
+

Re: [Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

2016-11-09 Thread Marek Olšák 
On Wed, Nov 9, 2016 at 11:41 PM, Francisco Jerez  wrote:
> @@ -3622,7 +3636,6 @@ piglit_init(int argc, char **argv)
> sso_in_use = false;
> prog_err_info = NULL;
> vao = 0;
> -   fbo = 0;

 This breaks some piglit tests with --process-isolation 0. Adding
 "draw_fbo = 0" here fixes that. Is that the right fix?

>>>
>>> Interesting, I guess the test breaks while trying to deallocate the FBO
>>> From the previous test?  Or does it break in some other way?  In the
>>> former case I guess that setting 'draw_fbo = 0' would work around the
>>> issue, but it would probably also lead to FBO object leaks.
>>
>> No idea what's causing the failures, but releasing the FBO should be
>> really easy at the end of that block.
>>
>
> Does the attached patch fix the problem for you?  I didn't get a full
> piglit run to work with --process-isolation 0, so I couldn't confirm
> whether it fixes the issue.

Yes, it fixes the issue. Thanks.

Reviewed-by: Marek Olšák 

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

Re: [Piglit] [PATCH] tests/quick.py: reduce the number of vs_in tests in quick.py

2016-11-09 Thread Dylan Baker 
Bump.

Anyone interested in this?

Dylan

Quoting Dylan Baker (2016-10-31 14:40:35)
> There are 18000 of these tests, quick.py has 53000 tests, which is 34%
> of the total tests. That's just too many for quick.py.
> 
> This patch takes the somewhat naive approach of just filtering out 80%
> of those tests from quick.py, which reduces the total number of tests to
> 38000, which makes a pretty big dent in the amount of time it takes to
> run quick.py. The approach is deterministic, but random.
> 
> cc: Andres Gomez 
> Signed-off-by: 
> ---
>  tests/quick.py | 27 +++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tests/quick.py b/tests/quick.py
> index 0e02f92..5f7250f 100644
> --- a/tests/quick.py
> +++ b/tests/quick.py
> @@ -1,8 +1,19 @@
>  # -*- coding: utf-8 -*-
>  
> +"""A quicker profile than all.
> +
> +This profile filters out a number of very slow tests, and tests that are very
> +exhaustively tested, since they add a good deal of runtime to piglit.
> +
> +There are 18000+ auto-generated tests that are exhaustive, but for quick.py 
> we
> +don't want that level of exhaustiveness, so this filter removes 80% in a 
> random
> +(but deterministic) way.
> +"""
> +
>  from __future__ import (
>  absolute_import, division, print_function, unicode_literals
>  )
> +import random
>  
>  from framework import grouptools
>  from framework.test import (GleanTest, PiglitGLTest)
> @@ -13,6 +24,21 @@ __all__ = ['profile']
>  # See the note in all.py about this warning
>  # pylint: disable=bad-continuation
>  
> +
> +class FilterVsIn(object):
> +"""Filter out 80% of the Vertex Attrib 64 vs_in tests."""
> +
> +def __init__(self):
> +self.random = random.Random()
> +self.random.seed(42)
> +
> +def __call__(self, name, _):
> +if 'vs_in' in name:
> +# 20%
> +return self.random.random() <= .2
> +return True
> +
> +
>  GleanTest.GLOBAL_PARAMS += ["--quick"]
>  
>  # Set the --quick flag on a few image_load_store_tests
> @@ -37,3 +63,4 @@ with profile.group_manager(
>  
>  # These take too long
>  profile.filter_tests(lambda n, _: '-explosion' not in n)
> +profile.filter_tests(FilterVsIn())
> -- 
> 2.10.2
> 


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

Re: [Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

2016-11-09 Thread Francisco Jerez 
Marek Olšák  writes:

> On Wed, Nov 9, 2016 at 8:10 PM, Francisco Jerez  wrote:
>> Marek Olšák  writes:
>>
>>> On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez  
>>> wrote:
 This refactors the implementation of the various "fb" commands to be
 part of a single 'if (parse_str(line, "fb ", ...)) {}' block in order
 to make code-sharing easier among fb subcommands.  Will be more useful
 when we start introducing additional fb subcommands.
 ---
  tests/shaders/shader_runner.c | 67 
 ++-
  1 file changed, 40 insertions(+), 27 deletions(-)

 diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
 index ab2b907..4a2c807 100644
 --- a/tests/shaders/shader_runner.c
 +++ b/tests/shaders/shader_runner.c
 @@ -140,7 +140,7 @@ static bool prog_in_use = false;
  static bool sso_in_use = false;
  static GLchar *prog_err_info = NULL;
  static GLuint vao = 0;
 -static GLuint fbo = 0;
 +static GLuint draw_fbo = 0;
  static GLint render_width, render_height;

  static bool report_subtests = false;
 @@ -2959,13 +2959,16 @@ piglit_display(void)
 do_enable_disable(rest, true);
 } else if (sscanf(line, "depthfunc %31s", s) == 1) {
 glDepthFunc(piglit_get_gl_enum_from_name(s));
 -   } else if (sscanf(line, "fb tex 2d %d", &tex) == 1) {
 -   GLenum status;
 +   } else if (parse_str(line, "fb ", &rest)) {
 +   GLuint fbo = 0;
>>>
>>> Wrong indentation. (it can lead to buggy code)
>>>
>>
>> Yeah, the whole block is intentionally indented one tab to the left,
>> it's cleaned up in the next commit.
>>

 -   if (fbo == 0) {
 -   glGenFramebuffers(1, &fbo);
 -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 -   }
 +   if (parse_str(rest, "tex 2d ", &rest)) {
 +   glGenFramebuffers(1, &fbo);
 +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 +
 +   REQUIRE(parse_int(rest, &tex, &rest),
 +   "Framebuffer binding command not "
 +   "understood at: %s\n", rest);

 glFramebufferTexture2D(GL_FRAMEBUFFER,
GL_COLOR_ATTACHMENT0,
 @@ -2976,21 +2979,12 @@ piglit_display(void)
 piglit_report_result(PIGLIT_FAIL);
 }

 -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
 -   if (status != GL_FRAMEBUFFER_COMPLETE) {
 -   fprintf(stderr, "incomplete fbo (status 
 0x%x)\n", status);
 -   piglit_report_result(PIGLIT_FAIL);
 -   }
 -
 -   render_width = get_texture_binding(tex)->width;
 -   render_height = get_texture_binding(tex)->height;
 -   } else if (sscanf(line, "fb tex layered %d", &tex) == 1) {
 -   GLenum status;
 +   w = get_texture_binding(tex)->width;
 +   h = get_texture_binding(tex)->height;

 -   if (fbo == 0) {
 -   glGenFramebuffers(1, &fbo);
 -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
 -   }
 +   } else if (sscanf(rest, "tex layered %d", &tex) == 1) {
 +   glGenFramebuffers(1, &fbo);
 +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);

 glFramebufferTexture(GL_FRAMEBUFFER,
  GL_COLOR_ATTACHMENT0,
 @@ -3000,11 +2994,31 @@ piglit_display(void)
 piglit_report_result(PIGLIT_FAIL);
 }

 -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
 -   if (status != GL_FRAMEBUFFER_COMPLETE) {
 -   fprintf(stderr, "incomplete fbo (status 
 0x%x)\n", status);
 -   piglit_report_result(PIGLIT_FAIL);
 -   }
 +   w = get_texture_binding(tex)->width;
 +   h = get_texture_binding(tex)->height;
 +
 +   } else {
 +   fprintf(stderr, "Unknown fb bind subcommand "
 +   "\"%s\"\n", rest);
 +   piglit_report_result(PIGLIT_FAIL);
 +   }
 +
 +

Re: [Piglit] [PATCH] draw-vertices-half-float: Add coverage for GL_OES_vertex_half_float

2016-11-09 Thread Kenneth Graunke 
On Monday, October 10, 2016 2:25:02 PM PST Kevin Strasser wrote:
> Add ES2 implementation to the test to provide coverage for
> GL_OES_vertex_half_float extension. The tests will continue to pass on mesa
> for desktop GL as well as ES2 once supported is added.
> 
> Signed-off-by: Kevin Strasser 

Pushed with Tapani's review.  Thanks!


signature.asc
Description: This is a digitally signed message part.
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

2016-11-09 Thread Marek Olšák 
On Wed, Nov 9, 2016 at 8:10 PM, Francisco Jerez  wrote:
> Marek Olšák  writes:
>
>> On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez  
>> wrote:
>>> This refactors the implementation of the various "fb" commands to be
>>> part of a single 'if (parse_str(line, "fb ", ...)) {}' block in order
>>> to make code-sharing easier among fb subcommands.  Will be more useful
>>> when we start introducing additional fb subcommands.
>>> ---
>>>  tests/shaders/shader_runner.c | 67 
>>> ++-
>>>  1 file changed, 40 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>>> index ab2b907..4a2c807 100644
>>> --- a/tests/shaders/shader_runner.c
>>> +++ b/tests/shaders/shader_runner.c
>>> @@ -140,7 +140,7 @@ static bool prog_in_use = false;
>>>  static bool sso_in_use = false;
>>>  static GLchar *prog_err_info = NULL;
>>>  static GLuint vao = 0;
>>> -static GLuint fbo = 0;
>>> +static GLuint draw_fbo = 0;
>>>  static GLint render_width, render_height;
>>>
>>>  static bool report_subtests = false;
>>> @@ -2959,13 +2959,16 @@ piglit_display(void)
>>> do_enable_disable(rest, true);
>>> } else if (sscanf(line, "depthfunc %31s", s) == 1) {
>>> glDepthFunc(piglit_get_gl_enum_from_name(s));
>>> -   } else if (sscanf(line, "fb tex 2d %d", &tex) == 1) {
>>> -   GLenum status;
>>> +   } else if (parse_str(line, "fb ", &rest)) {
>>> +   GLuint fbo = 0;
>>
>> Wrong indentation. (it can lead to buggy code)
>>
>
> Yeah, the whole block is intentionally indented one tab to the left,
> it's cleaned up in the next commit.
>
>>>
>>> -   if (fbo == 0) {
>>> -   glGenFramebuffers(1, &fbo);
>>> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>>> -   }
>>> +   if (parse_str(rest, "tex 2d ", &rest)) {
>>> +   glGenFramebuffers(1, &fbo);
>>> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>>> +
>>> +   REQUIRE(parse_int(rest, &tex, &rest),
>>> +   "Framebuffer binding command not "
>>> +   "understood at: %s\n", rest);
>>>
>>> glFramebufferTexture2D(GL_FRAMEBUFFER,
>>>GL_COLOR_ATTACHMENT0,
>>> @@ -2976,21 +2979,12 @@ piglit_display(void)
>>> piglit_report_result(PIGLIT_FAIL);
>>> }
>>>
>>> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
>>> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
>>> -   fprintf(stderr, "incomplete fbo (status 
>>> 0x%x)\n", status);
>>> -   piglit_report_result(PIGLIT_FAIL);
>>> -   }
>>> -
>>> -   render_width = get_texture_binding(tex)->width;
>>> -   render_height = get_texture_binding(tex)->height;
>>> -   } else if (sscanf(line, "fb tex layered %d", &tex) == 1) {
>>> -   GLenum status;
>>> +   w = get_texture_binding(tex)->width;
>>> +   h = get_texture_binding(tex)->height;
>>>
>>> -   if (fbo == 0) {
>>> -   glGenFramebuffers(1, &fbo);
>>> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>>> -   }
>>> +   } else if (sscanf(rest, "tex layered %d", &tex) == 1) {
>>> +   glGenFramebuffers(1, &fbo);
>>> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>>>
>>> glFramebufferTexture(GL_FRAMEBUFFER,
>>>  GL_COLOR_ATTACHMENT0,
>>> @@ -3000,11 +2994,31 @@ piglit_display(void)
>>> piglit_report_result(PIGLIT_FAIL);
>>> }
>>>
>>> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
>>> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
>>> -   fprintf(stderr, "incomplete fbo (status 
>>> 0x%x)\n", status);
>>> -   piglit_report_result(PIGLIT_FAIL);
>>> -   }
>>> +   w = get_texture_binding(tex)->width;
>>> +   h = get_texture_binding(tex)->height;
>>> +
>>> +   } else {
>>> +   fprintf(stderr, "Unknown fb bind subcommand "
>>> +   "\"%s\"\n", rest);
>>> +   piglit_report_result(PIGLIT_FAIL);
>>> +   }
>>> +
>>> +   const GLenum status = 
>>> glCheckFramebufferStatus(GL_FRAMEBUFFER);
>>> +   if (status != GL_FRAMEBUFFER_COMPLETE)

Re: [Piglit] [Patch 1/1] texelFetch, textureSize: replace deprecated gl_FragColor

2016-11-09 Thread Kenneth Graunke 
On Wednesday, November 9, 2016 3:18:36 PM PST Sebastian Olender wrote:
> According to GLSLLangSpec: Use of gl_FragData and gl_FragColor
> is deprecated in version 1.3, using user-defined out instead.
> ---
>  tests/texturing/shaders/texelFetch.c  | 9 ++---
>  tests/texturing/shaders/textureSize.c | 9 ++---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/texturing/shaders/texelFetch.c 
> b/tests/texturing/shaders/texelFetch.c
> index 0291a12..2e1d223 100644
> --- a/tests/texturing/shaders/texelFetch.c
> +++ b/tests/texturing/shaders/texelFetch.c
> @@ -627,9 +627,10 @@ generate_GLSL(enum shader_target test_stage)
>"#version %d\n"
>"flat in %s color;\n"
>"uniform vec4 divisor;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
> -  "gl_FragColor = vec4(color)/divisor;\n"
> +  "fragColor = vec4(color)/divisor;\n"
>"}\n",
>shader_version,
>sampler.return_type);
> @@ -676,9 +677,10 @@ generate_GLSL(enum shader_target test_stage)
>"#version %d\n"
>"flat in %s color;\n"
>"uniform vec4 divisor;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
> -  "gl_FragColor = vec4(color)/divisor;\n"
> +  "fragColor = vec4(color)/divisor;\n"
>"}\n",
>shader_version,
>sampler.return_type);
> @@ -703,10 +705,11 @@ generate_GLSL(enum shader_target test_stage)
>"flat in ivec4 tc;\n"
>"uniform vec4 divisor;\n"
>"uniform %s tex;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
>"vec4 color = texelFetch%s(tex, ivec%d(tc)%s%s);\n"
> -  "gl_FragColor = color/divisor;\n"
> +  "fragColor = color/divisor;\n"
>"}\n",
>shader_version,
>has_samples() ? "#extension 
> GL_ARB_texture_multisample: require" : "",
> diff --git a/tests/texturing/shaders/textureSize.c 
> b/tests/texturing/shaders/textureSize.c
> index c02c566..ac3df04 100644
> --- a/tests/texturing/shaders/textureSize.c
> +++ b/tests/texturing/shaders/textureSize.c
> @@ -284,9 +284,10 @@ generate_GLSL(enum shader_target test_stage)
>"#define ivec1 int\n"
>"#define vec1 float\n"
>"flat in ivec%d size;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
> -  "gl_FragColor = vec4(0.01 * size,%s 1);\n"
> +  "fragColor = vec4(0.01 * size,%s 1);\n"
>"}\n",
>shader_version, size, zeroes[3 - size]);
>   break;
> @@ -325,9 +326,10 @@ generate_GLSL(enum shader_target test_stage)
>"#define ivec1 int\n"
>"#define vec1 float\n"
>"flat in ivec%d size;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
> -  "gl_FragColor = vec4(0.01 * size,%s 1);\n"
> +  "fragColor = vec4(0.01 * size,%s 1);\n"
>"}\n",
>shader_version, size, zeroes[3 - size]);
>   break;
> @@ -345,10 +347,11 @@ generate_GLSL(enum shader_target test_stage)
>"#define ivec1 int\n"
>"uniform int lod;\n"
>"uniform %s tex;\n"
> +  "out vec4 fragColor;\n"
>"void main()\n"
>"{\n"
>"ivec%d size = textureSize(tex%s);\n"
> -  "gl_FragColor = vec4(0.01 * size,%s 1);\n"
> +  "fragColor = vec4(0.01 * size,%s 1);\n"
>"}\n",
>shader_version, extension, sampler.name, size, lod_arg,
>zeroes[3 - size]);
> 

I don't have a problem with this patch, but out of curiosity, is there
some reason you need to make this change?  gl_FragColor may be deprecated 
in 1.30 but it didn't actually go away until 4.20.

--Ken


signature.asc
Description: This is a digitally signed message part.
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 24/27] framework/profile: Move group_manager from TestProfile to TestDict

2016-11-09 Thread Dylan Baker 
This move is going to allow us to supplement the TestDict with a
different class that can be used instead that loads xml.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 104 ++
 tests/all.py| 508 ++---
 tests/cl.py |   8 +-
 tests/quick.py  |   8 +-
 tests/sanity.py |   2 +-
 unittests/framework/test_profile.py | 153 -
 6 files changed, 385 insertions(+), 398 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 7e09081..23abc6d 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -170,52 +170,6 @@ class TestDict(collections.MutableMapping):
 def __iter__(self):
 return iter(self.__container)
 
-@property
-@contextlib.contextmanager
-def allow_reassignment(self):
-"""Context manager that allows keys to be reassigned.
-
-Normally reassignment happens in error, but sometimes one actually
-wants to do reassignment, say to add extra options in a reduced
-profile. This method allows reassignment, but only within its context,
-making it an explicit choice to do so.
-
-It is safe to nest this contextmanager.
-
-This is not thread safe, or even co-routine safe.
-"""
-self.__allow_reassignment += 1
-yield
-self.__allow_reassignment -= 1
-
-
-class TestProfile(object):
-"""Class that holds a list of tests for execution.
-
-This class represents a single testsuite, it has a mapping (dictionary-like
-object) of tests attached (TestDict). This is a mapping of :
-(python 3 str, python 2 unicode), and the key is delimited by
-grouptools.SEPARATOR.
-
-The group_manager method provides a context_manager to make adding test to
-the test_list easier, by doing more validation and enforcement.
->>> t = TestProfile()
->>> with t.group_manager(Test, 'foo@bar') as g:
-... g(['foo'])
-
-This class does not provide a way to execute itself, instead that is
-handled by the run function in this module, which is able to process and
-run multiple TestProfile objects at once.
-"""
-def __init__(self):
-self.test_list = TestDict()
-self.forced_test_list = []
-self.filters = []
-self.options = {
-'dmesg': get_dmesg(False),
-'monitor': Monitoring(False),
-}
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, **default_args):
 """A context manager to make working with flat groups simple.
@@ -255,16 +209,15 @@ class TestProfile(object):
 """Helper function that actually adds the tests.
 
 Arguments:
-args -- arguments to be passed to the test_class constructor.
-This must be appropriate for the underlying class
+args   -- arguments to be passed to the test_class constructor.
+  This must be appropriate for the underlying class
 
 Keyword Arguments:
-name -- If this is a a truthy value that value will be used as the
-key for the test. If name is falsy then args will be
-' '.join'd and used as name. Default: None
+name   -- If this is a a truthy value that value will be used as
+  the key for the test. If name is falsy then args will be
+  ' '.join'd and used as name. Default: None
 kwargs -- Any additional args will be passed directly to the test
   constructor as keyword args.
-
 """
 # If there is no name, then either
 # a) join the arguments list together to make the name
@@ -281,7 +234,7 @@ class TestProfile(object):
 assert isinstance(name, six.string_types)
 lgroup = grouptools.join(group, name)
 
-self.test_list[lgroup] = test_class(
+self[lgroup] = test_class(
 args,
 **dict(itertools.chain(six.iteritems(default_args),
six.iteritems(kwargs
@@ -291,9 +244,48 @@ class TestProfile(object):
 @property
 @contextlib.contextmanager
 def allow_reassignment(self):
-"""A convenience wrapper around self.test_list.allow_reassignment."""
-with self.test_list.allow_reassignment:
-yield
+"""Context manager that allows keys to be reassigned.
+
+Normally reassignment happens in error, but sometimes one actually
+wants to do reassignment, say to add extra options in a reduced
+profile. This method allows reassignment, but only within its context,
+making it an explicit choice to do so.
+
+It is safe to nest this contextmanager.
+
+This is not thread safe, or even co-routine safe.
+"""
+self.__allow_reass

[Piglit] [PATCH v4 21/27] framework/profile: Split try/except block

2016-11-09 Thread Dylan Baker 
This will avoid catching AttributeErrors when importing, and only catch
them if mod.profile doesn't exist.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 042cf98..8449e7a 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -388,15 +388,18 @@ def load_test_profile(filename):
 try:
 mod = importlib.import_module('tests.{0}'.format(
 os.path.splitext(os.path.basename(filename))[0]))
+except ImportError:
+raise exceptions.PiglitFatalError(
+'Failed to import "{}", there is either something wrong with the '
+'module or it doesn\'t exist. Check your spelling?'.format(
+filename))
+
+try:
 return mod.profile
 except AttributeError:
 raise exceptions.PiglitFatalError(
-'There is not profile attribute in module {}.\n'
+'There is no "profile" attribute in module {}.\n'
 'Did you specify the right file?'.format(filename))
-except ImportError:
-raise exceptions.PiglitFatalError(
-'There is no test profile called "{}".\n'
-'Check your spelling?'.format(filename))
 
 
 def run(profiles, logger, backend, concurrency):
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 3/27] framework/profile: make run hooks public

2016-11-09 Thread Dylan Baker 
These are used in a couple of places, and since they're meant to be
changed by subclasses they need to be public.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 28 
 tests/igt.py |  3 ++-
 tests/xts.py |  8 
 3 files changed, 14 insertions(+), 25 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 7b0cb07..0d2e1cf 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -287,22 +287,6 @@ class TestProfile(object):
 raise exceptions.PiglitFatalError(
 'There are no tests scheduled to run. Aborting run.')
 
-def _pre_run_hook(self):
-""" Hook executed at the start of TestProfile.run
-
-To make use of this hook one will need to subclass TestProfile, and
-set this to do something, as be default it will no-op
-"""
-pass
-
-def _post_run_hook(self):
-""" Hook executed at the end of TestProfile.run
-
-To make use of this hook one will need to subclass TestProfile, and
-set this to do something, as be default it will no-op
-"""
-pass
-
 def run(self, logger, backend):
 """ Runs all tests using Thread pool
 
@@ -321,8 +305,6 @@ class TestProfile(object):
 
 """
 
-self._pre_run_hook()
-
 chunksize = 1
 
 self._prepare_test_list()
@@ -350,6 +332,7 @@ class TestProfile(object):
 single = multiprocessing.dummy.Pool(1)
 multi = multiprocessing.dummy.Pool()
 
+self.setup()
 try:
 if options.OPTIONS.concurrent == "all":
 run_threads(multi, six.iteritems(self.test_list))
@@ -371,8 +354,7 @@ class TestProfile(object):
 # then die. Pressing C-c again will kill immediately.
 log.get().summary()
 raise
-
-self._post_run_hook()
+self.teardown()
 
 if self._monitoring.abort_needed:
 raise exceptions.PiglitAbort(self._monitoring.error_message)
@@ -482,6 +464,12 @@ class TestProfile(object):
 with self.test_list.allow_reassignment:
 yield
 
+def setup(self):
+"""Method to do pre-run setup."""
+
+def teardown(self):
+"""Method to od post-run teardown."""
+
 
 def load_test_profile(filename):
 """Load a python module and return it's profile attribute.
diff --git a/tests/igt.py b/tests/igt.py
index 7ebb036..69c91d1 100644
--- a/tests/igt.py
+++ b/tests/igt.py
@@ -91,7 +91,8 @@ else:
 
 class IGTTestProfile(TestProfile):
 """Test profile for intel-gpu-tools tests."""
-def _pre_run_hook(self):
+
+def setup(self):
 if options.OPTIONS.execute:
 try:
 check_environment()
diff --git a/tests/xts.py b/tests/xts.py
index f3cbbe8..715ecfa 100644
--- a/tests/xts.py
+++ b/tests/xts.py
@@ -41,12 +41,12 @@ X_TEST_SUITE = core.PIGLIT_CONFIG.required_get('xts', 
'path')
 
 class XTSProfile(TestProfile):  # pylint: disable=too-few-public-methods
 """ A subclass of TestProfile that provides a setup hook for XTS """
-def _pre_run_hook(self):
-""" This hook sets the XTSTest.results_path variable
 
-Setting this variable allows images created by XTS to moved into the
-results directory
+def setup(self):
+"""This hook sets the XTSTest.results_path variable.
 
+Setting this variable allows images created by XTS to moved into the
+results directory.
 """
 XTSTest.RESULTS_PATH = self.results_dir
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 26/27] framework/test/glsl_parser_test.py: split the parser out of the class

2016-11-09 Thread Dylan Baker 
This pulls the code for parsing the test file out of GLSLParserTest and
puts them in a new Parser class. This is going be useful later for
allowing the parsing to be done during the compile stage, and not
require searching the file system at runtime.

Signed-off-by: Dylan Baker 
---
 framework/test/glsl_parser_test.py | 85 ---
 1 file changed, 46 insertions(+), 39 deletions(-)

diff --git a/framework/test/glsl_parser_test.py 
b/framework/test/glsl_parser_test.py
index b1acb18..22eb6ce 100644
--- a/framework/test/glsl_parser_test.py
+++ b/framework/test/glsl_parser_test.py
@@ -73,56 +73,45 @@ class 
GLSLParserInternalError(exceptions.PiglitInternalError):
 pass
 
 
-class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
-""" Read the options in a glsl parser test and create a Test object
+class Parser(object):
+"""Find and parse the config block of a GLSLParserTest.
 
 Specifically it is necessary to parse a glsl_parser_test to get information
 about it before actually creating a PiglitTest. Even though this could
 be done with a function wrapper, making it a distinct class makes it easier
 to sort in the profile.
-
-Arguments:
-filepath -- the path to a glsl_parser_test which must end in .vert,
-.tesc, .tese, .geom or .frag
-
 """
 _CONFIG_KEYS = frozenset(['expect_result', 'glsl_version',
   'require_extensions', 'check_link'])
 
 def __init__(self, filepath):
-os.stat(filepath)
-
 # a set that stores a list of keys that have been found already
 self.__found_keys = set()
+self.gl_required = set()
+self.glsl_es_version = None
+self.glsl_version = None
 
-# Parse the config file and get the config section, then write this
-# section to a StringIO and pass that to ConfigParser
 try:
 with io.open(filepath, mode='r', encoding='utf-8') as testfile:
 testfile = testfile.read()
-config = self.__parser(testfile, filepath)
-command = self.__get_command(config, filepath)
+self.config = self.parse(testfile, filepath)
+self.command = self.get_command(filepath)
 except GLSLParserInternalError as e:
 raise exceptions.PiglitFatalError(
 'In file "{}":\n{}'.format(filepath, six.text_type(e)))
 
-super(GLSLParserTest, self).__init__(command, run_concurrent=True)
-
-self.__set_skip_conditions(config)
+self.set_skip_conditions()
 
-def __set_skip_conditions(self, config):
+def set_skip_conditions(self):
 """Set OpenGL and OpenGL ES fast skipping conditions."""
-glsl = config.get('glsl_version')
-if glsl:
-if _is_gles_version(glsl):
-self.glsl_es_version = float(glsl[:3])
-else:
-self.glsl_version = float(glsl)
+glsl = self.config['glsl_version']
+if _is_gles_version(glsl):
+self.glsl_es_version = float(glsl[:3])
+else:
+self.glsl_version = float(glsl)
 
-req = config.get('require_extensions')
-if req:
-self.gl_required = set(
-r for r in req.split() if not r.startswith('!'))
+req = self.config['require_extensions']
+self.gl_required = set(r for r in req.split() if not r.startswith('!'))
 
 # If GLES is requested, but piglit was not built with a gles version,
 # then ARB_ES3_compatibility is required. Add it to
@@ -138,10 +127,10 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 ver = '3_2'
 ext = 'ARB_ES{}_compatibility'.format(ver)
 self.gl_required.add(ext)
-self._command.append(ext)
+self.command.append(ext)
 
 @staticmethod
-def __pick_binary(version):
+def pick_binary(version):
 """Pick the correct version of glslparsertest to use.
 
 This will try to select glslparsertest_gles2 for OpenGL ES tests, and
@@ -162,7 +151,7 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 else:
 return 'None'
 
-def __get_command(self, config, filepath):
+def get_command(self, filepath):
 """ Create the command argument to pass to super()
 
 This private helper creates a configparser object, then reads in the
@@ -173,26 +162,26 @@ class GLSLParserTest(FastSkipMixin, PiglitBaseTest):
 
 """
 for opt in ['expect_result', 'glsl_version']:
-if not config.get(opt):
+if not self.config.get(opt):
 raise GLSLParserInternalError("Missing required section {} "
   "from config".format(opt))
 
 # Create the command and pass it into a PiglitTest()
-glsl = config['glsl_version']
+glsl = self.config['glsl_version']
 command = [
-

[Piglit] [PATCH v4 27/27] tests/all.py: Make add_fbo_depthstencil_tests take an adder

2016-11-09 Thread Dylan Baker 
This helper is the only one that doesn't take an adder function, which
is a consistency issue. But more importantly with the move of the
group_manager to TestDict instead of TestProfile, it makes an unsafe
assumption about the availability of the group_manager method, since a
TestDict could be initialized outside of a TestProfile instance. Which,
coincidently is something that the series to serialize the TestDict
does.

Signed-off-by: Dylan Baker 
---
 tests/all.py | 95 +
 1 file changed, 39 insertions(+), 56 deletions(-)

diff --git a/tests/all.py b/tests/all.py
index 4416d01..297aeda 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -63,16 +63,16 @@ def add_fbo_stencil_tests(adder, format):
   'fbo-stencil-{}-blit'.format(format))
 
 
-def add_fbo_depthstencil_tests(group, format, num_samples):
+def add_fbo_depthstencil_tests(adder, format, num_samples):
 assert format, \
 'add_fbo_depthstencil_tests argument "format" cannot be empty'
 
 if format == 'default_fb':
 prefix = ''
-create_test = lambda a: PiglitGLTest(a, run_concurrent=False)
+concurrent = False
 else:
 prefix = 'fbo-'
-create_test = PiglitGLTest
+concurrent = True
 
 if int(num_samples) > 1:
 suffix = ' samples=' + num_samples
@@ -81,51 +81,39 @@ def add_fbo_depthstencil_tests(group, format, num_samples):
 suffix = ''
 psamples = ''
 
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-clear{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'clear', format, psamples])
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-readpixels-FLOAT-and-USHORT{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format,
-'FLOAT-and-USHORT', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-readpixels-24_8{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format, '24_8',
-psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-readpixels-32F_24_8_REV{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'readpixels', format,
-'32F_24_8_REV', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-FLOAT-and-USHORT{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format,
- 'FLOAT-and-USHORT', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-24_8{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format, '24_8',
-psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-drawpixels-32F_24_8_REV{}'.format(
-prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'drawpixels', format,
-'32F_24_8_REV', psamples])
-profile.test_list[grouptools.join(
-group,
-'{}depthstencil-{}-copypixels{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'copypixels', format, psamples])
-profile.test_list[grouptools.join(
-group, '{}depthstencil-{}-blit{}'.format(prefix, format, suffix))] = \
-create_test(['fbo-depthstencil', 'blit', format, psamples])
+adder(['fbo-depthstencil', 'clear', format, psamples],
+  '{}depthstencil-{}-clear{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, 'FLOAT-and-USHORT',
+   psamples],
+  '{}depthstencil-{}-readpixels-FLOAT-and-USHORT{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, '24_8', psamples],
+  '{}depthstencil-{}-readpixels-24_8{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'readpixels', format, '32F_24_8_REV', psamples],
+  '{}depthstencil-{}-readpixels-32F_24_8_REV{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', format, 'FLOAT-and-USHORT',
+   psamples],
+  '{}depthstencil-{}-drawpixels-FLOAT-and-USHORT{}'.format(
+  prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', format, '24_8', psamples],
+  '{}depthstencil-{}-drawpixels-24_8{}'.format(prefix, format, suffix),
+  run_concurrent=concurrent)
+adder(['fbo-depthstencil', 'drawpixels', fo

[Piglit] [PATCH v4 25/27] framework/test/base: Remove timeout parameter

2016-11-09 Thread Dylan Baker 
This doesn't work and never has, setting it would cause an exception,
and I'm not exactly sure how to fix it. It's also unused, so we can
bring it back later if we need it.

Signed-off-by: Dylan Baker 
---
 framework/test/base.py | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/framework/test/base.py b/framework/test/base.py
index 6b7cab6..4e7c8b2 100644
--- a/framework/test/base.py
+++ b/framework/test/base.py
@@ -175,7 +175,7 @@ class Test(object):
 __slots__ = ['run_concurrent', 'env', 'result', 'cwd', '_command']
 timeout = None
 
-def __init__(self, command, run_concurrent=False, timeout=None):
+def __init__(self, command, run_concurrent=False):
 assert isinstance(command, list), command
 
 self.run_concurrent = run_concurrent
@@ -183,9 +183,6 @@ class Test(object):
 self.env = {}
 self.result = TestResult()
 self.cwd = None
-if timeout is not None:
-assert isinstance(timeout, int)
-self.timeout = timeout
 
 def execute(self, path, log, options):
 """ Run a test
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 22/27] framework/profile: replace Testprofile.{dmesg, monitoring} with dict

2016-11-09 Thread Dylan Baker 
This allows a significant amount of cleanup to happen. It allows
removing attributes from the global OPTIONS, removing tests that no
longer apply, and because of the split run method it allows more values
to simply be passed.

Signed-off-by: Dylan Baker 
---
 framework/options.py |  4 +--
 framework/profile.py | 48 +++--
 framework/programs/run.py| 21 -
 framework/test/base.py   | 31 ++
 tests/igt.py |  7 +--
 unittests/framework/test/test_base.py| 14 +++---
 unittests/framework/test/test_shader_test.py |  2 +-
 unittests/framework/test_profile.py  | 24 +---
 8 files changed, 45 insertions(+), 106 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index db4bf76..211159a 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -48,8 +48,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 Options are as follows:
 execute -- False for dry run
 valgrind -- True if valgrind is to be used
-dmesg -- True if dmesg checking is desired. This forces concurrency off
-monitored -- True if monitoring is desired. This forces concurrency off
 env -- environment variables set for each test before run
 deqp_mustpass -- True to enable the use of the deqp mustpass list feature.
 """
@@ -57,8 +55,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 def __init__(self):
 self.execute = True
 self.valgrind = False
-self.dmesg = False
-self.monitored = False
 self.sync = False
 self.deqp_mustpass = False
 self.process_isolation = True
diff --git a/framework/profile.py b/framework/profile.py
index 8449e7a..1bde8fb 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -211,42 +211,10 @@ class TestProfile(object):
 self.test_list = TestDict()
 self.forced_test_list = []
 self.filters = []
-# Sets a default of a Dummy
-self._dmesg = None
-self.dmesg = False
-self.results_dir = None
-self._monitoring = None
-self.monitoring = False
-
-@property
-def dmesg(self):
-""" Return dmesg """
-return self._dmesg
-
-@dmesg.setter
-def dmesg(self, not_dummy):
-"""Use dmesg.
-
-Arguments:
-not_dummy -- Get a platform dependent Dmesg class if True, otherwise
- get a DummyDmesg.
-"""
-self._dmesg = get_dmesg(not_dummy)
-
-@property
-def monitoring(self):
-""" Return monitoring """
-return self._monitoring
-
-@monitoring.setter
-def monitoring(self, monitored):
-"""Set monitoring.
-
-Arguments:
-monitored -- if Truthy Monitoring will enable monitoring according the
- defined rules
-"""
-self._monitoring = Monitoring(monitored)
+self.options = {
+'dmesg': get_dmesg(False),
+'monitor': Monitoring(False),
+}
 
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
@@ -431,9 +399,9 @@ def run(profiles, logger, backend, concurrency):
 def test(name, test, profile, this_pool=None):
 """Function to call test.execute from map"""
 with backend.write_test(name) as w:
-test.execute(name, log.get(), profile.dmesg, profile.monitoring)
+test.execute(name, log.get(), profile.options)
 w(test.result)
-if profile.monitoring.abort_needed:
+if profile.options['monitor'].abort_needed:
 this_pool.terminate()
 
 def run_threads(pool, profile, test_list, filterby=None):
@@ -484,5 +452,5 @@ def run(profiles, logger, backend, concurrency):
 log.get().summary()
 
 for p, _ in profiles:
-if p.monitoring.abort_needed:
-raise exceptions.PiglitAbort(p.monitoring.error_message)
+if p.options['monitor'].abort_needed:
+raise exceptions.PiglitAbort(p.options['monitor'].error_message)
diff --git a/framework/programs/run.py b/framework/programs/run.py
index 2ef3b4e..9d9538f 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -34,6 +34,8 @@ import time
 import six
 
 from framework import core, backends, exceptions, options
+from framework import dmesg
+from framework import monitoring
 from framework import profile
 from framework.results import TimeAttribute
 from . import parsers
@@ -226,6 +228,8 @@ def _create_metadata(args, name):
 opts['concurrent'] = args.concurrency
 opts['include_filter'] = args.include_tests
 opts['exclude_filter'] = args.exclude_tests
+opts['dmesg'] = args.dmesg
+opts['monitoring'] = args.monitored
 if args.platform:
 opts['platform'] = args.platform
 
@@ -282,8 +286,6

[Piglit] [PATCH v4 12/27] framework/programs/run: remove redundant list() around comprehension

2016-11-09 Thread Dylan Baker 
This is a list comprehension, it already results in a list. There is no
need to wrap it in the list() constructor call.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index 9e82bba..c957895 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -324,7 +324,7 @@ def run(input_):
 
 with open(args.test_list) as test_list:
 # Strip newlines
-profiles[0].forced_test_list = list([t.strip() for t in test_list])
+profiles[0].forced_test_list = [t.strip() for t in test_list]
 
 # Set the dmesg type
 if args.dmesg:
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 9/27] tests: Copy profiles to allow them to be run in parallel

2016-11-09 Thread Dylan Baker 
Some of these would be rather silly to run in parallel (xts and
xts-render, for example), but this will help avoid copy and pasting
things leading to the propagation of bad code.

Signed-off-by: Dylan Baker 
---
 tests/cpu.py| 4 +++-
 tests/glslparser.py | 4 +++-
 tests/gpu.py| 4 +++-
 tests/llvmpipe.py   | 4 +++-
 tests/quick.py  | 4 +++-
 tests/quick_cl.py   | 4 +++-
 tests/shader.py | 4 +++-
 tests/xts-render.py | 7 ---
 8 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/tests/cpu.py b/tests/cpu.py
index 34fb6f8..7fc905e 100644
--- a/tests/cpu.py
+++ b/tests/cpu.py
@@ -13,11 +13,13 @@ hardware.
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-from tests.quick import profile
+from tests.quick import profile as _profile
 from framework.test import GLSLParserTest
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def filter_gpu(name, test):
 """Remove all tests that are run on the GPU."""
diff --git a/tests/glslparser.py b/tests/glslparser.py
index 60442a2..fccc353 100644
--- a/tests/glslparser.py
+++ b/tests/glslparser.py
@@ -5,8 +5,10 @@ from __future__ import (
 )
 
 from framework.test import GLSLParserTest
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 profile.filter_tests(lambda _, t: isinstance(t, GLSLParserTest))
diff --git a/tests/gpu.py b/tests/gpu.py
index 01bca25..c9e3d15 100644
--- a/tests/gpu.py
+++ b/tests/gpu.py
@@ -6,11 +6,13 @@ from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 
-from tests.quick import profile
+from tests.quick import profile as _profile
 from framework.test import GLSLParserTest
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 # Remove all parser tests, as they are compiler test
 profile.filter_tests(lambda p, t: not isinstance(t, GLSLParserTest))
 profile.filter_tests(lambda n, _: not n.startswith('asmparsertest'))
diff --git a/tests/llvmpipe.py b/tests/llvmpipe.py
index f02755c..0ebd88b 100644
--- a/tests/llvmpipe.py
+++ b/tests/llvmpipe.py
@@ -8,10 +8,12 @@ import platform
 import sys
 
 from framework.grouptools import join
-from tests.gpu import profile
+from tests.gpu import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def remove(key):
 try:
diff --git a/tests/quick.py b/tests/quick.py
index 0e02f92..7af9e82 100644
--- a/tests/quick.py
+++ b/tests/quick.py
@@ -6,13 +6,15 @@ from __future__ import (
 
 from framework import grouptools
 from framework.test import (GleanTest, PiglitGLTest)
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
 # See the note in all.py about this warning
 # pylint: disable=bad-continuation
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 GleanTest.GLOBAL_PARAMS += ["--quick"]
 
 # Set the --quick flag on a few image_load_store_tests
diff --git a/tests/quick_cl.py b/tests/quick_cl.py
index 9f7c8f3..831e8fd 100644
--- a/tests/quick_cl.py
+++ b/tests/quick_cl.py
@@ -28,8 +28,10 @@ from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
 
-from tests.cl import profile
+from tests.cl import profile as _profile
 from framework.test import add_opencv_tests, add_oclconform_tests
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 add_opencv_tests(profile)
 add_oclconform_tests(profile)
diff --git a/tests/shader.py b/tests/shader.py
index 3d67679..ed5635a 100644
--- a/tests/shader.py
+++ b/tests/shader.py
@@ -5,8 +5,10 @@ from __future__ import (
 )
 
 from framework.test.shader_test import ShaderTest, MultiShaderTest
-from tests.all import profile
+from tests.all import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 profile.filter_tests(lambda _, t: isinstance(t, (ShaderTest, MultiShaderTest)))
diff --git a/tests/xts-render.py b/tests/xts-render.py
index ee644be..234fb2f 100644
--- a/tests/xts-render.py
+++ b/tests/xts-render.py
@@ -22,11 +22,13 @@
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-from framework import core
-from framework.profile import load_test_profile
+
+from tests.xts import profile as _profile
 
 __all__ = ['profile']
 
+profile = _profile.copy()  # pylint: disable=invalid-name
+
 
 def xts_render_filter(path, test):
 # Keep any tests that aren't from xts.
@@ -37,5 +39,4 @@ def xts_render_filter(path, test):
 return 'xlib9' in path
 
 
-profile = load_test_profile("xts")
 profile.filter_tests(xts_render_filter)
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 23/27] framework/profile: remove unused argument from TestProfile.group_manager

2016-11-09 Thread Dylan Baker 
Signed-off-by: Dylan Baker 
---
 framework/profile.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/profile.py b/framework/profile.py
index 1bde8fb..7e09081 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -217,7 +217,7 @@ class TestProfile(object):
 }
 
 @contextlib.contextmanager
-def group_manager(self, test_class, group, prefix=None, **default_args):
+def group_manager(self, test_class, group, **default_args):
 """A context manager to make working with flat groups simple.
 
 This provides a simple way to replace add_plain_test,
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 8/27] framework/profile: add a copy method to profile

2016-11-09 Thread Dylan Baker 
This will allow a profile to be copied and "subclassed" without
affecting the original profile. This will allow a long-standing bug that
made it impossible to run two subclasses of all.py (say shader.py and
glslparser.py) at the same time, since they would both try to modify the
all.py profile in incompatible ways.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 16 +-
 framework/programs/print_commands.py |  2 +-
 unittests/framework/test_profile.py  | 52 +-
 3 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/framework/profile.py b/framework/profile.py
index 4d111bf..a64855e 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -31,6 +31,7 @@ from __future__ import (
 )
 import collections
 import contextlib
+import copy
 import importlib
 import itertools
 import multiprocessing
@@ -398,6 +399,21 @@ class TestProfile(object):
 def teardown(self):
 """Method to od post-run teardown."""
 
+def copy(self):
+"""Create a copy of the TestProfile.
+
+This method creates a copy with references to the original instance
+(using copy.copy), except for the test_list attribute, which is copied
+using copy.deepcopy, which is necessary to ensure that filter_tests
+only affects the right instance. This allows profiles to be
+"subclassed" by other profiles, without modifying the original.
+"""
+new = copy.copy(self)
+new.test_list = copy.deepcopy(self.test_list)
+new.forced_test_list = copy.copy(self.forced_test_list)
+new.filters = copy.copy(self.filters)
+return new
+
 
 def load_test_profile(filename):
 """Load a python module and return it's profile attribute.
diff --git a/framework/programs/print_commands.py 
b/framework/programs/print_commands.py
index 033ca87..6e68eb5 100644
--- a/framework/programs/print_commands.py
+++ b/framework/programs/print_commands.py
@@ -95,7 +95,7 @@ def main(input_):
 
 profile_ = profile.load_test_profile(args.testProfile)
 
-profile_._prepare_test_list()
+profile_.prepare_test_list()
 for name, test in six.iteritems(profile_.test_list):
 assert isinstance(test, Test)
 print(args.format_string.format(
diff --git a/unittests/framework/test_profile.py 
b/unittests/framework/test_profile.py
index 6671349..5ef95e4 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -285,6 +285,58 @@ class TestTestProfile(object):
 
 assert grouptools.join('foo', 'abc') in self.profile.test_list
 
+class TestCopy(object):
+"""Tests for the copy method."""
+
+@pytest.fixture
+def fixture(self):
+orig = profile.TestProfile()
+orig.test_list['foo'] = utils.Test(['foo'])
+orig.test_list['bar'] = utils.Test(['bar'])
+orig.filters = [lambda name, _: name != 'bar']
+orig.forced_test_list = ['foo']
+return orig
+
+def test_filters(self, fixture):
+"""The filters attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.filters == new.filters
+assert fixture.filters is not new.filters
+
+# And double check by modifying one of them and asserting that the
+# other has not changed.
+new.filters.append(lambda name, _: name != 'oink')
+assert len(fixture.filters) == 1
+
+def test_forced_test_list(self, fixture):
+"""The forced_test_list attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.forced_test_list == new.forced_test_list
+assert fixture.forced_test_list is not new.forced_test_list
+
+# And double check by modifying one of them and asserting that the
+# other has not changed.
+del new.forced_test_list[0]
+assert fixture.forced_test_list[0] == 'foo'
+
+def test_test_list(self, fixture):
+"""The test_list attribute is copied correctly."""
+new = fixture.copy()
+
+# Assert that the fixtures are equivalent, but not the same
+assert fixture.test_list == new.test_list
+assert fixture.test_list is not new.test_list
+
+def test_prepare_test_list(self, fixture):
+"""The prepare_test_list method doesn't affect both."""
+new = fixture.copy()
+new.prepare_test_list()
+assert new.test_list != fixture.test_list
+
 
 class TestTestDict(object):
 """Tests for the TestDict object."""
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinf

[Piglit] [PATCH v4 14/27] framework/programs/run: import framework.profile as profile

2016-11-09 Thread Dylan Baker 
Since there isn't a conflict in the name anymore, this just makes the
code cleaner and easier to read.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index c957895..fa12c09 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -33,8 +33,8 @@ import shutil
 import six
 
 from framework import core, backends, exceptions, options
-import framework.results
-import framework.profile
+from framework import profile
+from framework.results import TimeAttribute
 from . import parsers
 
 __all__ = ['run',
@@ -312,7 +312,7 @@ def run(input_):
 backend.initialize(_create_metadata(
 args, args.name or path.basename(args.results_path)))
 
-profiles = [framework.profile.load_test_profile(p) for p in 
args.test_profile]
+profiles = [profile.load_test_profile(p) for p in args.test_profile]
 for p in profiles:
 p.results_dir = args.results_path
 
@@ -335,9 +335,9 @@ def run(input_):
 for p in profiles:
 p.monitoring = args.monitored
 
-time_elapsed = framework.results.TimeAttribute(start=time.time())
+time_elapsed = TimeAttribute(start=time.time())
 
-framework.profile.run(profiles, args.log_level, backend, args.concurrency)
+profile.run(profiles, args.log_level, backend, args.concurrency)
 
 time_elapsed.end = time.time()
 backend.finalize({'time_elapsed': time_elapsed.to_json()})
@@ -395,7 +395,7 @@ def resume(input_):
 if args.no_retry or result.result != 'incomplete':
 options.OPTIONS.exclude_tests.add(name)
 
-profiles = [framework.profile.load_test_profile(p)
+profiles = [profile.load_test_profile(p)
 for p in results.options['profile']]
 for p in profiles:
 p.results_dir = args.results_path
@@ -407,7 +407,7 @@ def resume(input_):
 p.monitoring = options.OPTIONS.monitored
 
 # This is resumed, don't bother with time since it won't be accurate anyway
-framework.profile.run(
+profile.run(
 profiles,
 results.options['log_level'],
 backend,
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 7/27] framework/programs/run: Remove use of TestrunResult

2016-11-09 Thread Dylan Baker 
We basically only used it to set the name and the time elapsed. That's
silly, just do those things directly. It needs less code and doesn't
require creating a big object.

This is leftover from the days before atomic writes, when results were
all stored in memory until the end of the run.

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 19 ++-
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index 880d748..b92de99 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -305,20 +305,12 @@ def run(input_):
 'Cannot overwrite existing folder without the -o/--overwrite '
 'option being set.')
 
-results = framework.results.TestrunResult()
-backends.set_meta(args.backend, results)
-
-# Set results.name
-if args.name is not None:
-results.name = args.name
-else:
-results.name = path.basename(args.results_path)
-
 backend = backends.get_backend(args.backend)(
 args.results_path,
 junit_suffix=args.junit_suffix,
 junit_subtests=args.junit_subtests)
-backend.initialize(_create_metadata(args, results.name))
+backend.initialize(_create_metadata(
+args, args.name or path.basename(args.results_path)))
 
 profile = framework.profile.merge_test_profiles(args.test_profile)
 profile.results_dir = args.results_path
@@ -328,7 +320,6 @@ def run(input_):
 # Strip newlines
 profile.forced_test_list = list([t.strip() for t in test_list])
 
-results.time_elapsed.start = time.time()
 # Set the dmesg type
 if args.dmesg:
 profile.dmesg = args.dmesg
@@ -336,10 +327,12 @@ def run(input_):
 if args.monitored:
 profile.monitoring = args.monitored
 
+time_elapsed = framework.results.TimeAttribute(start=time.time())
+
 framework.profile.run(profile, args.log_level, backend, args.concurrency)
 
-results.time_elapsed.end = time.time()
-backend.finalize({'time_elapsed': results.time_elapsed.to_json()})
+time_elapsed.end = time.time()
+backend.finalize({'time_elapsed': time_elapsed.to_json()})
 
 print('Thank you for running Piglit!\n'
   'Results have been written to ' + args.results_path)
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 19/27] framework/profile: Update __all__

2016-11-09 Thread Dylan Baker 
Signed-off-by: Dylan Baker 
---
 framework/profile.py | 2 ++
 1 file changed, 2 insertions(+), 0 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 9f72718..5f4657a 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -49,8 +49,10 @@ from framework.test.base import Test
 
 __all__ = [
 'RegexFilter',
+'TestDict',
 'TestProfile',
 'load_test_profile',
+'run',
 ]
 
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 11/27] framework/profile: Don't merge profiles

2016-11-09 Thread Dylan Baker 
Because we can copy profiles, we don't need to merge them to run more
than one of them. Instead we can simply have a list of profiles, and run
them one by one. One side effect of this is that tests will be run one
profile at a time, so if running with out the -1/--no-concurrency or
-c/--all-concurrent options tests will run in a sort of zipper pattern:
, , , etc.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 98 --
 framework/programs/run.py   | 33 ---
 framework/summary/feature.py|  3 +-
 unittests/framework/summary/test_feature.py | 19 +---
 unittests/framework/test_profile.py | 17 +
 5 files changed, 67 insertions(+), 103 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index a64855e..e4b8308 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -49,7 +49,6 @@ from framework.test.base import Test
 __all__ = [
 'TestProfile',
 'load_test_profile',
-'merge_test_profiles'
 ]
 
 
@@ -298,21 +297,6 @@ class TestProfile(object):
 """
 self.filters.append(function)
 
-def update(self, *profiles):
-""" Updates the contents of this TestProfile instance with another
-
-This method overwrites key:value pairs in self with those in the
-provided profiles argument. This allows multiple TestProfiles to be
-called in the same run; which could be used to run piglit and external
-suites at the same time.
-
-Arguments:
-profiles -- one or more TestProfile-like objects to be merged.
-
-"""
-for profile in profiles:
-self.test_list.update(profile.test_list)
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
 """A context manager to make working with flat groups simple.
@@ -447,24 +431,7 @@ def load_test_profile(filename):
 'Check your spelling?'.format(filename))
 
 
-def merge_test_profiles(profiles):
-""" Helper for loading and merging TestProfile instances
-
-Takes paths to test profiles as arguments and returns a single merged
-TestProfile instance.
-
-Arguments:
-profiles -- a list of one or more paths to profile files.
-
-"""
-profile = load_test_profile(profiles.pop())
-with profile.allow_reassignment:
-for p in profiles:
-profile.update(load_test_profile(p))
-return profile
-
-
-def run(profile, logger, backend, concurrency):
+def run(profiles, logger, backend, concurrency):
 """Runs all tests using Thread pool.
 
 When called this method will flatten out self.tests into self.test_list,
@@ -478,30 +445,52 @@ def run(profile, logger, backend, concurrency):
 Finally it will print a final summary of the tests.
 
 Arguments:
-profile -- a Profile ojbect.
-logger  -- a log.LogManager instance.
-backend -- a results.Backend derived instance.
+profiles -- a list of Profile instances.
+logger   -- a log.LogManager instance.
+backend  -- a results.Backend derived instance.
 """
 chunksize = 1
 
-profile.prepare_test_list()
-log = LogManager(logger, len(profile.test_list))
+for p in profiles:
+p.prepare_test_list()
+log = LogManager(logger, sum(len(p.test_list) for p in profiles))
 
-def test(pair, this_pool=None):
+def test(name, test, profile, this_pool=None):
 """Function to call test.execute from map"""
-name, test = pair
 with backend.write_test(name) as w:
 test.execute(name, log.get(), profile.dmesg, profile.monitoring)
 w(test.result)
 if profile.monitoring.abort_needed:
 this_pool.terminate()
 
-def run_threads(pool, testlist):
+def run_threads(pool, profile, filterby=None):
 """ Open a pool, close it, and join it """
-pool.imap(lambda pair: test(pair, pool), testlist, chunksize)
+iterable = six.iteritems(profile.test_list)
+if filterby:
+iterable = (x for x in iterable if filterby(x))
+
+pool.imap(lambda pair: test(pair[0], pair[1], profile, pool),
+  iterable, chunksize)
 pool.close()
 pool.join()
 
+def run_profile(profile):
+"""Run an individual profile."""
+profile.setup()
+if concurrency == "all":
+run_threads(multi, profile)
+elif concurrency == "none":
+run_threads(single, profile)
+else:
+assert concurrency == "some"
+# Filter and return only thread safe tests to the threaded pool
+run_threads(multi, profile, lambda x: x[1].run_concurrent)
+
+# Filter and return the non thread safe tests to the single
+# pool
+run_threads(single, profile, lambda x: not x[1].run_concurrent)
+profile.teardown()
+
 # Multiprocessing.dummy is a wrapper around Thread

[Piglit] [PATCH v4 20/27] framework/profile: Update docstrings

2016-11-09 Thread Dylan Baker 
Signed-off-by: Dylan Baker 
---
 framework/profile.py | 101 
 1 file changed, 48 insertions(+), 53 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 5f4657a..042cf98 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -19,11 +19,12 @@
 # OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
 # DEALINGS IN THE SOFTWARE.
 
-""" Provides Profiles for test groups
-
-Each set of tests, both native Piglit profiles and external suite integration,
-are represented by a TestProfile or a TestProfile derived object.
+"""Classes dealing with groups of Tests.
 
+In piglit tests are grouped into "profiles", which are equivalent to "suites"
+in some other testing nomenclature. A profile is a way to tell the framework
+that you have a group of tests you want to run, here are the names of those
+tests, and the Test instance.
 """
 
 from __future__ import (
@@ -95,18 +96,15 @@ class RegexFilter(object):
 class TestDict(collections.MutableMapping):
 """A special kind of dict for tests.
 
-This dict lowers the names of keys by default.
-
-This class intentionally doesn't accept keyword arguments. This is
-intentional to avoid breakages.
+This mapping lowers the names of keys by default, and enforces that keys be
+strings (not bytes) and that values are Test derived objects. It is also a
+wrapper around collections.OrderedDict.
 
+This class doesn't accept keyword arguments, this is intentional. This is
+because the TestDict class is ordered, and keyword arguments are unordered,
+which is a design mismatch.
 """
 def __init__(self):
-# This is because it had special __setitem__ and __getitem__ protocol
-# methods, and simply passing *args and **kwargs into self.__container
-# will bypass these methods. It will also break the ordering, since a
-# regular dictionary or keyword arguments are inherintly unordered
-#
 # This counter is incremented once when the allow_reassignment context
 # manager is opened, and decremented each time it is closed. This
 # allows stacking of the context manager
@@ -116,15 +114,13 @@ class TestDict(collections.MutableMapping):
 def __setitem__(self, key, value):
 """Enforce types on set operations.
 
-Keys should only be strings, and values should only be more Trees
-or Tests.
+Keys should only be strings, and values should only be Tests.
 
 This method makes one additional requirement, it lowers the key before
 adding it. This solves a couple of problems, namely that we want to be
-able to use filesystem heirarchies as groups in some cases, and those
+able to use file-system hierarchies as groups in some cases, and those
 are assumed to be all lowercase to avoid problems on case insensitive
-filesystems.
-
+file-systems.
 """
 # keys should be strings
 if not isinstance(key, six.text_type):
@@ -182,12 +178,11 @@ class TestDict(collections.MutableMapping):
 Normally reassignment happens in error, but sometimes one actually
 wants to do reassignment, say to add extra options in a reduced
 profile. This method allows reassignment, but only within its context,
-making it an explict choice to do so.
+making it an explicit choice to do so.
 
 It is safe to nest this contextmanager.
 
-It is not safe to use this context manager in a threaded application
-
+This is not thread safe, or even co-routine safe.
 """
 self.__allow_reassignment += 1
 yield
@@ -195,22 +190,22 @@ class TestDict(collections.MutableMapping):
 
 
 class TestProfile(object):
-""" Class that holds a list of tests for execution
-
-This class provides a container for storing tests in either a nested
-dictionary structure (deprecated), or a flat dictionary structure with '/'
-delimited groups.
-
-Once a TestProfile object is created tests can be added to the test_list
-name as a key/value pair, the key should be a fully qualified name for the
-test, including it's group hierarchy and should be '/' delimited, with no
-leading or trailing '/', the value should be an exectest.Test derived
-object.
-
-When the test list is filled calling TestProfile.run() will set the
-execution of these tests off, and will flatten the nested group hierarchy
-of self.tests and merge it with self.test_list
-
+"""Class that holds a list of tests for execution.
+
+This class represents a single testsuite, it has a mapping (dictionary-like
+object) of tests attached (TestDict). This is a mapping of :
+(python 3 str, python 2 unicode), and the key is delimited by
+grouptools.SEPARATOR.
+
+The group_manager method provides a context_manager to make adding test to
+the test_list easier, by doing mor

[Piglit] [PATCH v4 10/27] framework/programs/run: Only allow --test-list if one profile

2016-11-09 Thread Dylan Baker 
This limitation has basically always existed, but for the next patch to
function this is a requirement. This limitation will be properly fixed
in a follow up series (that was originally part of this series, but the
size and scope of the series got too big so this was split.)

Signed-off-by: Dylan Baker 
---
 framework/programs/run.py | 4 
 1 file changed, 4 insertions(+), 0 deletions(-)

diff --git a/framework/programs/run.py b/framework/programs/run.py
index b92de99..8af8448 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -316,6 +316,10 @@ def run(input_):
 profile.results_dir = args.results_path
 # If a test list is provided then set the forced_test_list value.
 if args.test_list:
+if len(args.test_profiles) != 1:
+raise exceptions.PiglitFatalError(
+'Unable to force a test list with more than one profile')
+
 with open(args.test_list) as test_list:
 # Strip newlines
 profile.forced_test_list = list([t.strip() for t in test_list])
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 18/27] framework/profile: Don't alter the TestProfile before running

2016-11-09 Thread Dylan Baker 
This patch changes the way that the tests from TestProfile are
processed. Rather than having a process_test_list method that modifies
TestProfile.test_list, a new itertests() method creates an iterator that
yields tests that are not excluded by filters.

This saves a bit of code, increases the memory usage, and reduces
surprise.

It would be nice if there wasn't a need to create a concrete test list,
but there wouldn't be any way to get the number of tests otherwise.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 112 ++--
 framework/programs/print_commands.py |   3 +-
 framework/summary/feature.py |  11 +---
 unittests/framework/test_profile.py  |   6 +--
 4 files changed, 45 insertions(+), 87 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index a6843ae..9f72718 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -191,35 +191,6 @@ class TestDict(collections.MutableMapping):
 yield
 self.__allow_reassignment -= 1
 
-def filter(self, callable):
-"""Filter tests out of the testdict before running.
-
-This method destructively filters results out of the test test
-dictionary list using the callable provided.
-
-Arguments:
-callable -- a callable object that returns truthy if the item remains,
-falsy if it should be removed
-
-"""
-for k, v in list(six.iteritems(self)):
-if not callable(k, v):
-del self[k]
-
-def reorder(self, order):
-"""Reorder the TestDict to match the order of the provided list."""
-new = collections.OrderedDict()
-try:
-for k in order:
-new[k] = self.__container[k]
-except KeyError:
-# If there is a name in order that isn't available in self there
-# will be a KeyError, this is expected. In this case fail
-# gracefully and report the error to the user.
-raise exceptions.PiglitFatalError(
-'Cannot reorder test: "{}", it is not in the 
profile.'.format(k))
-self.__container = new
-
 
 class TestProfile(object):
 """ Class that holds a list of tests for execution
@@ -282,28 +253,6 @@ class TestProfile(object):
 """
 self._monitoring = Monitoring(monitored)
 
-def prepare_test_list(self):
-""" Prepare tests for running
-
-Flattens the nested group hierarchy into a flat dictionary using '/'
-delimited groups by calling self.flatten_group_hierarchy(), then
-runs it's own filters plus the filters in the self.filters name
-
-"""
-if self.forced_test_list:
-# Remove all tests not in the test list, then reorder the tests to
-# match the testlist. This still allows additional filters to be
-# run afterwards.
-self.test_list.filter(lambda n, _: n in self.forced_test_list)
-self.test_list.reorder(self.forced_test_list)
-
-# Filter out unwanted tests
-self.test_list.filter(lambda n, t: all(f(n, t) for f in self.filters))
-
-if not self.test_list:
-raise exceptions.PiglitFatalError(
-'There are no tests scheduled to run. Aborting run.')
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
 """A context manager to make working with flat groups simple.
@@ -395,9 +344,8 @@ class TestProfile(object):
 
 This method creates a copy with references to the original instance
 (using copy.copy), except for the test_list attribute, which is copied
-using copy.deepcopy, which is necessary to ensure that
-prepare_test_list only affects the right instance. This allows profiles
-to be "subclassed" by other profiles, without modifying the original.
+using copy.deepcopy. This allows profiles to be "subclassed" by other
+profiles, without modifying the original.
 """
 new = copy.copy(self)
 new.test_list = copy.deepcopy(self.test_list)
@@ -405,6 +353,22 @@ class TestProfile(object):
 new.filters = copy.copy(self.filters)
 return new
 
+def itertests(self):
+"""Iterate over tests while filtering.
+
+This iterator is non-destructive.
+"""
+if self.forced_test_list:
+opts = collections.OrderedDict()
+for n in self.forced_test_list:
+opts[n] = self.test_list[n]
+else:
+opts = self.test_list  # pylint: disable=redefined-variable-type
+
+for k, v in six.iteritems(opts):
+if all(f(k, v) for f in self.filters):
+yield k, v
+
 
 def load_test_profile(filename):
 """Load a python module and return it's profile attribute.
@@ -458,9 +422,11 @@ def run(profiles, logger, backend, concurrency):
 """
 chunksi

[Piglit] [PATCH v4 4/27] framework/profile: Drop unneeded except

2016-11-09 Thread Dylan Baker 
A try/finally works better here anyway.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 0d2e1cf..f63180a 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -346,14 +346,8 @@ class TestProfile(object):
 # pool
 run_threads(single, (x for x in six.iteritems(self.test_list)
  if not x[1].run_concurrent))
-
-log.get().summary()
-except (KeyboardInterrupt, Exception):
-# In the event that C-c is pressed, or any sort of exception would
-# generate a stacktrace, print the status line so that it's clear,
-# then die. Pressing C-c again will kill immediately.
+finally:
 log.get().summary()
-raise
 self.teardown()
 
 if self._monitoring.abort_needed:
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 17/27] framework/profile: Factor out check_all closure

2016-11-09 Thread Dylan Baker 
This does away with the check_all closure, by simplifying it down to an
equivalent lambda expression.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index 17d17db..a6843ae 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -203,7 +203,7 @@ class TestDict(collections.MutableMapping):
 
 """
 for k, v in list(six.iteritems(self)):
-if not callable((k, v)):
+if not callable(k, v):
 del self[k]
 
 def reorder(self, order):
@@ -290,23 +290,15 @@ class TestProfile(object):
 runs it's own filters plus the filters in the self.filters name
 
 """
-def check_all(item):
-""" Checks group and test name against all filters """
-path, test = item
-for f in self.filters:
-if not f(path, test):
-return False
-return True
-
 if self.forced_test_list:
 # Remove all tests not in the test list, then reorder the tests to
 # match the testlist. This still allows additional filters to be
 # run afterwards.
-self.test_list.filter(lambda i: i[0] in self.forced_test_list)
+self.test_list.filter(lambda n, _: n in self.forced_test_list)
 self.test_list.reorder(self.forced_test_list)
 
 # Filter out unwanted tests
-self.test_list.filter(check_all)
+self.test_list.filter(lambda n, t: all(f(n, t) for f in self.filters))
 
 if not self.test_list:
 raise exceptions.PiglitFatalError(
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 5/27] framework: Split the run method out of profile.

2016-11-09 Thread Dylan Baker 
There are a couple of reasons for doing this. First, profile is a big
complex mess that does entirely too much, and this helps with that.
Second, there are bugs in the way two profiles run at the same time
work, and this is going to fix that.

Signed-off-by: Dylan Baker 
---
 framework/profile.py| 135 ++---
 framework/programs/run.py   |   4 +-
 framework/summary/feature.py|   2 +-
 unittests/framework/test_profile.py |  10 +-
 4 files changed, 76 insertions(+), 75 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index f63180a..1b9448f 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -244,7 +244,7 @@ class TestProfile(object):
 """
 self._monitoring = Monitoring(monitored)
 
-def _prepare_test_list(self):
+def prepare_test_list(self):
 """ Prepare tests for running
 
 Flattens the nested group hierarchy into a flat dictionary using '/'
@@ -287,72 +287,6 @@ class TestProfile(object):
 raise exceptions.PiglitFatalError(
 'There are no tests scheduled to run. Aborting run.')
 
-def run(self, logger, backend):
-""" Runs all tests using Thread pool
-
-When called this method will flatten out self.tests into
-self.test_list, then will prepare a logger, and begin executing tests
-through it's Thread pools.
-
-Based on the value of options.OPTIONS.concurrent it will either run all
-the tests concurrently, all serially, or first the thread safe tests
-then the serial tests.
-
-Finally it will print a final summary of the tests
-
-Arguments:
-backend -- a results.Backend derived instance
-
-"""
-
-chunksize = 1
-
-self._prepare_test_list()
-log = LogManager(logger, len(self.test_list))
-
-def test(pair, this_pool=None):
-"""Function to call test.execute from map"""
-name, test = pair
-with backend.write_test(name) as w:
-test.execute(name, log.get(), self.dmesg, self.monitoring)
-w(test.result)
-if self._monitoring.abort_needed:
-this_pool.terminate()
-
-def run_threads(pool, testlist):
-""" Open a pool, close it, and join it """
-pool.imap(lambda pair: test(pair, pool), testlist, chunksize)
-pool.close()
-pool.join()
-
-# Multiprocessing.dummy is a wrapper around Threading that provides a
-# multiprocessing compatible API
-#
-# The default value of pool is the number of virtual processor cores
-single = multiprocessing.dummy.Pool(1)
-multi = multiprocessing.dummy.Pool()
-
-self.setup()
-try:
-if options.OPTIONS.concurrent == "all":
-run_threads(multi, six.iteritems(self.test_list))
-elif options.OPTIONS.concurrent == "none":
-run_threads(single, six.iteritems(self.test_list))
-else:
-# Filter and return only thread safe tests to the threaded pool
-run_threads(multi, (x for x in six.iteritems(self.test_list)
-if x[1].run_concurrent))
-# Filter and return the non thread safe tests to the single
-# pool
-run_threads(single, (x for x in six.iteritems(self.test_list)
- if not x[1].run_concurrent))
-finally:
-log.get().summary()
-self.teardown()
-
-if self._monitoring.abort_needed:
-raise exceptions.PiglitAbort(self._monitoring.error_message)
-
 def filter_tests(self, function):
 """Filter out tests that return false from the supplied function
 
@@ -512,3 +446,70 @@ def merge_test_profiles(profiles):
 for p in profiles:
 profile.update(load_test_profile(p))
 return profile
+
+
+def run(profile, logger, backend):
+"""Runs all tests using Thread pool.
+
+When called this method will flatten out self.tests into self.test_list,
+then will prepare a logger, and begin executing tests through it's Thread
+pools.
+
+Based on the value of options.OPTIONS.concurrent it will either run all the
+tests concurrently, all serially, or first the thread safe tests then the
+serial tests.
+
+Finally it will print a final summary of the tests.
+
+Arguments:
+profile -- a Profile ojbect.
+logger  -- a log.LogManager instance.
+backend -- a results.Backend derived instance.
+"""
+chunksize = 1
+
+profile.prepare_test_list()
+log = LogManager(logger, len(profile.test_list))
+
+def test(pair, this_pool=None):
+"""Function to call test.execute from map"""
+name, test = pair
+with backend.write_test(name) as w:
+test.execute(name, log.get(), profile.dmes

[Piglit] [PATCH v4 1/27] framework/result: TestrunResult.options should be a dict

2016-11-09 Thread Dylan Baker 
This changes the default type from None to a dict, which is what it is
when setup in framework/programs/run.

Signed-off-by: Dylan Baker 
---
 framework/results.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/framework/results.py b/framework/results.py
index 3c2bb2a..1a73c11 100644
--- a/framework/results.py
+++ b/framework/results.py
@@ -295,7 +295,7 @@ class TestrunResult(object):
 def __init__(self):
 self.name = None
 self.uname = None
-self.options = None
+self.options = {}
 self.glxinfo = None
 self.wglinfo = None
 self.clinfo = None
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 16/27] framework: Pull {include, exclude}_filter out of Options

2016-11-09 Thread Dylan Baker 
Since these are also just special cases of filters for the standard
TestProfile filtering mechanism, and they have a lot of unique classes.
This is just a waste, the same can be achieved with a much simpler class
structure.

Signed-off-by: Dylan Baker 
---
 framework/options.py | 137 +--
 framework/profile.py |  54 +---
 framework/programs/print_commands.py |  10 +-
 framework/programs/run.py|  30 +++--
 framework/summary/feature.py |  20 +--
 unittests/framework/test_options.py  | 178 +
 unittests/framework/test_profile.py  | 126 ++--
 7 files changed, 119 insertions(+), 436 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index dc97c38..db4bf76 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -28,9 +28,7 @@ is that while you can mutate
 from __future__ import (
 absolute_import, division, print_function, unicode_literals
 )
-import collections
 import os
-import re
 
 import six
 
@@ -39,129 +37,6 @@ __all__ = ['OPTIONS']
 # pylint: disable=too-few-public-methods
 
 
-_RETYPE = type(re.compile(''))
-
-
-class _ReList(collections.MutableSequence):
-"""A list-like container that only holds RegexObjects.
-
-This class behaves identically to a list, except that all objects are
-forced to be RegexObjects with a flag of re.IGNORECASE (2 if one inspects
-the object).
-
-If inputs do not match this object, they will be coerced to becoming such
-an object, or they assignment will fail.
-
-"""
-def __init__(self, iterable=None):
-self._wrapped = []
-if iterable is not None:
-self.extend(iterable)
-
-@staticmethod
-def __compile(value):
-"""Ensure that the object is properly compiled.
-
-If the object is not a RegexObject then compile it to one, setting the
-proper flag. If it is a RegexObject, and the flag is incorrect
-recompile it to have the proper flags. Otherwise return it.
-
-"""
-if not isinstance(value, _RETYPE):
-return re.compile(value, re.IGNORECASE)
-elif value.flags != re.IGNORECASE:
-return re.compile(value.pattern, re.IGNORECASE)
-return value
-
-def __getitem__(self, index):
-return self._wrapped[index]
-
-def __setitem__(self, index, value):
-self._wrapped[index] = self.__compile(value)
-
-def __delitem__(self, index):
-del self._wrapped[index]
-
-def __len__(self):
-return len(self._wrapped)
-
-def insert(self, index, value):
-self._wrapped.insert(index, self.__compile(value))
-
-def __eq__(self, other):
-"""Two ReList instances are the same if their wrapped list are 
equal."""
-if isinstance(other, _ReList):
-# There doesn't seem to be a better way to do this.
-return self._wrapped == other._wrapped  # pylint: 
disable=protected-access
-raise TypeError('Cannot compare _ReList and non-_ReList object')
-
-def __ne__(self, other):
-return not self == other
-
-def to_json(self):
-"""Allow easy JSON serialization.
-
-This returns the pattern (the string or unicode used to create the re)
-of each re object in a list rather than the RegexObject itself. This is
-critical for JSON serialization, and thanks to the piglit_encoder this
-is all we need to serialize this class.
-
-"""
-return [l.pattern for l in self]
-
-
-class _FilterReList(_ReList):
-"""A version of ReList that handles group madness.
-
-Groups are printed with '/' as a separator, but internally something else
-may be used. This version replaces '/' with '.'.
-
-"""
-def __setitem__(self, index, value):
-# Replace '/' with '.', this solves the problem of '/' not matching
-# grouptools.SEPARATOR, but without needing to import grouptools
-super(_FilterReList, self).__setitem__(index, value.replace('/', '.'))
-
-def insert(self, index, value):
-super(_FilterReList, self).insert(index, value.replace('/', '.'))
-
-
-class _ReListDescriptor(object):
-"""A Descriptor than ensures reassignment of _{in,ex}clude_filter is an
-_ReList
-
-Without this some behavior's can get very strange. This descriptor is
-mostly hit by testing code, but may be of use outside of testing at some
-point.
-
-"""
-def __init__(self, name, type_=_ReList):
-self.__name = name
-self.__type = type_
-
-def __get__(self, instance, cls):
-try:
-return getattr(instance, self.__name)
-except AttributeError as e:
-new = _ReList()
-try:
-setattr(instance, self.__name, new)
-except Exception:
-raise e
-return new
-
-def __set__(self, instance, value):
-assert isinstance(v

[Piglit] [PATCH v4 6/27] framework: remove concurrent from OPTIONS, pass directly to profile

2016-11-09 Thread Dylan Baker 
This removes a value out of the global OPTIONS, which is nice. It's very
trivial to pass this instead of putting it in options.

Signed-off-by: Dylan Baker 
---
 framework/options.py  |  2 --
 framework/profile.py  |  8 +---
 framework/programs/run.py | 11 +++
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index 5cb88aa..46e37ee 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -171,7 +171,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 the configuration file.
 
 Options are as follows:
-concurrent -- one of: ["all", "some", "none"]. Default: "some"
 execute -- False for dry run
 include_filter -- list of compiled regex which include exclusively tests
   that match
@@ -187,7 +186,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 exclude_filter = _ReListDescriptor('_exclude_filter', type_=_FilterReList)
 
 def __init__(self):
-self.concurrent = "some"
 self.execute = True
 self._include_filter = _ReList()
 self._exclude_filter = _ReList()
diff --git a/framework/profile.py b/framework/profile.py
index 1b9448f..4d111bf 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -448,7 +448,7 @@ def merge_test_profiles(profiles):
 return profile
 
 
-def run(profile, logger, backend):
+def run(profile, logger, backend, concurrency):
 """Runs all tests using Thread pool.
 
 When called this method will flatten out self.tests into self.test_list,
@@ -495,11 +495,13 @@ def run(profile, logger, backend):
 
 profile.setup()
 try:
-if options.OPTIONS.concurrent == "all":
+if concurrency == "all":
 run_threads(multi, six.iteritems(profile.test_list))
-elif options.OPTIONS.concurrent == "none":
+elif concurrency == "none":
 run_threads(single, six.iteritems(profile.test_list))
 else:
+assert concurrency == "some"
+
 # Filter and return only thread safe tests to the threaded pool
 run_threads(multi, (x for x in six.iteritems(profile.test_list)
 if x[1].run_concurrent))
diff --git a/framework/programs/run.py b/framework/programs/run.py
index 023aa2e..880d748 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -222,6 +222,7 @@ def _create_metadata(args, name):
 opts = dict(options.OPTIONS)
 opts['profile'] = args.test_profile
 opts['log_level'] = args.log_level
+opts['concurrent'] = args.concurrency
 if args.platform:
 opts['platform'] = args.platform
 
@@ -276,7 +277,6 @@ def run(input_):
 args.concurrency = "none"
 
 # Pass arguments into Options
-options.OPTIONS.concurrent = args.concurrency
 options.OPTIONS.exclude_filter = args.exclude_tests
 options.OPTIONS.include_filter = args.include_tests
 options.OPTIONS.execute = args.execute
@@ -336,7 +336,7 @@ def run(input_):
 if args.monitored:
 profile.monitoring = args.monitored
 
-framework.profile.run(profile, args.log_level, backend)
+framework.profile.run(profile, args.log_level, backend, args.concurrency)
 
 results.time_elapsed.end = time.time()
 backend.finalize({'time_elapsed': results.time_elapsed.to_json()})
@@ -365,7 +365,6 @@ def resume(input_):
 _disable_windows_exception_messages()
 
 results = backends.load(args.results_path)
-options.OPTIONS.concurrent = results.options['concurrent']
 options.OPTIONS.exclude_filter = results.options['exclude_filter']
 options.OPTIONS.include_filter = results.options['include_filter']
 options.OPTIONS.execute = results.options['execute']
@@ -404,7 +403,11 @@ def resume(input_):
 profile.monitoring = options.OPTIONS.monitored
 
 # This is resumed, don't bother with time since it won't be accurate anyway
-framework.profile.run(profile, results.options['log_level'], backend)
+framework.profile.run(
+profile,
+results.options['log_level'],
+backend,
+results.options['concurrent'])
 
 backend.finalize()
 
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 13/27] framework/profile: Drop TestProfile.filter_tests

2016-11-09 Thread Dylan Baker 
This was just a thin wrapper around the already public
TestProfile.filters list. It seems silly to have such a method when it
does nothing but call self.filters.append.

Signed-off-by: Dylan Baker 
---
 framework/profile.py | 16 +++-
 tests/all.py |  2 +-
 tests/cpu.py |  2 +-
 tests/glslparser.py  |  2 +-
 tests/gpu.py |  4 ++--
 tests/quick.py   |  2 +-
 tests/shader.py  |  2 +-
 tests/xts-render.py  |  2 +-
 8 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/framework/profile.py b/framework/profile.py
index e4b8308..e016a36 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -287,16 +287,6 @@ class TestProfile(object):
 raise exceptions.PiglitFatalError(
 'There are no tests scheduled to run. Aborting run.')
 
-def filter_tests(self, function):
-"""Filter out tests that return false from the supplied function
-
-Arguments:
-function -- a callable that takes two parameters: path, test and
-returns whether the test should be included in the test
-run or not.
-"""
-self.filters.append(function)
-
 @contextlib.contextmanager
 def group_manager(self, test_class, group, prefix=None, **default_args):
 """A context manager to make working with flat groups simple.
@@ -388,9 +378,9 @@ class TestProfile(object):
 
 This method creates a copy with references to the original instance
 (using copy.copy), except for the test_list attribute, which is copied
-using copy.deepcopy, which is necessary to ensure that filter_tests
-only affects the right instance. This allows profiles to be
-"subclassed" by other profiles, without modifying the original.
+using copy.deepcopy, which is necessary to ensure that
+prepare_test_list only affects the right instance. This allows profiles
+to be "subclassed" by other profiles, without modifying the original.
 """
 new = copy.copy(self)
 new.test_list = copy.deepcopy(self.test_list)
diff --git a/tests/all.py b/tests/all.py
index 938013a..f1b9e5d 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4908,4 +4908,4 @@ with profile.group_manager(
 g(['arb_compute_variable_group_size-minmax'], 'minmax')
 
 if platform.system() is 'Windows':
-profile.filter_tests(lambda p, _: not p.startswith('glx'))
+profile.filters.append(lambda p, _: not p.startswith('glx'))
diff --git a/tests/cpu.py b/tests/cpu.py
index 7fc905e..65d9990 100644
--- a/tests/cpu.py
+++ b/tests/cpu.py
@@ -28,4 +28,4 @@ def filter_gpu(name, test):
 return False
 
 
-profile.filter_tests(filter_gpu)
+profile.filters.append(filter_gpu)
diff --git a/tests/glslparser.py b/tests/glslparser.py
index fccc353..5d0facb 100644
--- a/tests/glslparser.py
+++ b/tests/glslparser.py
@@ -11,4 +11,4 @@ __all__ = ['profile']
 
 profile = _profile.copy()  # pylint: disable=invalid-name
 
-profile.filter_tests(lambda _, t: isinstance(t, GLSLParserTest))
+profile.filters.append(lambda _, t: isinstance(t, GLSLParserTest))
diff --git a/tests/gpu.py b/tests/gpu.py
index c9e3d15..fce550c 100644
--- a/tests/gpu.py
+++ b/tests/gpu.py
@@ -14,5 +14,5 @@ __all__ = ['profile']
 profile = _profile.copy()  # pylint: disable=invalid-name
 
 # Remove all parser tests, as they are compiler test
-profile.filter_tests(lambda p, t: not isinstance(t, GLSLParserTest))
-profile.filter_tests(lambda n, _: not n.startswith('asmparsertest'))
+profile.filters.append(lambda p, t: not isinstance(t, GLSLParserTest))
+profile.filters.append(lambda n, _: not n.startswith('asmparsertest'))
diff --git a/tests/quick.py b/tests/quick.py
index 7af9e82..3a02df7 100644
--- a/tests/quick.py
+++ b/tests/quick.py
@@ -38,4 +38,4 @@ with profile.group_manager(
 g(['arb_shader_image_size-builtin', '--quick'], 'builtin')
 
 # These take too long
-profile.filter_tests(lambda n, _: '-explosion' not in n)
+profile.filters.append(lambda n, _: '-explosion' not in n)
diff --git a/tests/shader.py b/tests/shader.py
index ed5635a..d283a57 100644
--- a/tests/shader.py
+++ b/tests/shader.py
@@ -11,4 +11,4 @@ __all__ = ['profile']
 
 profile = _profile.copy()  # pylint: disable=invalid-name
 
-profile.filter_tests(lambda _, t: isinstance(t, (ShaderTest, MultiShaderTest)))
+profile.filters.append(lambda _, t: isinstance(t, (ShaderTest, 
MultiShaderTest)))
diff --git a/tests/xts-render.py b/tests/xts-render.py
index 234fb2f..d2cd843 100644
--- a/tests/xts-render.py
+++ b/tests/xts-render.py
@@ -39,4 +39,4 @@ def xts_render_filter(path, test):
 return 'xlib9' in path
 
 
-profile.filter_tests(xts_render_filter)
+profile.filters.append(xts_render_filter)
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 15/27] framework: Remove exclude_tests from options.OPTIONS

2016-11-09 Thread Dylan Baker 
Rather than putting this in a global variable, just add a filter for
this in the runner. Far simpler, and removes more globals.

Signed-off-by: Dylan Baker 
---
 framework/options.py|  1 -
 framework/profile.py|  1 -
 framework/programs/run.py   |  5 -
 unittests/framework/test_profile.py | 17 -
 4 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/framework/options.py b/framework/options.py
index 46e37ee..dc97c38 100644
--- a/framework/options.py
+++ b/framework/options.py
@@ -189,7 +189,6 @@ class _Options(object):  # pylint: 
disable=too-many-instance-attributes
 self.execute = True
 self._include_filter = _ReList()
 self._exclude_filter = _ReList()
-self.exclude_tests = set()
 self.valgrind = False
 self.dmesg = False
 self.monitored = False
diff --git a/framework/profile.py b/framework/profile.py
index e016a36..e00fbc4 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -260,7 +260,6 @@ class TestProfile(object):
 """Filter for user-specified restrictions"""
 return ((not options.OPTIONS.include_filter or
  matches_any_regexp(path, options.OPTIONS.include_filter))
-and path not in options.OPTIONS.exclude_tests
 and not matches_any_regexp(path, 
options.OPTIONS.exclude_filter))
 
 filters = self.filters + [test_matches]
diff --git a/framework/programs/run.py b/framework/programs/run.py
index fa12c09..20a036e 100644
--- a/framework/programs/run.py
+++ b/framework/programs/run.py
@@ -391,9 +391,10 @@ def resume(input_):
 
 # Don't re-run tests that have already completed, incomplete status tests
 # have obviously not completed.
+exclude_tests = set()
 for name, result in six.iteritems(results.tests):
 if args.no_retry or result.result != 'incomplete':
-options.OPTIONS.exclude_tests.add(name)
+exclude_tests.add(name)
 
 profiles = [profile.load_test_profile(p)
 for p in results.options['profile']]
@@ -406,6 +407,8 @@ def resume(input_):
 if options.OPTIONS.monitored:
 p.monitoring = options.OPTIONS.monitored
 
+p.filters.append(lambda n, _: n not in exclude_tests)
+
 # This is resumed, don't bother with time since it won't be accurate anyway
 profile.run(
 profiles,
diff --git a/unittests/framework/test_profile.py 
b/unittests/framework/test_profile.py
index 4ffabfa..f2aa5b5 100644
--- a/unittests/framework/test_profile.py
+++ b/unittests/framework/test_profile.py
@@ -154,23 +154,6 @@ class TestTestProfile(object):
 
 assert dict(profile_.test_list) == baseline
 
-def test_matches_env_exclude(self):
-"""profile.TestProfile.prepare_test_list: 'not path in
-env.exclude_tests'.
-"""
-# Pylint can't figure out that self.opts isn't a dict, but an
-# options._Option object.
-self.opts.exclude_tests.add(grouptools.join('group3', 'test5'))  # 
pylint: disable=no-member
-
-baseline = copy.deepcopy(self.data)
-del baseline[grouptools.join('group3', 'test5')]
-
-profile_ = profile.TestProfile()
-profile_.test_list = self.data
-profile_.prepare_test_list()
-
-assert dict(profile_.test_list) == dict(baseline)
-
 def test_matches_exclude_mar(self):
 """profile.TestProfile.prepare_test_list: 'not
 matches_any_regexp()'.
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 2/27] unittests: Add tests for the feature completeness summary

2016-11-09 Thread Dylan Baker 
This adds a few simple tests for the feature completeness summary
object. Nothing to strenuous but much better than nothing.

Signed-off-by: Dylan Baker 
---
 unittests/framework/summary/test_feature.py | 116 +-
 1 file changed, 116 insertions(+), 0 deletions(-)
 create mode 100644 unittests/framework/summary/test_feature.py

diff --git a/unittests/framework/summary/test_feature.py 
b/unittests/framework/summary/test_feature.py
new file mode 100644
index 000..370b360
--- /dev/null
+++ b/unittests/framework/summary/test_feature.py
@@ -0,0 +1,116 @@
+# encoding=utf-8
+# Copyright ?? 2016 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 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.
+
+"""Tests for the feature summary module."""
+
+from __future__ import (
+absolute_import, division, print_function, unicode_literals
+)
+try:
+import simplejson as json
+except ImportError:
+import json
+try:
+import mock
+except ImportError:
+from unittest import mock
+
+import pytest
+import six
+
+from framework import grouptools
+from framework import results
+from framework import profile
+from framework.summary import feature
+
+from .. import utils
+
+DATA = {
+'spec@gl-1.0': {
+'include_tests': 'gl-1.0',
+'exclude_tests': '',
+'target_rate': 50,
+},
+'spec@gl-2.0': {
+'include_tests': 'gl-2.0',
+'exclude_tests': '',
+'target_rate': 50,
+},
+}
+
+
+def _maketest(res):
+"""Helper utility to make a test with a result."""
+t = utils.Test(['foo'])
+t.result.result = res
+return t
+
+
+PROFILE = profile.TestProfile()
+PROFILE.test_list = {
+'spec@gl-1.0@a': _maketest('pass'),
+'spec@gl-1.0@b': _maketest('warn'),
+'spec@gl-1.0@c': _maketest('pass'),
+'spec@gl-1.0@d': _maketest('fail'),
+'spec@gl-2.0@a': _maketest('fail'),
+'spec@gl-2.0@b': _maketest('crash'),
+'spec@gl-2.0@c': _maketest('pass'),
+'spec@gl-2.0@d': _maketest('fail'),
+}
+
+
+class TestFeatResult(object):
+"""Tests fro the FeatResult class."""
+
+@pytest.fixture(scope='session')
+def feature(self, tmpdir_factory):
+p = tmpdir_factory.mktemp('feature').join('p')
+# each write to p will replace the contents, so using json.dump which
+# makes a number of small writes will fail to produce anything useful.
+p.write(json.dumps(DATA))
+
+result = results.TestrunResult()
+for n, s in six.iteritems(PROFILE.test_list):
+result.tests[n] = s.result
+result.options['profile'] = [None]
+result.name = 'foo'
+
+with mock.patch('framework.summary.feature.profile.load_test_profile',
+mock.Mock(return_value=PROFILE)):
+return feature.FeatResults([result], six.text_type(p))
+
+def test_basic(self, feature):
+"""The fixture works."""
+pass
+
+def test_features(self, feature):
+"""The features set is populated."""
+assert feature.features == {'spec@gl-1.0', 'spec@gl-2.0'}
+
+def test_feat_fractions(self, feature):
+"""feat_fraction is populated."""
+assert feature.feat_fractions == \
+{'foo': {'spec@gl-1.0': (2, 4), 'spec@gl-2.0': (1, 4)}}
+
+def test_feat_status(self, feature):
+"""feat_status is populated."""
+assert feature.feat_status == \
+{'foo': {'spec@gl-1.0': 'pass', 'spec@gl-2.0': 'fail'}}
-- 
git-series 0.8.10
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH v4 0/27] Various cleanups for profiles

2016-11-09 Thread Dylan Baker 
This series is a large collections of cleanups for th profile module and
particularly for the TestProfile and TestDict classes. These began their
lives as the start of a series to serialize the TestDict at compile time
and to move startup overhead.

This series is just a bunch of cleanups and removes a *lot* of code, and
simplifies a number of things, as well as fixing a few long standing
bugs in piglit. The biggest of these fixes is that it is now possible to
pass two derivatives of all.py (say glslparser.py and shader.py) at the
same time.

This series passes our CI system with no changes or regressions.

This has gone out several times with no comments, I'm planning to push first
thing in the morning if no one objects.

Changes in v2:
- Rebase on master, including one patch that was part of this
  series, but was merged since it fixed a bug
- Don't add filters that do nothing in framework/programs/run.py.
  This doesn't really affect this series, but it does have
  ramifications in some other work that isn't ready to go to the
  list yet.

Changes in v3:
- Don't close the pools until all of the profiles have finished
  running
- Don't remove the _{pre,post}_run_hook methods, instead rename them
  to setup and teardown, making them public. They actually are used
  by XTS and IGT, which I didn't notice. I still think I'd like to
  get rid of them, but not while they're in use and there isn't an
  obviously good way to do so.
- Store images in the JSON file, rather than externally
- the XTS profile uses a temporary directory to store it's results,
  which (along with the previous change) removes the need to pass
  the results directory to its tests.
- Fix RegexFilter to work for both the include filter (-t) and the
  exclude filter (-x), it previously only worked for one or the
  other. Also add tests.

Changes in v4:
- Add tests for summary-feature
- Don't break summary feature tests with changes (previously this
  was component was completely broken)
- Fix the inverse flag in RegexFilter (in framework/program/run.py),
  which was being passed to the wrong function.
- Remove RFC patch to use enum34 (and fix changes resulting from that
  removal)
- Fix args.concurrent, which should be args.concurrency, in programs/run.py
- Fix check for monitoring errors in profile.run

Dylan Baker (27):
  framework/result: TestrunResult.options should be a dict
  unittests: Add tests for the feature completeness summary
  framework/profile: make run hooks public
  framework/profile: Drop unneeded except
  framework: Split the run method out of profile.
  framework: remove concurrent from OPTIONS, pass directly to profile
  framework/programs/run: Remove use of TestrunResult
  framework/profile: add a copy method to profile
  tests: Copy profiles to allow them to be run in parallel
  framework/programs/run: Only allow --test-list if one profile
  framework/profile: Don't merge profiles
  framework/programs/run: remove redundant list() around comprehension
  framework/profile: Drop TestProfile.filter_tests
  framework/programs/run: import framework.profile as profile
  framework: Remove exclude_tests from options.OPTIONS
  framework: Pull {include,exclude}_filter out of Options
  framework/profile: Factor out check_all closure
  framework/profile: Don't alter the TestProfile before running
  framework/profile: Update __all__
  framework/profile: Update docstrings
  framework/profile: Split try/except block
  framework/profile: replace Testprofile.{dmesg,monitoring} with dict
  framework/profile: remove unused argument from TestProfile.group_manager
  framework/profile: Move group_manager from TestProfile to TestDict
  framework/test/base: Remove timeout parameter
  framework/test/glsl_parser_test.py: split the parser out of the class
  tests/all.py: Make add_fbo_depthstencil_tests take an adder

 framework/options.py | 144 +-
 framework/profile.py | 564 ---
 framework/programs/print_commands.py |  13 +-
 framework/programs/run.py| 113 ++--
 framework/results.py |   2 +-
 framework/summary/feature.py |  30 +-
 framework/test/base.py   |  36 +-
 framework/test/glsl_parser_test.py   |  85 +--
 tests/all.py | 605 +---
 tests/cl.py  |   8 +-
 tests/cpu.py |   6 +-
 tests/glslparser.py  |   6 +-
 tests/gpu.py |   8 +-
 tests/igt.py |  10 +-
 tests/llvmpipe.py|   4 +-
 tests/quick.py   |  14 +-
 tests/quick_cl.py|   4 +-
 tests/sanity.py  |   2 +-
 t

Re: [Piglit] [PATCH 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Dylan Baker 
Quoting Dylan Baker (2016-11-09 12:08:34)
> Quoting Lionel Landwerlin (2016-11-09 03:39:33)
> > This tests that the compiler accepts the new layout formats introduced by
> > NV_image_formats and also it's correct interaction with the
> > EXT_texture_norm16 specification.
> > 
> > Signed-off-by: Lionel Landwerlin 
> > ---
> >  generated_tests/CMakeLists.txt |   4 +
> >  .../gen_shader_image_nv_image_formats_tests.py | 230 
> > +
> >  2 files changed, 234 insertions(+)
> >  create mode 100644 
> > generated_tests/gen_shader_image_nv_image_formats_tests.py
> > 
> > diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
> > index ff43af5..fd38afe 100644
> > --- a/generated_tests/CMakeLists.txt
> > +++ b/generated_tests/CMakeLists.txt
> > @@ -163,6 +163,9 @@ piglit_make_generated_tests(
> > shader_image_load_store_tests.list
> > gen_shader_image_load_store_tests.py)
> >  piglit_make_generated_tests(
> > +   shader_image_nv_image_formats_tests.list
> > +   gen_shader_image_nv_image_formats_tests.py)
> > +piglit_make_generated_tests(
> > variable_index_read_tests.list
> > gen_variable_index_read_tests.py
> > templates/gen_variable_index_read_tests/vs.shader_test.mako
> > @@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
> > conversion_fp64.list
> > shader_precision_tests.list
> > shader_image_load_store_tests.list
> > +   shader_image_nv_image_formats_tests.list
> > variable_index_read_tests.list
> > gen_extensions_defined.list
> > vp-tex.list
> > diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py 
> > b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> > new file mode 100644
> > index 000..9b1861d
> > --- /dev/null
> > +++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> > @@ -0,0 +1,230 @@
> > +# coding=utf-8
> > +#
> > +# Copyright (C) 2016 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.
> 
> A Short docstring here explaining what the generator does wouldn't be amiss.
> You could probably just copy your commit message.
> 
> > +
> > +from __future__ import print_function, division, absolute_import
> > +import os.path
> > +from mako.template import Template
> > +from textwrap import dedent
> 
> please sort these as:
> __future__
> python builtins
> 
> 3rd party modules
> 
> local modules
> 
> (in this case mako is the only 3rd party module, everything else is correct.)
> 
> > +
> > +from modules import utils
> > +
> > +
> > +def gen_header(status, norm16):
> > +"""
> > +Generate a GLSL program header.
> 
> Most of piglit puts the """ and the first lone of the docstring on the same 
> line
> 
> > +
> > +Generate header code for ARB_shader_image_load_store GLSL parser
> > +tests that are expected to give status as result.
> > +"""
> > +return dedent("""
> > +/*
> > + * [config]
> > + * expect_result: {0}
> > + * glsl_version: 3.10 es
> > + * require_extensions: GL_NV_image_formats {1}
> > + * [end config]
> > + */
> > +#version 310 es
> > +#extension GL_NV_image_formats : enable
> > +""".format(status, "GL_EXT_texture_norm16" if norm16 else 
> > "!GL_EXT_texture_norm16"))
> 
> This line is really long
> 
> > +
> > +
> > +def gen(name, src, tests):
> > +"""
> > +Expand a source template for the provided list of test definitions.
> > +
> > +Generate a GLSL parser test for each of the elements of the
> > +'tests' iterable, each of them should be a dictionary of
> > +definitions that will be used as environment to render the

Re: [Piglit] [PATCH 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Dylan Baker 
Quoting Lionel Landwerlin (2016-11-09 03:39:33)
> This tests that the compiler accepts the new layout formats introduced by
> NV_image_formats and also it's correct interaction with the
> EXT_texture_norm16 specification.
> 
> Signed-off-by: Lionel Landwerlin 
> ---
>  generated_tests/CMakeLists.txt |   4 +
>  .../gen_shader_image_nv_image_formats_tests.py | 230 
> +
>  2 files changed, 234 insertions(+)
>  create mode 100644 generated_tests/gen_shader_image_nv_image_formats_tests.py
> 
> diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
> index ff43af5..fd38afe 100644
> --- a/generated_tests/CMakeLists.txt
> +++ b/generated_tests/CMakeLists.txt
> @@ -163,6 +163,9 @@ piglit_make_generated_tests(
> shader_image_load_store_tests.list
> gen_shader_image_load_store_tests.py)
>  piglit_make_generated_tests(
> +   shader_image_nv_image_formats_tests.list
> +   gen_shader_image_nv_image_formats_tests.py)
> +piglit_make_generated_tests(
> variable_index_read_tests.list
> gen_variable_index_read_tests.py
> templates/gen_variable_index_read_tests/vs.shader_test.mako
> @@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
> conversion_fp64.list
> shader_precision_tests.list
> shader_image_load_store_tests.list
> +   shader_image_nv_image_formats_tests.list
> variable_index_read_tests.list
> gen_extensions_defined.list
> vp-tex.list
> diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py 
> b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> new file mode 100644
> index 000..9b1861d
> --- /dev/null
> +++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> @@ -0,0 +1,230 @@
> +# coding=utf-8
> +#
> +# Copyright (C) 2016 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.

A Short docstring here explaining what the generator does wouldn't be amiss.
You could probably just copy your commit message.

> +
> +from __future__ import print_function, division, absolute_import
> +import os.path
> +from mako.template import Template
> +from textwrap import dedent

please sort these as:
__future__
python builtins

3rd party modules

local modules

(in this case mako is the only 3rd party module, everything else is correct.)

> +
> +from modules import utils
> +
> +
> +def gen_header(status, norm16):
> +"""
> +Generate a GLSL program header.

Most of piglit puts the """ and the first lone of the docstring on the same line

> +
> +Generate header code for ARB_shader_image_load_store GLSL parser
> +tests that are expected to give status as result.
> +"""
> +return dedent("""
> +/*
> + * [config]
> + * expect_result: {0}
> + * glsl_version: 3.10 es
> + * require_extensions: GL_NV_image_formats {1}
> + * [end config]
> + */
> +#version 310 es
> +#extension GL_NV_image_formats : enable
> +""".format(status, "GL_EXT_texture_norm16" if norm16 else 
> "!GL_EXT_texture_norm16"))

This line is really long

> +
> +
> +def gen(name, src, tests):
> +"""
> +Expand a source template for the provided list of test definitions.
> +
> +Generate a GLSL parser test for each of the elements of the
> +'tests' iterable, each of them should be a dictionary of
> +definitions that will be used as environment to render the source
> +template.
> +
> +The file name of each test will be the concatenation of the 'name'
> +argument with the 'name' item from the respective test dictionary.
> +"""
> +template = Template(dedent(src))
> +
> +for t in product([{'name': name}], tests):
> +filename = os.path

Re: [Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

2016-11-09 Thread Francisco Jerez 
Marek Olšák  writes:

> On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez  
> wrote:
>> This refactors the implementation of the various "fb" commands to be
>> part of a single 'if (parse_str(line, "fb ", ...)) {}' block in order
>> to make code-sharing easier among fb subcommands.  Will be more useful
>> when we start introducing additional fb subcommands.
>> ---
>>  tests/shaders/shader_runner.c | 67 
>> ++-
>>  1 file changed, 40 insertions(+), 27 deletions(-)
>>
>> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
>> index ab2b907..4a2c807 100644
>> --- a/tests/shaders/shader_runner.c
>> +++ b/tests/shaders/shader_runner.c
>> @@ -140,7 +140,7 @@ static bool prog_in_use = false;
>>  static bool sso_in_use = false;
>>  static GLchar *prog_err_info = NULL;
>>  static GLuint vao = 0;
>> -static GLuint fbo = 0;
>> +static GLuint draw_fbo = 0;
>>  static GLint render_width, render_height;
>>
>>  static bool report_subtests = false;
>> @@ -2959,13 +2959,16 @@ piglit_display(void)
>> do_enable_disable(rest, true);
>> } else if (sscanf(line, "depthfunc %31s", s) == 1) {
>> glDepthFunc(piglit_get_gl_enum_from_name(s));
>> -   } else if (sscanf(line, "fb tex 2d %d", &tex) == 1) {
>> -   GLenum status;
>> +   } else if (parse_str(line, "fb ", &rest)) {
>> +   GLuint fbo = 0;
>
> Wrong indentation. (it can lead to buggy code)
>

Yeah, the whole block is intentionally indented one tab to the left,
it's cleaned up in the next commit.

>>
>> -   if (fbo == 0) {
>> -   glGenFramebuffers(1, &fbo);
>> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>> -   }
>> +   if (parse_str(rest, "tex 2d ", &rest)) {
>> +   glGenFramebuffers(1, &fbo);
>> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>> +
>> +   REQUIRE(parse_int(rest, &tex, &rest),
>> +   "Framebuffer binding command not "
>> +   "understood at: %s\n", rest);
>>
>> glFramebufferTexture2D(GL_FRAMEBUFFER,
>>GL_COLOR_ATTACHMENT0,
>> @@ -2976,21 +2979,12 @@ piglit_display(void)
>> piglit_report_result(PIGLIT_FAIL);
>> }
>>
>> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
>> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
>> -   fprintf(stderr, "incomplete fbo (status 
>> 0x%x)\n", status);
>> -   piglit_report_result(PIGLIT_FAIL);
>> -   }
>> -
>> -   render_width = get_texture_binding(tex)->width;
>> -   render_height = get_texture_binding(tex)->height;
>> -   } else if (sscanf(line, "fb tex layered %d", &tex) == 1) {
>> -   GLenum status;
>> +   w = get_texture_binding(tex)->width;
>> +   h = get_texture_binding(tex)->height;
>>
>> -   if (fbo == 0) {
>> -   glGenFramebuffers(1, &fbo);
>> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>> -   }
>> +   } else if (sscanf(rest, "tex layered %d", &tex) == 1) {
>> +   glGenFramebuffers(1, &fbo);
>> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>>
>> glFramebufferTexture(GL_FRAMEBUFFER,
>>  GL_COLOR_ATTACHMENT0,
>> @@ -3000,11 +2994,31 @@ piglit_display(void)
>> piglit_report_result(PIGLIT_FAIL);
>> }
>>
>> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
>> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
>> -   fprintf(stderr, "incomplete fbo (status 
>> 0x%x)\n", status);
>> -   piglit_report_result(PIGLIT_FAIL);
>> -   }
>> +   w = get_texture_binding(tex)->width;
>> +   h = get_texture_binding(tex)->height;
>> +
>> +   } else {
>> +   fprintf(stderr, "Unknown fb bind subcommand "
>> +   "\"%s\"\n", rest);
>> +   piglit_report_result(PIGLIT_FAIL);
>> +   }
>> +
>> +   const GLenum status = 
>> glCheckFramebufferStatus(GL_FRAMEBUFFER);
>> +   if (status != GL_FRAMEBUFFER_COMPLETE) {
>> +   fprintf(stderr, "incomplete fbo (status 0x%x)\n", 
>> status);
>> +   piglit_report_result(PIGLIT_FAIL);
>> +

Re: [Piglit] [PATCH v2 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Ilia Mirkin 
On Wed, Nov 9, 2016 at 12:15 PM, Lionel Landwerlin
 wrote:
> This tests that the compiler accepts the new layout formats introduced by
> NV_image_formats and also it's correct interaction with the
> EXT_texture_norm16 specification.
>
> v2: Only disallow normalized formats when EXT_texture_norm16 is not
> available (Ilia)
>
> Signed-off-by: Lionel Landwerlin 
> ---
>  generated_tests/CMakeLists.txt |   4 +
>  .../gen_shader_image_nv_image_formats_tests.py | 239 
> +
>  2 files changed, 243 insertions(+)
>  create mode 100644 generated_tests/gen_shader_image_nv_image_formats_tests.py
>
> diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
> index ff43af5..fd38afe 100644
> --- a/generated_tests/CMakeLists.txt
> +++ b/generated_tests/CMakeLists.txt
> @@ -163,6 +163,9 @@ piglit_make_generated_tests(
> shader_image_load_store_tests.list
> gen_shader_image_load_store_tests.py)
>  piglit_make_generated_tests(
> +   shader_image_nv_image_formats_tests.list
> +   gen_shader_image_nv_image_formats_tests.py)
> +piglit_make_generated_tests(
> variable_index_read_tests.list
> gen_variable_index_read_tests.py
> templates/gen_variable_index_read_tests/vs.shader_test.mako
> @@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
> conversion_fp64.list
> shader_precision_tests.list
> shader_image_load_store_tests.list
> +   shader_image_nv_image_formats_tests.list
> variable_index_read_tests.list
> gen_extensions_defined.list
> vp-tex.list
> diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py 
> b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> new file mode 100644
> index 000..de6d244
> --- /dev/null
> +++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> @@ -0,0 +1,239 @@
> +# coding=utf-8
> +#
> +# Copyright (C) 2016 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.
> +
> +from __future__ import print_function, division, absolute_import
> +import os.path
> +from mako.template import Template
> +from textwrap import dedent
> +
> +from modules import utils
> +
> +
> +def gen_header(status, norm16):
> +"""
> +Generate a GLSL program header.
> +
> +Generate header code for ARB_shader_image_load_store GLSL parser
> +tests that are expected to give status as result.
> +"""
> +return dedent("""
> +/*
> + * [config]
> + * expect_result: {0}
> + * glsl_version: 3.10 es
> + * require_extensions: GL_NV_image_formats {1}
> + * [end config]
> + */
> +#version 310 es
> +#extension GL_NV_image_formats : enable

I'd sleep better at night with a :require here rather than :enable.

> +""".format(status, "GL_EXT_texture_norm16" if norm16 else 
> "!GL_EXT_texture_norm16"))
> +
> +
> +def gen(name, src, tests):
> +"""
> +Expand a source template for the provided list of test definitions.
> +
> +Generate a GLSL parser test for each of the elements of the
> +'tests' iterable, each of them should be a dictionary of
> +definitions that will be used as environment to render the source
> +template.
> +
> +The file name of each test will be the concatenation of the 'name'
> +argument with the 'name' item from the respective test dictionary.
> +"""
> +template = Template(dedent(src))
> +
> +for t in product([{'name': name}], tests):
> +filename = os.path.join('spec',
> +'nv_image_formats',
> +'compiler',
> +'{0}.{1}'.format(t['name'],
> +

[Piglit] [PATCH v2 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Lionel Landwerlin 
This tests that the compiler accepts the new layout formats introduced by
NV_image_formats and also it's correct interaction with the
EXT_texture_norm16 specification.

v2: Only disallow normalized formats when EXT_texture_norm16 is not
available (Ilia)

Signed-off-by: Lionel Landwerlin 
---
 generated_tests/CMakeLists.txt |   4 +
 .../gen_shader_image_nv_image_formats_tests.py | 239 +
 2 files changed, 243 insertions(+)
 create mode 100644 generated_tests/gen_shader_image_nv_image_formats_tests.py

diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
index ff43af5..fd38afe 100644
--- a/generated_tests/CMakeLists.txt
+++ b/generated_tests/CMakeLists.txt
@@ -163,6 +163,9 @@ piglit_make_generated_tests(
shader_image_load_store_tests.list
gen_shader_image_load_store_tests.py)
 piglit_make_generated_tests(
+   shader_image_nv_image_formats_tests.list
+   gen_shader_image_nv_image_formats_tests.py)
+piglit_make_generated_tests(
variable_index_read_tests.list
gen_variable_index_read_tests.py
templates/gen_variable_index_read_tests/vs.shader_test.mako
@@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
conversion_fp64.list
shader_precision_tests.list
shader_image_load_store_tests.list
+   shader_image_nv_image_formats_tests.list
variable_index_read_tests.list
gen_extensions_defined.list
vp-tex.list
diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py 
b/generated_tests/gen_shader_image_nv_image_formats_tests.py
new file mode 100644
index 000..de6d244
--- /dev/null
+++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
@@ -0,0 +1,239 @@
+# coding=utf-8
+#
+# Copyright (C) 2016 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.
+
+from __future__ import print_function, division, absolute_import
+import os.path
+from mako.template import Template
+from textwrap import dedent
+
+from modules import utils
+
+
+def gen_header(status, norm16):
+"""
+Generate a GLSL program header.
+
+Generate header code for ARB_shader_image_load_store GLSL parser
+tests that are expected to give status as result.
+"""
+return dedent("""
+/*
+ * [config]
+ * expect_result: {0}
+ * glsl_version: 3.10 es
+ * require_extensions: GL_NV_image_formats {1}
+ * [end config]
+ */
+#version 310 es
+#extension GL_NV_image_formats : enable
+""".format(status, "GL_EXT_texture_norm16" if norm16 else 
"!GL_EXT_texture_norm16"))
+
+
+def gen(name, src, tests):
+"""
+Expand a source template for the provided list of test definitions.
+
+Generate a GLSL parser test for each of the elements of the
+'tests' iterable, each of them should be a dictionary of
+definitions that will be used as environment to render the source
+template.
+
+The file name of each test will be the concatenation of the 'name'
+argument with the 'name' item from the respective test dictionary.
+"""
+template = Template(dedent(src))
+
+for t in product([{'name': name}], tests):
+filename = os.path.join('spec',
+'nv_image_formats',
+'compiler',
+'{0}.{1}'.format(t['name'],
+ t['shader_stage']))
+print(filename)
+
+dirname = os.path.dirname(filename)
+utils.safe_makedirs(dirname)
+
+with open(filename, 'w') as f:
+f.write(template.render(header = gen_header, **t))
+
+
+shader_stages = [
+{'shader_stage': 'frag'},
+{'shader_stage': 'vert'}
+]
+
+
+image_types = [
+{
+

[Piglit] [PATCH v2 1/2] gen_extensions_defined.py: add NV_image_formats

2016-11-09 Thread Lionel Landwerlin 
This extensions is written against OpenGL ES 3.1 and 3.10 of the OpenGL ES
Shading Language specification.

v2: Fix missing 'GL_' prefix (Ilia)

Signed-off-by: Lionel Landwerlin 
---
 generated_tests/gen_extensions_defined.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/generated_tests/gen_extensions_defined.py 
b/generated_tests/gen_extensions_defined.py
index d42e6ad..3b34443 100644
--- a/generated_tests/gen_extensions_defined.py
+++ b/generated_tests/gen_extensions_defined.py
@@ -127,6 +127,7 @@ EXTENSIONS = [
 ("GL_EXT_texture_buffer", "310 es"),
 ("GL_OES_texture_buffer", "310 es"),
 ("GL_EXT_clip_cull_distance", "300 es"),
+("GL_NV_image_formats", "310 es"),
 ]
 EXTENSIONS = [(n, glsl.Version(v)) for n, v in EXTENSIONS]

--
2.10.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH] dir-locals.el: Adds whitespace support

2016-11-09 Thread Andres Gomez 
On Tue, 2016-11-08 at 12:59 -0500, Ilia Mirkin wrote:
> Could you update the change description to say precisely what each of
> these things do? It's hard to review otherwise since I have to go out
> and look up the things I don't know or have since forgotten.

Done and sent again for review.

I hope the changelog is explanatory enough ☺

> 
>   -ilia
> 
> On Sat, Oct 22, 2016 at 5:19 PM, Andres Gomez  wrote:
> > Provides support for highlighting incorrect indentation.
> > 
> > v2: python-mode inherits from prog-mode
> > v3: Removed too long lines trail highlighting, as suggested by Ilia
> > Mirkin.
> > 
> > Signed-off-by: Andres Gomez 
> > ---
> >  .dir-locals.el | 17 +
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> > 
> > diff --git a/.dir-locals.el b/.dir-locals.el
> > index 3bdca17..2a37aaf 100644
> > --- a/.dir-locals.el
> > +++ b/.dir-locals.el
> > @@ -1,11 +1,20 @@
> >  ((nil . ((indent-tabs-mode . t)
> >  (tab-width . 8)
> > -(show-trailing-whitespace . t)))
> > +(show-trailing-whitespace . t)
> > +(whitespace-style face indentation)
> > +(whitespace-line-column . 79)))
> >   (prog-mode .
> > -   ((c-file-style . "linux")))
> > +   ((c-file-style . "linux")
> > +(eval ignore-errors
> > +  (require 'whitespace)
> > +  (whitespace-mode 1
> >   (cmake-mode .
> > -((cmake-tab-width . 8)))
> > +((cmake-tab-width . 8)
> > + (eval ignore-errors
> > +   (require 'whitespace)
> > +   (whitespace-mode 1
> >   (python-mode .
> >   ((indent-tabs-mode . nil)
> > -  (tab-width . 4)))
> > +  (tab-width . 4)
> > +  (whitespace-line-column . 80)))
> >   )
> > --
> > 2.9.3
> > 
> 
> 
> 
-- 
Br,

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

Re: [Piglit] [PATCH] dir-locals.el: Adds White Space support

2016-11-09 Thread Ilia Mirkin 
Acked-by: Ilia Mirkin 

I won't bother asking why one whitespace-line-column is set to 79 and
the other to 80. I'm sure the answer would just disappoint me.

On Wed, Nov 9, 2016 at 11:29 AM, Andres Gomez  wrote:
> The White Space package, which is available since GNU Emacs 22, is
> loaded and activated locally in prog-mode and cmake-mode.
>
> Also, using White Space variables, we set highlighting through faces
> on wrong indentation and the maximum length of a coding line.
>
> Notice that:
>  - The highlighting for the characters beyond the set length of a
>coding line is not activated by default, only for wrong
>indentations.
>  - If the White Space package is not available, errors on loading or
>activation are ignored.
>  - If the White Space mode is not activated the set variables would
>not have any effect.
>
> v2: python-mode inherits from prog-mode
> v3: Removed too long lines trail highlighting, as suggested by Ilia
> Mirkin.
>
> Signed-off-by: Andres Gomez 
> ---
>  .dir-locals.el | 17 +
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/.dir-locals.el b/.dir-locals.el
> index 3bdca17..2a37aaf 100644
> --- a/.dir-locals.el
> +++ b/.dir-locals.el
> @@ -1,11 +1,20 @@
>  ((nil . ((indent-tabs-mode . t)
>  (tab-width . 8)
> -(show-trailing-whitespace . t)))
> +(show-trailing-whitespace . t)
> +(whitespace-style face indentation)
> +(whitespace-line-column . 79)))
>   (prog-mode .
> -   ((c-file-style . "linux")))
> +   ((c-file-style . "linux")
> +(eval ignore-errors
> +  (require 'whitespace)
> +  (whitespace-mode 1
>   (cmake-mode .
> -((cmake-tab-width . 8)))
> +((cmake-tab-width . 8)
> + (eval ignore-errors
> +   (require 'whitespace)
> +   (whitespace-mode 1
>   (python-mode .
>   ((indent-tabs-mode . nil)
> -  (tab-width . 4)))
> +  (tab-width . 4)
> +  (whitespace-line-column . 80)))
>   )
> --
> 2.9.3
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH] dir-locals.el: Adds White Space support

2016-11-09 Thread Andres Gomez 
The White Space package, which is available since GNU Emacs 22, is
loaded and activated locally in prog-mode and cmake-mode.

Also, using White Space variables, we set highlighting through faces
on wrong indentation and the maximum length of a coding line.

Notice that:
 - The highlighting for the characters beyond the set length of a
   coding line is not activated by default, only for wrong
   indentations.
 - If the White Space package is not available, errors on loading or
   activation are ignored.
 - If the White Space mode is not activated the set variables would
   not have any effect.

v2: python-mode inherits from prog-mode
v3: Removed too long lines trail highlighting, as suggested by Ilia
Mirkin.

Signed-off-by: Andres Gomez 
---
 .dir-locals.el | 17 +
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/.dir-locals.el b/.dir-locals.el
index 3bdca17..2a37aaf 100644
--- a/.dir-locals.el
+++ b/.dir-locals.el
@@ -1,11 +1,20 @@
 ((nil . ((indent-tabs-mode . t)
 (tab-width . 8)
-(show-trailing-whitespace . t)))
+(show-trailing-whitespace . t)
+(whitespace-style face indentation)
+(whitespace-line-column . 79)))
  (prog-mode .
-   ((c-file-style . "linux")))
+   ((c-file-style . "linux")
+(eval ignore-errors
+  (require 'whitespace)
+  (whitespace-mode 1
  (cmake-mode .
-((cmake-tab-width . 8)))
+((cmake-tab-width . 8)
+ (eval ignore-errors
+   (require 'whitespace)
+   (whitespace-mode 1
  (python-mode .
  ((indent-tabs-mode . nil)
-  (tab-width . 4)))
+  (tab-width . 4)
+  (whitespace-line-column . 80)))
  )
-- 
2.9.3

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

[Piglit] [Patch 1/1] texelFetch, textureSize: replace deprecated gl_FragColor

2016-11-09 Thread Sebastian Olender 
According to GLSLLangSpec: Use of gl_FragData and gl_FragColor
is deprecated in version 1.3, using user-defined out instead.
---
 tests/texturing/shaders/texelFetch.c  | 9 ++---
 tests/texturing/shaders/textureSize.c | 9 ++---
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/tests/texturing/shaders/texelFetch.c 
b/tests/texturing/shaders/texelFetch.c
index 0291a12..2e1d223 100644
--- a/tests/texturing/shaders/texelFetch.c
+++ b/tests/texturing/shaders/texelFetch.c
@@ -627,9 +627,10 @@ generate_GLSL(enum shader_target test_stage)
 "#version %d\n"
 "flat in %s color;\n"
 "uniform vec4 divisor;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
-"gl_FragColor = vec4(color)/divisor;\n"
+"fragColor = vec4(color)/divisor;\n"
 "}\n",
 shader_version,
 sampler.return_type);
@@ -676,9 +677,10 @@ generate_GLSL(enum shader_target test_stage)
 "#version %d\n"
 "flat in %s color;\n"
 "uniform vec4 divisor;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
-"gl_FragColor = vec4(color)/divisor;\n"
+"fragColor = vec4(color)/divisor;\n"
 "}\n",
 shader_version,
 sampler.return_type);
@@ -703,10 +705,11 @@ generate_GLSL(enum shader_target test_stage)
 "flat in ivec4 tc;\n"
 "uniform vec4 divisor;\n"
 "uniform %s tex;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
 "vec4 color = texelFetch%s(tex, ivec%d(tc)%s%s);\n"
-"gl_FragColor = color/divisor;\n"
+"fragColor = color/divisor;\n"
 "}\n",
 shader_version,
 has_samples() ? "#extension 
GL_ARB_texture_multisample: require" : "",
diff --git a/tests/texturing/shaders/textureSize.c 
b/tests/texturing/shaders/textureSize.c
index c02c566..ac3df04 100644
--- a/tests/texturing/shaders/textureSize.c
+++ b/tests/texturing/shaders/textureSize.c
@@ -284,9 +284,10 @@ generate_GLSL(enum shader_target test_stage)
 "#define ivec1 int\n"
 "#define vec1 float\n"
 "flat in ivec%d size;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
-"gl_FragColor = vec4(0.01 * size,%s 1);\n"
+"fragColor = vec4(0.01 * size,%s 1);\n"
 "}\n",
 shader_version, size, zeroes[3 - size]);
break;
@@ -325,9 +326,10 @@ generate_GLSL(enum shader_target test_stage)
 "#define ivec1 int\n"
 "#define vec1 float\n"
 "flat in ivec%d size;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
-"gl_FragColor = vec4(0.01 * size,%s 1);\n"
+"fragColor = vec4(0.01 * size,%s 1);\n"
 "}\n",
 shader_version, size, zeroes[3 - size]);
break;
@@ -345,10 +347,11 @@ generate_GLSL(enum shader_target test_stage)
 "#define ivec1 int\n"
 "uniform int lod;\n"
 "uniform %s tex;\n"
+"out vec4 fragColor;\n"
 "void main()\n"
 "{\n"
 "ivec%d size = textureSize(tex%s);\n"
-"gl_FragColor = vec4(0.01 * size,%s 1);\n"
+"fragColor = vec4(0.01 * size,%s 1);\n"
 "}\n",
 shader_version, extension, sampler.name, size, lod_arg,
 zeroes[3 - size]);
-- 
2.7.4

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

Re: [Piglit] [PATCH 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Ilia Mirkin 
The extra ext is only required for unorm and snorm formats, not all 16-bit
ones.

On Nov 9, 2016 6:39 AM, "Lionel Landwerlin" 
wrote:

> This tests that the compiler accepts the new layout formats introduced by
> NV_image_formats and also it's correct interaction with the
> EXT_texture_norm16 specification.
>
> Signed-off-by: Lionel Landwerlin 
> ---
>  generated_tests/CMakeLists.txt |   4 +
>  .../gen_shader_image_nv_image_formats_tests.py | 230
> +
>  2 files changed, 234 insertions(+)
>  create mode 100644 generated_tests/gen_shader_
> image_nv_image_formats_tests.py
>
> diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.
> txt
> index ff43af5..fd38afe 100644
> --- a/generated_tests/CMakeLists.txt
> +++ b/generated_tests/CMakeLists.txt
> @@ -163,6 +163,9 @@ piglit_make_generated_tests(
> shader_image_load_store_tests.list
> gen_shader_image_load_store_tests.py)
>  piglit_make_generated_tests(
> +   shader_image_nv_image_formats_tests.list
> +   gen_shader_image_nv_image_formats_tests.py)
> +piglit_make_generated_tests(
> variable_index_read_tests.list
> gen_variable_index_read_tests.py
> templates/gen_variable_index_read_tests/vs.shader_test.mako
> @@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
> conversion_fp64.list
> shader_precision_tests.list
> shader_image_load_store_tests.list
> +   shader_image_nv_image_formats_tests.list
> variable_index_read_tests.list
> gen_extensions_defined.list
> vp-tex.list
> diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py
> b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> new file mode 100644
> index 000..9b1861d
> --- /dev/null
> +++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
> @@ -0,0 +1,230 @@
> +# coding=utf-8
> +#
> +# Copyright (C) 2016 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.
> +
> +from __future__ import print_function, division, absolute_import
> +import os.path
> +from mako.template import Template
> +from textwrap import dedent
> +
> +from modules import utils
> +
> +
> +def gen_header(status, norm16):
> +"""
> +Generate a GLSL program header.
> +
> +Generate header code for ARB_shader_image_load_store GLSL parser
> +tests that are expected to give status as result.
> +"""
> +return dedent("""
> +/*
> + * [config]
> + * expect_result: {0}
> + * glsl_version: 3.10 es
> + * require_extensions: GL_NV_image_formats {1}
> + * [end config]
> + */
> +#version 310 es
> +#extension GL_NV_image_formats : enable
> +""".format(status, "GL_EXT_texture_norm16" if norm16 else
> "!GL_EXT_texture_norm16"))
> +
> +
> +def gen(name, src, tests):
> +"""
> +Expand a source template for the provided list of test definitions.
> +
> +Generate a GLSL parser test for each of the elements of the
> +'tests' iterable, each of them should be a dictionary of
> +definitions that will be used as environment to render the source
> +template.
> +
> +The file name of each test will be the concatenation of the 'name'
> +argument with the 'name' item from the respective test dictionary.
> +"""
> +template = Template(dedent(src))
> +
> +for t in product([{'name': name}], tests):
> +filename = os.path.join('spec',
> +'nv_image_formats',
> +'compiler',
> +'{0}.{1}'.format(t['name'],
> + t['shader_stage']))
> +print(filename)
> +
> +dirname = o

Re: [Piglit] [PATCH 1/2] gen_extensions_defined.py: add NV_image_formats

2016-11-09 Thread Ilia Mirkin 
On Nov 9, 2016 6:39 AM, "Lionel Landwerlin" 
wrote:
>
> This extensions is written against OpenGL ES 3.1 and 3.10 of the OpenGL ES
> Shading Language specification.
>
> Signed-off-by: Lionel Landwerlin 
> ---
>  generated_tests/gen_extensions_defined.py | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/generated_tests/gen_extensions_defined.py
b/generated_tests/gen_extensions_defined.py
> index d42e6ad..42a8791 100644
> --- a/generated_tests/gen_extensions_defined.py
> +++ b/generated_tests/gen_extensions_defined.py
> @@ -127,6 +127,7 @@ EXTENSIONS = [
>  ("GL_EXT_texture_buffer", "310 es"),
>  ("GL_OES_texture_buffer", "310 es"),
>  ("GL_EXT_clip_cull_distance", "300 es"),
> +("NV_image_formats", "310 es"),

GL_NV_...

>  ]
>  EXTENSIONS = [(n, glsl.Version(v)) for n, v in EXTENSIONS]
>
> --
> 2.10.2
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Re: [Piglit] [PATCH 06/18] shader_runner: Refactor handling of fb commands.

2016-11-09 Thread Marek Olšák 
On Wed, Oct 19, 2016 at 1:36 AM, Francisco Jerez  wrote:
> This refactors the implementation of the various "fb" commands to be
> part of a single 'if (parse_str(line, "fb ", ...)) {}' block in order
> to make code-sharing easier among fb subcommands.  Will be more useful
> when we start introducing additional fb subcommands.
> ---
>  tests/shaders/shader_runner.c | 67 
> ++-
>  1 file changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/tests/shaders/shader_runner.c b/tests/shaders/shader_runner.c
> index ab2b907..4a2c807 100644
> --- a/tests/shaders/shader_runner.c
> +++ b/tests/shaders/shader_runner.c
> @@ -140,7 +140,7 @@ static bool prog_in_use = false;
>  static bool sso_in_use = false;
>  static GLchar *prog_err_info = NULL;
>  static GLuint vao = 0;
> -static GLuint fbo = 0;
> +static GLuint draw_fbo = 0;
>  static GLint render_width, render_height;
>
>  static bool report_subtests = false;
> @@ -2959,13 +2959,16 @@ piglit_display(void)
> do_enable_disable(rest, true);
> } else if (sscanf(line, "depthfunc %31s", s) == 1) {
> glDepthFunc(piglit_get_gl_enum_from_name(s));
> -   } else if (sscanf(line, "fb tex 2d %d", &tex) == 1) {
> -   GLenum status;
> +   } else if (parse_str(line, "fb ", &rest)) {
> +   GLuint fbo = 0;

Wrong indentation. (it can lead to buggy code)

>
> -   if (fbo == 0) {
> -   glGenFramebuffers(1, &fbo);
> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> -   }
> +   if (parse_str(rest, "tex 2d ", &rest)) {
> +   glGenFramebuffers(1, &fbo);
> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> +
> +   REQUIRE(parse_int(rest, &tex, &rest),
> +   "Framebuffer binding command not "
> +   "understood at: %s\n", rest);
>
> glFramebufferTexture2D(GL_FRAMEBUFFER,
>GL_COLOR_ATTACHMENT0,
> @@ -2976,21 +2979,12 @@ piglit_display(void)
> piglit_report_result(PIGLIT_FAIL);
> }
>
> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
> -   fprintf(stderr, "incomplete fbo (status 
> 0x%x)\n", status);
> -   piglit_report_result(PIGLIT_FAIL);
> -   }
> -
> -   render_width = get_texture_binding(tex)->width;
> -   render_height = get_texture_binding(tex)->height;
> -   } else if (sscanf(line, "fb tex layered %d", &tex) == 1) {
> -   GLenum status;
> +   w = get_texture_binding(tex)->width;
> +   h = get_texture_binding(tex)->height;
>
> -   if (fbo == 0) {
> -   glGenFramebuffers(1, &fbo);
> -   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
> -   }
> +   } else if (sscanf(rest, "tex layered %d", &tex) == 1) {
> +   glGenFramebuffers(1, &fbo);
> +   glBindFramebuffer(GL_FRAMEBUFFER, fbo);
>
> glFramebufferTexture(GL_FRAMEBUFFER,
>  GL_COLOR_ATTACHMENT0,
> @@ -3000,11 +2994,31 @@ piglit_display(void)
> piglit_report_result(PIGLIT_FAIL);
> }
>
> -   status = glCheckFramebufferStatus(GL_FRAMEBUFFER);
> -   if (status != GL_FRAMEBUFFER_COMPLETE) {
> -   fprintf(stderr, "incomplete fbo (status 
> 0x%x)\n", status);
> -   piglit_report_result(PIGLIT_FAIL);
> -   }
> +   w = get_texture_binding(tex)->width;
> +   h = get_texture_binding(tex)->height;
> +
> +   } else {
> +   fprintf(stderr, "Unknown fb bind subcommand "
> +   "\"%s\"\n", rest);
> +   piglit_report_result(PIGLIT_FAIL);
> +   }
> +
> +   const GLenum status = 
> glCheckFramebufferStatus(GL_FRAMEBUFFER);
> +   if (status != GL_FRAMEBUFFER_COMPLETE) {
> +   fprintf(stderr, "incomplete fbo (status 0x%x)\n", 
> status);
> +   piglit_report_result(PIGLIT_FAIL);
> +   }
> +
> +   render_width = w;
> +   render_height = h;
> +
> +   /* Delete the previous draw FB in case
> +* it's no longer reachable.
> +*/
> +   if

Re: [Piglit] [PATCH] glsl-1.50: do not link interface-blocks-containing-unsized-arrays.geom

2016-11-09 Thread Iago Toral 
If nobody has comments about this, I'd like to push it tomorrow
together with the corresponding patch in Mesa.

Iago

On Fri, 2016-11-04 at 13:33 +0100, Iago Toral Quiroga wrote:
> Linking means that we will validate in/out blocks between consumer
> and
> producer stages. In this case it should fail because for geometry
> shaders
> piglit will include a dummy vertex shader that does not have a
> matching
> output block.
> 
> This has not been a problem until now because the linker was not
> detecting
> this situation properly and did not produce the expected link error.
> I am
> sending a patch to Mesa to fix this and once that lands this will
> start
> to fail without this patch.
> ---
>  .../compiler/interface-blocks-containing-unsized-arrays.geom   | 
> 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/spec/glsl-1.50/compiler/interface-blocks-
> containing-unsized-arrays.geom b/tests/spec/glsl-
> 1.50/compiler/interface-blocks-containing-unsized-arrays.geom
> index 169122d..a11f291 100644
> --- a/tests/spec/glsl-1.50/compiler/interface-blocks-containing-
> unsized-arrays.geom
> +++ b/tests/spec/glsl-1.50/compiler/interface-blocks-containing-
> unsized-arrays.geom
> @@ -1,12 +1,17 @@
>  // [config]
>  // expect_result: pass
>  // glsl_version: 1.50
> -// check_link: true
> +// check_link: false
>  // [end config]
>  //
>  // Test that an interface block may contain members which are
> unsized
>  // arrays.  Both GLSL 1.10 and GLSL 1.20 style array declarations
> are
>  // tested.
> +//
> +// We cannot check linking because with geometry shaders, piglit
> adds
> +// a dummy vertex shader and in this case, for linking to work, we
> need
> +// that dummy vertex shader to include a matching output block,
> which
> +// it doesn't.
>  
>  #version 150
>  layout(triangles) in;
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 2/2] generated_tests: new compiler tests for NV_image_formats

2016-11-09 Thread Lionel Landwerlin 
This tests that the compiler accepts the new layout formats introduced by
NV_image_formats and also it's correct interaction with the
EXT_texture_norm16 specification.

Signed-off-by: Lionel Landwerlin 
---
 generated_tests/CMakeLists.txt |   4 +
 .../gen_shader_image_nv_image_formats_tests.py | 230 +
 2 files changed, 234 insertions(+)
 create mode 100644 generated_tests/gen_shader_image_nv_image_formats_tests.py

diff --git a/generated_tests/CMakeLists.txt b/generated_tests/CMakeLists.txt
index ff43af5..fd38afe 100644
--- a/generated_tests/CMakeLists.txt
+++ b/generated_tests/CMakeLists.txt
@@ -163,6 +163,9 @@ piglit_make_generated_tests(
shader_image_load_store_tests.list
gen_shader_image_load_store_tests.py)
 piglit_make_generated_tests(
+   shader_image_nv_image_formats_tests.list
+   gen_shader_image_nv_image_formats_tests.py)
+piglit_make_generated_tests(
variable_index_read_tests.list
gen_variable_index_read_tests.py
templates/gen_variable_index_read_tests/vs.shader_test.mako
@@ -242,6 +245,7 @@ add_custom_target(gen-gl-tests
conversion_fp64.list
shader_precision_tests.list
shader_image_load_store_tests.list
+   shader_image_nv_image_formats_tests.list
variable_index_read_tests.list
gen_extensions_defined.list
vp-tex.list
diff --git a/generated_tests/gen_shader_image_nv_image_formats_tests.py 
b/generated_tests/gen_shader_image_nv_image_formats_tests.py
new file mode 100644
index 000..9b1861d
--- /dev/null
+++ b/generated_tests/gen_shader_image_nv_image_formats_tests.py
@@ -0,0 +1,230 @@
+# coding=utf-8
+#
+# Copyright (C) 2016 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.
+
+from __future__ import print_function, division, absolute_import
+import os.path
+from mako.template import Template
+from textwrap import dedent
+
+from modules import utils
+
+
+def gen_header(status, norm16):
+"""
+Generate a GLSL program header.
+
+Generate header code for ARB_shader_image_load_store GLSL parser
+tests that are expected to give status as result.
+"""
+return dedent("""
+/*
+ * [config]
+ * expect_result: {0}
+ * glsl_version: 3.10 es
+ * require_extensions: GL_NV_image_formats {1}
+ * [end config]
+ */
+#version 310 es
+#extension GL_NV_image_formats : enable
+""".format(status, "GL_EXT_texture_norm16" if norm16 else 
"!GL_EXT_texture_norm16"))
+
+
+def gen(name, src, tests):
+"""
+Expand a source template for the provided list of test definitions.
+
+Generate a GLSL parser test for each of the elements of the
+'tests' iterable, each of them should be a dictionary of
+definitions that will be used as environment to render the source
+template.
+
+The file name of each test will be the concatenation of the 'name'
+argument with the 'name' item from the respective test dictionary.
+"""
+template = Template(dedent(src))
+
+for t in product([{'name': name}], tests):
+filename = os.path.join('spec',
+'nv_image_formats',
+'compiler',
+'{0}.{1}'.format(t['name'],
+ t['shader_stage']))
+print(filename)
+
+dirname = os.path.dirname(filename)
+utils.safe_makedirs(dirname)
+
+with open(filename, 'w') as f:
+f.write(template.render(header = gen_header, **t))
+
+
+shader_stages = [
+{'shader_stage': 'frag'},
+{'shader_stage': 'vert'}
+]
+
+
+image_types = [
+{
+'name': '2d',
+'image_type': 'image2D',
+},
+{
+'name': '3d',
+

[Piglit] [PATCH 0/2] Tests for NV_image_formats

2016-11-09 Thread Lionel Landwerlin 
Hi all,

We do have some interest to have support for R8 images in GLES on
Intel hardware for ChromeOS [1] and it seems enabling the
NV_image_formats extension is the way to go.

I'm not quite sure how much this needs to be tested. Any direction
would be really helpful. For now, here are some basic tests to verify
that the compiler doesn't rejects new formats with the
NV_image_formats [2] extension.

Cheers,

[1]: https://bugs.freedesktop.org/show_bug.cgi?id=98480
[2]: https://www.khronos.org/registry/gles/extensions/NV/NV_image_formats.txt

Lionel Landwerlin (2):
  gen_extensions_defined.py: add NV_image_formats
  generated_tests: new compiler tests for NV_image_formats

 generated_tests/CMakeLists.txt |   4 +
 generated_tests/gen_extensions_defined.py  |   1 +
 .../gen_shader_image_nv_image_formats_tests.py | 230 +
 3 files changed, 235 insertions(+)
 create mode 100644 generated_tests/gen_shader_image_nv_image_formats_tests.py

--
2.10.2
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

[Piglit] [PATCH 1/2] gen_extensions_defined.py: add NV_image_formats

2016-11-09 Thread Lionel Landwerlin 
This extensions is written against OpenGL ES 3.1 and 3.10 of the OpenGL ES
Shading Language specification.

Signed-off-by: Lionel Landwerlin 
---
 generated_tests/gen_extensions_defined.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/generated_tests/gen_extensions_defined.py 
b/generated_tests/gen_extensions_defined.py
index d42e6ad..42a8791 100644
--- a/generated_tests/gen_extensions_defined.py
+++ b/generated_tests/gen_extensions_defined.py
@@ -127,6 +127,7 @@ EXTENSIONS = [
 ("GL_EXT_texture_buffer", "310 es"),
 ("GL_OES_texture_buffer", "310 es"),
 ("GL_EXT_clip_cull_distance", "300 es"),
+("NV_image_formats", "310 es"),
 ]
 EXTENSIONS = [(n, glsl.Version(v)) for n, v in EXTENSIONS]
 
-- 
2.10.2

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

Re: [Piglit] [PATCH] framework/test/shader_test: Fix MultiShaderRunner handling of directories

2016-11-09 Thread Marek Olšák 
Tested-by: Marek Olšák 

Marek

On Wed, Nov 9, 2016 at 12:51 AM, Dylan Baker  wrote:
> Currently MultiShaderRunner (used with --process-isolation false)
> expects each directory will contain either GL, GLES2, or GLES3 shaders.
> The barrier between GLES2 and GLES3 is somewhat artificial, since they
> can be promoted to GLES3. This patch allows GLES2 and GLES3 shaders to
> be run together but still enforces a separation between GLES and GL
> shaders.
>
> cc: Marek Olšák 
> Signed-off-by: Dylan Baker 
> ---
>  framework/test/shader_test.py | 16 +++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/framework/test/shader_test.py b/framework/test/shader_test.py
> index 447e8c0..1966149 100644
> --- a/framework/test/shader_test.py
> +++ b/framework/test/shader_test.py
> @@ -200,7 +200,21 @@ class MultiShaderTest(ReducedProcessMixin, 
> PiglitBaseTest):
>  subtest = os.path.basename(os.path.splitext(each)[0]).lower()
>
>  if prog is not None:
> -assert parser.prog == prog
> +# This allows mixing GLES2 and GLES3 shader test files
> +# together. Since GLES2 profiles can be promoted to GLES3, 
> this
> +# is fine.
> +if parser.prog != prog:
> +# Pylint can't figure out that prog is not None.
> +if 'gles' in parser.prog and 'gles' in prog:  # pylint: 
> disable=unsupported-membership-test
> +prog = max(parser.prog, prog)
> +else:
> +# The only way we can get here is if one is GLES and
> +# one is not, since there is only one desktop runner
> +# and so it will never fail the is parser.prog != 
> prog
> +# check
> +raise exceptions.PiglitInternalError(
> +'GLES and GL shaders in the same command!\n'
> +'Cannot pick a shader_runner binary!')
>  else:
>  prog = parser.prog
>
> --
> 2.10.2
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit