Re: [Piglit] [PATCH] glx: make sure contexts are destroyed before we exit the test

[Eric Anholt]

Timothy Arceri  writes:

> Without this the last two contexts that are created will not
> have been destroyed when exiting the test. This creates a race
> condition in Mesa between any threads that might be using
> glsl_types and the atexit() callback that destroys these types.

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH] cmake: Do not build tests using designated initializers with older GCC.

[Eric Anholt]

Vinson Lee  writes:

> Designated initializers are not supported in older versions of GCC.

I don't think we should be doing this.  piglit is for development, if
you want to run it on old distros, then pull down a new compiler and
build it with that.


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

Re: [Piglit] [PATCH 2/2] glx: add test for GLX_ARB_create_context_no_error

[Eric Anholt]

Adam Jackson  writes:

> On Thu, 2019-02-07 at 14:53 -0800, Eric Anholt wrote:
>> Adam Jackson  writes:
>> 
>> > +static void
>> > +fold_results(enum piglit_result *a, enum piglit_result b)
>> > +{
>> > +  if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
>> > +  *a = PIGLIT_FAIL;
>> > +  else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
>> > +  *a = PIGLIT_PASS;
>> > +  else
>> > +  *a = PIGLIT_SKIP;
>> > +}
>> 
>> This is just piglit_merge_result()
>
> Indeed.
>
>> > +static enum piglit_result check_no_error(bool flag, bool debug, bool 
>> > robust)
>> > +{
>> > +  int ctx_flags = 0;
>> > +  ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
>> > +  ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
>> > +  const int attribs[] = {
>> > +  GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
>> > +  GLX_CONTEXT_MINOR_VERSION_ARB, 0,
>> > +  GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
>> > +  GLX_CONTEXT_FLAGS_ARB, ctx_flags,
>> > +  None
>> > +  };
>> > +  static bool is_dispatch_init = false;
>> > +  GLXContext ctx;
>> > +  enum piglit_result pass = PIGLIT_SKIP;
>> > +
>> > +  printf("info: no_error=%s, debug=%s, robustness=%s\n",
>> > + BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
>> > +
>> > +  ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
>> > +  XSync(dpy, 0);
>> 
>> Needs to check that we have robusness or debug and skip if they're
>> missing.
>
> There's not a separate extension string for debug, it's part of
> GLX_ARB_create_context. And asking for a debug context is allowed to be
> a no-op, so you can't even query the context bit afterward. True enough
> about robustness though.
>
>> > +  /* The number of texture units is a small, unsigned number. Craft an
>> > +   * illegal call by using a very large number that should fail on any
>> > +   * OpenGL implementation in practice.
>> > +   */
>> > +  glActiveTexture(-1);
>> > +  if (glGetError() != 0 && flag) {
>> > +  printf("error: error observed with KHR_no_error enabled\n");
>> > +  pass = PIGLIT_FAIL;
>> > +  goto done;
>> > +  }
>> 
>> I'm a reluctant to trigger undefined behavior that allows anything
>> including application termination in a testcase.
>
> Yeah, this seems like a mistake. Removed.
>
>> > +int main(int argc, char **argv)
>> > +{
>> > +  enum piglit_result pass = PIGLIT_SKIP;
>> > +
>> > +  GLX_ARB_create_context_setup();
>> > +  piglit_require_glx_extension(dpy, "GLX_ARB_create_context_no_error");
>> > +
>> > +  /* "Control group": Check that errors are indeed generated without
>> > +   * KHR_no_error enabled. */
>> > +  fold_results(, check_no_error(false, false, false));
>> > +  fold_results(, check_no_error(false, false, false));
>> > +  fold_results(, check_no_error(false, true, false));
>> > +  fold_results(, check_no_error(false, true, false));
>> 
>> We don't actually verify that errors are generated without KHR_no_error
>> -- the checks are under if (flag).  However, the rest of piglit covers
>> that, so let's just delete these cases and drop the first arg to
>> check_no_error.
>
> Will do.
>
>> > +
>> > +  /* Check that KHR_no_error gets enabled and its interaction with debug 
>> > and
>> > +   * robustness context flags. */
>> > +  fold_results(, check_no_error(true, false, false));
>> > +  fold_results(, check_no_error(true, false, false));
>> > +  fold_results(, check_no_error(true, true, false));
>> > +  fold_results(, check_no_error(true, true, false));
>> > +  fold_results(, check_no_error(true, false, true));
>> > +  fold_results(, check_no_error(true, true, true));
>> 
>> Looks like there are 2 duplicated cases here.
>
> Will do this too.
>
> The above review seems to apply equally to the EGL test (which also has
> at least one logic error). Will resubmit both with the above feedback
> addressed.

Sounds good!


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

Re: [Piglit] [PATCH 2/2] glx: add test for GLX_ARB_create_context_no_error

[Eric Anholt]

Adam Jackson  writes:

> From: Grigori Goronzy 
>
> ---
>  .../glx_arb_create_context/CMakeLists.gl.txt  |   1 +
>  tests/spec/glx_arb_create_context/common.h|   1 +
>  tests/spec/glx_arb_create_context/no-error.c  | 159 ++
>  3 files changed, 161 insertions(+)
>  create mode 100644 tests/spec/glx_arb_create_context/no-error.c
>
> diff --git a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt 
> b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> index f5c5e7cbc..91a87f95d 100644
> --- a/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> +++ b/tests/spec/glx_arb_create_context/CMakeLists.gl.txt
> @@ -41,6 +41,7 @@ IF(PIGLIT_BUILD_GLX_TESTS)
>   piglit_add_executable (glx-create-context-valid-attribute-empty 
> valid-attribute-empty.c common.c)
>   piglit_add_executable (glx-create-context-valid-attribute-null 
> valid-attribute-null.c common.c)
>   piglit_add_executable (glx-create-context-valid-flag-forward-compatible 
> valid-flag-forward-compatible.c common.c)
> + piglit_add_executable (glx-create-context-no-error no-error.c common.c)
>  ENDIF(PIGLIT_BUILD_GLX_TESTS)
>  
>  # vim: ft=cmake:
> diff --git a/tests/spec/glx_arb_create_context/common.h 
> b/tests/spec/glx_arb_create_context/common.h
> index 2e58b16db..e1ac28e6f 100644
> --- a/tests/spec/glx_arb_create_context/common.h
> +++ b/tests/spec/glx_arb_create_context/common.h
> @@ -30,6 +30,7 @@ extern GLXFBConfig fbconfig;
>  extern XVisualInfo *visinfo;
>  extern Window win;
>  extern GLXWindow glxWin;
> +extern int glx_error_code;
>  
>  extern bool parse_version_string(const char *string, int *major, int *minor);
>  
> diff --git a/tests/spec/glx_arb_create_context/no-error.c 
> b/tests/spec/glx_arb_create_context/no-error.c
> new file mode 100644
> index 0..d3b7fa470
> --- /dev/null
> +++ b/tests/spec/glx_arb_create_context/no-error.c
> @@ -0,0 +1,159 @@
> +/* Copyright 2017 Grigori Goronzy 
> + *
> + * 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.h"
> +#include "piglit-util-gl.h"
> +#include "piglit-glx-util.h"
> +#include "common.h"
> +
> +#define BOOLSTR(x) ((x) ? "yes" : "no")
> +
> +static void
> +fold_results(enum piglit_result *a, enum piglit_result b)
> +{
> + if (*a == PIGLIT_FAIL || b == PIGLIT_FAIL)
> + *a = PIGLIT_FAIL;
> + else if (*a == PIGLIT_PASS || b == PIGLIT_PASS)
> + *a = PIGLIT_PASS;
> + else
> + *a = PIGLIT_SKIP;
> +}

This is just piglit_merge_result()

> +static enum piglit_result check_no_error(bool flag, bool debug, bool robust)
> +{
> + int ctx_flags = 0;
> + ctx_flags |= debug ? GLX_CONTEXT_DEBUG_BIT_ARB : 0;
> + ctx_flags |= robust ? GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB : 0;
> + const int attribs[] = {
> + GLX_CONTEXT_MAJOR_VERSION_ARB, 2,
> + GLX_CONTEXT_MINOR_VERSION_ARB, 0,
> + GLX_CONTEXT_OPENGL_NO_ERROR_ARB, flag,
> + GLX_CONTEXT_FLAGS_ARB, ctx_flags,
> + None
> + };
> + static bool is_dispatch_init = false;
> + GLXContext ctx;
> + enum piglit_result pass = PIGLIT_SKIP;
> +
> + printf("info: no_error=%s, debug=%s, robustness=%s\n",
> +BOOLSTR(flag), BOOLSTR(debug), BOOLSTR(robust));
> +
> + ctx = glXCreateContextAttribsARB(dpy, fbconfig, NULL, True, attribs);
> + XSync(dpy, 0);

Needs to check that we have robusness or debug and skip if they're
missing.

> +
> + if (ctx == NULL) {
> + if (glx_error_code != 0) {
> + if ((debug || robust) && flag) {
> + /* KHR_no_error doesn't allow the no error mode 
> to be enabled
> +  * with KHR_debug or ARB_robustness, so context 
> creation is
> +  * expected to fail in these cases.
> +  */
> +  

Re: [Piglit] [PATCH] tex-miplevel-selection: force compatibility when using 430 shader

[Eric Anholt]

Ilia Mirkin  writes:

> Mesa will only consider the shader a compat shader if it has an explicit
> "compatibility" token in the version. Without this, we don't get
> gl_TexCoord, used by the test.
>
> Signed-off-by: Ilia Mirkin 

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH v1 1/2] piglit-framework-gl: add possibility to deinit test

[Eric Anholt]

Sergii Romantsov  writes:

> Hello,
> is there any chance to get review?
> In the goal is idea to get a simple mechanism to control resources freeing
> (any extra memory allocations, files etc.) of any piglit-test on the exit.

I gave some feedback, and I haven't seen a new version.


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

Re: [Piglit] Piglit website in sphinx

[Eric Anholt]

Jordan Justen  writes:

> On 2018-12-17 22:07:21, Tapani Pälli wrote:
>> Hi;
>> 
>> On 12/17/18 11:50 AM, Jordan Justen wrote:
>> > Although I doubt it generates much traffic, I wanted to try to convert
>> > the https://piglit.freedesktop.org/ homepage into an 'auto-building'
>> > gitlab branch.
>> > 
>> > I think the current site was built by ikiwiki, but I was after
>> > spending way more time that I hoped, I still was unable to reproduce
>> > the site without lots of extra undesirable links.
>> > 
>> > So, I tried converting it to sphinx. Of course, I expected it would
>> > look somewhat different with sphinx. This change proved much easier,
>> > and I think looks pretty good:
>> > 
>> > https://jljusten.pages.freedesktop.org/piglit/
>> 
>> Looks nice and clean! Maybe at same time the documentation could be 
>> improved. Some of the Piglit options seem to be missing, as example I 
>> never saw PIGLIT_PLATFORM documented, there is no list of platforms so 
>> only way is to grep the source ... or are they somewhere?
>
> Eric pointed out that it could be good to make the website sources
> appear under the docs directory, like Mesa does. I made the changes to
> do that here:
>
> https://gitlab.freedesktop.org/jljusten/piglit/commits/master
>
> With this version, the website would be rebuilt whenever the docs
> directory changes on the master branch.
>
> I guess my point is that by putting the website under the docs
> directory of the main branch, maybe it will allow others to contribute
> changes (like this) more easily. :)
>
> Another change that we might consider is whether to use merge requests
> in piglit. That would also require a change to the website.

This looks pretty nice to me.

Acked-by: Eric Anholt 


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

Re: [Piglit] Piglit website in sphinx

[Eric Anholt]

Jordan Justen  writes:

> Although I doubt it generates much traffic, I wanted to try to convert
> the https://piglit.freedesktop.org/ homepage into an 'auto-building'
> gitlab branch.
>
> I think the current site was built by ikiwiki, but I was after
> spending way more time that I hoped, I still was unable to reproduce
> the site without lots of extra undesirable links.
>
> So, I tried converting it to sphinx. Of course, I expected it would
> look somewhat different with sphinx. This change proved much easier,
> and I think looks pretty good:
>
> https://jljusten.pages.freedesktop.org/piglit/
>
> The source code is here in this branch:
>
> https://gitlab.freedesktop.org/jljusten/piglit/commits/website
>
> It is setup such that we could add a small 'website' branch to the
> piglit repo. The branch's history would be detacted from the source
> code history.
>
> Then, if we need to change the piglit website again, in, say a decade
> or so :), it'll be a bit simpler.
>
> Does it look ok, and does it seem like an ok plan?

Looks nice to me, only nit is the title is "Piglit 1 documentation" --
where did that "1" come from?

I kind of like having the website in master so that some of the contents
on the site can be documentation in-tree as well, but I'm happy with it
at least being easily modifiable.


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

Re: [Piglit] [PATCH v1 2/2] test: fix resources usage for depthstencil-render-miplevels

[Eric Anholt]

Sergii Romantsov  writes:

> Usage test 'depthstencil-render-miplevels 200 s=z24_s8' causes core dump on 
> exit.
> Issues:
> 1. Allocation of memory many times to the same variable
> 2. Not complete array to store pointers
> 3. Absence of memory freeing

Can you explain how not freeing memory could cause a core dump on exit?
And, in general, I would recommend just freeing data after usage, rather
than having a deinit function.


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

Re: [Piglit] [PATCH] EGL_EXT_image_dma_buf_import: set RGBA visual for yuv sampling tests

[Eric Anholt]

Lionel Landwerlin  writes:

> Depending on the platform you're running on, the test might fail with
> the AYUV format (or another format with alpha channel). To make this
> reliable, force to visual to RGBA.
>
> Signed-off-by: Lionel Landwerlin 
> Suggested-by: Eric Anholt 

r-b.


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

Re: [Piglit] [PATCH v2] EGL_EXT_image_dma_buf_import: add fbo option to yuv formats with alpha channel

[Eric Anholt]

Lionel Landwerlin  writes:

> Depending on the platform you're running on, the test might fail with
> the AYUV format. To make this reliable, force to render to fbo.

It should be reliable with just the RGBA change.

Separately, I think you should be able to get the -fbo argument (and
thus fewer windows flickering during a test run) by just removing the
run_concurrent=False from the test setup.  That will improve the
performance of piglit runs while making these tests less special.


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

Re: [Piglit] [PATCH v2] EGL_EXT_image_dma_buf_import: fix error check with unknown drm format

[Eric Anholt]

Lionel Landwerlin  writes:

> According to the EGL_EXT_image_dma_buf_import spec, creating an EGL
> image with a DRM format not supported should yield the BAD_MATCH
> error :
>
> "
>* If  is EGL_LINUX_DMA_BUF_EXT, and the 
> EGL_LINUX_DRM_FOURCC_EXT
>  attribute is set to a format not supported by the EGL, EGL_BAD_MATCH
>  is generated.
> "
>
> v2: Add spec quote (Eric)

r-b


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

Re: [Piglit] [PATCH] EGL_EXT_image_dma_buf_import: fix error check with unknown drm format

[Eric Anholt]

Lionel Landwerlin  writes:

> According to the EGL_EXT_image_dma_buf_import spec, creating an EGL
> image with a DRM format not supported should yield the BAD_MATCH
> error :
>
> "
>* If  is EGL_LINUX_DMA_BUF_EXT, and the 
> EGL_LINUX_DRM_FOURCC_EXT
>  attribute is set to a format not supported by the EGL, EGL_BAD_MATCH
>  is generated.
> "
>
> Signed-off-by: Lionel Landwerlin 
> Fixes: 0ee445dbbc161f ("tests/spec: EXT_image_dma_buf_import invalid 
> attributes")

If you include that spec citation as a comment in the same style as say,
test_buffer_not_null()'s, it gets my r-b.


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

Re: [Piglit] [PATCH 2/2] EGL_EXT_image_dma_buf_import: add fbo option to yuv formats with alpha channel

[Eric Anholt]

Lionel Landwerlin  writes:

> Depending on the platform you're running on, the test might fail with
> the AYUV format. To make this reliable, force to render to fbo.

While more fbo tests are good, I think you're missing

config.window_visual = PIGLIT_GL_VISUAL_RGBA;

in sample_yuv.c


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

Re: [Piglit] [PATCH] glsl-1.10: test some arithmetic on non-existing struct member

[Eric Anholt]

Tapani Pälli  writes:

> Signed-off-by: Tapani Pälli 
> https://bugs.freedesktop.org/show_bug.cgi?id=108012

Reviewed-by: Eric Anholt 


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

[Piglit] [PATCH] glsl-sdl-sincos-accuracy: Add a new test for a vc4 bug with SDL2.

[Eric Anholt]

Our sin(0.0) and cos(0.0) were too inaccurate, causing twisting in
SDL2's rendering.
---
 .../glsl-sdl-sincos-accuracy.shader_test  | 36 +++
 1 file changed, 36 insertions(+)
 create mode 100644 tests/shaders/glsl-sdl-sincos-accuracy.shader_test

diff --git a/tests/shaders/glsl-sdl-sincos-accuracy.shader_test 
b/tests/shaders/glsl-sdl-sincos-accuracy.shader_test
new file mode 100644
index ..c6ca7e1ae596
--- /dev/null
+++ b/tests/shaders/glsl-sdl-sincos-accuracy.shader_test
@@ -0,0 +1,36 @@
+[require]
+GLSL >= 1.10
+
+[vertex shader]
+void main()
+{
+   gl_Position = gl_Vertex;
+}
+
+[fragment shader]
+/* SDL2 prior to 12156:e5a666405750 (Aug 28) would render a bunch of
+ * its contents using cos/sin in the VS to handle a user-passed angle
+ * value (See SDL's RenderCopyEx, for example).  If our error at angle
+ * == 0.0 is too much, we'll slightly rotate the images even with
+ * nearest filtering.
+ */
+uniform float angle;
+void main()
+{
+   float sin_err = abs(sin(angle) - 0.0);
+   float cos_err = abs(cos(angle) - 1.0);
+   /* Allow a tolerance of half a pixel of error at a 2048x2048
+* window (viewport is from -1 to 1).
+*/
+   float tolerance = 2.0 / 2048.0 / 2.0;
+
+   gl_FragColor = vec4(sin_err >= tolerance,
+   sin_err < tolerance && cos_err < tolerance,
+   cos_err >= tolerance,
+   0.0);
+}
+
+[test]
+uniform float angle 0.0
+draw rect -1 -1 2 2
+probe all rgba 0.0 1.0 0.0 0.0
-- 
2.18.0

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

Re: [Piglit] [PATCH v2 3/7] wflinfo: Reimplement wflinfo separating the various API's

[Eric Anholt]

Dylan Baker  writes:

> Currently fast skipping is implemented such that it assumes there's a
> single version of ES, a single version of desktop, and all extensions
> are shared between them. This has basically worked because 1) there are
> basically no gles1 tests, and 2) piglit didn't have compat profile. But
> worked and correct are two different things.
>
> With the addition of compat profiles it's time to re-evaluate how fast
> skipping works. Namely we need to have different attributes for ES1,
> ES1+, core, compat, and I've added on for "legacy" (pre-profile), since
> waffle supports that.
>
> This maintains legacy interfaces so that existing code continues to
> work.
>
> v2: - Fix versions < 3.1 on implementations without core profile
> ---

I haven't wanted to look at this series because I don't like python, but
I had a few minutes today so I started to review.

>  framework/wflinfo.py| 320 
>  unittests/framework/test_wflinfo.py |  23 +-
>  2 files changed, 201 insertions(+), 142 deletions(-)
>
> diff --git a/framework/wflinfo.py b/framework/wflinfo.py
> index 8c7da084a..d3a79437d 100644
> --- a/framework/wflinfo.py
> +++ b/framework/wflinfo.py
> @@ -25,12 +25,14 @@ import errno
>  import os
>  import subprocess
>  import sys
> +import threading
>  
>  import six
>  
> -from framework import exceptions, core
> +from framework import exceptions
> +from framework.core import lazy_property
>  from framework.options import OPTIONS
> -from framework.test import piglit_test
> +# from framework.test import piglit_test

Commented out line meant to be removed?

