Re: [Piglit] Rounding mode in Piglit

2017-06-27 Thread Matt Turner
On Tue, Jun 27, 2017 at 3:43 AM, tournier.elie  wrote:
> Hello list,
>
> The GLSL spec said "The rounding mode cannot be set and is undefined"
> but in Piglit, we implicitly define a rounding mode.
> Indeed, in some test like
> "generated_tests/spec/arb_gpu_shader_fp64/execution/conversion/vert-conversion-explicit-double-uint.shader_test",
> we check if uint(1.9467) == 1.
> Depending on the rounding mode, we can have uint(1,9467) = 1 or
> uint(1,9467) = 2.
>
> Is it normal or should we change the tests to allow these 2 values?

The rounding mode is undefined, but the GLSL spec explicitly says (in
5.4.1 Conversion and Scalar Constructors):

When constructors are used to convert any floating-point type to an
integer type, the fractional part of the
floating-point value is dropped. It is undefined to convert a negative
floating-point value to an uint.

So uint(1.9467) is always 1u.
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] ext_packed_depth_stencil: set KHR_no_error compatibility

2017-06-27 Thread Timothy Arceri

On 28/06/17 00:20, Samuel Pitoiset wrote:


Signed-off-by: Samuel Pitoiset 
---
  tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c | 1 +
  tests/spec/ext_packed_depth_stencil/errors.c| 8 ++--
  tests/spec/ext_packed_depth_stencil/getteximage.c   | 1 +
  tests/spec/ext_packed_depth_stencil/readdrawpixels.c| 1 +
  tests/spec/ext_packed_depth_stencil/readpixels-24_8.c   | 1 +
  tests/spec/ext_packed_depth_stencil/texsubimage.c   | 1 +
  6 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c 
b/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
index 62f461a47..64c1893f6 100644
--- a/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
+++ b/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
@@ -53,6 +53,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
  #endif
  
  	config.window_visual = PIGLIT_GL_VISUAL_RGBA;

+   config.khr_no_error_support = PIGLIT_HAS_ERRORS;
  
  PIGLIT_GL_TEST_CONFIG_END
  
diff --git a/tests/spec/ext_packed_depth_stencil/errors.c b/tests/spec/ext_packed_depth_stencil/errors.c

index 74d40cca8..d1e51c1e1 100644
--- a/tests/spec/ext_packed_depth_stencil/errors.c
+++ b/tests/spec/ext_packed_depth_stencil/errors.c
@@ -33,6 +33,8 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.window_visual = (PIGLIT_GL_VISUAL_RGBA |
PIGLIT_GL_VISUAL_DEPTH |
PIGLIT_GL_VISUAL_STENCIL);
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
  PIGLIT_GL_TEST_CONFIG_END
  
  
@@ -93,8 +95,10 @@ piglit_init(int argc, char **argv)
  
  	piglit_require_extension("GL_EXT_packed_depth_stencil");
  
-	pass = test_drawpixels() && pass;

-   pass = test_readpixels() && pass;
+   if (!piglit_khr_no_error) {
+   pass = test_drawpixels() && pass;
+   pass = test_readpixels() && pass;
+   }


I'm not sure the 3rd test belongs here, but it's already there so:

Reviewed-by: Timothy Arceri 

  
  	/* The EXT_packed_depth_stencil spec says:

 *
diff --git a/tests/spec/ext_packed_depth_stencil/getteximage.c 
b/tests/spec/ext_packed_depth_stencil/getteximage.c
index 2302e6f82..40a12af4c 100644
--- a/tests/spec/ext_packed_depth_stencil/getteximage.c
+++ b/tests/spec/ext_packed_depth_stencil/getteximage.c
@@ -37,6 +37,7 @@
  PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 12;
config.window_visual = PIGLIT_GL_VISUAL_RGBA;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
  PIGLIT_GL_TEST_CONFIG_END
  
  
diff --git a/tests/spec/ext_packed_depth_stencil/readdrawpixels.c b/tests/spec/ext_packed_depth_stencil/readdrawpixels.c

index 10c8a6182..7be6664c5 100644
--- a/tests/spec/ext_packed_depth_stencil/readdrawpixels.c
+++ b/tests/spec/ext_packed_depth_stencil/readdrawpixels.c
@@ -33,6 +33,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.window_visual = (PIGLIT_GL_VISUAL_RGBA |
PIGLIT_GL_VISUAL_DEPTH |
PIGLIT_GL_VISUAL_STENCIL);
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
  PIGLIT_GL_TEST_CONFIG_END
  
  
diff --git a/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c b/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c

index 7fb9ad8ac..db71dba8b 100644
--- a/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c
+++ b/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c
@@ -37,6 +37,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
  
  	config.supports_gl_compat_version = 10;

config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
  
  PIGLIT_GL_TEST_CONFIG_END
  
diff --git a/tests/spec/ext_packed_depth_stencil/texsubimage.c b/tests/spec/ext_packed_depth_stencil/texsubimage.c

index 6b9a917ab..999d2e0bc 100644
--- a/tests/spec/ext_packed_depth_stencil/texsubimage.c
+++ b/tests/spec/ext_packed_depth_stencil/texsubimage.c
@@ -36,6 +36,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 13;
  
  	config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;

+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
  
  PIGLIT_GL_TEST_CONFIG_END
  


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


Re: [Piglit] [PATCH 3/3] framework: Exit if a filter removes all tests from a profile

2017-06-27 Thread Martin Peres



On 27/06/17 19:31, Dylan Baker wrote:

Quoting Martin Peres (2017-06-27 03:38:33)

On 22/06/17 22:12, Dylan Baker wrote:

Quoting Petri Latvala (2017-06-21 01:37:21)

On 06/20/2017 08:59 PM, Dylan Baker wrote:

Quoting Petri Latvala (2017-06-20 05:41:11)

On 04/13/2017 09:46 PM, Dylan Baker wrote:

Quoting Brian Paul (2017-04-12 13:04:59)

On 04/12/2017 11:55 AM, Dylan Baker wrote:

It doesn't makes sense to run if a user has removed all tests from a
selected profile, and currently if all tests are removed, then an
assertion will be hit in the backend that isn't extremely clear about
what went wrong. This should be much easier to understand.

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

diff --git a/framework/profile.py b/framework/profile.py
index 4604367e1..ce0b24ce8 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -389,6 +389,13 @@ def run(profiles, logger, backend, concurrency):
  profiles = [(p, list(p.itertests())) for p in profiles]
  log = LogManager(logger, sum(len(l) for _, l in profiles))

+# check that after the filters are run there are actually tests to run
+for p, l in profiles:
+if not l:
+raise exceptions.PiglitUserError(
+'After running filters there are no tests in '
+'profile "{}"'.format(p.name))

Bumped into an issue caused by this commit with IGT tests.

When the last test of a run never finishes and you attempt to

  piglit resume -n test-results-path

this exception is raised instead of the expected result of finishing up
the run.


--
Petri Latvala


That is expected with the command line you've supplied.

The -n/--no-retry option instructs piglit to not retry tests that started but
didn't finish, if you get the same behavior without the -n option that is a bug.

Dylan



What is the supported method then to loop resume until results.json.bz2
gets generated? Previous behaviour was that piglit reported that there's
nothing more to do and generated that.


--
Petri Latvala



I would use `piglit summary aggregate` to build the results.json.foo file.


Hi Dylan,

Petri is on vacation for some time, so let me provide a little more
background on why this patch is problematic for our use-case.

For automated IGT testing, we have a testlist and reboot the machine as
soon as we get a certain error condition or if the test takes more than
a specified timeout, which reboots the machine immediately (watchdog).

Once the machine has rebooted, we re-deploy the right kernel and resume
piglit testing, using the -n option. When the testing is over, piglit
generates the results.json.foo file and returns 0. At this point,
ezbench considers the execution as done and does its own things.

However, since this patch got introduced, if the last test got
interrupted, then piglit will raise the PiglitUserError exception which
is hard to interpret from outside as a way to say that all the tests
have been run and that we should be generating the report. This also
introduces an ill-tested codepath, so Petri and I were wondering why not
make it less confusing for the user by just realizing that there are no
tests left to run, and just generate the report at this point.

Any suggestion as to what we should be doing? Maybe we could only send
this error when no tests have been run already and there are no runs to
be run?

Thanks,
Martin


Hi Martin,

I really don't think we should remove the exception, the majority of piglit
users are not wrapping piglit in more infrastructure, I think at this point the
Intel kernel and mesa teams are the only ones doing so, and it does report a
real error.


Sure, but let's not make it harder for other companies to use piglit, 
especially since piglit is the prefered runner for IGT which is becoming 
less intel-specific (AMD and vc4).




I do think that it would work if `piglit resume` would essentially become
`piglit summary aggregate` if there are no tests left to run, since tests have
actually run and a result would be non-empty.


Yes, that is all we are asking and it would make wrapping into more 
infrastructure easier :)


Feel like cooking the patch or should I do it?

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


Re: [Piglit] [PATCH v3] gl30basic: add some extra suspected extension

2017-06-27 Thread Dylan Baker
Quoting Sandra Koroniewska (2017-06-27 06:52:02)
> WGL_EXT_SWAP_CONTROL is added for historical reasons.
> ---
>  tests/general/gl30basic.c | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/general/gl30basic.c b/tests/general/gl30basic.c
> index 2207787bb..d6bcd34ad 100644
> --- a/tests/general/gl30basic.c
> +++ b/tests/general/gl30basic.c
> @@ -95,12 +95,13 @@ test_extension_list(void)
>const GLubyte *ext = glGetStringi(GL_EXTENSIONS, k);
>if (0)
>   printf("Ext[%d] = %s\n", k, (char *) ext);
> -  if (!ext ||
> -  ext[0] != 'G' ||
> -  ext[1] != 'L' ||
> -  ext[2] != '_') {
> - printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
> - return PIGLIT_FAIL;
> +  if (!ext || strncmp(ext, "GL_", 3))
> +  {
> +  if (strncmp(ext, "WGL_EXT_swap_control", 20))
> +  {
> +  printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
> +  return PIGLIT_FAIL;
> +  }
>}

The logic here seems dubious to me. How can the inner if statement ever be true,
given that you've already checked that the extension string is empty or starts
with 'GL_'? Also, unless I'm misreading this the PIGLIT_FAIL will only be
generated if the extension is WGL_EXT_swap_control, which also seems wrong.

I think this should be:

if (!ext || strncmp(ext, "GL_", 3) || strncmp(ext, "WGL_EXT_swap_control", 20)) 
{
...
}

Dylan


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


Re: [Piglit] [PATCH 3/3] framework: Exit if a filter removes all tests from a profile

2017-06-27 Thread Dylan Baker
Quoting Martin Peres (2017-06-27 03:38:33)
> On 22/06/17 22:12, Dylan Baker wrote:
> > Quoting Petri Latvala (2017-06-21 01:37:21)
> >> On 06/20/2017 08:59 PM, Dylan Baker wrote:
> >>> Quoting Petri Latvala (2017-06-20 05:41:11)
>  On 04/13/2017 09:46 PM, Dylan Baker wrote:
> > Quoting Brian Paul (2017-04-12 13:04:59)
> >> On 04/12/2017 11:55 AM, Dylan Baker wrote:
> >>> It doesn't makes sense to run if a user has removed all tests from a
> >>> selected profile, and currently if all tests are removed, then an
> >>> assertion will be hit in the backend that isn't extremely clear about
> >>> what went wrong. This should be much easier to understand.
> >>>
> >>> Signed-off-by: Dylan Baker 
> >>> ---
> >>>  framework/profile.py | 7 +++
> >>>  1 file changed, 7 insertions(+)
> >>>
> >>> diff --git a/framework/profile.py b/framework/profile.py
> >>> index 4604367e1..ce0b24ce8 100644
> >>> --- a/framework/profile.py
> >>> +++ b/framework/profile.py
> >>> @@ -389,6 +389,13 @@ def run(profiles, logger, backend, concurrency):
> >>>  profiles = [(p, list(p.itertests())) for p in profiles]
> >>>  log = LogManager(logger, sum(len(l) for _, l in profiles))
> >>>
> >>> +# check that after the filters are run there are actually tests 
> >>> to run
> >>> +for p, l in profiles:
> >>> +if not l:
> >>> +raise exceptions.PiglitUserError(
> >>> +'After running filters there are no tests in '
> >>> +'profile "{}"'.format(p.name))
>  Bumped into an issue caused by this commit with IGT tests.
> 
>  When the last test of a run never finishes and you attempt to
> 
>   piglit resume -n test-results-path
> 
>  this exception is raised instead of the expected result of finishing up
>  the run.
> 
> 
>  -- 
>  Petri Latvala
> 
> >>> That is expected with the command line you've supplied.
> >>>
> >>> The -n/--no-retry option instructs piglit to not retry tests that started 
> >>> but
> >>> didn't finish, if you get the same behavior without the -n option that is 
> >>> a bug.
> >>>
> >>> Dylan
> >>
> >>
> >> What is the supported method then to loop resume until results.json.bz2
> >> gets generated? Previous behaviour was that piglit reported that there's
> >> nothing more to do and generated that.
> >>
> >>
> >> -- 
> >> Petri Latvala
> >>
> > 
> > I would use `piglit summary aggregate` to build the results.json.foo file.
> 
> Hi Dylan,
> 
> Petri is on vacation for some time, so let me provide a little more 
> background on why this patch is problematic for our use-case.
> 
> For automated IGT testing, we have a testlist and reboot the machine as 
> soon as we get a certain error condition or if the test takes more than 
> a specified timeout, which reboots the machine immediately (watchdog).
> 
> Once the machine has rebooted, we re-deploy the right kernel and resume 
> piglit testing, using the -n option. When the testing is over, piglit 
> generates the results.json.foo file and returns 0. At this point, 
> ezbench considers the execution as done and does its own things.
> 
> However, since this patch got introduced, if the last test got 
> interrupted, then piglit will raise the PiglitUserError exception which 
> is hard to interpret from outside as a way to say that all the tests 
> have been run and that we should be generating the report. This also 
> introduces an ill-tested codepath, so Petri and I were wondering why not 
> make it less confusing for the user by just realizing that there are no 
> tests left to run, and just generate the report at this point.
> 
> Any suggestion as to what we should be doing? Maybe we could only send 
> this error when no tests have been run already and there are no runs to 
> be run?
> 
> Thanks,
> Martin

Hi Martin,

I really don't think we should remove the exception, the majority of piglit
users are not wrapping piglit in more infrastructure, I think at this point the
Intel kernel and mesa teams are the only ones doing so, and it does report a
real error.

I do think that it would work if `piglit resume` would essentially become
`piglit summary aggregate` if there are no tests left to run, since tests have
actually run and a result would be non-empty.

Dylan


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


[Piglit] [PATCH] ext_packed_depth_stencil: set KHR_no_error compatibility

2017-06-27 Thread Samuel Pitoiset
Signed-off-by: Samuel Pitoiset 
---
 tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c | 1 +
 tests/spec/ext_packed_depth_stencil/errors.c| 8 ++--
 tests/spec/ext_packed_depth_stencil/getteximage.c   | 1 +
 tests/spec/ext_packed_depth_stencil/readdrawpixels.c| 1 +
 tests/spec/ext_packed_depth_stencil/readpixels-24_8.c   | 1 +
 tests/spec/ext_packed_depth_stencil/texsubimage.c   | 1 +
 6 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c 
b/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
index 62f461a47..64c1893f6 100644
--- a/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
+++ b/tests/spec/ext_packed_depth_stencil/depth-stencil-texture.c
@@ -53,6 +53,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
 #endif
 
config.window_visual = PIGLIT_GL_VISUAL_RGBA;
+   config.khr_no_error_support = PIGLIT_HAS_ERRORS;
 
 PIGLIT_GL_TEST_CONFIG_END
 
diff --git a/tests/spec/ext_packed_depth_stencil/errors.c 
b/tests/spec/ext_packed_depth_stencil/errors.c
index 74d40cca8..d1e51c1e1 100644
--- a/tests/spec/ext_packed_depth_stencil/errors.c
+++ b/tests/spec/ext_packed_depth_stencil/errors.c
@@ -33,6 +33,8 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.window_visual = (PIGLIT_GL_VISUAL_RGBA |
PIGLIT_GL_VISUAL_DEPTH |
PIGLIT_GL_VISUAL_STENCIL);
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
+
 PIGLIT_GL_TEST_CONFIG_END
 
 
@@ -93,8 +95,10 @@ piglit_init(int argc, char **argv)
 
piglit_require_extension("GL_EXT_packed_depth_stencil");
 
-   pass = test_drawpixels() && pass;
-   pass = test_readpixels() && pass;
+   if (!piglit_khr_no_error) {
+   pass = test_drawpixels() && pass;
+   pass = test_readpixels() && pass;
+   }
 
/* The EXT_packed_depth_stencil spec says:
 *
diff --git a/tests/spec/ext_packed_depth_stencil/getteximage.c 
b/tests/spec/ext_packed_depth_stencil/getteximage.c
index 2302e6f82..40a12af4c 100644
--- a/tests/spec/ext_packed_depth_stencil/getteximage.c
+++ b/tests/spec/ext_packed_depth_stencil/getteximage.c
@@ -37,6 +37,7 @@
 PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 12;
config.window_visual = PIGLIT_GL_VISUAL_RGBA;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
 PIGLIT_GL_TEST_CONFIG_END
 
 
diff --git a/tests/spec/ext_packed_depth_stencil/readdrawpixels.c 
b/tests/spec/ext_packed_depth_stencil/readdrawpixels.c
index 10c8a6182..7be6664c5 100644
--- a/tests/spec/ext_packed_depth_stencil/readdrawpixels.c
+++ b/tests/spec/ext_packed_depth_stencil/readdrawpixels.c
@@ -33,6 +33,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.window_visual = (PIGLIT_GL_VISUAL_RGBA |
PIGLIT_GL_VISUAL_DEPTH |
PIGLIT_GL_VISUAL_STENCIL);
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
 PIGLIT_GL_TEST_CONFIG_END
 
 
diff --git a/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c 
b/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c
index 7fb9ad8ac..db71dba8b 100644
--- a/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c
+++ b/tests/spec/ext_packed_depth_stencil/readpixels-24_8.c
@@ -37,6 +37,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
 
config.supports_gl_compat_version = 10;
config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
 
 PIGLIT_GL_TEST_CONFIG_END
 
diff --git a/tests/spec/ext_packed_depth_stencil/texsubimage.c 
b/tests/spec/ext_packed_depth_stencil/texsubimage.c
index 6b9a917ab..999d2e0bc 100644
--- a/tests/spec/ext_packed_depth_stencil/texsubimage.c
+++ b/tests/spec/ext_packed_depth_stencil/texsubimage.c
@@ -36,6 +36,7 @@ PIGLIT_GL_TEST_CONFIG_BEGIN
config.supports_gl_compat_version = 13;
 
config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
+   config.khr_no_error_support = PIGLIT_NO_ERRORS;
 
 PIGLIT_GL_TEST_CONFIG_END
 
-- 
2.13.2

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


Re: [Piglit] [PATCH v3] gl30basic: add some extra suspected extension

2017-06-27 Thread Brian Paul

On 06/27/2017 07:52 AM, Sandra Koroniewska wrote:

WGL_EXT_SWAP_CONTROL is added for historical reasons.
---
  tests/general/gl30basic.c | 13 +++--
  1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/general/gl30basic.c b/tests/general/gl30basic.c
index 2207787bb..d6bcd34ad 100644
--- a/tests/general/gl30basic.c
+++ b/tests/general/gl30basic.c
@@ -95,12 +95,13 @@ test_extension_list(void)
const GLubyte *ext = glGetStringi(GL_EXTENSIONS, k);
if (0)
   printf("Ext[%d] = %s\n", k, (char *) ext);
-  if (!ext ||
-  ext[0] != 'G' ||
-  ext[1] != 'L' ||
-  ext[2] != '_') {
- printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
- return PIGLIT_FAIL;
+  if (!ext || strncmp(ext, "GL_", 3))
+  {
+  if (strncmp(ext, "WGL_EXT_swap_control", 20))
+  {
+  printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
+  return PIGLIT_FAIL;
+  }
}
if (strchr((char *) ext, ' ')) {
   printf("%s: extension string [%d] contains a space: %s\n", Prog, k, 
ext);



The opening braces should go on the same line as the if statements.

I think I suggested last time to add a comment about why we're testing 
for WGL_EXT_swap_control.


-Brian

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


[Piglit] [PATCH v3] gl30basic: add some extra suspected extension

2017-06-27 Thread Sandra Koroniewska
WGL_EXT_SWAP_CONTROL is added for historical reasons.
---
 tests/general/gl30basic.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tests/general/gl30basic.c b/tests/general/gl30basic.c
index 2207787bb..d6bcd34ad 100644
--- a/tests/general/gl30basic.c
+++ b/tests/general/gl30basic.c
@@ -95,12 +95,13 @@ test_extension_list(void)
   const GLubyte *ext = glGetStringi(GL_EXTENSIONS, k);
   if (0)
  printf("Ext[%d] = %s\n", k, (char *) ext);
-  if (!ext ||
-  ext[0] != 'G' ||
-  ext[1] != 'L' ||
-  ext[2] != '_') {
- printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
- return PIGLIT_FAIL;
+  if (!ext || strncmp(ext, "GL_", 3))
+  {
+  if (strncmp(ext, "WGL_EXT_swap_control", 20))
+  {
+  printf("%s: bad extension string [%d]: %s\n", Prog, k, ext);
+  return PIGLIT_FAIL;
+  }
   }
   if (strchr((char *) ext, ' ')) {
  printf("%s: extension string [%d] contains a space: %s\n", Prog, k, 
ext);
-- 
2.11.0.windows.1

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


[Piglit] Rounding mode in Piglit

2017-06-27 Thread tournier.elie
Hello list,

The GLSL spec said "The rounding mode cannot be set and is undefined"
but in Piglit, we implicitly define a rounding mode.
Indeed, in some test like
"generated_tests/spec/arb_gpu_shader_fp64/execution/conversion/vert-conversion-explicit-double-uint.shader_test",
we check if uint(1.9467) == 1.
Depending on the rounding mode, we can have uint(1,9467) = 1 or
uint(1,9467) = 2.

Is it normal or should we change the tests to allow these 2 values?

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


Re: [Piglit] [PATCH 3/3] framework: Exit if a filter removes all tests from a profile

2017-06-27 Thread Martin Peres

On 22/06/17 22:12, Dylan Baker wrote:

Quoting Petri Latvala (2017-06-21 01:37:21)

On 06/20/2017 08:59 PM, Dylan Baker wrote:

Quoting Petri Latvala (2017-06-20 05:41:11)

On 04/13/2017 09:46 PM, Dylan Baker wrote:

Quoting Brian Paul (2017-04-12 13:04:59)

On 04/12/2017 11:55 AM, Dylan Baker wrote:

It doesn't makes sense to run if a user has removed all tests from a
selected profile, and currently if all tests are removed, then an
assertion will be hit in the backend that isn't extremely clear about
what went wrong. This should be much easier to understand.

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

diff --git a/framework/profile.py b/framework/profile.py
index 4604367e1..ce0b24ce8 100644
--- a/framework/profile.py
+++ b/framework/profile.py
@@ -389,6 +389,13 @@ def run(profiles, logger, backend, concurrency):
 profiles = [(p, list(p.itertests())) for p in profiles]
 log = LogManager(logger, sum(len(l) for _, l in profiles))

+# check that after the filters are run there are actually tests to run
+for p, l in profiles:
+if not l:
+raise exceptions.PiglitUserError(
+'After running filters there are no tests in '
+'profile "{}"'.format(p.name))

Bumped into an issue caused by this commit with IGT tests.

When the last test of a run never finishes and you attempt to

 piglit resume -n test-results-path

this exception is raised instead of the expected result of finishing up
the run.


--
Petri Latvala


That is expected with the command line you've supplied.

The -n/--no-retry option instructs piglit to not retry tests that started but
didn't finish, if you get the same behavior without the -n option that is a bug.

Dylan



What is the supported method then to loop resume until results.json.bz2
gets generated? Previous behaviour was that piglit reported that there's
nothing more to do and generated that.


--
Petri Latvala



I would use `piglit summary aggregate` to build the results.json.foo file.


Hi Dylan,

Petri is on vacation for some time, so let me provide a little more 
background on why this patch is problematic for our use-case.


For automated IGT testing, we have a testlist and reboot the machine as 
soon as we get a certain error condition or if the test takes more than 
a specified timeout, which reboots the machine immediately (watchdog).


Once the machine has rebooted, we re-deploy the right kernel and resume 
piglit testing, using the -n option. When the testing is over, piglit 
generates the results.json.foo file and returns 0. At this point, 
ezbench considers the execution as done and does its own things.


However, since this patch got introduced, if the last test got 
interrupted, then piglit will raise the PiglitUserError exception which 
is hard to interpret from outside as a way to say that all the tests 
have been run and that we should be generating the report. This also 
introduces an ill-tested codepath, so Petri and I were wondering why not 
make it less confusing for the user by just realizing that there are no 
tests left to run, and just generate the report at this point.


Any suggestion as to what we should be doing? Maybe we could only send 
this error when no tests have been run already and there are no runs to 
be run?


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


Re: [Piglit] [PATCH] fbo: set KHR_no_error compatibility for remaining fbo tests

2017-06-27 Thread Timothy Arceri

On 27/06/17 17:58, Samuel Pitoiset wrote:




On 06/27/2017 02:19 AM, Timothy Arceri wrote:

On 26/06/17 19:04, Samuel Pitoiset wrote:


Looks like you forgot to update fbo-storage-formats.c.


It's updated. Did you mean something else?


I meant, there is a GL_INVALID_ENUM which is not handled correctly, no?

Ok, I see it this time. Thanks.







With that fixed, patch is:

Reviewed-by: Samuel Pitoiset 

Thanks!

On 06/26/2017 08:00 AM, Timothy Arceri wrote:

---
  tests/fbo/fbo-1d.c |  1 +
  tests/fbo/fbo-3d.c |  1 +
  tests/fbo/fbo-alpha.c  |  1 +
  tests/fbo/fbo-alphatest-formats.c  |  1 +
  tests/fbo/fbo-alphatest-nocolor-ff.c   |  1 +
  tests/fbo/fbo-alphatest-nocolor.c  |  1 +
  tests/fbo/fbo-array.c  |  1 +
  tests/fbo/fbo-bind-renderbuffer.c  |  9 --
  tests/fbo/fbo-blending-formats.c   |  1 +
  tests/fbo/fbo-blit-d24s8.c |  1 +
  tests/fbo/fbo-blit-stretch.cpp |  1 +
  tests/fbo/fbo-blit.c   |  1 +
  tests/fbo/fbo-clear-formats.c  |  1 +
  tests/fbo/fbo-clearmipmap.c|  1 +
  tests/fbo/fbo-colormask-formats.c  |  1 +
  tests/fbo/fbo-copypix.c|  1 +
  tests/fbo/fbo-copyteximage-simple.c|  1 +
  tests/fbo/fbo-copyteximage.c   |  1 +
  tests/fbo/fbo-cubemap.c|  1 +
  tests/fbo/fbo-depth-array.c|  1 +
  tests/fbo/fbo-depth-tex1d.c|  1 +
  tests/fbo/fbo-depth.c  |  1 +
  tests/fbo/fbo-depthtex.c   |  1 +
  tests/fbo/fbo-deriv.c  |  1 +
  tests/fbo/fbo-draw-buffers-blend.c |  1 +
  tests/fbo/fbo-drawbuffers-arbfp.c  |  1 +
  tests/fbo/fbo-drawbuffers-blend-add.c  |  1 +
  tests/fbo/fbo-drawbuffers-fragcolor.c  |  1 +
  tests/fbo/fbo-drawbuffers-maxtargets.c |  1 +
  tests/fbo/fbo-drawbuffers.c|  1 +
  tests/fbo/fbo-drawbuffers2-blend.c |  1 +
  tests/fbo/fbo-drawbuffers2-colormask.c |  1 +
  tests/fbo/fbo-finish-deleted.c |  1 +
  tests/fbo/fbo-flushing-2.c |  1 +
  tests/fbo/fbo-flushing.c   |  1 +
  tests/fbo/fbo-fragcoord.c  |  1 +
  tests/fbo/fbo-fragcoord2.c |  1 +
  tests/fbo/fbo-generatemipmap-filtering.c   |  1 +
  tests/fbo/fbo-generatemipmap-formats.c |  1 +
  tests/fbo/fbo-generatemipmap-noimage.c |  1 +
  tests/fbo/fbo-generatemipmap-nonsquare.c   |  1 +
  tests/fbo/fbo-generatemipmap-npot.c|  1 +
  tests/fbo/fbo-generatemipmap-scissor.c |  1 +
  tests/fbo/fbo-generatemipmap-swizzle.c |  1 +
  tests/fbo/fbo-generatemipmap-viewport.c|  1 +
  tests/fbo/fbo-generatemipmap.c |  1 +
  .../fbo/fbo-getframebufferattachmentparameter-01.c | 36 
+-

  tests/fbo/fbo-gl_pointcoord.c  |  1 +
  tests/fbo/fbo-incomplete-invalid-texture.c |  1 +
  tests/fbo/fbo-incomplete-texture-01.c  |  1 +
  tests/fbo/fbo-incomplete-texture-02.c  |  1 +
  tests/fbo/fbo-incomplete-texture-03.c  |  1 +
  tests/fbo/fbo-incomplete-texture-04.c  |  1 +
  tests/fbo/fbo-incomplete.cpp   |  1 +
  tests/fbo/fbo-integer.c|  1 +
  tests/fbo/fbo-luminance-alpha.c|  1 +
  tests/fbo/fbo-maxsize.c|  1 +
  tests/fbo/fbo-mipmap-copypix.c |  1 +
  tests/fbo/fbo-mrt-alphatest.c  |  1 +
  tests/fbo/fbo-mrt-new-bind.c   |  1 +
  tests/fbo/fbo-nodepth-test.c   |  1 +
  tests/fbo/fbo-nostencil-test.c |  1 +
  tests/fbo/fbo-pbo-readpixels-small.c   |  1 +
  tests/fbo/fbo-readdrawpix.c|  1 +
  tests/fbo/fbo-readpixels-depth-formats.c   |  1 +
  tests/fbo/fbo-readpixels.c |  1 +
  tests/fbo/fbo-rg.c |  1 +
  tests/fbo/fbo-scissor-bitmap.c |  1 +
  tests/fbo/fbo-scissor-blit.c   |  1 +
  tests/fbo/fbo-srgb.c   |  1 +
  tests/fbo/fbo-stencil.c|  1 +
  tests/fbo/fbo-storage-completeness.c   |  5 +--
  tests/fbo/fbo-storage-formats.c|  5 +--
  tests/fbo/fbo-sys-blit.c   |  1 +
  tests/fbo/fbo-sys-sub-blit.c   |  

Re: [Piglit] [PATCH] fbo: set KHR_no_error compatibility for remaining fbo tests

2017-06-27 Thread Samuel Pitoiset



On 06/27/2017 02:19 AM, Timothy Arceri wrote:

On 26/06/17 19:04, Samuel Pitoiset wrote:


Looks like you forgot to update fbo-storage-formats.c.


It's updated. Did you mean something else?


I meant, there is a GL_INVALID_ENUM which is not handled correctly, no?





With that fixed, patch is:

Reviewed-by: Samuel Pitoiset 

Thanks!

On 06/26/2017 08:00 AM, Timothy Arceri wrote:

---
  tests/fbo/fbo-1d.c |  1 +
  tests/fbo/fbo-3d.c |  1 +
  tests/fbo/fbo-alpha.c  |  1 +
  tests/fbo/fbo-alphatest-formats.c  |  1 +
  tests/fbo/fbo-alphatest-nocolor-ff.c   |  1 +
  tests/fbo/fbo-alphatest-nocolor.c  |  1 +
  tests/fbo/fbo-array.c  |  1 +
  tests/fbo/fbo-bind-renderbuffer.c  |  9 --
  tests/fbo/fbo-blending-formats.c   |  1 +
  tests/fbo/fbo-blit-d24s8.c |  1 +
  tests/fbo/fbo-blit-stretch.cpp |  1 +
  tests/fbo/fbo-blit.c   |  1 +
  tests/fbo/fbo-clear-formats.c  |  1 +
  tests/fbo/fbo-clearmipmap.c|  1 +
  tests/fbo/fbo-colormask-formats.c  |  1 +
  tests/fbo/fbo-copypix.c|  1 +
  tests/fbo/fbo-copyteximage-simple.c|  1 +
  tests/fbo/fbo-copyteximage.c   |  1 +
  tests/fbo/fbo-cubemap.c|  1 +
  tests/fbo/fbo-depth-array.c|  1 +
  tests/fbo/fbo-depth-tex1d.c|  1 +
  tests/fbo/fbo-depth.c  |  1 +
  tests/fbo/fbo-depthtex.c   |  1 +
  tests/fbo/fbo-deriv.c  |  1 +
  tests/fbo/fbo-draw-buffers-blend.c |  1 +
  tests/fbo/fbo-drawbuffers-arbfp.c  |  1 +
  tests/fbo/fbo-drawbuffers-blend-add.c  |  1 +
  tests/fbo/fbo-drawbuffers-fragcolor.c  |  1 +
  tests/fbo/fbo-drawbuffers-maxtargets.c |  1 +
  tests/fbo/fbo-drawbuffers.c|  1 +
  tests/fbo/fbo-drawbuffers2-blend.c |  1 +
  tests/fbo/fbo-drawbuffers2-colormask.c |  1 +
  tests/fbo/fbo-finish-deleted.c |  1 +
  tests/fbo/fbo-flushing-2.c |  1 +
  tests/fbo/fbo-flushing.c   |  1 +
  tests/fbo/fbo-fragcoord.c  |  1 +
  tests/fbo/fbo-fragcoord2.c |  1 +
  tests/fbo/fbo-generatemipmap-filtering.c   |  1 +
  tests/fbo/fbo-generatemipmap-formats.c |  1 +
  tests/fbo/fbo-generatemipmap-noimage.c |  1 +
  tests/fbo/fbo-generatemipmap-nonsquare.c   |  1 +
  tests/fbo/fbo-generatemipmap-npot.c|  1 +
  tests/fbo/fbo-generatemipmap-scissor.c |  1 +
  tests/fbo/fbo-generatemipmap-swizzle.c |  1 +
  tests/fbo/fbo-generatemipmap-viewport.c|  1 +
  tests/fbo/fbo-generatemipmap.c |  1 +
  .../fbo/fbo-getframebufferattachmentparameter-01.c | 36 
+-

  tests/fbo/fbo-gl_pointcoord.c  |  1 +
  tests/fbo/fbo-incomplete-invalid-texture.c |  1 +
  tests/fbo/fbo-incomplete-texture-01.c  |  1 +
  tests/fbo/fbo-incomplete-texture-02.c  |  1 +
  tests/fbo/fbo-incomplete-texture-03.c  |  1 +
  tests/fbo/fbo-incomplete-texture-04.c  |  1 +
  tests/fbo/fbo-incomplete.cpp   |  1 +
  tests/fbo/fbo-integer.c|  1 +
  tests/fbo/fbo-luminance-alpha.c|  1 +
  tests/fbo/fbo-maxsize.c|  1 +
  tests/fbo/fbo-mipmap-copypix.c |  1 +
  tests/fbo/fbo-mrt-alphatest.c  |  1 +
  tests/fbo/fbo-mrt-new-bind.c   |  1 +
  tests/fbo/fbo-nodepth-test.c   |  1 +
  tests/fbo/fbo-nostencil-test.c |  1 +
  tests/fbo/fbo-pbo-readpixels-small.c   |  1 +
  tests/fbo/fbo-readdrawpix.c|  1 +
  tests/fbo/fbo-readpixels-depth-formats.c   |  1 +
  tests/fbo/fbo-readpixels.c |  1 +
  tests/fbo/fbo-rg.c |  1 +
  tests/fbo/fbo-scissor-bitmap.c |  1 +
  tests/fbo/fbo-scissor-blit.c   |  1 +
  tests/fbo/fbo-srgb.c   |  1 +
  tests/fbo/fbo-stencil.c|  1 +
  tests/fbo/fbo-storage-completeness.c   |  5 +--
  tests/fbo/fbo-storage-formats.c|  5 +--
  tests/fbo/fbo-sys-blit.c   |  1 +
  tests/fbo/fbo-sys-sub-blit.c   |  1 +
  tests/fbo/fbo-viewport.c   |  1 +
  76 files chan