>  class StopWflinfo(exceptions.PiglitException):
> @@ -40,6 +42,15 @@ class StopWflinfo(exceptions.PiglitException):
>  self.reason = reason
>  
>  
> +class ProfileInfo(object):
> +"""Information about a single profile (core, compat, es1, es2, etc)."""
> +
> +def __init__(self, shader_version, language_version, extensions):
> +self.shader_version = shader_version
> +self.api_version = language_version
> +self.extensions = extensions
> +
> +
>  class WflInfo(object):
>  """Class representing platform information as provided by wflinfo.
>  
> @@ -56,6 +67,15 @@ class WflInfo(object):
>  
>  """
>  __shared_state = {}
> +__core_init = False
> +__core_lock = threading.Lock()
> +__compat_init = False
> +__compat_lock = threading.Lock()
> +__es1_init = False
> +__es1_lock = threading.Lock()
> +__es2_init = False
> +__es2_lock = threading.Lock()
> +

All this init/locking stuff feels tremendously excessive, and if it was
necessary it ought to be part of lazy_property.  But I expect that you
could just accept the race to create two of the object representing the
driver's capabilities.

>  def __new__(cls, *args, **kwargs): # Implement the borg pattern:
> #
> https://code.activestate.com/recipes/66531-singleton-we-dont-need-no-stinkin-singleton-the-bo/
> @@ -94,9 +114,9 @@ class WflInfo(object):
>  
>  # setup execution environment where we extend the PATH env 
> var
>  # to include the piglit TEST_BIN_DIR
> -new_env = os.environ
> -new_env['PATH'] = ':'.join([piglit_test.TEST_BIN_DIR,
> -os.environ['PATH']])
> +new_env = os.environ.copy()
> +# new_env['PATH'] = ':'.join([piglit_test.TEST_BIN_DIR,
> +# os.environ['PATH']])

What's going on with this being commented out?  Why is it changing in
this patch?


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

Re: [Piglit] [PATCH 1/2] Test glMultiDrawArraysIndirect with non-VBO attribs

[Eric Anholt]

Marek Olšák  writes:

> From: Marek Olšák 
>
> ---
>  tests/opengl.py  |  6 ++
>  tests/spec/gl-3.0/multidrawarrays-vertexid.c | 64 +++-
>  2 files changed, 56 insertions(+), 14 deletions(-)

Both patches are:

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH] tfp: Also test with GL_TEXTURE_RECTANGLE

[Eric Anholt]

Jason Ekstrand  writes:

> ---
>  tests/glx/glx-tfp.c | 102 +++-
>  1 file changed, 62 insertions(+), 40 deletions(-)

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH] mesa_tile_raster_order: Add a test for the extension.

[Eric Anholt]

Eric Anholt  writes:

> This doesn't test repeated copies with TextureBarrier()s in between,
> but that doesn't seem like important new behavior in this extension.

I still have this test laying around my tree, and I'd like to push it.
Ack from someone?


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

[Piglit] [PATCH] deqp: Add Mesa version overrides for khr_gl* testlist generation.

[Eric Anholt]

The runner will assertion fail if it can't create an appropriate
context, so just force the version so that we can get our testlists
generated.
---
 framework/test/deqp.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/framework/test/deqp.py b/framework/test/deqp.py
index 2c37f6974ebf..ab3350c3e7e7 100644
--- a/framework/test/deqp.py
+++ b/framework/test/deqp.py
@@ -117,9 +117,13 @@ def gen_caselist_txt(bin_, caselist, extra_args):
 
 # TODO: need to catch some exceptions here...
 with open(os.devnull, 'w') as d:
+env = os.environ.copy()
+env['MESA_GL_VERSION_OVERRIDE'] = '4.6'
+env['MESA_GLES_VERSION_OVERRIDE'] = '3.2'
+
 subprocess.check_call(
 [bin_, '--deqp-runmode=txt-caselist'] + extra_args, cwd=basedir,
-stdout=d, stderr=d)
+stdout=d, stderr=d, env=env)
 assert os.path.exists(caselist_path)
 return caselist_path
 
-- 
2.17.0

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

Re: [Piglit] [PATCH] framework: Add a PIGLIT_TEST_TIMEOUT environment variable

[Eric Anholt]

Jason Ekstrand  writes:

> This commit adds a new PIGLIT_TESTS_TIMEOUT environment variable which
> overrides the pre-test timeout specified by the test runner.
> ---
>
> This is almost certainly not the right solution but it gets the discussion
> started.  A couple of known issues:
>
>  - No documentation
>  - It overrides.  Maybe it should be a max timeout?
>  - Maybe we want a flag instead of an environment variable?
>  - Maybe Jason has no clue what he's doing inside the piglit framework and
>someone else should write the patch?

This would save me some hassle dealing with manually filtering out slow
tests for my simulator runs.  I don't have a strong preference on
whether it should be a clamp vs override.


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

Re: [Piglit] [PATCH 2/4] README.md: Remove not about .py on profiles

[Eric Anholt]

Dylan Baker  writes:

> It is still valid, but the behavior of profiles specified without an
> extension is always preferable, since some profiles are available as
> compressed xml and as python. The XML versions will start faster and
> use less memory than the python version. Other profiles are only
> available as python, so just stop using it.

in the subject: "note"

Other than that, 1, 2, 3 are:

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH 3/4] README.md: Remove PIGLIT_FORCE_GLSLPASER_DESKTOP env variable

[Eric Anholt]

Dylan Baker  writes:

> This is no longer honored, instead use the glslparser_arb_compat
> profile.

I still see it in the tree here.


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

Re: [Piglit] [PATCH 5/5] textureGather: Use piglit_probe_rect_rgba_varying

[Eric Anholt]

Jason Ekstrand  writes:

> Instead of doing 900 piglit_proble_pixel_rgba calls (one for each pixel)
> which each call glReadPixels, use the new helper which only invokes
> glReadPixels once.

This series is:

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH] Use _exit() instead of exit() in child processes

[Eric Anholt]

Michel Dänzer  writes:

> From: Michel Dänzer 
>
> A child process which doesn't call exec() shouldn't use exit(), as that
> will attempt to run any atexit handlers of the parent, which may break.
> It actually results in crashing with the Mesa radeonsi driver.

Reviewed-by: Eric Anholt 


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

Re: [Piglit] GitLab migration of Piglit

[Eric Anholt]

Jason Ekstrand  writes:

> All,
>
> Sorry for the mess of GitLab e-mails but there are a lot of questions to
> ask as this process moves forward.  Today, we're discussing piglit.  I've
> included both the mesa and piglit list in the hopes that people will
> actually see this e-mail.
>
> Honestly, I expect the migration of piglit to have much less impact on most
> people's daily lives than moving the mesa repo.
>
> The biggest question I have is whether we actually want to continue to have
> a separate "piglit" group.  With GitLab, we can already give someone
> developer access to piglit without giving them developer access to mesa.
> Mostly, this is a question of whether we consider piglit to be it's own
> project on freedesktop or a sub-project of mesa.  I don't know the answer
> to that question.

So far, having it be a separate group has just been a pain in getting
people to contribute to piglit, when we mistakenly forget to add mesa
devs to it.  I don't think we need it to be a separate committer group.


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

Re: [Piglit] [PATCH] glsl-1.10: test loop unrolling when induction variable is inside if branch

[Eric Anholt]

Timothy Arceri  writes:

> ---
>  ...induction-variable-inside-if-branch.shader_test | 61 
> ++
>  1 file changed, 61 insertions(+)
>  create mode 100644 
> tests/spec/glsl-1.10/execution/vs-loop-simple-unroll-induction-variable-inside-if-branch.shader_test
>
> diff --git 
> a/tests/spec/glsl-1.10/execution/vs-loop-simple-unroll-induction-variable-inside-if-branch.shader_test
>  
> b/tests/spec/glsl-1.10/execution/vs-loop-simple-unroll-induction-variable-inside-if-branch.shader_test
> new file mode 100644
> index 0..1990db8ea
> --- /dev/null
> +++ 
> b/tests/spec/glsl-1.10/execution/vs-loop-simple-unroll-induction-variable-inside-if-branch.shader_test
> @@ -0,0 +1,61 @@
> +# This tests unrolling of a loop with a single exit point.
> +#
> +# Here we test that loop is correctly unrolled when the induction variable is
> +# inside an if branch.
> +[require]
> +GLSL >= 1.10
> +
> +[vertex shader]
> +void main()
> +{
> +  gl_Position = gl_Vertex;
> +
> +  vec4 colour = vec4(1.0, 0.0, 0.0, 1.0);
> +
> +  int i = 1;
> +  int j = 0; // we use this so the if doesn't get reduced to a series of 
> bcsel
> +  do {
> +
> +j++;
> +
> +if (i >= 3) {
> +  if (i == 3) {
> + colour = vec4(0.0, 1.0, 0.0, 1.0);
> + j++;
> + if (j != 6)
> +   colour = vec4(1.0, 0.0, 1.0, 1.0);
> +  } else {
> +colour = vec4(1.0, 1.0, 0.0, 1.0);
> +  }
> +  break;
> +} else {
> +  if (i != 1) {
> + j++;
> +  }
> +}
> +
> +   if (i >= 5) {
> +  j++; // unreachable
> +  break;
> +} else {

mismatched indentation

> +  if (i != 1) {
> + j++;
> +  }
> +  i++;
> +}
> +  } while (i < 4);
> +
> +  gl_FrontColor = colour;
> +}

Thanks for making tests for bugs like this!

Reviewed-by: Eric Anholt 


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

Re: [Piglit] [PATCH] glsl-uniform-initializer-7: Fixed uninitialized uniform.

[Eric Anholt]

mathias.froehl...@gmx.net writes:

> From: Mathias Fröhlich <mathias.froehl...@gmx.net>
>
> Hi all,
>
> please review, and push if ok!
>
> best
>
> Mathias
>
>
>
>
> This fixes a valgrind error in executing shader_runner on this test.

Reviewed-by: Eric Anholt <e...@anholt.net>

And you also have piglit push access now, not sure how that was missed.


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

Re: [Piglit] [PATCH] piglit-util: use _exit to skip context destruction

[Eric Anholt]

Marek Olšák  writes:

> From: Marek Olšák 

You should have a "why" in your commit message.


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

Re: [Piglit] [PATCH] ext_texture_norm16-render: refactor subtest execution

[Eric Anholt]

Tapani Pälli <tapani.pa...@intel.com> writes:

> Patch moves subtest execution in a separate function that
> returns total result for each format, this fixes issues in test
> with current pass/fail reporting.
>
> Also remove reporting from verify_contents_float, this caused
> duplicate subtest result reporting.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v2] ext_texture_norm16-render: test for GL_EXT_texture_norm16

[Eric Anholt]

Tapani Pälli  writes:

> Test includes:
>- texture uploads
>- mipmap generation
>- framebuffer creation
>- rendering to
>- reading from
>- interaction with GL_EXT_copy_image
>
> v2: code cleanup
>
> Signed-off-by: Tapani Pälli 
> ---
>  tests/all.py   |   5 +
>  tests/spec/CMakeLists.txt  |   1 +
>  tests/spec/ext_texture_norm16/CMakeLists.gles2.txt |   7 +
>  tests/spec/ext_texture_norm16/CMakeLists.txt   |   1 +
>  tests/spec/ext_texture_norm16/render.c | 374 
> +
>  5 files changed, 388 insertions(+)
>  create mode 100644 tests/spec/ext_texture_norm16/CMakeLists.gles2.txt
>  create mode 100644 tests/spec/ext_texture_norm16/CMakeLists.txt
>  create mode 100644 tests/spec/ext_texture_norm16/render.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 26638cd82..101a6c149 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3187,6 +3187,11 @@ with profile.test_list.group_manager(
>  
>  with profile.test_list.group_manager(
>  PiglitGLTest,
> +grouptools.join('spec', 'ext_texture_norm16')) as g:
> +g(['ext_texture_norm16-render'], 'render')
> +
> +with profile.test_list.group_manager(
> +PiglitGLTest,
>  grouptools.join('spec', 'ext_frag_depth')) as g:
>  g(['fragdepth_gles2'])
>  
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index dc14beb4e..405d35a53 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -178,3 +178,4 @@ add_subdirectory (arb_fragment_shader_interlock)
>  add_subdirectory (ext_occlusion_query_boolean)
>  add_subdirectory (ext_disjoint_timer_query)
>  add_subdirectory (intel_blackhole_render)
> +add_subdirectory (ext_texture_norm16)
> diff --git a/tests/spec/ext_texture_norm16/CMakeLists.gles2.txt 
> b/tests/spec/ext_texture_norm16/CMakeLists.gles2.txt
> new file mode 100644
> index 0..e9cebd101
> --- /dev/null
> +++ b/tests/spec/ext_texture_norm16/CMakeLists.gles2.txt
> @@ -0,0 +1,7 @@
> +link_libraries (
> + piglitutil_${piglit_target_api}
> +)
> +
> +piglit_add_executable (ext_texture_norm16-render render.c)
> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/ext_texture_norm16/CMakeLists.txt 
> b/tests/spec/ext_texture_norm16/CMakeLists.txt
> new file mode 100644
> index 0..144a306f4
> --- /dev/null
> +++ b/tests/spec/ext_texture_norm16/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/ext_texture_norm16/render.c 
> b/tests/spec/ext_texture_norm16/render.c
> new file mode 100644
> index 0..f99b1fd9c
> --- /dev/null
> +++ b/tests/spec/ext_texture_norm16/render.c
> @@ -0,0 +1,374 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * @file
> + * Basic tests for formats added by GL_EXT_texture_norm16 extension
> + *
> + * 
> https://www.khronos.org/registry/OpenGL/extensions/EXT/EXT_texture_norm16.txt
> + *
> + * Test includes:
> + *   - texture uploads
> + *   - mipmap generation
> + *   - framebuffer creation
> + *   - rendering to
> + *   - reading from
> + *   - interaction with GL_EXT_copy_image
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> + config.supports_gl_es_version = 31;
> + config.window_visual = PIGLIT_GL_VISUAL_RGBA;
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char vs_source[] =
> + "#version 310 es\n"
> + "layout(location = 0) in highp vec4 vertex;\n"
> + "layout(location = 1) in highp vec4 uv;\n"
> + "out highp vec2 tex_coord;\n"
> + "\n"
> + "void main()\n"
> + "{\n"
> + "   gl_Position = vertex;\n"
> + "   tex_coord = uv.st;\n"
> + "}\n";
> +
> +static const char fs_source[] =
> + 

Re: [Piglit] [PATCH 7/7] nv_fog_distance: Test interactions with GL_FOG_COORDINATE

[Eric Anholt]

;
> +#include "minmax-test.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + static const float fog_color[4] = { 0.0, 1.0, 0.0, 1.0 };
> + static const float draw_color[4] = { 1.0, 0.0, 0.0, 1.0 };
> + static const float mix_color[4] = { 0.5, 0.5, 0.0, 1.0 };
> + bool pass = true;
> +
> + glViewport(0, 0, piglit_width, piglit_height);
> +
> + /* Pick a Z value for the mesh.  Select fog start and stop distances
> +  * such that the middle of the window (the closest point) will have
> +  * zero fog and the corners (the farthest points) will have full fog.
> +  */

Drop this comment, since we're just drawing a pair of tris and not
having any verts in the middle at all?

> + float z = 0.5;
> + float fog_start = z;
> + float fog_end = sqrt(1.0 + 1.0 + (z * z));
> +
> + glFogf(GL_FOG_START, fog_start);
> + glFogf(GL_FOG_END, fog_end);
> + glFogfv(GL_FOG_COLOR, fog_color);
> +
> + glFogCoordf((fog_end + fog_start) / 2.0);
> + glColor3fv(draw_color);
> +
> + glBegin(GL_TRIANGLE_FAN);
> + glVertex3f(-1.0, -1.0, z);
> + glVertex3f(-1.0,  1.0, z);
> + glVertex3f( 1.0,  1.0, z);
> + glVertex3f( 1.0, -1.0, z);
> + glEnd();

piglit_draw_rect_z()?

> +
> + pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> +
> + pass = piglit_probe_rect_rgb(0, 0, piglit_width, piglit_height,
> +  mix_color);
> +
> + piglit_present_results();
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + piglit_require_extension("GL_NV_fog_distance");
> + piglit_require_extension("GL_EXT_fog_coord");
> + piglit_require_extension("GL_ARB_vertex_buffer_object");
> +
> + glEnable(GL_FOG);
> + glFogi(GL_FOG_MODE, GL_LINEAR);
> + glFogi(GL_FOG_COORD_SRC, GL_FOG_COORDINATE);
> + glFogi(GL_FOG_DISTANCE_MODE_NV, GL_EYE_RADIAL_NV);

Indentation weirdness.

With whatever of these fixes,

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH 6/7] nv_fog_distance: Simple rendering test

[Eric Anholt]

Ian Romanick <i...@freedesktop.org> writes:
> diff --git a/tests/spec/nv_fog_distance/simple-draw.c 
> b/tests/spec/nv_fog_distance/simple-draw.c
> new file mode 100644
> index 0..e7be6bf5e
> --- /dev/null
> +++ b/tests/spec/nv_fog_distance/simple-draw.c
> @@ -0,0 +1,245 @@
> +/* Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/** @file simple-draw.c
> + * Simple rendering tests of GL_NV_fog_distance
> + */
> +
> +#include "piglit-util-gl.h"
> +#include "minmax-test.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static GLenum distance_mode = GL_EYE_RADIAL_NV;
> +static GLuint vbo[2];
> +
> +/* Generate an NxM grid of vertices spanning [-1,-1]x[1,1]
> + */
> +static unsigned
> +generate_mesh(unsigned n, unsigned m, float z, GLuint vbo[2])
> +{
> + /* Generate the grid of vertices by linearly interpolating across
> +  * [-1,1]x[-1,1].  This is the easy part.
> +  *
> +  * Each vertex is 3 floats.  There are N*M vertices.
> +  */
> + glBindBuffer(GL_ARRAY_BUFFER, vbo[0]);
> + glBufferData(GL_ARRAY_BUFFER, sizeof(float) * 3 * n * m,
> +  NULL, GL_STATIC_DRAW);
> +
> + float *verts = glMapBuffer(GL_ARRAY_BUFFER, GL_READ_ONLY);
> + for (unsigned i = 0; i < m; i++) {
> + const float y = -1.0 + ((2.0 * i) / (m - 1));
> +
> + for (unsigned j = 0; j < n; j++) {
> + const float x = -1.0 + ((2.0 * j) / (n - 1));
> +
> + verts[0] = x;
> + verts[1] = y;
> + verts[2] = z;
> + verts += 3;
> + }
> + }
> +
> + glUnmapBuffer(GL_ARRAY_BUFFER);

This all feels overly complicated for a testcase -- how bad would a
double loop of piglit_draw_rect_z() be, I wonder?  I'm betting that
would let your testcase work on the old nvidia drivers.

However, it also sounds like it works on Mesa, so either way,

Acked-by: Eric Anholt <e...@anholt.net>

> + if (argc > 1) {
> + unsigned i;
> +
> + for (i = 0; i < ARRAY_SIZE(modes); i++) {
> + if (strcmp(modes[i].name, argv[1]) == 0) {
> + distance_mode = modes[i].mode;
> + break;
> + }
> + }
> +
> + if (i == ARRAY_SIZE(modes)) {
> + printf("Unknown distance mode \"%s\".\n",
> +argv[1]);
> + piglit_report_result(PIGLIT_FAIL);
> + }
> + }
> +
> + glEnableClientState(GL_VERTEX_ARRAY);
> + glEnable(GL_FOG);

Funny indentation


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

Re: [Piglit] [PATCH 5/7] nv_fog_distance: Simple touch test for the enums

[Eric Anholt]

Ian Romanick  writes:

> From: Ian Romanick 
>
> Signed-off-by: Ian Romanick 
> ---
>  tests/all.py |   5 +
>  tests/spec/CMakeLists.txt|   1 +
>  tests/spec/nv_fog_distance/CMakeLists.gl.txt |  11 ++
>  tests/spec/nv_fog_distance/CMakeLists.txt|   1 +
>  tests/spec/nv_fog_distance/coverage.c| 246 
> +++
>  5 files changed, 264 insertions(+)
>  create mode 100644 tests/spec/nv_fog_distance/CMakeLists.gl.txt
>  create mode 100644 tests/spec/nv_fog_distance/CMakeLists.txt
>  create mode 100644 tests/spec/nv_fog_distance/coverage.c
>
> diff --git a/tests/all.py b/tests/all.py
> index 26638cd82..41e9dda10 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3914,6 +3914,11 @@ with profile.test_list.group_manager(
>  grouptools.join('spec', 'nv_fill_rectangle')) as g:
>  g(['nv_fill_rectangle-invalid-draw-mode'], 'invalid-draw-mode')
>  
> +with profile.test_list.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', 'nv_fog_distance')) as g:
> +g(['nv_fog_distance-coverage'], 'coverage')
> +
>  with profile.test_list.group_manager(
>  PiglitGLTest,
>  grouptools.join('spec', 'oes_matrix_get')) as g:
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index dc14beb4e..794843d8b 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -102,6 +102,7 @@ add_subdirectory (ext_timer_query)
>  add_subdirectory (ext_transform_feedback)
>  add_subdirectory (nv_conditional_render)
>  add_subdirectory (nv_fill_rectangle)
> +add_subdirectory (nv_fog_distance)
>  add_subdirectory (nv_image_formats)
>  add_subdirectory (nv_texture_barrier)
>  add_subdirectory (nv_texture_env_combine4)
> diff --git a/tests/spec/nv_fog_distance/CMakeLists.gl.txt 
> b/tests/spec/nv_fog_distance/CMakeLists.gl.txt
> new file mode 100644
> index 0..9abea3bef
> --- /dev/null
> +++ b/tests/spec/nv_fog_distance/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 (nv_fog_distance-coverage coverage.c)
> diff --git a/tests/spec/nv_fog_distance/CMakeLists.txt 
> b/tests/spec/nv_fog_distance/CMakeLists.txt
> new file mode 100644
> index 0..144a306f4
> --- /dev/null
> +++ b/tests/spec/nv_fog_distance/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/nv_fog_distance/coverage.c 
> b/tests/spec/nv_fog_distance/coverage.c
> new file mode 100644
> index 0..7a27a1633
> --- /dev/null
> +++ b/tests/spec/nv_fog_distance/coverage.c
> @@ -0,0 +1,246 @@
> +/* Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/** @file coverage.c
> + * Positive and negative enum coverage test.
> + */
> +
> +#include "piglit-util-gl.h"
> +#include "minmax-test.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> +
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* UNREACHED */
> + return PIGLIT_FAIL;
> +}
> +
> +static bool
> +check_initial_state(void)
> +{
> + union {
> + GLfloat f;
> + GLboolean b;
> + GLint i;
> + } values;
> + bool pass = true;
> + const GLenum expected_error =
> + piglit_is_extension_supported("GL_NV_fog_distance")
> + ? GL_NO_ERROR : GL_INVALID_ENUM;
> +
> + printf("Check getting initial state...\n");
> + values.i = 0xDEADBEEF;
> + 

Re: [Piglit] [PATCH 1/7] gl-1.0-blend: Fix problems with blend extension detection

[Eric Anholt]

Ian Romanick  writes:

> From: Ian Romanick 
>
> There were two bugs here.  First, GL_EXT_blend_color and
> GL_EXT_blend_func_separate were only tested if the version is 1.4.  This
> is unnecessary because those extensions are part of OpenGL 1.4.  Second,
> GL_EXT_blend_minmax was typoed GL_EXT_blend_min_max.

Looks like GL_EXT_blend_color at least is only in 1.4 if the imaging
subset is supported, which it looks like we don't do in Mesa.  So I
think we should disentangle these two changes, and r-b from me on the
typo fix.


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

Re: [Piglit] [PATCH 2/7] gl-1.0-blend: Enable EXT_blend_subtract tests on implementations that lack EXT_blend_minmax

[Eric Anholt]

Ian Romanick  writes:

> From: Ian Romanick 
>
> And vice versa.
>
> Signed-off-by: Ian Romanick 
> ---
>  tests/spec/gl-1.0/blend.c | 40 ++--
>  1 file changed, 26 insertions(+), 14 deletions(-)
>
> diff --git a/tests/spec/gl-1.0/blend.c b/tests/spec/gl-1.0/blend.c
> index 0cacf69cb..278ef75ac 100644
> --- a/tests/spec/gl-1.0/blend.c
> +++ b/tests/spec/gl-1.0/blend.c
> @@ -115,14 +115,20 @@ static const GLenum dst_factors[] = {
>   GL_CONSTANT_ALPHA,
>   GL_ONE_MINUS_CONSTANT_ALPHA
>  };
> -static const GLenum operators[] = {
> +
> +static const GLenum subtract_operators[] = {
>   GL_FUNC_ADD,
>   GL_FUNC_SUBTRACT,
> - GL_FUNC_REVERSE_SUBTRACT,
> + GL_FUNC_REVERSE_SUBTRACT
> +};
> +
> +static const GLenum minmax_operators[] = {
>   GL_MIN,
>   GL_MAX
>  };
>  
> +GLenum operators[ARRAY_SIZE(subtract_operators) + 
> ARRAY_SIZE(minmax_operators)];
> +
>  struct image {
>   GLuint name;
>   GLfloat *data;
> @@ -714,14 +720,8 @@ run_all_factor_sets(void)
>   have_blend_color = true;
>   have_sep_func = true;
>   } else {
> - /* glBlendEquation is added by either extension.
> -  * However, there is only one table of operators to
> -  * test, and it includes the operators from both
> -  * extensions.  In Mesa, only R100 supports
> -  * GL_EXT_blend_subtract but not GL_EXT_blend_minmax.
> -  */
>   have_blend_equation =
> - piglit_is_extension_supported("GL_EXT_blend_subtract") 
> &&
> + piglit_is_extension_supported("GL_EXT_blend_subtract") 
> ||
>   piglit_is_extension_supported("GL_EXT_blend_minmax");
>   have_blend_color =
>   piglit_is_extension_supported("GL_EXT_blend_color");
> @@ -753,15 +753,27 @@ run_all_factor_sets(void)
>   num_dst_factors_sep = 1;
>   }
>  
> - if (have_blend_equation) {
> - num_operators_rgb = ARRAY_SIZE(operators);
> - num_operators_a = ARRAY_SIZE(operators);
> + num_operators_rgb = 0;
> + if (piglit_is_extension_supported("GL_EXT_blend_subtract")) {
> + memcpy([num_operators_rgb],
> +subtract_operators,
> +ARRAY_SIZE(subtract_operators) * sizeof(operators[0]));
> + num_operators_rgb += ARRAY_SIZE(subtract_operators);
>   }
>   else {
> - num_operators_rgb = 1; /* just ADD */
> - num_operators_a = 1; /* just ADD */
> + num_operators_rgb = 1;
> + operators[0] = GL_FUNC_ADD;
> + }
> +
> + if (piglit_is_extension_supported("GL_EXT_blend_minmax")) {
> + memcpy([num_operators_rgb],
> +minmax_operators,
> +ARRAY_SIZE(minmax_operators) * sizeof(operators[0]));
> + num_operators_rgb += ARRAY_SIZE(minmax_operators);
>   }

This all feels like it could be *so* much simpler.

operators[num_operators_rgb++] = GL_FUNC_ADD;

if (piglit_is_extension_supported("GL_EXT_blend_subtract"))
operators[num_operators_rgb++] = GL_FUNC_SUBTRACT;
operators[num_operators_rgb++] = GL_FUNC_REVERSE_SUBTRACT;

if (piglit_is_extension_supported("GL_EXT_blend_minmax")) {
operators[num_operators_rgb++] = GL_MIN;
operators[num_operators_rgb++] = GL_MAX;
}

assert(num_operators_rgb <= ARRAY_SIZE(operators);

num_operators_a = num_operators_rgb;


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

Re: [Piglit] [PATCH] draw-vertices-2101010: Don't test BGRA if we don't have the ext.

[Eric Anholt]

Ilia Mirkin <imir...@alum.mit.edu> writes:

> On Thu, Apr 26, 2018 at 1:21 PM, Eric Anholt <e...@anholt.net> wrote:
>> The 2101010 spec says to remove BGRA references if this extension
>> isn't present, so don't test them.
>> ---
>>  .../draw-vertices-2101010.c   | 22 ++-
>>  1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git 
>> a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c 
>> b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
>> index 5ce4c0655626..a672bb7d5520 100644
>> --- a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
>> +++ b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
>> @@ -89,7 +89,7 @@ static GLuint vboColorPointer(GLint size, GLenum type, 
>> GLsizei stride,
>>  return id;
>>  }
>>
>> -static void test_packed_int_color_vertices(float x1, float y1, float x2, 
>> float y2, int index)
>> +static bool test_packed_int_color_vertices(float x1, float y1, float x2, 
>> float y2, int index)
>>  {
>>  unsigned int v[3];
>>  unsigned int c[3];
>> @@ -111,6 +111,10 @@ static void test_packed_int_color_vertices(float x1, 
>> float y1, float x2, float y
>>
>>  glVertexPointer(4, GL_INT_2_10_10_10_REV, 4, v);
>>
>> +if (index >= 2 &&
>> +   !piglit_is_extension_supported("GL_EXT_vertex_array_bgra"))
>> +   return false;
>
> Would it make sense to move this further towards the top of the function?
>
> Either way,
>
> Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu>

I was going for "as close as possible to the switch statement that turns
the magic numbers into BGRA usage"


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

Re: [Piglit] [PATCH] texture-integer-glsl130: Make sure we use usampler for ui textures.

[Eric Anholt]

Ilia Mirkin <imir...@alum.mit.edu> writes:

> On Thu, Apr 26, 2018 at 1:01 PM, Eric Anholt <e...@anholt.net> wrote:
>> Fixes failure on VC5 because we would sign-extend 16ui.  Sampling ui
>> with isampler is undefined according to the table under "8.7 Texture
>> Lookup Functions" in the 1.30 spec.
>> ---
>>  .../texture-integer-glsl130.c | 40 ---
>>  1 file changed, 26 insertions(+), 14 deletions(-)
>>
>> diff --git a/tests/spec/ext_texture_integer/texture-integer-glsl130.c 
>> b/tests/spec/ext_texture_integer/texture-integer-glsl130.c
>> index 2247207173a1..06332b66f2cf 100644
>> --- a/tests/spec/ext_texture_integer/texture-integer-glsl130.c
>> +++ b/tests/spec/ext_texture_integer/texture-integer-glsl130.c
>> @@ -44,7 +44,7 @@ static const char *TestName = "texture-integer";
>>  static GLint TexWidth = 16, TexHeight = 16;
>>  static GLuint Texture;
>>
>> -static GLint BiasUniform = -1, TexUniform = -1;
>> +static GLint BiasUniform[2] = {-1, -1}, TexUniform[2] = {-1, -1};
>>
>>  struct format_info
>>  {
>> @@ -131,18 +131,28 @@ static const struct format_info rgb10_formats[] = {
>> { GL_RGB10_A2UI, GL_BGRA_INTEGER_EXT, 10, GL_TRUE },
>>  };
>>
>> -static const char *FragShaderText =
>> +/* The programs will be indexed by whether the texture is signed or not. */
>> +static const char *FragShaderText[2] = {
>> "#version 130\n"
>> "uniform vec4 bias; \n"
>> -   "uniform isampler2D tex; \n"
>> +   "uniform usampler2D tex; \n"
>> "void main() \n"
>> "{ \n"
>> "   vec4 t = vec4(texture(tex, gl_TexCoord[0].xy)); \n"
>> "   gl_FragColor = t + bias; \n"
>> -   "} \n";
>> +   "} \n",
>>
>> -static GLuint Program;
>> +   "#version 130\n"
>> +   "uniform vec4 bias; \n"
>> +   "uniform isampler2D tex; \n"
>> +   "void main() \n"
>> +   "{ \n"
>> +   "   vec4 t = vec4(texture(tex, gl_TexCoord[0].xy)); \n"
>> +   "   gl_FragColor = t + bias; \n"
>> +   "} \n",
>> +};
>>
>> +static GLuint Program[2];
>>
>>  static int
>>  get_max_val(const struct format_info *info)
>> @@ -440,12 +450,14 @@ test_format(const struct format_info *info)
>> ;
>> }
>>
>> +   glUseProgram(Program[info->Signed]);
>> +
>> /* compute, set test bias */
>> bias[0] = expected[0] - value[0];
>> bias[1] = expected[1] - value[1];
>> bias[2] = expected[2] - value[2];
>> bias[3] = expected[3] - value[3];
>> -   glUniform4fv(BiasUniform, 1, bias);
>> +   glUniform4fv(BiasUniform[info->Signed], 1, bias);
>
> I'd feel very slightly better if this said BiasUniform[!!info->Signed]
> or some variant thereof (GL_TRUE is 1, but ... who knows). And same
> for Program[] above.
>
> Reviewed-by: Ilia Mirkin <imir...@alum.mit.edu>

No thanks.  true has always been 1, and won't change.  !! just makes
things more confusing.


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

[Piglit] [PATCH] draw-vertices-2101010: Don't test BGRA if we don't have the ext.

[Eric Anholt]

The 2101010 spec says to remove BGRA references if this extension
isn't present, so don't test them.
---
 .../draw-vertices-2101010.c   | 22 ++-
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c 
b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
index 5ce4c0655626..a672bb7d5520 100644
--- a/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
+++ b/tests/spec/arb_vertex_type_2_10_10_10_rev/draw-vertices-2101010.c
@@ -89,7 +89,7 @@ static GLuint vboColorPointer(GLint size, GLenum type, 
GLsizei stride,
 return id;
 }
 
-static void test_packed_int_color_vertices(float x1, float y1, float x2, float 
y2, int index)
+static bool test_packed_int_color_vertices(float x1, float y1, float x2, float 
y2, int index)
 {
 unsigned int v[3];
 unsigned int c[3];
@@ -111,6 +111,10 @@ static void test_packed_int_color_vertices(float x1, float 
y1, float x2, float y
 
 glVertexPointer(4, GL_INT_2_10_10_10_REV, 4, v);
 
+if (index >= 2 &&
+   !piglit_is_extension_supported("GL_EXT_vertex_array_bgra"))
+   return false;
+
 glEnableClientState(GL_COLOR_ARRAY);
 switch (index) {
 case 0: vbo = vboColorPointer(4, GL_INT_2_10_10_10_REV, 4, c, 
sizeof(c), 0); break;
@@ -123,9 +127,11 @@ static void test_packed_int_color_vertices(float x1, float 
y1, float x2, float y
 
 glDisableClientState(GL_COLOR_ARRAY);
 glDeleteBuffers(1, );
+
+return true;
 }
 
-static void test_packed_int_vertices(float x1, float y1, float x2, float y2, 
int index)
+static bool test_packed_int_vertices(float x1, float y1, float x2, float y2, 
int index)
 {
 unsigned int v[3];
 GLuint vbo;
@@ -148,9 +154,11 @@ static void test_packed_int_vertices(float x1, float y1, 
float x2, float y2, int
 glDrawArrays(GL_TRIANGLES, 0, 3);
 
 glDeleteBuffers(1, );
+
+return true;
 }
 
-static void test_int_vertices_abi(float x1, float y1, float x2, float y2, int 
index)
+static bool test_int_vertices_abi(float x1, float y1, float x2, float y2, int 
index)
 {
 GLuint v[3];
 GLuint c[3];
@@ -182,10 +190,12 @@ static void test_int_vertices_abi(float x1, float y1, 
float x2, float y2, int in
 glEnd();
 
 glEnableClientState(GL_VERTEX_ARRAY);
+
+return true;
 }
 
 struct test {
-void (*test)(float x1, float y1, float x2, float y2, int index);
+bool (*test)(float x1, float y1, float x2, float y2, int index);
 int index;
 float expected_color_equation_22[4];
 float expected_color_equation_23[4];
@@ -230,7 +240,9 @@ piglit_display(void)
 for (i = 0; tests[i].test; i++) {
 glBindBuffer(GL_ARRAY_BUFFER, 0);
 
-tests[i].test(x, y, x+20, y+20, tests[i].index);
+   if (!tests[i].test(x, y, x+20, y+20, tests[i].index))
+   continue;
+
 if (!piglit_check_gl_error(GL_NO_ERROR))
piglit_report_result(PIGLIT_FAIL);
 
-- 
2.17.0

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

[Piglit] [PATCH] arb_vertex_type_2_10_10_10: Don't test BGRA if we don't have the ext.

[Eric Anholt]

The 2101010 spec says to remove BGRA references if this extension
isn't present, so don't test them.
---
 tests/spec/arb_vertex_type_2_10_10_10_rev/array_types.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/spec/arb_vertex_type_2_10_10_10_rev/array_types.c 
b/tests/spec/arb_vertex_type_2_10_10_10_rev/array_types.c
index 4c7b199fa22c..e08c79bfc1ff 100644
--- a/tests/spec/arb_vertex_type_2_10_10_10_rev/array_types.c
+++ b/tests/spec/arb_vertex_type_2_10_10_10_rev/array_types.c
@@ -160,6 +160,10 @@ test(int x1, int y1, int x2, int y2, const struct 
test_info *test)
GLuint v[3], c[3];
bool pass;
 
+   if (test->size == GL_BGRA &&
+   !piglit_is_extension_supported("GL_EXT_vertex_array_bgra"))
+   return true;
+
printf("testing: %s\n", test->name);
 
/* vertex positions */
-- 
2.17.0

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

[Piglit] [PATCH] texture-integer-glsl130: Make sure we use usampler for ui textures.

[Eric Anholt]

Fixes failure on VC5 because we would sign-extend 16ui.  Sampling ui
with isampler is undefined according to the table under "8.7 Texture
Lookup Functions" in the 1.30 spec.
---
 .../texture-integer-glsl130.c | 40 ---
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/tests/spec/ext_texture_integer/texture-integer-glsl130.c 
b/tests/spec/ext_texture_integer/texture-integer-glsl130.c
index 2247207173a1..06332b66f2cf 100644
--- a/tests/spec/ext_texture_integer/texture-integer-glsl130.c
+++ b/tests/spec/ext_texture_integer/texture-integer-glsl130.c
@@ -44,7 +44,7 @@ static const char *TestName = "texture-integer";
 static GLint TexWidth = 16, TexHeight = 16;
 static GLuint Texture;
 
-static GLint BiasUniform = -1, TexUniform = -1;
+static GLint BiasUniform[2] = {-1, -1}, TexUniform[2] = {-1, -1};
 
 struct format_info
 {
@@ -131,18 +131,28 @@ static const struct format_info rgb10_formats[] = {
{ GL_RGB10_A2UI, GL_BGRA_INTEGER_EXT, 10, GL_TRUE },
 };
 
-static const char *FragShaderText =
+/* The programs will be indexed by whether the texture is signed or not. */
+static const char *FragShaderText[2] = {
"#version 130\n"
"uniform vec4 bias; \n"
-   "uniform isampler2D tex; \n"
+   "uniform usampler2D tex; \n"
"void main() \n"
"{ \n"
"   vec4 t = vec4(texture(tex, gl_TexCoord[0].xy)); \n"
"   gl_FragColor = t + bias; \n"
-   "} \n";
+   "} \n",
 
-static GLuint Program;
+   "#version 130\n"
+   "uniform vec4 bias; \n"
+   "uniform isampler2D tex; \n"
+   "void main() \n"
+   "{ \n"
+   "   vec4 t = vec4(texture(tex, gl_TexCoord[0].xy)); \n"
+   "   gl_FragColor = t + bias; \n"
+   "} \n",
+};
 
+static GLuint Program[2];
 
 static int
 get_max_val(const struct format_info *info)
@@ -440,12 +450,14 @@ test_format(const struct format_info *info)
;
}
 
+   glUseProgram(Program[info->Signed]);
+
/* compute, set test bias */
bias[0] = expected[0] - value[0];
bias[1] = expected[1] - value[1];
bias[2] = expected[2] - value[2];
bias[3] = expected[3] - value[3];
-   glUniform4fv(BiasUniform, 1, bias);
+   glUniform4fv(BiasUniform[info->Signed], 1, bias);
 
/* draw */
glClearColor(0, 1, 1, 0);
@@ -584,17 +596,17 @@ piglit_display(void)
 void
 piglit_init(int argc, char **argv)
 {
+   int i;
+
piglit_require_extension("GL_EXT_texture_integer");
piglit_require_GLSL_version(130);
 
-   Program = piglit_build_simple_program(NULL, FragShaderText);
-
-   glUseProgram(Program);
-
-   BiasUniform = glGetUniformLocation(Program, "bias");
-   TexUniform = glGetUniformLocation(Program, "tex");
-
-   glUniform1i(TexUniform, 0);  /* tex unit zero */
+   for (i = 0; i < 2; i++) {
+   Program[i] = piglit_build_simple_program(NULL,
+FragShaderText[i]);
+   BiasUniform[i] = glGetUniformLocation(Program[i], "bias");
+   TexUniform[i] = glGetUniformLocation(Program[i], "tex");
+   }
 
(void) check_error(__FILE__, __LINE__);
 
-- 
2.17.0

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

Re: [Piglit] [PATCH] add tests for using immediates in common operations

[Eric Anholt]

Ilia Mirkin <imir...@alum.mit.edu> writes:

> On Mon, Apr 23, 2018 at 4:49 PM, Eric Anholt <e...@anholt.net> wrote:
>> Ilia Mirkin <imir...@alum.mit.edu> writes:
>>
>>> NVIDIA hardware has "short" immediates, and some instructions have both
>>> long and short immediate variants. Nouveau was picking the wrong one
>>> under certain conditions. This adds tests which are likely to cause
>>> these kinds of issues to surface in a backend.
>>
>> My only concern is the integer multiply tests on non-native-integer
>> hardware or things where the number of integer bits is too small.  That
>> said, whoever tries to support one of those will have plenty of problems
>> already and need to define some shader_runner precision requirement
>> annotations.
>>
>> Reviewed-by: Eric Anholt <e...@anholt.net>
>
> Hmmm ... I didn't think of that. Should I bump these up to GLSL 1.30?
> I don't think you could do GLSL 1.30 without proper integers.

Looks like 1.30 does give you exactly 32-bit ints, so that would cover
my concern.


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

Re: [Piglit] [PATCH] framework: update deqp mustpass list for text based mustpass

[Eric Anholt]

Dylan Baker <dy...@pnwbakers.com> writes:

> The current code has bit-rotted quite a bit, in late 2016 the format was
> changed from XML based to txt based, but the code was never updated.
> ---

I think you can remove the elementtree import now.

Other than that,

Reviewed-by: Eric Anholt <e...@anholt.net>

(though deqp seems much less important to me than VK-GL-CTS, given that
you need to be running from VK-GL-CTS for conformance submissions as far
as I understand)


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

Re: [Piglit] [PATCH 00/35] Serialize profiles into XML at build time

[Eric Anholt]

Marek Olšák  writes:

> Is this use case affected?
>
> piglit run --deqp-mustpass-list --process-isolation 0 -p gbm -c quick
> cts_gl45 deqp_gles2 deqp_gles3 deqp_gles31
>
> Yes, that is just 1 command to run all those test suites at the same time.
>
> I use my personal "deqp" piglit branch that also disables process isolation
> for glcts and deqp, and parses the deqp mustpass lists which are in txt
> files.

Parsing the mustpass lists sounds really useful.  Trying to construct an
appropriate command line otherwise has been quite a challenge.


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

Re: [Piglit] [PATCH] AMD_performance_monitor: add VC4 specific tests

[Eric Anholt]

Boris Brezillon  writes:

> From: Boris Brezillon 
>
> This adds a specific test for the VC4 GPU.
> Right now, it only checks that FEP-valid-quads and
> QPU-total-clk-cycles-waiting-TMU are consistent after executing well
> know operations (draw a rectangle or a texture).
>
> More tests will be added over time.
>
> Signed-off-by: Boris Brezillon 

I added it to all.py, fixed a compiler warning (skipping subtests didn't
skip, oops!), and pushed.  Thanks!


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

Re: [Piglit] [PATCH 1/2] fbo-drawbuffers2-blend: fix incorrect comments

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> ---
>  tests/fbo/fbo-drawbuffers2-blend.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

These 2 are:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH] AMD_performance_monitor: add VC4 specific tests

[Eric Anholt]

Boris Brezillon <boris.brezil...@bootlin.com> writes:

> On Fri, 09 Feb 2018 21:12:20 +0000
> Eric Anholt <e...@anholt.net> wrote:
>
>> Boris Brezillon <boris.brezil...@bootlin.com> writes:
>> 
>> > From: Boris Brezillon <boris.brezil...@free-electrons.com>
>> >
>> > This adds a specific test for the VC4 GPU.
>> > Right now, it only checks that FEP-valid-quads and
>> > QPU-total-clk-cycles-waiting-TMU are consistent after executing well
>> > know operations (draw a rectangle or a texture).
>> >
>> > More tests will be added over time.  
>> 
>> I'd still like to put together some tests of state management (to catch
>> things like the flushing bugs we had in the first revs), but this seems
>> like a good start.
>> 
>> > +#define FEP_VALID_QUADS_REF_VAL   6440
>> > +
>> > +static void draw_rect(const struct perfmon_test *test)
>> > +{
>> > +  piglit_draw_rect(-1, -1, 3, 3);
>> > +}
>> > +
>> > +static void draw_tex(const struct perfmon_test *test)
>> > +{
>> > +  GLuint tex;
>> > +
>> > +  tex = piglit_rgbw_texture(GL_RGBA, 64, 64, false, true,
>> > +GL_UNSIGNED_BYTE);
>> > +  verify(piglit_check_gl_error(GL_NO_ERROR));
>> > +
>> > +  glEnable(GL_TEXTURE_2D);
>> > +  glBindTexture(GL_TEXTURE_2D, tex);
>> > +  piglit_draw_rect_tex(-1, -1, 2, 2, 0, 0, 1, 1);
>> > +  glDisable(GL_TEXTURE_2D);
>> > +  glDeleteTextures(1, );
>> > +}
>> > +
>> > +static bool fep_valid_quads_check_res(uint64_t res)
>> > +{
>> > +  return res == FEP_VALID_QUADS_REF_VAL;  
>> 
>> We should probably have the number of valid quads either be some math
>> based on the config.window_width/height, or explicitly set the
>> window_width/height so it's not dependent on whatever defaults the
>> framework chooses.
>> 
>> The valid quads is a pretty magic value because we're drawing this:
>> 
>> +---+
>> |  /|
>> | / |
>> |/  |
>> +---+
>> 
>> so some of the 2x2 quads get drawn twice along the middle edge between
>> the tris.  If we drew the triangle outside of the window:
>> 
>> +---+---+
>> |   |  /
>> |   | /
>> |   |/
>> +---/
>> |  /
>> | /
>> |/
>> +
>> 
>> with piglit_draw_rect(-1, -2, 4, 4), then I don't think any of the quads
>> would be drawn twice, and the magic REF_VAL would become
>> align(window_width, 2) * align(window_height, 2) / 4.
>
> So I picked win.height=width=64 and I have a mismatch. According to
> your formula I should have fep_valid_quads=1024, but I get 1040.

Huh, maybe we go past the guard band and get clipped into a quad?  I
guess just hardcode the value, then.


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

Re: [Piglit] [PATCH] AMD_performance_monitor: add VC4 specific tests

[Eric Anholt]

Boris Brezillon  writes:

> From: Boris Brezillon 
>
> This adds a specific test for the VC4 GPU.
> Right now, it only checks that FEP-valid-quads and
> QPU-total-clk-cycles-waiting-TMU are consistent after executing well
> know operations (draw a rectangle or a texture).
>
> More tests will be added over time.

I'd still like to put together some tests of state management (to catch
things like the flushing bugs we had in the first revs), but this seems
like a good start.

> +#define FEP_VALID_QUADS_REF_VAL  6440
> +
> +static void draw_rect(const struct perfmon_test *test)
> +{
> + piglit_draw_rect(-1, -1, 3, 3);
> +}
> +
> +static void draw_tex(const struct perfmon_test *test)
> +{
> + GLuint tex;
> +
> + tex = piglit_rgbw_texture(GL_RGBA, 64, 64, false, true,
> +   GL_UNSIGNED_BYTE);
> + verify(piglit_check_gl_error(GL_NO_ERROR));
> +
> + glEnable(GL_TEXTURE_2D);
> + glBindTexture(GL_TEXTURE_2D, tex);
> + piglit_draw_rect_tex(-1, -1, 2, 2, 0, 0, 1, 1);
> + glDisable(GL_TEXTURE_2D);
> + glDeleteTextures(1, );
> +}
> +
> +static bool fep_valid_quads_check_res(uint64_t res)
> +{
> + return res == FEP_VALID_QUADS_REF_VAL;

We should probably have the number of valid quads either be some math
based on the config.window_width/height, or explicitly set the
window_width/height so it's not dependent on whatever defaults the
framework chooses.

The valid quads is a pretty magic value because we're drawing this:

+---+
|  /|
| / |
|/  |
+---+

so some of the 2x2 quads get drawn twice along the middle edge between
the tris.  If we drew the triangle outside of the window:

+---+---+
|   |  /
|   | /
|   |/
+---/
|  /
| /
|/
+

with piglit_draw_rect(-1, -2, 4, 4), then I don't think any of the quads
would be drawn twice, and the magic REF_VAL would become
align(window_width, 2) * align(window_height, 2) / 4.


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

Re: [Piglit] [PATCH v4] travis: add docker based cmake build job

[Eric Anholt]

Andres Gomez <ago...@igalia.com> writes:

> Until now we were only running the python unit tests.
>
> It seems desirable to also check that the CMake based build compiles
> successfully. We do that now using docker.
>
> The docker build can be tweaked with some environment variables and,
> also, be stored in the docker hub if desired. Check the changes for
> extra details regarding these variables.
>
> v2: Removed other build possibilities other than just from inside
> Travis-CI, as suggested by Juan.
> v3: Replaced the "RELEASE" parameter to create the docker image with
> "PARENT" and removed some unneeded documentation after v2, as
> suggested by Juan.
> v4:
>   - Use DOCKER_PARENT, DOCKER_IMAGE and DOCKER_TAG variables from the
> custom Travis-CI setup to define the PARENT, IMAGE and TAG
> parameters during the docker image creation.
>   - Upload the image only if DOCKER_IMAGE and DOCKER_TAG are set.
>
> Cc: Dylan Baker <dy...@pnwbakers.com>
> Cc: Juan A. Suarez <jasua...@igalia.com>
> Signed-off-by: Andres Gomez <ago...@igalia.com>
> Reviewed-by: Juan A. Suarez <jasua...@igalia.com>

If this build has proven to be fast enough, I should probably take a
look at doing this for the xserver as well.  (For xserver, the issue was
all the package installs per Travis run, so I have an image I manage on
dockerhub instead)

Acked-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v3] travis: add docker based cmake build job

[Eric Anholt]

Andres Gomez  writes:

> Until now we were only running the python unit tests.
>
> It seems desirable to also check that the CMake based build compiles
> successfully. We do that now using docker.
>
> The docker build can be tweaked with some environment variables and,
> also, be stored in the docker hub if desired. Check the changes for
> extra details regarding these variables.
>
> v2: Removed other build possibilities other than just from inside
> Travis-CI, as suggested by Juan.
> v3: Replaced the "RELEASE" parameter to create the docker image with
> "PARENT" and removed some unneeded documentation after v2, as
> suggested by Juan.
>
> Cc: Dylan Baker 
> Cc: Juan A. Suarez 
> Signed-off-by: Andres Gomez 
> Reviewed-by: Juan A. Suarez 

I'm definitely interested in extending our tests using Travis, but could
you add some comments explaining what this system does?  Do we do a
bunch of image build work (numpy/six/mako install) on each Travis build,
or does it get cached?  Do I need to do any setup in Travis in order for
this to take effect?


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

Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> On 01/18/2018 01:27 PM, Eric Anholt wrote:
>> Brian Paul <bri...@vmware.com> writes:
>> 
>>> To avoid an infinite loop.  See code comments for details.
>> 
>> Skipping a failing test and returning pass is wrong to me.
>
> It's not ideal.  But the bug is in LLVM and cannot readily be fixed in 
> llvmpipe.
>
> I could have the test return a WARN result in this situation.  Would 
> that be better?

It's still a bug in the driver, even if it's because the driver's using
a buggy external library.  It should be a fail.


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

Re: [Piglit] [PATCH] gl-1.0-blend-func: skip some blend tests when using LLVM 3.8

[Eric Anholt]

Brian Paul  writes:

> To avoid an infinite loop.  See code comments for details.

Skipping a failing test and returning pass is wrong to me.


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

Re: [Piglit] [PATCH 0/3] cleanups for ATI_fs tests

[Eric Anholt]

Miklós Máté  writes:

> Please do so. These are nice cleanups.
>
> MM

I finally got around to pushing these.  Once again, thanks for your work
on this!


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

Re: [Piglit] [PATCH] arb_texture_view: fix order of vertices in draw_3d_depth()

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> The vertices are drawn with a GL_TRIANGLE_STRIP.  As it was, the
> order of the 3rd and 4th vertices caused us to render a "twisted"
> quad.  With this change, a true quad is drawn.
>
> llvmpipe passes as it did before.  NVIDIA's driver still fails the test.
> ---
>  tests/spec/arb_texture_view/common.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tests/spec/arb_texture_view/common.c 
> b/tests/spec/arb_texture_view/common.c
> index fc18bbc..f2453f5 100644
> --- a/tests/spec/arb_texture_view/common.c
> +++ b/tests/spec/arb_texture_view/common.c
> @@ -105,13 +105,13 @@ void
>  draw_3d_depth(float x, float y, float w, float h, int depth)
>  {
>   const GLfloat vertices[16] =  {x, y, depth, 0.0,
> -  x+w, y, depth, 0.0,
> -  x+w, y+h, depth, 0.0,
> -  x, y+h, depth, 0.0};
> +x+w, y, depth, 0.0,
> +x, y+h, depth, 0.0,
> +x+w, y+h, depth, 0.0};
>   const GLfloat texcoords[8] = {0.0, 0.0,
> -  1.0, 0.0,
> -  1.0, 1.0,
> -  0.0, 1.0};
> +   1.0, 0.0,
> +   0.0, 1.0,
> +   1.0, 1.0};

This matches the ordering that other callers of
piglit_draw_rect_from_arrays() use.

Reviewed-by: Eric Anholt <e...@anholt.net>




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

Re: [Piglit] [PATCH v2] ext_disjoint_timer_query-simple: basic API test

[Eric Anholt]

Tapani Pälli <tapani.pa...@intel.com> writes:

> v2: cleanup code, remove some GL_NO_ERROR checks
> add to all.py (Eric)
>
> Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>

> diff --git a/tests/all.py b/tests/all.py
> index 0c99bff0e..4e41aebdd 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -3295,6 +3295,11 @@ with profile.test_list.group_manager(
>  grouptools.join('spec', 'oes_required_internalformat')) as g:
>  g(['oes_required_internalformat-renderbuffer'], 'renderbuffer')
>  
> +with profile.test_list.group_manager(
> +PiglitGLTest,
> +grouptools.join('spec', 'ext_disjoint_timer_query')) as g:
> +g(['ext_disjoint_timer_query-simple'], 'query')

Can we s/'query'/'simple'/ to match the binary's name?  With that fixed,

Reviewed-by: Eric Anholt <e...@anholt.net>


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

[Piglit] [PATCH 2/3] ati_fs: Use enums instead of magic numbers in the render tests.

[Eric Anholt]

Signed-off-by: Eric Anholt <e...@anholt.net>
---
 tests/spec/ati_fragment_shader/render-notexture.c | 13 +---
 tests/spec/ati_fragment_shader/render-sources.c   | 36 +--
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/tests/spec/ati_fragment_shader/render-notexture.c 
b/tests/spec/ati_fragment_shader/render-notexture.c
index 12dfe6de6afe..d85d030dee81 100644
--- a/tests/spec/ati_fragment_shader/render-notexture.c
+++ b/tests/spec/ati_fragment_shader/render-notexture.c
@@ -40,6 +40,11 @@ static const float color1[] = {0.2, 0.3, 0.8};
 static const float texcoord[] = {0.2, 0.7, 0.4};
 static const float black[] = {0.0, 0.0, 0.0};
 
+enum {
+   SHADER_TEXCOORD = 1,
+   SHADER_TEX,
+};
+
 enum piglit_result
 piglit_display(void)
 {
@@ -54,9 +59,9 @@ piglit_display(void)
glTexCoord3fv(texcoord);
 
glEnable(GL_FRAGMENT_SHADER_ATI);
-   glBindFragmentShaderATI(6);
+   glBindFragmentShaderATI(SHADER_TEXCOORD);
piglit_draw_rect(0, 0, piglit_width / 4, piglit_height);
-   glBindFragmentShaderATI(42);
+   glBindFragmentShaderATI(SHADER_TEX);
piglit_draw_rect(piglit_width / 4, 0, piglit_width / 4, piglit_height);
glGenTextures(1, );
glBindTexture(GL_TEXTURE_2D, tex);
@@ -89,14 +94,14 @@ piglit_init(int argc, char **argv)
 {
piglit_require_extension("GL_ATI_fragment_shader");
 
-   glBindFragmentShaderATI(6);
+   glBindFragmentShaderATI(SHADER_TEXCOORD);
glBeginFragmentShaderATI();
glPassTexCoordATI(GL_REG_1_ATI, GL_TEXTURE0_ARB, GL_SWIZZLE_STR_ATI);
glColorFragmentOp1ATI(GL_MOV_ATI, GL_REG_0_ATI, GL_NONE, GL_NONE,
GL_REG_1_ATI, GL_NONE, GL_NONE);
glEndFragmentShaderATI();
 
-   glBindFragmentShaderATI(42);
+   glBindFragmentShaderATI(SHADER_TEX);
glBeginFragmentShaderATI();
glSampleMapATI(GL_REG_0_ATI, GL_TEXTURE0_ARB, GL_SWIZZLE_STR_ATI);
glColorFragmentOp1ATI(GL_MOV_ATI, GL_REG_0_ATI, GL_NONE, GL_NONE,
diff --git a/tests/spec/ati_fragment_shader/render-sources.c 
b/tests/spec/ati_fragment_shader/render-sources.c
index da70b0cce14c..ccc4247c70c8 100644
--- a/tests/spec/ati_fragment_shader/render-sources.c
+++ b/tests/spec/ati_fragment_shader/render-sources.c
@@ -48,12 +48,18 @@ static const float color2[] = {0.9, 0.8, 0.3};
 static const float texcoord[] = {0.2, 0.7, 0.4};
 static const float texcolor[] = {0.8, 0.1, 0.7};
 
-static float result6[] = {0.0, 0.0, 0.0};
-static float result13[] = {0.0, 0.0, 0.0};
-static float result42[] = {0.0, 0.0, 0.0};
+static float result_const[] = {0.0, 0.0, 0.0};
+static float result_tex[] = {0.0, 0.0, 0.0};
+static float result_color[] = {0.0, 0.0, 0.0};
 
 static GLuint tex;
 
+enum {
+   SHADER_TEX = 1,
+   SHADER_CONST,
+   SHADER_COLOR,
+};
+
 enum piglit_result
 piglit_display(void)
 {
@@ -73,20 +79,20 @@ piglit_display(void)
glEnable(GL_FRAGMENT_SHADER_ATI);
glBindFragmentShaderATI(0);
piglit_draw_rect(0, 0, piglit_width / 4, piglit_height);
-   glBindFragmentShaderATI(6);
+   glBindFragmentShaderATI(SHADER_CONST);
piglit_draw_rect(piglit_width / 4, 0, piglit_width / 4, piglit_height);
-   glBindFragmentShaderATI(42);
+   glBindFragmentShaderATI(SHADER_COLOR);
piglit_draw_rect(2 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height);
-   glBindFragmentShaderATI(13);
+   glBindFragmentShaderATI(SHADER_TEX);
piglit_draw_rect(3 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height);
glDisable(GL_FRAGMENT_SHADER_ATI);
 
/* Mesa uses fixed-function when the shader is invalid, but
 * it's undefined */
/*pass = pass && piglit_probe_rect_rgb(0, 0, piglit_width / 4, 
piglit_height, color1*texcolor);*/
-   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0, piglit_width 
/ 4, piglit_height, result6);
-   pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result42);
-   pass = pass && piglit_probe_rect_rgb(3 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result13);
+   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0, piglit_width 
/ 4, piglit_height, result_const);
+   pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result_color);
+   pass = pass && piglit_probe_rect_rgb(3 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result_tex);
 
piglit_present_results();
 
@@ -106,7 +112,7 @@ piglit_init(int argc, char **argv)
 * texcoord, sample, const, zero, one, pri color
 */
 
-   glBindFragmentShaderATI(13);
+   glBindFragmentShaderATI(SHADER_TEX);
glBeginFragmentShaderATI();
glPassTexCoordATI(GL_REG_1_ATI, GL_TEXTURE0_ARB, GL_SWIZZLE_STR_ATI);
glSampleMapATI(GL_REG_0_ATI, GL_TEX

[Piglit] [PATCH 3/3] ati_fs: Do some column wrapping in the render tests.

[Eric Anholt]

---
 tests/spec/ati_fragment_shader/render-constants.c  | 25 ++-
 tests/spec/ati_fragment_shader/render-default.c| 24 ++-
 tests/spec/ati_fragment_shader/render-fog.c| 28 -
 tests/spec/ati_fragment_shader/render-notexture.c  | 24 ++-
 tests/spec/ati_fragment_shader/render-precedence.c | 25 ++-
 tests/spec/ati_fragment_shader/render-sources.c| 36 --
 tests/spec/ati_fragment_shader/render-textargets.c | 29 +++--
 7 files changed, 136 insertions(+), 55 deletions(-)

diff --git a/tests/spec/ati_fragment_shader/render-constants.c 
b/tests/spec/ati_fragment_shader/render-constants.c
index fd90a943405a..c5ee2c8d8365 100644
--- a/tests/spec/ati_fragment_shader/render-constants.c
+++ b/tests/spec/ati_fragment_shader/render-constants.c
@@ -64,18 +64,29 @@ piglit_display(void)
glBindFragmentShaderATI(s_global);
piglit_draw_rect(0, 0, piglit_width / 4, piglit_height); /* 2+3 */
glBindFragmentShaderATI(s_local);
-   piglit_draw_rect(piglit_width / 4, 0, piglit_width / 4, piglit_height); 
/* 1+3 */
+   piglit_draw_rect(piglit_width / 4, 0,
+piglit_width / 4, piglit_height); /* 1+3 */
glBindFragmentShaderATI(s_global);
glSetFragmentShaderConstantATI(GL_CON_7_ATI, color4);
-   piglit_draw_rect(2 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height); /* 4+3 */
+   piglit_draw_rect(2 * piglit_width / 4, 0,
+piglit_width / 4, piglit_height); /* 4+3 */
glBindFragmentShaderATI(s_local);
-   piglit_draw_rect(3 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height); /*1+3 */
+   piglit_draw_rect(3 * piglit_width / 4, 0,
+piglit_width / 4, piglit_height); /*1+3 */
glDisable(GL_FRAGMENT_SHADER_ATI);
 
-   pass = pass && piglit_probe_rect_rgb(0, 0, piglit_width / 4, 
piglit_height, result2p3);
-   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0, piglit_width 
/ 4, piglit_height, result1p3);
-   pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result4p3);
-   pass = pass && piglit_probe_rect_rgb(3 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result1p3);
+   pass = pass && piglit_probe_rect_rgb(0, 0,
+piglit_width / 4,
+piglit_height, result2p3);
+   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0,
+piglit_width / 4,
+piglit_height, result1p3);
+   pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0,
+piglit_width / 4,
+piglit_height, result4p3);
+   pass = pass && piglit_probe_rect_rgb(3 * piglit_width / 4, 0,
+piglit_width / 4,
+piglit_height, result1p3);
 
piglit_present_results();
 
diff --git a/tests/spec/ati_fragment_shader/render-default.c 
b/tests/spec/ati_fragment_shader/render-default.c
index 89e8cb09d13e..e760e065da2d 100644
--- a/tests/spec/ati_fragment_shader/render-default.c
+++ b/tests/spec/ati_fragment_shader/render-default.c
@@ -56,14 +56,24 @@ piglit_display(void)
glEnable(GL_FRAGMENT_SHADER_ATI);
piglit_draw_rect(piglit_width / 4, 0, piglit_width / 4, piglit_height);
glDisable(GL_FRAGMENT_SHADER_ATI);
-   piglit_draw_rect(2 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height);
+   piglit_draw_rect(2 * piglit_width / 4, 0,
+piglit_width / 4, piglit_height);
glEnable(GL_FRAGMENT_SHADER_ATI);
-   piglit_draw_rect(3 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height);
-
-   pass = pass && piglit_probe_rect_rgb(0, 0, piglit_width / 4, 
piglit_height, color);
-   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0, piglit_width 
/ 4, piglit_height, texcoord);
-   pass = pass && piglit_probe_rect_rgb(2*piglit_width / 4, 0, 
piglit_width / 4, piglit_height, color);
-   pass = pass && piglit_probe_rect_rgb(3*piglit_width / 4, 0, 
piglit_width / 4, piglit_height, texcoord);
+   piglit_draw_rect(3 * piglit_width / 4, 0,
+piglit_width / 4, piglit_height);
+
+   pass = pass && piglit_probe_rect_rgb(0, 0,
+piglit_width / 4,
+piglit_height, color);
+   pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0,
+piglit_width / 4,
+piglit_height, texcoord);
+   pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0,
+piglit_width / 4,
+   

[Piglit] [PATCH 0/3] cleanups for ATI_fs tests

[Eric Anholt]

Here are a few little cleanups I made while reviewing the series.
Miklós, if you are OK with these, then I'll squash them into your
patches and push the whole set.

Eric Anholt (3):
  ati_fs: Wrap some comments in render tests.
  ati_fs: Use enums instead of magic numbers in the render tests.
  ati_fs: Do some column wrapping in the render tests.

 tests/spec/ati_fragment_shader/render-constants.c  | 25 +---
 tests/spec/ati_fragment_shader/render-default.c| 31 +++---
 tests/spec/ati_fragment_shader/render-fog.c| 28 ++---
 tests/spec/ati_fragment_shader/render-notexture.c  | 37 +++
 tests/spec/ati_fragment_shader/render-precedence.c | 25 +---
 tests/spec/ati_fragment_shader/render-sources.c| 71 +++---
 tests/spec/ati_fragment_shader/render-textargets.c | 32 +++---
 7 files changed, 174 insertions(+), 75 deletions(-)

-- 
2.15.0

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

[Piglit] [PATCH 1/3] ati_fs: Wrap some comments in render tests.

[Eric Anholt]

This would be squashed into patch 2.
---
 tests/spec/ati_fragment_shader/render-default.c| 7 +--
 tests/spec/ati_fragment_shader/render-sources.c| 7 +--
 tests/spec/ati_fragment_shader/render-textargets.c | 3 ++-
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/tests/spec/ati_fragment_shader/render-default.c 
b/tests/spec/ati_fragment_shader/render-default.c
index a2e2cecfffeb..89e8cb09d13e 100644
--- a/tests/spec/ati_fragment_shader/render-default.c
+++ b/tests/spec/ati_fragment_shader/render-default.c
@@ -22,7 +22,8 @@
  */
 
 /**
- * Tests rendering with GL_ATI_fragment_shader: enable, disable, default 
fragment shader.
+ * Tests rendering with GL_ATI_fragment_shader: enable, disable,
+ * default fragment shader.
  */
 
 #include "piglit-util-gl.h"
@@ -76,7 +77,9 @@ piglit_init(int argc, char **argv)
 {
piglit_require_extension("GL_ATI_fragment_shader");
 
-   /* create a default shader that does something different than 
fixed-function */
+   /* create a default shader that does something different than
+* fixed-function
+*/
glBeginFragmentShaderATI();
glPassTexCoordATI(GL_REG_1_ATI, GL_TEXTURE0_ARB, GL_SWIZZLE_STR_ATI);
glColorFragmentOp1ATI(GL_MOV_ATI, GL_REG_0_ATI, GL_NONE, GL_NONE,
diff --git a/tests/spec/ati_fragment_shader/render-sources.c 
b/tests/spec/ati_fragment_shader/render-sources.c
index be0de33073c7..da70b0cce14c 100644
--- a/tests/spec/ati_fragment_shader/render-sources.c
+++ b/tests/spec/ati_fragment_shader/render-sources.c
@@ -81,7 +81,8 @@ piglit_display(void)
piglit_draw_rect(3 * piglit_width / 4, 0, piglit_width / 4, 
piglit_height);
glDisable(GL_FRAGMENT_SHADER_ATI);
 
-   /* Mesa uses fixed-function when the shader is invalid, but it's 
undefined */
+   /* Mesa uses fixed-function when the shader is invalid, but
+* it's undefined */
/*pass = pass && piglit_probe_rect_rgb(0, 0, piglit_width / 4, 
piglit_height, color1*texcolor);*/
pass = pass && piglit_probe_rect_rgb(piglit_width / 4, 0, piglit_width 
/ 4, piglit_height, result6);
pass = pass && piglit_probe_rect_rgb(2 * piglit_width / 4, 0, 
piglit_width / 4, piglit_height, result42);
@@ -101,7 +102,9 @@ piglit_init(int argc, char **argv)
 
piglit_require_extension("GL_ATI_fragment_shader");
 
-   /* create shaders that use all possible input sources: texcoord, 
sample, const, zero, one, pri color */
+   /* create shaders that use all possible input sources:
+* texcoord, sample, const, zero, one, pri color
+*/
 
glBindFragmentShaderATI(13);
glBeginFragmentShaderATI();
diff --git a/tests/spec/ati_fragment_shader/render-textargets.c 
b/tests/spec/ati_fragment_shader/render-textargets.c
index 88fec9742b4a..82cfd7a4d181 100644
--- a/tests/spec/ati_fragment_shader/render-textargets.c
+++ b/tests/spec/ati_fragment_shader/render-textargets.c
@@ -22,7 +22,8 @@
  */
 
 /**
- * Tests rendering with GL_ATI_fragment_shader: using different texture 
targets with the same shader.
+ * Tests rendering with GL_ATI_fragment_shader: using different
+ * texture targets with the same shader.
  */
 
 #include "piglit-util-gl.h"
-- 
2.15.0

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

Re: [Piglit] [PATCH] fbo-blending-snorm: new test for testing snorm blend behavior

[Eric Anholt]

srol...@vmware.com writes:

> From: Roland Scheidegger 
>
> The existing fbo-blending-formats test is mostly useless for anything but
> unorm formats, since it does not test any values outside [0,1] (neither for
> src, dst, intermeidate calculations, blend result).
> I tried to enhance it but it got too complex, in particular because I really
> want to test snorm, not floats (which aren't really validated much neither),
> because if you actually use int math for them it's difficult to handle and
> llvmpipe had lots of bugs. And it's not even obvious from the GL spec when
> clamping actually happens (in particular, the inverse blend factors will be
> in range [0,2]). snorm blending is sort of semi-supported in GL, the
> presence of EXT_texture_snorm doesn't guarantee it (and nvidia doesn't support
> the extension, presumably because they can't or don't want to deal with the
> legacy alpha/luminance/intensity formats), and while GL 3.1 introduces the
> snorm formats too it isn't guaranteed for rendering neither (for GLES OTOH
> there's a EXT_render_snorm extension), so the format handling of the
> fbo-blending-formats test isn't really sufficient and not easily extensible.
> So, this test will test actual blend behavior with values which will need
> clamping, and where the intermediate and final values will be negative (and,
> for the inverse blend factors, be even larger than one).
> This passes (now) on llvmpipe, and nvidia blob. softpipe is a complete
> failure (if there's clamping it will always clamp to [0,1] for starters),
> and as a matter of fact, softpipe doesn't get the clamping right even with
> unorm neither, since values outside [0,1] won't get clamped in the tile
> cache when there's no clamping, hence they'll enter the blend later when
> blend is enabled unclamped - but there's no test for this (note this is
> only an issue if the fragment color clamp is disabled).

I thought the ARB_color_buffer_float/render test was trying to cover
this.


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

Re: [Piglit] [PATCH 2/4] ATI_fs: add render tests

[Eric Anholt]

Miklós Máté <mtm...@gmail.com> writes:

> On 21/11/17 21:17, Eric Anholt wrote:
>> Miklós Máté <mtm...@gmail.com> writes:
>>> diff --git a/tests/spec/ati_fragment_shader/render-notexture.c 
>>> b/tests/spec/ati_fragment_shader/render-notexture.c
>>> new file mode 100644
>>> index 0..2d9aefe09
>>> --- /dev/null
>>> +++ b/tests/spec/ati_fragment_shader/render-notexture.c
>>> @@ -0,0 +1,71 @@
>>> +/* TODO license header */
>>> +
>>> +/**
>>> + * Tests rendering with GL_ATI_fragment_shader: when no texture is bound
>>> + * - glPassTexCoordATI() should work as normal
>>> + * - glSampleMapATI() should return all zeros (technically this is 
>>> undefined)
>> I don't think that would be undefined -- you'd have the default texture
>> object used, which would sample as zeroes.
>
> Should I test the right hand side to be black then?

I think so.


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

Re: [Piglit] [PATCH 1/4] ATI_fs: add api error tests

[Eric Anholt]

Miklós Máté <mtm...@gmail.com> writes:

> On 21/11/17 21:09, Eric Anholt wrote:
>> Miklós Máté <mtm...@gmail.com> writes:
>>> +
>>> +   piglit_report_result(PIGLIT_PASS);
>>> +}
>>> diff --git a/tests/spec/ati_fragment_shader/error02-inside.c 
>>> b/tests/spec/ati_fragment_shader/error02-inside.c
>>> new file mode 100644
>>> index 0..5dee70cf6
>>> --- /dev/null
>>> +++ b/tests/spec/ati_fragment_shader/error02-inside.c
>>> @@ -0,0 +1,59 @@
>>> +/* TODO license header */
>>> +
>>> +/**
>>> + * Tests API errors for GL_ATI_fragment_shader.
>>> + * One for each paragraph in the Errors section.
>>> + */
>>> +
>>> +#include "piglit-util-gl.h"
>>> +
>>> +PIGLIT_GL_TEST_CONFIG_BEGIN
>>> +
>>> +   config.supports_gl_compat_version = 10;
>>> +   config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
>>> +
>>> +PIGLIT_GL_TEST_CONFIG_END
>>> +
>>> +enum piglit_result
>>> +piglit_display(void)
>>> +{
>>> +   /* UNREACHED */
>>> +   return PIGLIT_FAIL;
>>> +}
>>> +
>>> +#define check_gl_error(err) if (!piglit_check_gl_error(err)) 
>>> piglit_report_result(PIGLIT_FAIL)
>>> +
>>> +void
>>> +piglit_init(int argc, char **argv)
>>> +{
>>> +   piglit_require_extension("GL_ATI_fragment_shader");
>>> +
>>> +   /*
>>> +* Paragraph 2 of the Errors section:
>>> +*
>>> +* The error INVALID_OPERATION is generated if GenFragmentShadersATI,
>>> +* BindFragmentShaderATI, DeleteFragmentShaderATI, or
>>> +* BeginFragmentShaderATI are specified inside a
>>> +* Begin/EndFragmentShaderATI pair.
>>> +*/
>>> +
>>> +   glBeginFragmentShaderATI();
>>> +
>>> +   check_gl_error(GL_NO_ERROR);
>>> +   glGenFragmentShadersATI(1);
>>> +   check_gl_error(GL_INVALID_OPERATION);
>> Instead of the macro, we'll often have
>>
>> bool pass = true;
>>
>> ...
>>
>> pass = piglit_check_gl_error(GL_NO_ERROR) && pass
>> glGenFragmentShadersATI(1);
>> pass = piglit_check_gl_error(GL_INVALID_OPERATION) && pass
>>
>> ...
>>
>> piglit_report_result(pass ? PIGLIT_PASS : PIGLIT_FAIL);
>>
>> That way you get to see multiple errors if there are different cases
>> failing.
> First I did that, but I had a difficult time finding out where Mesa 
> failed to throw the expected error, so I made the check fatal. If people 
> prefer non-fatal checks I can change it back.
>

piglit_check_gl_error() prints the file and line number of the check
that failed, to help you find it.

I'm not requiring a change here, just suggesting a way this has been
handled elsewhere.

>>> +   check_gl_error(GL_NO_ERROR);
>> I'd drop the GL_NO_ERROR checks -- you know there's no error because the
>> last thing you did is check the errors.
> As far as I understand the errors are pushed into a stack. I do these no 
> error checks to make sure the stack is empty before the critical call. 
> If it's not possible for one gl call to push more than one error into 
> the stack, then the no error checks can indeed be trimmed down in this test.

You can see the path in _mesa_error() -> _mesa_record_error() -- it's
just a single value, not a stack.


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

Re: [Piglit] [PATCH] ext_disjoint_timer_query-simple: basic API test

[Eric Anholt]

Tapani Pälli  writes:

> Signed-off-by: Tapani Pälli 
> ---
>  tests/spec/CMakeLists.txt  |   1 +
>  .../ext_disjoint_timer_query/CMakeLists.gles2.txt  |   7 ++
>  tests/spec/ext_disjoint_timer_query/CMakeLists.txt |   1 +
>  tests/spec/ext_disjoint_timer_query/simple-query.c | 135 
> +

Don't forget to update all.py, or nobody will run your new test :)

>  4 files changed, 144 insertions(+)
>  create mode 100644 tests/spec/ext_disjoint_timer_query/CMakeLists.gles2.txt
>  create mode 100644 tests/spec/ext_disjoint_timer_query/CMakeLists.txt
>  create mode 100644 tests/spec/ext_disjoint_timer_query/simple-query.c
>
> diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
> index 99fa95f5f..b53411954 100644
> --- a/tests/spec/CMakeLists.txt
> +++ b/tests/spec/CMakeLists.txt
> @@ -169,3 +169,4 @@ add_subdirectory (intel_conservative_rasterization)
>  add_subdirectory (arb_post_depth_coverage)
>  add_subdirectory (arb_fragment_shader_interlock)
>  add_subdirectory (ext_occlusion_query_boolean)
> +add_subdirectory (ext_disjoint_timer_query)
> diff --git a/tests/spec/ext_disjoint_timer_query/CMakeLists.gles2.txt 
> b/tests/spec/ext_disjoint_timer_query/CMakeLists.gles2.txt
> new file mode 100644
> index 0..b8a07153e
> --- /dev/null
> +++ b/tests/spec/ext_disjoint_timer_query/CMakeLists.gles2.txt
> @@ -0,0 +1,7 @@
> +link_libraries (
> + piglitutil_${piglit_target_api}
> +)
> +
> +piglit_add_executable (ext_disjoint_timer_query-simple simple-query.c)
> +
> +# vim: ft=cmake:
> diff --git a/tests/spec/ext_disjoint_timer_query/CMakeLists.txt 
> b/tests/spec/ext_disjoint_timer_query/CMakeLists.txt
> new file mode 100644
> index 0..144a306f4
> --- /dev/null
> +++ b/tests/spec/ext_disjoint_timer_query/CMakeLists.txt
> @@ -0,0 +1 @@
> +piglit_include_target_api()
> diff --git a/tests/spec/ext_disjoint_timer_query/simple-query.c 
> b/tests/spec/ext_disjoint_timer_query/simple-query.c
> new file mode 100644
> index 0..4f5e7392b
> --- /dev/null
> +++ b/tests/spec/ext_disjoint_timer_query/simple-query.c
> @@ -0,0 +1,135 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER 
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +/**
> + * @file
> + * Tests GL_EXT_disjoint_timer_query extension. Test does not to cover
> + * the whole API as that is tested throughly by existing query tests for
> + * desktop GL. Main objective is to test that timer queries work on OpenGL
> + * ES 2.0 and we can get GL_GPU_DISJOINT_EXT value from the driver.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> + config.supports_gl_es_version = 20;
> + config.window_visual = PIGLIT_GL_VISUAL_RGBA;
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const char vs_source[] =
> + "attribute vec2 piglit_vertex;\n"
> + "\n"
> + "void main()\n"
> + "{\n"
> + "   gl_Position = vec4(piglit_vertex, 0.0, 1.0);\n"
> + "}\n";
> +
> +static const char fs_source[] =
> + "void main()\n"
> + "{\n"
> + "   gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);\n"
> + "}\n";
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + GLuint query;
> + GLint current, disjoint;
> + GLuint64 time = 0;
> +
> + glGenQueriesEXT(1, );
> +
> + if (!piglit_check_gl_error(GL_NO_ERROR))
> + piglit_report_result(PIGLIT_FAIL);
> +
> + /* Clear disjoint error state. */
> + glGetIntegerv(GL_GPU_DISJOINT_EXT, );
> +
> + if (!piglit_check_gl_error(GL_NO_ERROR))
> + piglit_report_result(PIGLIT_FAIL);
> +
> + glBeginQueryEXT(GL_TIME_ELAPSED_EXT, query);
> +
> + if (!piglit_check_gl_error(GL_NO_ERROR))
> + piglit_report_result(PIGLIT_FAIL);
> +
> +/* "The error INVALID_OPERATION is generated if QueryCounterEXT 

Re: [Piglit] [PATCH 2/4] ATI_fs: add render tests

[Eric Anholt]

Miklós Máté  writes:

> These mainly check the state machine. Compiler checks will come later.
>
> Signed-off-by: Miklós Máté 
> ---
>  tests/spec/ati_fragment_shader/render-constants.c  |  98 +++
>  tests/spec/ati_fragment_shader/render-default.c|  81 +
>  tests/spec/ati_fragment_shader/render-fog.c|  95 +++
>  tests/spec/ati_fragment_shader/render-notexture.c  |  71 +++
>  tests/spec/ati_fragment_shader/render-precedence.c | 118 ++
>  tests/spec/ati_fragment_shader/render-sources.c| 132 
> +
>  tests/spec/ati_fragment_shader/render-textargets.c |  89 ++
>  7 files changed, 684 insertions(+)
>  create mode 100644 tests/spec/ati_fragment_shader/render-constants.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-default.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-fog.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-notexture.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-precedence.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-sources.c
>  create mode 100644 tests/spec/ati_fragment_shader/render-textargets.c
>
> diff --git a/tests/spec/ati_fragment_shader/render-constants.c 
> b/tests/spec/ati_fragment_shader/render-constants.c
> new file mode 100644
> index 0..6e1e4fd8c
> --- /dev/null
> +++ b/tests/spec/ati_fragment_shader/render-constants.c
> @@ -0,0 +1,98 @@
> +/* TODO license header */
> +
> +/**
> + * Tests rendering with GL_ATI_fragment_shader:
> + * - using local and global constants
> + * - updating global constants
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +static const float color1[] = {0.4, 0.2, 0.6};
> +static const float color2[] = {0.7, 0.2, 0.3};
> +static const float color3[] = {0.1, 0.7, 0.2};
> +static const float color4[] = {0.8, 0.1, 0.7};
> +
> +static float result1p3[] = {0.0, 0.0, 0.0};
> +static float result2p3[] = {0.0, 0.0, 0.0};
> +static float result4p3[] = {0.0, 0.0, 0.0};
> +
> +#define check_gl_error(err) if (!piglit_check_gl_error(err)) 
> piglit_report_result(PIGLIT_FAIL)
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + bool pass = true;
> +
> + piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
> +
> + glClearColor(1.0, 0.0, 0.0, 1.0);
> + glClear(GL_COLOR_BUFFER_BIT);
> +
> + glEnable(GL_FRAGMENT_SHADER_ATI);
> + glSetFragmentShaderConstantATI(GL_CON_7_ATI, color2);
> + glSetFragmentShaderConstantATI(GL_CON_4_ATI, color3);
> + glBindFragmentShaderATI(13);

Could we get some top-level variables with names instead of "13" and
"42"?

> + piglit_draw_rect(0, 0, piglit_width/4, piglit_height); /* 2+3 */

In general, we ask for spaces around binary operators in the code, so

piglit_draw_rect(0, 0, piglit_width / 4, piglit_height); /* 2+3 */

here and elsewhere in your tests.

> + glBindFragmentShaderATI(42);
> + piglit_draw_rect(piglit_width/4, 0, piglit_width/4, piglit_height); /* 
> 1+3 */
> + glBindFragmentShaderATI(13);
> + glSetFragmentShaderConstantATI(GL_CON_7_ATI, color4);
> + piglit_draw_rect(2*piglit_width/4, 0, piglit_width/4, piglit_height); 
> /* 4+3 */
> + glBindFragmentShaderATI(42);
> + piglit_draw_rect(3*piglit_width/4, 0, piglit_width/4, piglit_height); 
> /*1+3 */
> + glDisable(GL_FRAGMENT_SHADER_ATI);
> +
> + pass = pass && piglit_probe_rect_rgb(0, 0, piglit_width/4, 
> piglit_height, result2p3);
> + pass = pass && piglit_probe_rect_rgb(piglit_width/4, 0, piglit_width/4, 
> piglit_height, result1p3);
> + pass = pass && piglit_probe_rect_rgb(2*piglit_width/4, 0, 
> piglit_width/4, piglit_height, result4p3);
> + pass = pass && piglit_probe_rect_rgb(3*piglit_width/4, 0, 
> piglit_width/4, piglit_height, result1p3);
> +
> + piglit_present_results();
> +
> + check_gl_error(GL_NO_ERROR);
> +
> + return pass ? PIGLIT_PASS : PIGLIT_FAIL;
> +}
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + unsigned u;
> +
> + piglit_require_extension("GL_ATI_fragment_shader");
> +
> + /* create identical shaders, but one of them has a local constant */
> +
> + glBindFragmentShaderATI(13);
> + glBeginFragmentShaderATI();
> + glColorFragmentOp2ATI(GL_ADD_ATI, GL_REG_0_ATI, GL_NONE, GL_NONE,
> + GL_CON_7_ATI, GL_NONE, GL_NONE,
> + GL_CON_4_ATI, GL_NONE, GL_NONE);
> + glEndFragmentShaderATI();
> +
> + glBindFragmentShaderATI(42);
> + glBeginFragmentShaderATI();
> + glColorFragmentOp2ATI(GL_ADD_ATI, GL_REG_0_ATI, GL_NONE, GL_NONE,
> + GL_CON_7_ATI, GL_NONE, GL_NONE,
> + GL_CON_4_ATI, GL_NONE, GL_NONE);
> + 

Re: [Piglit] [PATCH 1/4] ATI_fs: add api error tests

[Eric Anholt]

Miklós Máté  writes:

> One for each paragraph in the Errors section of the spec.
>
> Signed-off-by: Miklós Máté 
> ---
>  tests/spec/ati_fragment_shader/error01-genzero.c   |  51 
>  tests/spec/ati_fragment_shader/error02-inside.c|  59 +
>  tests/spec/ati_fragment_shader/error03-outside.c   |  89 +++
>  tests/spec/ati_fragment_shader/error04-endshader.c |  93 
>  tests/spec/ati_fragment_shader/error05-passes.c| 123 ++
>  .../spec/ati_fragment_shader/error06-regswizzle.c  | 263 
> +
>  tests/spec/ati_fragment_shader/error07-instcount.c |  89 +++
>  tests/spec/ati_fragment_shader/error08-secondary.c |  82 +++
>  tests/spec/ati_fragment_shader/error09-allconst.c  |  78 ++
>  tests/spec/ati_fragment_shader/error10-dotx.c  | 115 +
>  .../spec/ati_fragment_shader/error11-invaliddst.c  | 171 ++
>  .../spec/ati_fragment_shader/error12-invalidsrc.c  | 151 
>  .../spec/ati_fragment_shader/error13-invalidarg.c  | 121 ++
>  .../spec/ati_fragment_shader/error14-invalidmod.c  | 129 ++

We tend to implement these API error tests as a single .c file.  As
you've experienced, this is a whole lot of copy and pasting, and it's
usually not too big of a deal to get the implementation to pass all of
them.

If it *is* hard to get the implementation to pass all of them, you may
look into piglit_report_subtest_result().

That said, there are some sizeable tests here and some nice usage of
subtests in a few of them, so I think you've generally made the right
choice.

Also, don't forget to add your tests to all.py!  If you don't, your
tests won't be run in everyone's regression testing.  Also, the build
system for the tests should be in the same commit as the tests
themselves.

> diff --git a/tests/spec/ati_fragment_shader/error01-genzero.c 
> b/tests/spec/ati_fragment_shader/error01-genzero.c
> new file mode 100644
> index 0..a19fb2ffc
> --- /dev/null
> +++ b/tests/spec/ati_fragment_shader/error01-genzero.c
> @@ -0,0 +1,51 @@
> +/* TODO license header */

You can see a sample of the license to use in the COPYING file -- just
copy and paste and change the date and "Intel Corporation" to your name.

> +
> +/**
> + * Tests API errors for GL_ATI_fragment_shader.
> + * One for each paragraph in the Errors section.
> + */

I'd move the comment from piglit_init() to here.

> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* UNREACHED */
> + return PIGLIT_FAIL;
> +}
> +
> +#define check_gl_error(err) if (!piglit_check_gl_error(err)) 
> piglit_report_result(PIGLIT_FAIL)
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + unsigned id = 42;

Unnecessary initializer.

> + piglit_require_extension("GL_ATI_fragment_shader");
> +
> + /*
> +  * Paragraph 1 of the Errors section:
> +  *
> +  * The error INVALID_VALUE is generated if GenFragmentShadersATI is
> +  * called where  is zero.
> +  */
> +
> + id = glGenFragmentShadersATI(0);
> + check_gl_error(GL_INVALID_VALUE);
> +
> + /* The spec also says that 0 is returned */
> + if (id != 0)
> + piglit_report_result(PIGLIT_FAIL);
> +
> + /* Note that the spec also says that no shaders are generated,
> +  * but there is no way of testing that */

Our general style is that the closing */ of a multi-line comment goes on
its own line.

> +
> + piglit_report_result(PIGLIT_PASS);
> +}
> diff --git a/tests/spec/ati_fragment_shader/error02-inside.c 
> b/tests/spec/ati_fragment_shader/error02-inside.c
> new file mode 100644
> index 0..5dee70cf6
> --- /dev/null
> +++ b/tests/spec/ati_fragment_shader/error02-inside.c
> @@ -0,0 +1,59 @@
> +/* TODO license header */
> +
> +/**
> + * Tests API errors for GL_ATI_fragment_shader.
> + * One for each paragraph in the Errors section.
> + */
> +
> +#include "piglit-util-gl.h"
> +
> +PIGLIT_GL_TEST_CONFIG_BEGIN
> +
> + config.supports_gl_compat_version = 10;
> + config.window_visual = PIGLIT_GL_VISUAL_DOUBLE | PIGLIT_GL_VISUAL_RGBA;
> +
> +PIGLIT_GL_TEST_CONFIG_END
> +
> +enum piglit_result
> +piglit_display(void)
> +{
> + /* UNREACHED */
> + return PIGLIT_FAIL;
> +}
> +
> +#define check_gl_error(err) if (!piglit_check_gl_error(err)) 
> piglit_report_result(PIGLIT_FAIL)
> +
> +void
> +piglit_init(int argc, char **argv)
> +{
> + piglit_require_extension("GL_ATI_fragment_shader");
> +
> + /*
> +  * Paragraph 2 of the Errors section:
> +  *
> +  * The error INVALID_OPERATION is generated if GenFragmentShadersATI,
> +  * BindFragmentShaderATI, DeleteFragmentShaderATI, or
> +  * BeginFragmentShaderATI are specified inside a
> +  * 

Re: [Piglit] [PATCH 0/4] WIP tests for GL_ATI_fragment_shader

[Eric Anholt]

Miklós Máté  writes:

> This series adds API sanity checks, error checks for the conditions listed in
> the specification, and render tests for validating the state machine.
> I think this is the point where the test set starts becoming useful.
>
> I plan to do more API sanity checks, validate error conditions that are not in
> the specification (e.g. invalid argRep), and render tests for validating the
> compiler.
>
> Note that I don't expect these to be committed as-is, I'm just showing
> my progress. See my corresponding Mesa patch series that fixes issues
> uncovered by these tests.

Thanks for writing tests!  I've got some feedback about general piglit
test writing and patch series structure, but I want to start out by
saying that this is a good start and I'd love to see them merged soon.


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

Re: [Piglit] [PATCH] glx: New test for GLX_EXT_no_config_context

[Eric Anholt]

Adam Jackson  writes:

> Create a no-config context, create one window of every visual, make it
> current against each of them. This is a bit of a touch test right now,
> this should be extended to create each kind of drawable for each config
> and to verify some rendering.

It's a good start.  Don't forget to add it to all.py, though.


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

[Piglit] [PATCH] glsl-fs-discard-mrt: Fix binding the winsys framebuffer in -fbo mode.

[Eric Anholt]

Fixes failure on vc5 where the alpha was forced to 1 due to an rgbx
window system buffer.
---
 tests/shaders/glsl-fs-discard-mrt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/shaders/glsl-fs-discard-mrt.c 
b/tests/shaders/glsl-fs-discard-mrt.c
index 8447cf270b96..68955fa06488 100644
--- a/tests/shaders/glsl-fs-discard-mrt.c
+++ b/tests/shaders/glsl-fs-discard-mrt.c
@@ -141,7 +141,7 @@ draw_fbo_to_screen_and_test(void)
float magenta[4] = {1, 0, 1, 0};
 
glUseProgram(0);
-   glBindFramebuffer(GL_FRAMEBUFFER, 0);
+   glBindFramebuffer(GL_FRAMEBUFFER, piglit_winsys_fbo);
 
glClearColor(0.5, 0.5, 0.5, 0.5);
glClear(GL_COLOR_BUFFER_BIT);
-- 
2.15.0

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

Re: [Piglit] Piglit test results

[Eric Anholt]

Dan Allen  writes:

> Hi,
>
> I reported a bug in relation to the R600 driver and have since found the 
> piglit tests. I ran the shader.py test and receive approximately 500 
> errors when this test is run (it varies slightly between each run).
>
> Are these test results of interest to someone, and if so, what should I 
> do with them? The card is quite old (Radeon HD 4890).

Generally, piglit is a test suite for the GL developers to use to find
bugs to work on and to prevent regressions.  Anyone working on the
driver will be running piglit regularly, so external submissions don't
do much.


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

Re: [Piglit] [PATCH] framework: specify GL_NEAREST filter for FBO textures

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> By setting the min/mag filters, we give a hint to the OpenGL driver
> that we probaby don't want mipmapped textures.  This results in the
> Mesa state tracker allocating single-level textures here instead of
> full mipmaps.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH] OES_required_internalformat: Add a test for renderbuffer sizing.

[Eric Anholt]

Eric Anholt <e...@anholt.net> writes:

> [ Unknown signature status ]
> Eric Anholt <e...@anholt.net> writes:
>
>> dEQP provides some coverage for texture and renderbuffer formats being
>> supported, but doesn't test that the storage reports appropriate
>> sizes.  We can't test textures easily becase GetTexLevelParameter
>> doesn't exist, but GetRenderbufferParameter can let us test
>> renderbuffers at least.
>
> Anybody up for reviewing this test?

Ping again, since I've enabled the extension in Mesa at this point.


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

Re: [Piglit] [PATCH] glsl-1.00: Add testcases from the spec about redeclarations.

[Eric Anholt]

Eric Anholt <e...@anholt.net> writes:

> Catches a regression in my recent redeclaration patch series
> (allowed-2) and failure I was trying to fix (disallowed-2).

I'd still love an ack on this one.


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

Re: [Piglit] [PATCH] framework: Do not run with an empty test list

[Eric Anholt]

Dylan Baker  writes:

> [ Unknown signature status ]
> Quoting Martin Peres (2017-10-20 00:38:18)
>> On 19/10/17 19:50, Dylan Baker wrote:
>> > Quoting Martin Peres (2017-10-19 07:17:25)
>> >> On 30/09/17 23:42, Dylan Baker wrote:
>> >>> Actually CC'ing him this time
>> >>>
>> >>> Quoting Dylan Baker (2017-09-29 20:29:34)
>>  Quoting Arkadiusz Hiler (2017-09-26 03:27:50)
>> > Because in Python we have `bool([]}) == False`, providing empty test
>> > list resulted in hitting the same code path as not providing it at all,
>> > meaning that we run everything.
>> >
>> > Let's just exit early with an appropriate message instead.
>> >
>> > This will get rid of the rather surprising behavior and will help 
>> > making
>> > the execution less prone to automated list generation errors (which has
>> > already bitten us) as well as human errors.
>> >
>> > Signed-off-by: Arkadiusz Hiler 
>> > ---
>> >framework/programs/run.py | 4 
>> >1 file changed, 4 insertions(+)
>> >
>> > diff --git a/framework/programs/run.py b/framework/programs/run.py
>> > index 4524f171b..0fec264ec 100644
>> > --- a/framework/programs/run.py
>> > +++ b/framework/programs/run.py
>> > @@ -327,6 +327,10 @@ def run(input_):
>> >stripped = (t.split('#')[0].strip() for t in test_list)
>> >forced_test_list = [t for t in stripped if t]
>> >
>> > +# to avoid running everything
>> > +if not forced_test_list:
>> > +raise exceptions.PiglitFatalError("Empty test list 
>> > provided")
>> > +
>> >backend = backends.get_backend(args.backend)(
>> >args.results_path,
>> >junit_suffix=args.junit_suffix,
>> > -- 
>> > 2.13.5
>> >
>> > ___
>> > Piglit mailing list
>> > Piglit@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/piglit
>> 
>>  Hmmm, there is a case that we do want to continue, and that's for 
>>  resume, CC'ing
>>  Martin to see if this breaks their use case.
>> >>
>> >> Sorry for the delay! Thanks for caring about CI :)
>> >>
>> >> So, I like the patch, but disagree on the "raise" here. Why not simply
>> >> print("Empty test list provided") and sys.exit(0)? There is nothing to
>> >> do after all, so why report that an error happened?
>> >>
>> >> If you still want to return an error, then please make sure it is one I
>> >> can test against.
>> >>
>> >> Thanks,
>> >> Martin
>> > 
>> > PiglitFatalError is actually caught by the main script, the error is 
>> > printed
>> > with 'Fatal Error: ' prepended and sys.exit(1) is called. So if that's 
>> > what you
>> > want than this is find as-is I think.
>> 
>> The problem is that execution errors, or abort-on also exit with error 
>> code 1. So either we introduce a new one for this particular error, or 
>> we just return 0 which means all went well.
>> 
>> Thinking more about it though, we should still generate results, just 
>> with nothing inside (since we asked for nothing). This way, we don't 
>> need to special case the fact that the test list is empty.
>> 
>> What do you think?
>
> I don't like generating empty results, I think it violates least surprise, you
> shouldn't get results if there's nothing to put in them. Especially since the
> most common case for empty results is user error, such as using -x and -t
> options such that no tests are run. Which, actually, is something that
> non-automated users have complained about in the past.
>
> We do have a PiglitUserError that has returncode 3, or we could extend
> PiglitFatalError to also take an int argument that is the returncode on exit.

Please just generate empty results without blowing up.  I use "-t
notarealtest" to cut out the piglit test component of X server testing
when I'm trying to get quick runs of the rest of the tests, but then I
have to also filter out the piglit framework dying to find my errors.


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

Re: [Piglit] [PATCH 0/7] framework, shader_runner: misc fixed and cleanups

[Eric Anholt]

Nicolai Hähnle <nhaeh...@gmail.com> writes:

> Hi all,
>
> Mostly no-op changes in comments, but a few functional changes as well.

This series is:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH 4/4] simple-readbuffer: simplify a conditional

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> I find this slightly easier to read.

Series is:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH 1/5] all.py: remove run_concurrent=False from most 1.1 tests

[Eric Anholt]

concurrent=False)
> -g(['polygon-mode'], run_concurrent=False)
> +g(['point-line-no-cull'])
> +g(['polygon-mode'])
>  g(['polygon-mode-facing'])
>  g(['polygon-mode-offset'])
> -g(['polygon-offset'], run_concurrent=False)
> +g(['polygon-offset'])
>  g(['push-pop-texture-state'])
>  g(['quad-invariance'])
>  g(['readpix-z'])
> -g(['roundmode-getintegerv'], run_concurrent=False)
> -g(['roundmode-pixelstore'], run_concurrent=False)
> -g(['select', 'gl11'], 'GL_SELECT - no test function', 
> run_concurrent=False)
> -g(['select', 'depth'], 'GL_SELECT - depth-test enabled',
> -  run_concurrent=False)
> -g(['select', 'stencil'], 'GL_SELECT - stencil-test enabled',
> -  run_concurrent=False)
> -g(['select', 'alpha'], 'GL_SELECT - alpha-test enabled',
> -  run_concurrent=False)
> -g(['select', 'scissor'], 'GL_SELECT - scissor-test enabled',
> -  run_concurrent=False)
> -g(['stencil-drawpixels'], run_concurrent=False)
> -g(['texgen'], run_concurrent=False)
> -g(['two-sided-lighting'], run_concurrent=False)
> -g(['user-clip'], run_concurrent=False)
> -g(['varray-disabled'], run_concurrent=False)
> -g(['windowoverlap'], run_concurrent=False)
> -g(['copyteximage-border'], run_concurrent=False)
> -g(['copyteximage-clipping'], run_concurrent=False)
> -g(['copytexsubimage'], run_concurrent=False)
> -g(['getteximage-formats'], run_concurrent=False)
> -g(['getteximage-luminance'], run_concurrent=False)
> -g(['getteximage-simple'], run_concurrent=False)
> -g(['getteximage-depth'], run_concurrent=True)
> +g(['roundmode-getintegerv'])
> +g(['roundmode-pixelstore'])
> +g(['select', 'gl11'], 'GL_SELECT - no test function')
> +g(['select', 'depth'], 'GL_SELECT - depth-test enabled')
> +g(['select', 'stencil'], 'GL_SELECT - stencil-test enabled')
> +g(['select', 'alpha'], 'GL_SELECT - alpha-test enabled')
> +g(['select', 'scissor'], 'GL_SELECT - scissor-test enabled')
> +g(['stencil-drawpixels'])
> +g(['texgen'])
> +g(['two-sided-lighting'])
> +g(['user-clip'])
> +g(['varray-disabled'])
> +g(['windowoverlap'])
> +g(['copyteximage-border'])
> +g(['copyteximage-clipping'])
> +g(['copytexsubimage'])
> +g(['getteximage-formats'])
> +g(['getteximage-luminance'])
> +g(['getteximage-simple'])
> +g(['getteximage-depth'])
>  g(['incomplete-texture', 'fixed'], 'incomplete-texture-fixed')
> -g(['max-texture-size'], run_concurrent=False)
> -g(['max-texture-size-level'])
>  g(['proxy-texture'])
>  g(['sized-texture-format-channels'])
> -g(['streaming-texture-leak'], run_concurrent=False)
> -g(['texredefine'], run_concurrent=False)
> +g(['texredefine'])
>  g(['texsubimage'])
>  g(['texsubimage-unpack'])
> -g(['texsubimage-depth-formats'], run_concurrent=False)
> -g(['texture-al'], run_concurrent=False)
> +g(['texsubimage-depth-formats'])
> +g(['texture-al'])
>  g(['triangle-guardband-viewport'])
>  g(['getteximage-targets', '1D'])
>  g(['getteximage-targets', '2D'])
>  g(['teximage-scale-bias'])
> -add_msaa_visual_plain_tests(g, ['draw-pixels'], run_concurrent=False)
> -add_msaa_visual_plain_tests(g, ['read-front'], run_concurrent=False)
> +add_msaa_visual_plain_tests(g, ['draw-pixels'])
> +add_msaa_visual_plain_tests(g, ['read-front'])

read-front should stay non-concurrent as well.

With those two changed, the series is:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH] glx: Rerun failing tests in auto mode if there is a pending expose event

[Eric Anholt]

Thomas Hellstrom <thellst...@vmware.com> writes:

> Tests that check frontbuffer contents after drawing may fail because
> the X server touch the front contents between drawing and checking. In those
> cases, there's typically a pending expose event. So check for that
> situation and optionally rerun the test.

Did this resolve your unstable test results?  Could you mention which
tests were failing, and your environment (a window manager, a
compositing manager, or none)?  With that,

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH 2/2] glean: remove pointless switch statements from fbo test

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> All switch cases did the same thing.  Spotted by Eric Anholt.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH] OES_required_internalformat: Add a test for renderbuffer sizing.

[Eric Anholt]

Eric Anholt <e...@anholt.net> writes:

> dEQP provides some coverage for texture and renderbuffer formats being
> supported, but doesn't test that the storage reports appropriate
> sizes.  We can't test textures easily becase GetTexLevelParameter
> doesn't exist, but GetRenderbufferParameter can let us test
> renderbuffers at least.

Anybody up for reviewing this test?


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

Re: [Piglit] [PATCH] glean: remove line drawing code from fbo test

[Eric Anholt]

Brian Paul  writes:

> The glean fbo test exercises rendering to 1D, 2D, 3D and cube images
> attached to FBOs.  For the 1D texture case, the rendering is done with
> a line, rather than a polygon.  Since the 1D image is one pixel high,
> the slightest mis-rasterization of the line will cause it to be clipped
> and the test to fail.
>
> Just fall through to the polygon drawing code to avoid that issue.
> Less code too.

If you just remove the whole switch statement around the POLYGON drawing
code, this would get my r-b. :)

I like this.  Test line rasterization only in something that's about
lines -- they're tricky to test well, and we've had plenty of trouble
with this draw call.


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

[Piglit] [PATCH] no-op-paths: Clear between each test.

[Eric Anholt]

This matches the behavior of the old glean test.  Without it, the
always-pass subtests would all pass if the first one had passed, and
if a single always-fail subtest failed then the rest would also fail.
---
 tests/spec/gl-1.0/no-op-paths.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/spec/gl-1.0/no-op-paths.c b/tests/spec/gl-1.0/no-op-paths.c
index 4576f35e9a23..7c86b7c30941 100644
--- a/tests/spec/gl-1.0/no-op-paths.c
+++ b/tests/spec/gl-1.0/no-op-paths.c
@@ -250,10 +250,10 @@ piglit_display(void)
piglit_ortho_projection(piglit_width, piglit_height, GL_FALSE);
 
glDisable(GL_DITHER);
-   glClear(GL_COLOR_BUFFER_BIT);

/* test always-pass paths */
for (i = 0; i < NUM_PATHS; i++) {
+   glClear(GL_COLOR_BUFFER_BIT);
set_path_state(i, ALWAYS_PASS);
 
/* draw polygon */
@@ -272,6 +272,7 @@ piglit_display(void)
 
/* enable all always-pass paths */
{
+   glClear(GL_COLOR_BUFFER_BIT);
for (i = 0; i < NUM_PATHS; i++) {
set_path_state(i, ALWAYS_PASS);
}
@@ -292,6 +293,7 @@ piglit_display(void)
 
/* test never-pass paths */
for (i = 0; i < NUM_PATHS; i++) {
+   glClear(GL_COLOR_BUFFER_BIT);
set_path_state(i, ALWAYS_FAIL);
 
/* draw polygon */
-- 
2.11.0

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

Re: [Piglit] [PATCH] arb_clear_texture-errors: add more error tests

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> Check for errors clearing other mipmap levels.
> Test clearing with a texture view.
> Update/add comments.

Nice.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

[Piglit] [PATCH] util: Avoid asking GL to transpose the ortho_uniform matrix.

[Eric Anholt]

From the GLES2 spec:

If the transpose parameter to any of the UniformMatrix* commands
is not FALSE, an INVALID_VALUE error is generated, and no uniform
values are changed.

Fixes spurious failures in draw-vertices-half-float_gles2.
---
 tests/util/piglit-util-gl.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/util/piglit-util-gl.c b/tests/util/piglit-util-gl.c
index 5ff13fedcf6e..5dc1bbc7d2fe 100644
--- a/tests/util/piglit-util-gl.c
+++ b/tests/util/piglit-util-gl.c
@@ -603,12 +603,12 @@ piglit_gen_ortho_uniform(GLint location, double l, double 
r, double b,
 double t, double n, double f)
 {
const GLfloat values[4][4] = {
-   { 2/(r-l),0,0,-(r+l)/(r-l) },
-   {0,2/(t-b), 0,-(t+b)/(t-b) },
-   {0,   0,-2/(f-n), -(f+n)/(f-n) },
-   {0,   0,0,  1  }
+   { 2/(r-l),  0,0,0 },
+   {0, 2/(t-b),  0,0 },
+   {0, 0,-2/(f-n), 0 },
+   { -(r+l)/(r-l), -(t+b)/(t-b), -(f+n)/(f-n), 1 }
};
-   glUniformMatrix4fv(location, 1, GL_TRUE, (const GLfloat *)values);
+   glUniformMatrix4fv(location, 1, GL_FALSE, (const GLfloat *)values);
 }
 
 
-- 
2.11.0

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

Re: [Piglit] [PATCH v3 0/8] Add tests for EGL_ANDROID_native_fence_sync.

[Eric Anholt]

Rafael Antognolli <rafael.antogno...@intel.com> writes:

> This time, reuse most of the code from egl_khr_fence_sync test.
>
> Cc: Eric Anholt <e...@anholt.net>
>
> Rafael Antognolli (8):
>   egl_khr_fence_sync: Prepare to support android native_sync.
>   egl_khr_fence_sync: add tests for android fence.
>   egl_khr_fence_sync: Add sw_sync lib.
>   egl_khr_fence_sync: Add test to create fence from fd.
>   egl_khr_fence_sync: Verify eglDupNativeFenceFDANDROID.
>   egl_khr_fence_sync: Verify that dup() fails on invalid fence.
>   egl_khr_fence_sync: Test for wait with zero timeout.
>   egl_khr_fence_sync: Enable android native fences extension test.

Other than one mid-series mistake I noted, which was already fixed in a
later commit in the series,

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v3 4/8] egl_khr_fence_sync: Add test to create fence from fd.

[Eric Anholt]

Rafael Antognolli  writes:

> Add a test that creates a sync file using sw_sync, and then creates an
> EGL fence sync from that sync file. It also verify that the fence sync
> EGL_SYNC_CONDITION_KHR is set to EGL_SYNC_NATIVE_FENCE_SIGNALED_ANDROID,
> and that EGL_SYNC_STATUS_KHR changes to EGL_SIGNALED_KHR once the fence
> is signaled.
>
> Signed-off-by: Rafael Antognolli 
> ---
>  .../spec/egl_khr_fence_sync/egl_khr_fence_sync.c   | 145 
> +
>  1 file changed, 145 insertions(+)
>
> diff --git a/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c 
> b/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c
> index d5353f2..f6ebc0b 100644
> --- a/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c
> +++ b/tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c
> @@ -55,6 +55,7 @@
>  
>  #include "piglit-util-egl.h"
>  #include "piglit-util-gl.h"
> +#include "sw_sync.h"
>  
>  /* Extension function pointers.
>   *
> @@ -1347,6 +1348,144 @@ static struct test_profile fence_android_native = {
>   .extension = "EGL_ANDROID_native_fence_sync",
>  };
>  
> +static EGLSyncKHR
> +test_create_fence_from_fd(int fd)
> +{
> + EGLint attrib_list[] = {
> + EGL_SYNC_NATIVE_FENCE_FD_ANDROID, fd,
> + EGL_NONE,
> + };
> +
> + return peglCreateSyncKHR(g_dpy, EGL_SYNC_NATIVE_FENCE_ANDROID, 
> attrib_list);
> +}
> +
> +static enum piglit_result
> +test_eglCreateSyncKHR_native_from_fd(void *test_data)
> +{
> + enum piglit_result result = PIGLIT_PASS;
> + EGLSyncKHR sync = 0;
> + EGLint sync_type = canary,
> +sync_status = canary,
> +sync_condition = canary;
> + int sync_fd = canary;
> + int timeline = canary;
> + bool ok = false;
> + struct test_profile *profile = test_data;
> +
> + result = test_setup(profile);
> + if (result != PIGLIT_PASS) {
> + return result;
> + }
> +
> + if (!sw_sync_is_supported()) {
> + result = PIGLIT_SKIP;
> + goto cleanup;
> + }
> +
> + /* Create timeline and sw_sync */
> + timeline = sw_sync_timeline_create();
> + if (timeline < 0) {
> + piglit_loge("sw_sync_timeline_create() failed");
> + result = PIGLIT_FAIL;
> + goto cleanup;
> + }
> +
> + sync_fd = sw_sync_fence_create(timeline, 1);
> + if (sync_fd < 0) {
> + piglit_loge("sw_sync_fence_create() failed");
> + result = PIGLIT_FAIL;
> + goto cleanup_timeline;
> + }
> +
> + sync = test_create_fence_from_fd(sync_fd);
> + if (sync == EGL_NO_SYNC_KHR) {
> + piglit_loge("eglCreateSyncKHR(%s) failed", profile->sync_str);
> + result = PIGLIT_FAIL;
> + sw_sync_fence_destroy(sync_fd);
> + goto cleanup_timeline;
> + }
> +
> + ok = peglGetSyncAttribKHR(g_dpy, sync, EGL_SYNC_TYPE_KHR, _type);
> + if (!ok) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_TYPE_KHR) failed");
> + result = PIGLIT_FAIL;
> + }
> + if (!piglit_check_egl_error(EGL_SUCCESS)) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_TYPE_KHR) emitted "
> + "an error");
> + result = PIGLIT_FAIL;
> + }
> + if (sync_type != profile->sync_type) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_TYPE_KHR) returned "
> + "0x%x but expected %s(0x%x)",
> + sync_type, profile->sync_str, profile->sync_type);
> + result = PIGLIT_FAIL;
> + }
> +
> + ok = peglGetSyncAttribKHR(g_dpy, sync, EGL_SYNC_STATUS_KHR, 
> _status);
> + if (!ok) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_STATUS_KHR) failed");
> + result = PIGLIT_FAIL;
> + }
> + if (!piglit_check_egl_error(EGL_SUCCESS)) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_STATUS_KHR) emitted "
> + "an error");
> + result = PIGLIT_FAIL;
> + }
> + if (sync_status != EGL_UNSIGNALED_KHR) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_STATUS_KHR) "
> + "returned 0x%x but expected "
> + "EGL_UNSIGNALED_KHR(0x%x)",
> + sync_status, EGL_UNSIGNALED_KHR);
> + result = PIGLIT_FAIL;
> + }
> +
> + ok = peglGetSyncAttribKHR(g_dpy, sync, EGL_SYNC_CONDITION_KHR, 
> _condition);
> + if (!ok) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_CONDITION_KHR) 
> failed");
> + result = PIGLIT_FAIL;
> + }
> + if (!piglit_check_egl_error(EGL_SUCCESS)) {
> + piglit_loge("eglGetSyncAttribKHR(EGL_SYNC_CONDITION_KHR) "
> + "emitted an error");
> + result = PIGLIT_FAIL;
> + }
> + if (sync_condition != EGL_SYNC_NATIVE_FENCE_SIGNALED_ANDROID) {
> + 

Re: [Piglit] [PATCH] piglit_drm_dma_buf: fix GPU offsets and strides

[Eric Anholt]

Marek Olšák <mar...@gmail.com> writes:

> From: Marek Olšák <marek.ol...@amd.com>
>
> Most of the original code is simply wrong.
>
> This patch makes sure that at least the returned GPU offsets and strides
> are correct. This doesn't fix the incorrect CPU upload path for non-zero
> planes. GBM doesn't seem to have the capability to map a specific plane
> for CPU access.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH] piglit_drm_dma_buf: fix the stride

[Eric Anholt]

Marek Olšák <mar...@gmail.com> writes:

> From: Marek Olšák <marek.ol...@amd.com>
>
> dst_stride is from gbm_bo_map (CPU mapping), but we need the real stride.
> This fixes piglit DMABUF tests with tiled surfaces.

If you also update the dst_strides used in buf->offset/stride fields of
the switch (fourcc) down below, you can add:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v2 10/12] egl_android_native_fence_sync: Test for wait with zero timeout.

[Eric Anholt]

Rafael Antognolli <rafael.antogno...@intel.com> writes:

> Verify that eglClientWaitSyncKHR() correctly handles zero timeout before
> and after the sw sync fence is signaled.
>
> Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com>
> ---
>  .../egl_android_native_fence_sync.c| 95 
> ++
>  1 file changed, 95 insertions(+)
>
> diff --git 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>  
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> index b1739a9..f31b213 100644
> --- 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> +++ 
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> @@ -786,6 +786,96 @@ cleanup:
>   return result;
>  }
>  
> +/**
> + * Verify that eglClientWaitSyncKHR() correctly handles zero timeout before 
> and
> + * after glFinish().

Here and in the error messages below,
s/glFinish()/sw_sync_timeline_inc()/

With that, patches 7-12 are:

Reviewed-by: Eric Anholt <e...@anholt.net>

I have some more tests I'm going to want to build before I trust an
implementation of the extension, but this is a good start and we should
actually get it into the tree.


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

Re: [Piglit] [PATCH v2 06/12] egl_android_native_fence_sync: Create fence from other display.

[Eric Anholt]

Rafael Antognolli  writes:

> Verify that EGL_NO_SYNC_KHR is returned and EGL_BAD_MATCH error is
> generated.
>
> Signed-off-by: Rafael Antognolli 
> ---
>  .../egl_android_native_fence_sync.c| 98 
> +-
>  1 file changed, 97 insertions(+), 1 deletion(-)
>
> diff --git 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>  
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> index eb01014..721bc44 100644
> --- 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> +++ 
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> @@ -515,7 +515,7 @@ cleanup:
>   * Verify that eglCreateSyncKHR emits correct error when given an invalid
>   * display.
>   *
> - * From the EGL_KHR_fence_sync spec:
> + * From the EGL_ANDROID_native_fence_sync spec:

The citation appears in both specs.  Let's pick one and use it in the
commit adding the citation, rather than changing it here.

>   *
>   * If  is not the name of a valid, initialized EGLDisplay,
>   * EGL_NO_SYNC_KHR is returned and an EGL_BAD_DISPLAY error is
> @@ -546,6 +546,97 @@ test_eglCreateSyncKHR_native_invalid_display(void 
> *test_data)
>   return result;
>  }
>  
> +static enum piglit_result
> +init_other_display(EGLDisplay *out_other_dpy, EGLDisplay orig_dpy)
> +{
> + enum piglit_result result = PIGLIT_PASS;
> + EGLDisplay other_dpy = 0;
> + int i;
> +
> + static const EGLint platforms[] = {
> + EGL_PLATFORM_X11_EXT,
> + EGL_PLATFORM_WAYLAND_EXT,
> + EGL_PLATFORM_GBM_MESA,
> + 0,
> + };
> +
> + for (i = 0; platforms[i] != 0; ++i) {
> + result = init_display(platforms[i], _dpy);
> + switch (result) {
> + case PIGLIT_SKIP:
> + break;
> + case PIGLIT_PASS:
> + if (other_dpy && other_dpy != orig_dpy) {
> + *out_other_dpy = other_dpy;
> + return PIGLIT_PASS;
> + } else {
> + result = PIGLIT_SKIP;
> + break;
> + }
> + default:
> + break;

Switch cases should be at the same indentation level as the switch
itself.

Actually, since this code looks like it's from
tests/egl/spec/egl_khr_fence_sync/egl_khr_fence_sync.c, could you just
move its init_other_display() function to a helper in
tests/util/piglit-util-egl.c?

I do wonder if it would be possible to share more of our test code
between these two tests.  Seems messy, though.

> + }
> + }
> +
> + return result;
> +}
> +
> +/**
> + * Verify that eglCreateSyncKHR() emits correct error when given a display 
> that
> + * does not match the display of the bound context.
> + *
> + * From the EGL_KHR_fence_sync spec:
> + *
> + *   * If  is EGL_SYNC_FENCE_KHR or EGL_SYNC_NATIVE_FENCE_ANDROID and
> + * no context is current for the bound API (i.e., eglGetCurrentContext
> + * returns EGL_NO_CONTEXT), EGL_NO_SYNC_KHR is returned and an
> + * EGL_BAD_MATCH error is generated.

This citation seems to be in EGL_ANDROID_native_fence_sync, not KHR.
Also, I think this is the wrong block of spec text -- you want the one
about "does not match the EGLDisplay of the currently bound context". I
was going to suggest gutting almost all of the test, given the original
text :)

> +static enum piglit_result
> +test_eglCreateSyncKHR_native_wrong_display_same_thread(void *test_data)
> +{
> + enum piglit_result result = PIGLIT_PASS;
> + EGLDisplay wrong_dpy = 0;
> + EGLSyncKHR sync = 0;
> +
> + result = test_setup();
> + if (result != PIGLIT_PASS) {
> + return result;
> + }
> +
> + piglit_logi("create second EGLDisplay");
> + result = init_other_display(_dpy, g_dpy);
> + if (result != PIGLIT_PASS) {
> + goto cleanup;
> + }
> +
> + piglit_require_egl_extension(wrong_dpy, "EGL_KHR_fence_sync");
> +
> + piglit_logi("try to create sync with second display");
> + sync = peglCreateSyncKHR(wrong_dpy, EGL_SYNC_NATIVE_FENCE_ANDROID, 
> NULL);
> + if (sync != EGL_NO_SYNC_KHR) {
> + piglit_loge("eglCreateSyncKHR() incorrectly succeeded");
> + result = PIGLIT_FAIL;
> + goto cleanup;
> + }
> + if (!piglit_check_egl_error(EGL_BAD_MATCH)) {
> + piglit_loge("eglCreateSyncKHR emitted wrong error");
> + result = PIGLIT_FAIL;
> + goto cleanup;
> + }
> +
> +cleanup:
> + if (wrong_dpy) {
> + eglTerminate(wrong_dpy);
> + }
> + test_cleanup(EGL_NO_SYNC_KHR, );
> + return result;
> +}
> +
>  static 

Re: [Piglit] [PATCH v2 05/12] egl_android_native_fence_sync: Create fence from invalid display.

[Eric Anholt]

Rafael Antognolli <rafael.antogno...@intel.com> writes:

> Verify that EGL_NO_SYNC_KHR is returned and an EGL_BAD_DISPLAY error is
> generated.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v2 04/12] egl_android_native_fence_sync: Verify eglDupNativeFenceFDANDROID.

[Eric Anholt]

Rafael Antognolli <rafael.antogno...@intel.com> writes:

> Verify that eglDupNativeFenceFDANDROID() correctly returns a new file
> descriptor.
>
> Signed-off-by: Rafael Antognolli <rafael.antogno...@intel.com>
> ---
>  .../egl_android_native_fence_sync.c| 45 
> ++
>  1 file changed, 45 insertions(+)
>
> diff --git 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
>  
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> index b32e26a..194e9d5 100644
> --- 
> a/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> +++ 
> b/tests/egl/spec/egl_android_native_fence_sync/egl_android_native_fence_sync.c
> @@ -471,6 +471,46 @@ cleanup:
>   return result;
>  }
>  
> +static enum piglit_result
> +test_eglCreateSyncKHR_native_dup_fence(void *test_data)
> +{
> + enum piglit_result result = PIGLIT_PASS;
> + EGLSyncKHR sync = 0;
> + int sync_fd = canary;
> +
> + result = test_setup();
> + if (result != PIGLIT_PASS) {
> + return result;
> + }
> +
> + sync = peglCreateSyncKHR(g_dpy, EGL_SYNC_NATIVE_FENCE_ANDROID, NULL);
> + if (sync == EGL_NO_SYNC_KHR) {
> + piglit_loge("eglCreateSyncKHR(EGL_SYNC_NATIVE_FENCE_ANDROID) 
> failed");
> + result = PIGLIT_FAIL;
> + goto cleanup;
> + }
> +
> + glFlush();
> +
> + if (result == PIGLIT_FAIL)
> + goto cleanup;

This check looks like it's unreachable.

Other than that, patches 3-4 are:

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] [PATCH v2 02/12] egl_android_native_fence_sync: Add sw_sync lib.

[Eric Anholt]

Rafael Antognolli  writes:

> Add a small library that helps manipulating software fences. They are
> useful for testing EGL Android fences, since the latter can be created
> out of them.
>
> Signed-off-by: Rafael Antognolli 
> ---
>  .../spec/egl_android_native_fence_sync/sw_sync.c   | 211 
> +
>  .../spec/egl_android_native_fence_sync/sw_sync.h   |  50 +
>  2 files changed, 261 insertions(+)
>  create mode 100644 tests/egl/spec/egl_android_native_fence_sync/sw_sync.c
>  create mode 100644 tests/egl/spec/egl_android_native_fence_sync/sw_sync.h
>
> diff --git a/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c 
> b/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c
> new file mode 100644
> index 000..4e8117f
> --- /dev/null
> +++ b/tests/egl/spec/egl_android_native_fence_sync/sw_sync.c
> @@ -0,0 +1,211 @@
> +/*
> + * Copyright 2012 Google, Inc
> + * Copyright © 2016 Collabora, Ltd.
> + *
> + * Based on the implementation from the Android Open Source Project
> + *
> + * 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.
> + *
> + * Authors:
> + *Robert Foss 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "sw_sync.h"
> +
> +#ifndef SW_SYNC_IOC_INC
> +struct sw_sync_create_fence_data {
> + __u32   value;
> + charname[32];
> + __s32   fence;
> +};
> +
> +#define SW_SYNC_IOC_MAGIC'W'
> +#define SW_SYNC_IOC_CREATE_FENCE _IOWR(SW_SYNC_IOC_MAGIC, 0,\
> + struct 
> sw_sync_create_fence_data)
> +#define SW_SYNC_IOC_INC  _IOW(SW_SYNC_IOC_MAGIC, 1, 
> __u32)
> +#endif
> +
> +#define DEVFS_SW_SYNC   "/dev/sw_sync"
> +#define DEBUGFS_SW_SYNC "/sys/kernel/debug/sync/sw_sync"
> +
> +bool sw_sync_is_supported(void)
> +{
> + if(access(DEVFS_SW_SYNC, R_OK | W_OK) != -1) {
> + return true;

space after "if"

> + } else if (access(DEBUGFS_SW_SYNC, R_OK | W_OK) != -1 ) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> +int sw_sync_fd_is_valid(int fd)
> +{
> + int status;
> +
> + if (fd < 0)
> + return 0;
> +
> + status = fcntl(fd, F_GETFD, 0);
> + return status >= 0;
> +}

Do we need to F_GETFD?  Couldn't all callers of this just "if (fd < 0)"?

> +
> +static
> +void sw_sync_fd_close(int fd)
> +{
> + if (!sw_sync_fd_is_valid(fd))
> + return;
> +
> + close(fd);

Is there a reason to call is_valid() before close()?  I think you could
drop this entire function and just close(fd) from
sw_sync_timeline_destroy() and fence_destroy().

> +}
> +
> +int sw_sync_timeline_create(void)
> +{
> + int fd = open(DEVFS_SW_SYNC, O_RDWR);
> +
> + if (!sw_sync_fd_is_valid(fd))
> + fd = open(DEBUGFS_SW_SYNC, O_RDWR);
> +
> + return fd;
> +}
> +
> +void sw_sync_timeline_destroy(int fd)
> +{
> + return sw_sync_fd_close(fd);
> +}
> +
> +void sw_sync_fence_destroy(int fd)
> +{
> + return sw_sync_fd_close(fd);
> +}
> +
> +int sw_sync_fence_create(int fd, int32_t seqno)
> +{
> + struct sw_sync_create_fence_data data = {};
> + data.value = seqno;
> +
> + if (fd >= 0) {
> + ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, );
> + return data.fence;
> + } else {
> + ioctl(fd, SW_SYNC_IOC_CREATE_FENCE, );
> + return -1;
> + }

Calling an ioctl on a negative fd?  Shouldn't you just early return -1
with no ioctl call?

> +static struct sync_file_info *sync_file_info(int fd)
> +{
> + struct sync_file_info *info;
> + struct sync_fence_info *fence_info;
> + int err, num_fences;
> +
> + info = calloc(1, sizeof(*info));
> + if (info == NULL)
> + return NULL;
> +
> + err = 

Re: [Piglit] [PATCH] util: call fflush() after printf() calls

[Eric Anholt]

Brian Paul  writes:

> Otherwise, on Windows, we don't see the printf() output until the program
> exits.  That's fine with the -auto option, but a problem w/out -auto.

That's pretty broken.  Could we just wrap printf/fprintf on windows to
call down and then fflush(), instead of doing it in all the callers?

You've hit the most commonly-called printf callers, but I don't want to
have to do this in all the tests, too.


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

Re: [Piglit] [PATCH] drawoverhead: new microbenchmark

[Eric Anholt]

Marek Olšák  writes:

> From: Marek Olšák 
>
> Based on a benchmark from mesa/demos, but rewritten and extended.
> It's a benchmark expected to be run separately, not a piglit test.
> So why piglit? Because it's a good framework for writing apps like this.
>
> mesa_glthread won't show an improvement here, because there is no app
> overhead.
>
> This is what the output looks like. The percentage is relative to
> the first test of the given draw call.
>
> The obvious thing there is that enabled vertex attribs decrease
> Mesa performance even if there are no state changes.

Since nobody else has replied,

I think piglit is the wrong place for this.  I agree that it's sorta
convenient, but mesa-demos or glmark2 are the right place.


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

[Piglit] [PATCH] glsl-1.00: Add testcases from the spec about redeclarations.

[Eric Anholt]

Catches a regression in my recent redeclaration patch series
(allowed-2) and failure I was trying to fix (disallowed-2).
---
 .../compiler/redeclaration-allowed-1.vert  | 26 ++
 .../compiler/redeclaration-allowed-2.vert  | 23 +++
 .../compiler/redeclaration-disallowed-1.vert   | 26 ++
 .../compiler/redeclaration-disallowed-2.vert   | 19 
 .../compiler/redeclaration-disallowed-3.vert   | 19 
 .../compiler/redeclaration-disallowed-4.vert   | 19 
 .../compiler/redeclaration-disallowed-5.vert   | 19 
 .../compiler/redeclaration-disallowed-6.vert   | 19 
 8 files changed, 170 insertions(+)
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-1.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-2.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-1.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-2.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-3.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-4.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-5.vert
 create mode 100644 
tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-6.vert

diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-1.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-1.vert
new file mode 100644
index ..7ca5677315e7
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-1.vert
@@ -0,0 +1,26 @@
+// [config]
+// expect_result: pass
+// glsl_version: 1.00
+// [end config]
+
+// See section 4.2.7 "Redeclarations and Redefinitions Within the Same
+// Scope"
+
+#version 100
+
+precision mediump float;
+
+int f(int a)
+{
+   return a;
+}
+
+float f(float a)
+{
+   return a;
+}
+
+void main()
+{
+   gl_Position = vec4(f(1.0)) + vec4(f(1));
+}
diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-2.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-2.vert
new file mode 100644
index ..1f6c0f851103
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-allowed-2.vert
@@ -0,0 +1,23 @@
+// [config]
+// expect_result: pass
+// glsl_version: 1.00
+// [end config]
+
+// See Section 4.2.7 "Redeclarations and Redefinitions Within the Same
+// Scope"
+
+#version 100
+
+precision mediump float;
+
+int f(int a);
+
+int f(int a)
+{
+   return a;
+}
+
+void main()
+{
+   gl_Position = vec4(f(1));
+}
diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-1.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-1.vert
new file mode 100644
index ..a5c4e1e7290a
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-1.vert
@@ -0,0 +1,26 @@
+// [config]
+// expect_result: fail
+// glsl_version: 1.00
+// [end config]
+
+// See Section 4.2.7 "Redeclarations and Redefinitions Within the Same
+// Scope"
+
+#version 100
+
+precision mediump float;
+
+int f(int a)
+{
+   return a;
+}
+
+int f(int a)
+{
+   return a;
+}
+
+void main()
+{
+   gl_Position = vec4(f(1));
+}
diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-2.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-2.vert
new file mode 100644
index ..779475a49168
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-2.vert
@@ -0,0 +1,19 @@
+// [config]
+// expect_result: fail
+// glsl_version: 1.00
+// [end config]
+
+// See Section 4.2.7 "Redeclarations and Redefinitions Within the Same
+// Scope"
+
+#version 100
+
+precision mediump float;
+
+int f(int a);
+int f(int a);
+
+void main()
+{
+   gl_Position = vec4(1.0);
+}
diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-3.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-3.vert
new file mode 100644
index ..2422abdd94d2
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-3.vert
@@ -0,0 +1,19 @@
+// [config]
+// expect_result: fail
+// glsl_version: 1.00
+// [end config]
+
+// See Section 4.2.7 "Redeclarations and Redefinitions Within the Same
+// Scope"
+
+#version 100
+
+precision mediump float;
+
+int f(int a);
+struct f { int x; };
+
+void main()
+{
+   gl_Position = vec4(1.0);
+}
diff --git a/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-4.vert 
b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-4.vert
new file mode 100644
index ..44d0d160be0a
--- /dev/null
+++ b/tests/spec/glsl-es-1.00/compiler/redeclaration-disallowed-4.vert
@@ -0,0 +1,19 @@
+// [config]
+// expect_result: fail
+// glsl_version: 1.00
+// [end config]
+
+// See Section 4.2.7 "Redeclarations and Redefinitions Within 

[Piglit] [PATCH] OES_required_internalformat: Add a test for renderbuffer sizing.

[Eric Anholt]

dEQP provides some coverage for texture and renderbuffer formats being
supported, but doesn't test that the storage reports appropriate
sizes.  We can't test textures easily becase GetTexLevelParameter
doesn't exist, but GetRenderbufferParameter can let us test
renderbuffers at least.
---
 tests/all.py   |   5 +
 tests/spec/CMakeLists.txt  |   1 +
 .../CMakeLists.gles2.txt   |  11 ++
 .../oes_required_internalformat/CMakeLists.txt |   1 +
 .../oes_required_internalformat/renderbuffer.c | 149 +
 5 files changed, 167 insertions(+)
 create mode 100644 tests/spec/oes_required_internalformat/CMakeLists.gles2.txt
 create mode 100644 tests/spec/oes_required_internalformat/CMakeLists.txt
 create mode 100644 tests/spec/oes_required_internalformat/renderbuffer.c

diff --git a/tests/all.py b/tests/all.py
index 38cc76d4d63a..010cf2203c4a 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -3214,6 +3214,11 @@ with profile.test_list.group_manager(
 
 with profile.test_list.group_manager(
 PiglitGLTest,
+grouptools.join('spec', 'oes_required_internalformat')) as g:
+g(['oes_required_internalformat-renderbuffer'], 'renderbuffer')
+
+with profile.test_list.group_manager(
+PiglitGLTest,
 grouptools.join('spec', 'ext_frag_depth')) as g:
 g(['fragdepth_gles2'])
 
diff --git a/tests/spec/CMakeLists.txt b/tests/spec/CMakeLists.txt
index 01a7935c736b..cce4dba33dab 100644
--- a/tests/spec/CMakeLists.txt
+++ b/tests/spec/CMakeLists.txt
@@ -93,6 +93,7 @@ add_subdirectory (nv_image_formats)
 add_subdirectory (nv_texture_barrier)
 add_subdirectory (oes_compressed_etc1_rgb8_texture)
 add_subdirectory (oes_compressed_paletted_texture)
+add_subdirectory (oes_required_internalformat)
 add_subdirectory (oes_matrix_get)
 add_subdirectory (arb_draw_elements_base_vertex)
 add_subdirectory (arb_fragment_program)
diff --git a/tests/spec/oes_required_internalformat/CMakeLists.gles2.txt 
b/tests/spec/oes_required_internalformat/CMakeLists.gles2.txt
new file mode 100644
index ..a641c8978e2a
--- /dev/null
+++ b/tests/spec/oes_required_internalformat/CMakeLists.gles2.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 (oes_required_internalformat-renderbuffer renderbuffer.c)
diff --git a/tests/spec/oes_required_internalformat/CMakeLists.txt 
b/tests/spec/oes_required_internalformat/CMakeLists.txt
new file mode 100644
index ..144a306f4e7d
--- /dev/null
+++ b/tests/spec/oes_required_internalformat/CMakeLists.txt
@@ -0,0 +1 @@
+piglit_include_target_api()
diff --git a/tests/spec/oes_required_internalformat/renderbuffer.c 
b/tests/spec/oes_required_internalformat/renderbuffer.c
new file mode 100644
index ..410e51b0ccc5
--- /dev/null
+++ b/tests/spec/oes_required_internalformat/renderbuffer.c
@@ -0,0 +1,149 @@
+/*
+ * Copyright © 2017 Broadcom
+ *
+ * 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.
+ */
+
+/* Tests the following language of GL_OES_required_internalformat:
+ *
+ * "An implementation must accept all of the values for
+ *   specified in Tables 3.4, 3.4.x, 3.4.y.
+ *  Furthermore, an implementation must respect the minimum
+ *  precision requirements of sized internal formats -- those with
+ *  explicit component resolutions -- by storing each component
+ *  with at least the number of bits prescribed."
+ */
+
+#include "piglit-util-gl.h"
+
+PIGLIT_GL_TEST_CONFIG_BEGIN
+
+   config.supports_gl_es_version = 20;
+
+   config.window_visual = PIGLIT_GL_VISUAL_RGB | PIGLIT_GL_VISUAL_DOUBLE;
+
+PIGLIT_GL_TEST_CONFIG_END
+
+static const GLenum size_tokens[] = {
+   GL_RENDERBUFFER_RED_SIZE,
+   

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

[Eric Anholt]

Sandra Koroniewska  writes:

> This fixes
> spec/gl30basic test on Windows Intel or Nvidia driver according to page 693 
> of openGL 4.4 spec ("In additional to OpenGL extensions, there are also ARB 
> extensions to the related GLX and WGL APIs.").

I'm reading that bit of spec text as saying that "extensions in the
Khronos ecosystem start with GL_ or GLX_ or WGL_...", not "extensions
returned from the GL extension list may start with WGL_".

If Khronos's intent is that window system extensions appear in this
extensions string, then we should get that clarified and actually test
for the correct behavior, not permit both behaviors.


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

Re: [Piglit] [PATCH] framework/test/shader_test: use -fbo option for shader tests

[Eric Anholt]

Brian Paul <bri...@vmware.com> writes:

> Running shader tests with a FBO instead of a window is faster in some
> environments.  For example, for a Windows7 VM (debug build of everything)
> the runtime of a large piglit run drops from to 3.7 hours to 3.2 hours.
> In a Linux guest, piglit-run.py -t built-in-functions drops from 19
> minutes to 15 minutes.
>
> No regressions found with VMware driver in Windows VM and Linux VM.

Reviewed-by: Eric Anholt <e...@anholt.net>


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

Re: [Piglit] faster piglit start-up

[Eric Anholt]

Brian Paul  writes:

> When we run piglit the all.py script traverses the entire tests/ and 
> generated/tests/ directories looking for *.shader_test, *.vert, *.frag, 
> etc. shaders to run.  In the generated_tests/ directory, there's about 
> 60,000 entries to scan (ls -R generated_tests|wc)
>
> This takes some time, especially so with MinGW in a Windows VM.  Between 
> that and other setup overhead it takes about 8 minutes on my system 
> before any tests are run.  Ugh.  Linux is much faster but there's still 
> a start-up lag.
>
> I'd like to trim this down.  The prime area I'm looking at is skipping 
> swaths of tests when we know the particular feature is not supported by 
> the GL driver.
>
> In framework/driver_classifier.py we run glxinfo to get the GL renderer 
> string, etc.  I'm looking at expanding this to get the driver's GL and 
> GLSL versions, extension list, etc.
>
> Then, in all.py when we're scanning the tests directory and we find 
> somthing like "spec/arb_gpu_shader_fp64" which may not be supported by 
> the driver, we can skip that directory.  For drivers which don't support 
> all the latest GL 4.x extensions (like VMware's) we can save a lot of 
> time by skipping subdirectory traversals and building the test list.
>
> I'm digging into this now but it'll probably be a few days before I'll 
> have something to review.
>
> I'd be interested to hear any other ideas to reduce start-up time.

vc4 is also terrible to test these days due to the explosion of
generated shader_tests we have -- I've got them hacked out of my builds
due to FS space used and the minutes it takes to start piglit when
they're present.

For a solution to the startup time, I wish we would just build our whole
test list using filesystem walks or whatever at piglit build time,
instead of executing python to build the test list per piglit run.


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