[Piglit] [PATCH v3] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Nanley Chery
From: Nanley Chery 

Enable Jenkins to report the result of each individual subtest
for the array and miptree ASTC tests. Modify the miptree test
to only run one subset of ASTC formats at a time.

v2. Modify miptree test to only check the given subtest.
Use the -subtest option to run a specific subtest.
v3. Indent function arguments and misc cleanups (Ilia).
Initialize texture objects to 0 (Ilia).

Signed-off-by: Nanley Chery 
---
 tests/all.py   | 12 ++--
 .../khr_compressed_astc-miptree.c  | 84 +-
 2 files changed, 42 insertions(+), 54 deletions(-)

diff --git a/tests/all.py b/tests/all.py
index cfafa71..29ee6a9 100644
--- a/tests/all.py
+++ b/tests/all.py
@@ -4223,12 +4223,16 @@ with profile.group_manager(
  PiglitGLTest,
  grouptools.join('spec', 'khr_texture_compression_astc')) as g:
 g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid formats')
-g(['khr_compressed_astc-array_gl'], 'array-gl')
-g(['khr_compressed_astc-array_gles3'], 'array-gles')
 g(['khr_compressed_astc-basic_gl'], 'basic-gl')
 g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
-g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
-g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
+
+for subtest in ('odd', 'even'):
+g(['khr_compressed_astc-array_gl', '-subtest', subtest])
+g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
+
+for subtest in ('ldr', 'srgb', 'hdr'):
+g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
+g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])
 
 with profile.group_manager(
  PiglitGLTest,
diff --git 
a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c 
b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
index 20f2415..6429c2e 100644
--- a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
+++ b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
@@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool check_srgb,
 enum piglit_result
 test_miptrees(void* input_type)
 {
-   int subtest =  0;
-   enum test_type * type = (enum test_type*) input_type;
-   bool is_srgb_test = *type == TEST_TYPE_SRGB;
-   bool is_hdr_test  = *type == TEST_TYPE_HDR;
+   const enum test_type subtest = *(enum test_type*) input_type;
+   const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
+   const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
 
static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
static const char * block_dim_str[14] = {
@@ -245,62 +244,47 @@ test_miptrees(void* input_type)
"12x12"
};
 
-   bool has_hdr = piglit_is_extension_supported(
-   "GL_KHR_texture_compression_astc_hdr");
-
-   /* If testing sRGB mode, fast-forward to the srgb test. */
-   if (is_srgb_test) {
-   subtest =  TEST_TYPE_SRGB;
-   } else {
-   /* Skip if on an HDR system not running the HDR test
-* or if on an LDR system running the HDR test.
-*/
-   if (has_hdr != is_hdr_test)
-   return PIGLIT_SKIP;
+   if (!is_srgb_test)
piglit_require_extension("GL_EXT_texture_sRGB_decode");
 
-   }
-
GLint pixel_offset_loc = glGetUniformLocation(prog, "pixel_offset");
GLint level_pixel_size_loc = glGetUniformLocation(prog,
"level_pixel_size");
 
-   /* Test each submode */
-   for (; subtest < ARRAY_SIZE(tests); ++subtest) {
-
-   /*  Check for error color if an LDR-only sys reading an HDR
-*  texture. No need to draw a reference mipmap in this case.
-*/
-   int check_error = !has_hdr && subtest == TEST_TYPE_HDR;
-   int block_dims = 0;
-   for (; block_dims < ARRAY_SIZE(block_dim_str); ++block_dims) {
-
-   /* Texture objects. */
-   GLuint tex_compressed;
-   GLuint tex_decompressed;
-
-   /* Load texture for current submode and block size */
-   load_texture("compressed", tests[subtest],
+   /*  Check for error color if an LDR-only sys reading an HDR
+*  texture. No need to draw a reference mipmap in this case.
+*/
+   const bool has_hdr = piglit_is_extension_supported(
+   "GL_KHR_texture_compression_astc_hdr");
+   const bool check_error = is_hdr_test && !has_hdr;
+   int block_dims;
+   for (block_dims = 0; block_dims < ARRAY_SIZE(block_dim_str); 
++block_dims) {
+
+   /* Texture objects. */
+   GLuint tex_compressed = 0;
+   GLuint tex_decompressed = 0;
+
+   /* Load texture for current 

Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Nanley Chery
On Mon, Nov 30, 2015 at 05:04:54PM -0800, Dylan Baker wrote:
> On Mon, Nov 30, 2015 at 04:34:40PM -0800, Nanley Chery wrote:
> > On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
> > > On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  
> > > wrote:
> > > > From: Nanley Chery 
> > > >
> > > > Enable Piglit to report the result of each individual subtest
> > > > for the array and miptree ASTC tests. Modify the miptree test
> > > > to only run one subset of ASTC formats at a time.
> > > >
> > > > v2. Modify miptree test to only check the given subtest.
> > > > Use the -subtest option to run a specific subtest.
> > > >
> > > > Signed-off-by: Nanley Chery 
> > > > ---
> > > >  tests/all.py   | 12 ++-
> > > >  .../khr_compressed_astc-miptree.c  | 90 
> > > > +-
> > > >  2 files changed, 45 insertions(+), 57 deletions(-)
> > > >
> > > > diff --git a/tests/all.py b/tests/all.py
> > > > index 82e6311..c63cf15 100644
> > > > --- a/tests/all.py
> > > > +++ b/tests/all.py
> > > > @@ -4182,12 +4182,16 @@ with profile.group_manager(
> > > >   PiglitGLTest,
> > > >   grouptools.join('spec', 'khr_texture_compression_astc')) as g:
> > > >  g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid 
> > > > formats')
> > > > -g(['khr_compressed_astc-array_gl'], 'array-gl')
> > > > -g(['khr_compressed_astc-array_gles3'], 'array-gles')
> > > >  g(['khr_compressed_astc-basic_gl'], 'basic-gl')
> > > >  g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
> > > > -g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
> > > > -g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
> > > > +
> > > > +for subtest in ('odd', 'even'):
> > > > +g(['khr_compressed_astc-array_gl', '-subtest', subtest])
> > > > +g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
> > > > +
> > > > +for subtest in ('ldr', 'srgb', 'hdr'):
> > > > +g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
> > > > +g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])
> > > 
> > > Won't it auto-run all the subtests on its own? It did that for me, I'm
> > > pretty sure.
> > > 
> > 
> > It does auto-run all of them by default, however it is useful to know
> > which specific case is causing the test to fail.
> 
> Alternatively you could use piglit_report_subtest_result when the test
> runs all subtests.
> 

The only place where piglit_report_result() is called is during the KTX
file loading function. Otherwise, piglit_report_subtest_result() is
called via piglit_run_selected_subtest(). By making the change in
all.py, it's easier to know which subtest is failing in Jenkins without
having to inspect the failure output. I suppose I should reword my
commit message.

-Nanley

> > 
> > > >
> > > >  with profile.group_manager(
> > > >   PiglitGLTest,
> > > > diff --git 
> > > > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c 
> > > > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > > index 20f2415..20d1c79 100644
> > > > --- 
> > > > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > > +++ 
> > > > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > > @@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool 
> > > > check_srgb,
> > > >  enum piglit_result
> > > >  test_miptrees(void* input_type)
> > > >  {
> > > > -   int subtest =  0;
> > > > -   enum test_type * type = (enum test_type*) input_type;
> > > > -   bool is_srgb_test = *type == TEST_TYPE_SRGB;
> > > > -   bool is_hdr_test  = *type == TEST_TYPE_HDR;
> > > > +   const enum test_type subtest = *(enum test_type*) input_type;
> > > > +   const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
> > > > +   const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
> > > >
> > > > static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
> > > > static const char * block_dim_str[14] = {
> > > > @@ -245,62 +244,47 @@ test_miptrees(void* input_type)
> > > > "12x12"
> > > > };
> > > >
> > > > -   bool has_hdr = piglit_is_extension_supported(
> > > > -   "GL_KHR_texture_compression_astc_hdr");
> > > > -
> > > > -   /* If testing sRGB mode, fast-forward to the srgb test. */
> > > > -   if (is_srgb_test) {
> > > > -   subtest =  TEST_TYPE_SRGB;
> > > > -   } else {
> > > > -   /* Skip if on an HDR system not running the HDR test
> > > > -* or if on an LDR system running the HDR test.
> > > > -*/
> > > > -   if (has_hdr != is_hdr_test)
> > > > -   return PIGLIT_SKIP;
> > > > +   if (!is_srgb_test)
> > > > piglit_require_extension("GL_EXT_texture_sRGB_decode");
> > > 
> > > Which we sadly don't expose in GLES. (Alt

[Piglit] [PATCH] arb_program_interface_query: Silence uninitialized variable warning.

2015-11-30 Thread Vinson Lee
Fixes Coverity "uninitialized scalar variable" defect.

Signed-off-by: Vinson Lee 
---
 tests/spec/arb_program_interface_query/getprogramresourceiv.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/tests/spec/arb_program_interface_query/getprogramresourceiv.c 
b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
index 769e29f..7ef72cc 100755
--- a/tests/spec/arb_program_interface_query/getprogramresourceiv.c
+++ b/tests/spec/arb_program_interface_query/getprogramresourceiv.c
@@ -668,6 +668,9 @@ check_prop(GLuint prog, GLenum programInterface, int index, 
const char *name,
case GL_BUFFER_VARIABLE:
pif = GL_SHADER_STORAGE_BLOCK;
break;
+   default:
+   assert(0);
+   pif = GL_NONE;
}
 
parent_idx = glGetProgramResourceIndex(prog, pif,
@@ -739,6 +742,9 @@ check_prop(GLuint prog, GLenum programInterface, int index, 
const char *name,
case GL_COMPUTE_SUBROUTINE_UNIFORM:
pif = GL_COMPUTE_SUBROUTINE;
break;
+   default:
+   assert(0);
+   pif = GL_NONE;
}
 
/* check that the return count is as expected */
-- 
2.6.3

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


[Piglit] [PATCH] arb_es2_compatibility: Check piglit_link_check_status result.

2015-11-30 Thread Vinson Lee
Fixes Coverity "unchecked return value" defect.

Signed-off-by: Vinson Lee 
---
 .../arb_es2_compatibility/arb_es2_compatibility-releaseshadercompiler.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/tests/spec/arb_es2_compatibility/arb_es2_compatibility-releaseshadercompiler.c
 
b/tests/spec/arb_es2_compatibility/arb_es2_compatibility-releaseshadercompiler.c
index b5c476e..ae5830d 100644
--- 
a/tests/spec/arb_es2_compatibility/arb_es2_compatibility-releaseshadercompiler.c
+++ 
b/tests/spec/arb_es2_compatibility/arb_es2_compatibility-releaseshadercompiler.c
@@ -70,7 +70,7 @@ draw(const float *color, float x_offset)
 
glBindAttribLocation(prog, 0, "vertex");
glLinkProgram(prog);
-   piglit_link_check_status(prog);
+   assert(piglit_link_check_status(prog));
 
glUseProgram(prog);
color_location = glGetUniformLocation(prog, "color");
-- 
2.6.3

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


Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Nanley Chery
On Mon, Nov 30, 2015 at 07:46:37PM -0500, Ilia Mirkin wrote:
> On Mon, Nov 30, 2015 at 7:34 PM, Nanley Chery  wrote:
> > On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
> >> On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  
> >> wrote:
> >> > +
> >> > +   /* Load texture for current submode and block size */
> >> > +   load_texture("compressed", tests[subtest],
> >> > +   block_dim_str[block_dims],
> >> > +   &tex_compressed);
> >> > +   if (!check_error) {
> >> > +   load_texture("decompressed", tests[subtest],
> >> > +   block_dim_str[block_dims],
> >>
> >> Here and elsewhere, this needs to be indented to the (, or past it...
> >> otherwise it's unreadable.
> >>
> >
> > Isn't this the coding style of Piglit? See this snippet in HACKING:
> > "* Indent with 8-column tabs"
> 
> By that logic, having 0 indentation on every line would be fine. If
> you have a function call, and you have arguments on the next line,
> those arguments need to be indented deeper than that function call.
> 

Oh, I thought you were referring to the '(' in the if statement.
I'll perform the indentation you suggested for the function calls.

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


Re: [Piglit] [PATCH] arb_separate_shader_objects: dont check for INVALID_OPERATION after validation

2015-11-30 Thread Tapani Pälli

Reviewed-by: Tapani Pälli 

On 12/01/2015 01:31 AM, Timothy Arceri wrote:

Validation should not generate an INVALID_OPERATION only the draw call should, 
this
was seems to have been working around a bug in Mesa.

https://bugs.freedesktop.org/show_bug.cgi?id=93180
---
  tests/spec/arb_separate_shader_objects/active-sampler-conflict.c | 2 --
  1 file changed, 2 deletions(-)

diff --git a/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c 
b/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
index 0a91f49..fbfa957 100644
--- a/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
+++ b/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
@@ -226,8 +226,6 @@ test_sampler_conflict(GLuint prog, GLuint pipe,
pass = false;
}
  
-	pass = piglit_check_gl_error(GL_INVALID_OPERATION) && pass;

-
/* Switch back to the valid configuration.  Without first calling
 * glValidateProgramPipeline, try to draw something.  Verify that
 * no error is generated.


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


Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Dylan Baker
On Mon, Nov 30, 2015 at 04:34:40PM -0800, Nanley Chery wrote:
> On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
> > On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  wrote:
> > > From: Nanley Chery 
> > >
> > > Enable Piglit to report the result of each individual subtest
> > > for the array and miptree ASTC tests. Modify the miptree test
> > > to only run one subset of ASTC formats at a time.
> > >
> > > v2. Modify miptree test to only check the given subtest.
> > > Use the -subtest option to run a specific subtest.
> > >
> > > Signed-off-by: Nanley Chery 
> > > ---
> > >  tests/all.py   | 12 ++-
> > >  .../khr_compressed_astc-miptree.c  | 90 
> > > +-
> > >  2 files changed, 45 insertions(+), 57 deletions(-)
> > >
> > > diff --git a/tests/all.py b/tests/all.py
> > > index 82e6311..c63cf15 100644
> > > --- a/tests/all.py
> > > +++ b/tests/all.py
> > > @@ -4182,12 +4182,16 @@ with profile.group_manager(
> > >   PiglitGLTest,
> > >   grouptools.join('spec', 'khr_texture_compression_astc')) as g:
> > >  g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid 
> > > formats')
> > > -g(['khr_compressed_astc-array_gl'], 'array-gl')
> > > -g(['khr_compressed_astc-array_gles3'], 'array-gles')
> > >  g(['khr_compressed_astc-basic_gl'], 'basic-gl')
> > >  g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
> > > -g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
> > > -g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
> > > +
> > > +for subtest in ('odd', 'even'):
> > > +g(['khr_compressed_astc-array_gl', '-subtest', subtest])
> > > +g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
> > > +
> > > +for subtest in ('ldr', 'srgb', 'hdr'):
> > > +g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
> > > +g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])
> > 
> > Won't it auto-run all the subtests on its own? It did that for me, I'm
> > pretty sure.
> > 
> 
> It does auto-run all of them by default, however it is useful to know
> which specific case is causing the test to fail.

Alternatively you could use piglit_report_subtest_result when the test
runs all subtests.

> 
> > >
> > >  with profile.group_manager(
> > >   PiglitGLTest,
> > > diff --git 
> > > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c 
> > > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > index 20f2415..20d1c79 100644
> > > --- 
> > > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > +++ 
> > > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > > @@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool 
> > > check_srgb,
> > >  enum piglit_result
> > >  test_miptrees(void* input_type)
> > >  {
> > > -   int subtest =  0;
> > > -   enum test_type * type = (enum test_type*) input_type;
> > > -   bool is_srgb_test = *type == TEST_TYPE_SRGB;
> > > -   bool is_hdr_test  = *type == TEST_TYPE_HDR;
> > > +   const enum test_type subtest = *(enum test_type*) input_type;
> > > +   const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
> > > +   const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
> > >
> > > static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
> > > static const char * block_dim_str[14] = {
> > > @@ -245,62 +244,47 @@ test_miptrees(void* input_type)
> > > "12x12"
> > > };
> > >
> > > -   bool has_hdr = piglit_is_extension_supported(
> > > -   "GL_KHR_texture_compression_astc_hdr");
> > > -
> > > -   /* If testing sRGB mode, fast-forward to the srgb test. */
> > > -   if (is_srgb_test) {
> > > -   subtest =  TEST_TYPE_SRGB;
> > > -   } else {
> > > -   /* Skip if on an HDR system not running the HDR test
> > > -* or if on an LDR system running the HDR test.
> > > -*/
> > > -   if (has_hdr != is_hdr_test)
> > > -   return PIGLIT_SKIP;
> > > +   if (!is_srgb_test)
> > > piglit_require_extension("GL_EXT_texture_sRGB_decode");
> > 
> > Which we sadly don't expose in GLES. (Although isn't the functionality
> > part of GLES3?)
> > 
> 
> Yes, we do no expose this in GLES. I'm not sure if it's part of GLES3.
> I think someone should implement this in mesa. I started to do so, but
> received some higher priority tasks.
> 
> > >
> > > -   }
> > > -
> > > GLint pixel_offset_loc = glGetUniformLocation(prog, 
> > > "pixel_offset");
> > > GLint level_pixel_size_loc = glGetUniformLocation(prog,
> > > 
> > > "level_pixel_size");
> > >
> > > -   /* Test each submode */
> > > -   for (; subtest < ARRAY_SIZE(tests); ++subtest) {
> > > -

Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Ilia Mirkin
On Mon, Nov 30, 2015 at 7:34 PM, Nanley Chery  wrote:
> On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
>> On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  wrote:
>> > +
>> > +   /* Load texture for current submode and block size */
>> > +   load_texture("compressed", tests[subtest],
>> > +   block_dim_str[block_dims],
>> > +   &tex_compressed);
>> > +   if (!check_error) {
>> > +   load_texture("decompressed", tests[subtest],
>> > +   block_dim_str[block_dims],
>>
>> Here and elsewhere, this needs to be indented to the (, or past it...
>> otherwise it's unreadable.
>>
>
> Isn't this the coding style of Piglit? See this snippet in HACKING:
> "* Indent with 8-column tabs"

By that logic, having 0 indentation on every line would be fine. If
you have a function call, and you have arguments on the next line,
those arguments need to be indented deeper than that function call.

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


Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Nanley Chery
On Mon, Nov 30, 2015 at 03:50:04PM -0500, Ilia Mirkin wrote:
> On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  wrote:
> > From: Nanley Chery 
> >
> > Enable Piglit to report the result of each individual subtest
> > for the array and miptree ASTC tests. Modify the miptree test
> > to only run one subset of ASTC formats at a time.
> >
> > v2. Modify miptree test to only check the given subtest.
> > Use the -subtest option to run a specific subtest.
> >
> > Signed-off-by: Nanley Chery 
> > ---
> >  tests/all.py   | 12 ++-
> >  .../khr_compressed_astc-miptree.c  | 90 
> > +-
> >  2 files changed, 45 insertions(+), 57 deletions(-)
> >
> > diff --git a/tests/all.py b/tests/all.py
> > index 82e6311..c63cf15 100644
> > --- a/tests/all.py
> > +++ b/tests/all.py
> > @@ -4182,12 +4182,16 @@ with profile.group_manager(
> >   PiglitGLTest,
> >   grouptools.join('spec', 'khr_texture_compression_astc')) as g:
> >  g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid 
> > formats')
> > -g(['khr_compressed_astc-array_gl'], 'array-gl')
> > -g(['khr_compressed_astc-array_gles3'], 'array-gles')
> >  g(['khr_compressed_astc-basic_gl'], 'basic-gl')
> >  g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
> > -g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
> > -g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
> > +
> > +for subtest in ('odd', 'even'):
> > +g(['khr_compressed_astc-array_gl', '-subtest', subtest])
> > +g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
> > +
> > +for subtest in ('ldr', 'srgb', 'hdr'):
> > +g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
> > +g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])
> 
> Won't it auto-run all the subtests on its own? It did that for me, I'm
> pretty sure.
> 

It does auto-run all of them by default, however it is useful to know
which specific case is causing the test to fail.

> >
> >  with profile.group_manager(
> >   PiglitGLTest,
> > diff --git 
> > a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c 
> > b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > index 20f2415..20d1c79 100644
> > --- a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > +++ b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> > @@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool 
> > check_srgb,
> >  enum piglit_result
> >  test_miptrees(void* input_type)
> >  {
> > -   int subtest =  0;
> > -   enum test_type * type = (enum test_type*) input_type;
> > -   bool is_srgb_test = *type == TEST_TYPE_SRGB;
> > -   bool is_hdr_test  = *type == TEST_TYPE_HDR;
> > +   const enum test_type subtest = *(enum test_type*) input_type;
> > +   const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
> > +   const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
> >
> > static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
> > static const char * block_dim_str[14] = {
> > @@ -245,62 +244,47 @@ test_miptrees(void* input_type)
> > "12x12"
> > };
> >
> > -   bool has_hdr = piglit_is_extension_supported(
> > -   "GL_KHR_texture_compression_astc_hdr");
> > -
> > -   /* If testing sRGB mode, fast-forward to the srgb test. */
> > -   if (is_srgb_test) {
> > -   subtest =  TEST_TYPE_SRGB;
> > -   } else {
> > -   /* Skip if on an HDR system not running the HDR test
> > -* or if on an LDR system running the HDR test.
> > -*/
> > -   if (has_hdr != is_hdr_test)
> > -   return PIGLIT_SKIP;
> > +   if (!is_srgb_test)
> > piglit_require_extension("GL_EXT_texture_sRGB_decode");
> 
> Which we sadly don't expose in GLES. (Although isn't the functionality
> part of GLES3?)
> 

Yes, we do no expose this in GLES. I'm not sure if it's part of GLES3.
I think someone should implement this in mesa. I started to do so, but
received some higher priority tasks.

> >
> > -   }
> > -
> > GLint pixel_offset_loc = glGetUniformLocation(prog, "pixel_offset");
> > GLint level_pixel_size_loc = glGetUniformLocation(prog,
> > "level_pixel_size");
> >
> > -   /* Test each submode */
> > -   for (; subtest < ARRAY_SIZE(tests); ++subtest) {
> > -
> > -   /*  Check for error color if an LDR-only sys reading an HDR
> > -*  texture. No need to draw a reference mipmap in this 
> > case.
> > -*/
> > -   int check_error = !has_hdr && subtest == TEST_TYPE_HDR;
> > -   int block_dims = 0;
> > -   for (; block_dims < ARRAY_SIZE(block_dim_str); 
> > ++block_dims) {
> > -

[Piglit] [PATCH] arb_separate_shader_objects: dont check for INVALID_OPERATION after validation

2015-11-30 Thread Timothy Arceri
Validation should not generate an INVALID_OPERATION only the draw call should, 
this
was seems to have been working around a bug in Mesa.

https://bugs.freedesktop.org/show_bug.cgi?id=93180
---
 tests/spec/arb_separate_shader_objects/active-sampler-conflict.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c 
b/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
index 0a91f49..fbfa957 100644
--- a/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
+++ b/tests/spec/arb_separate_shader_objects/active-sampler-conflict.c
@@ -226,8 +226,6 @@ test_sampler_conflict(GLuint prog, GLuint pipe,
pass = false;
}
 
-   pass = piglit_check_gl_error(GL_INVALID_OPERATION) && pass;
-
/* Switch back to the valid configuration.  Without first calling
 * glValidateProgramPipeline, try to draw something.  Verify that
 * no error is generated.
-- 
2.4.3

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


Re: [Piglit] [PATCH] khr_texture_compression_astc: Enable subtest reports

2015-11-30 Thread Ilia Mirkin
On Wed, Oct 28, 2015 at 7:55 PM, Nanley Chery  wrote:
> From: Nanley Chery 
>
> Enable Piglit to report the result of each individual subtest
> for the array and miptree ASTC tests. Modify the miptree test
> to only run one subset of ASTC formats at a time.
>
> v2. Modify miptree test to only check the given subtest.
> Use the -subtest option to run a specific subtest.
>
> Signed-off-by: Nanley Chery 
> ---
>  tests/all.py   | 12 ++-
>  .../khr_compressed_astc-miptree.c  | 90 
> +-
>  2 files changed, 45 insertions(+), 57 deletions(-)
>
> diff --git a/tests/all.py b/tests/all.py
> index 82e6311..c63cf15 100644
> --- a/tests/all.py
> +++ b/tests/all.py
> @@ -4182,12 +4182,16 @@ with profile.group_manager(
>   PiglitGLTest,
>   grouptools.join('spec', 'khr_texture_compression_astc')) as g:
>  g(['arb_texture_compression-invalid-formats', 'astc'], 'invalid formats')
> -g(['khr_compressed_astc-array_gl'], 'array-gl')
> -g(['khr_compressed_astc-array_gles3'], 'array-gles')
>  g(['khr_compressed_astc-basic_gl'], 'basic-gl')
>  g(['khr_compressed_astc-basic_gles2'], 'basic-gles')
> -g(['khr_compressed_astc-miptree_gl'], 'miptree-gl')
> -g(['khr_compressed_astc-miptree_gles2'], 'miptree-gles')
> +
> +for subtest in ('odd', 'even'):
> +g(['khr_compressed_astc-array_gl', '-subtest', subtest])
> +g(['khr_compressed_astc-array_gles3', '-subtest', subtest])
> +
> +for subtest in ('ldr', 'srgb', 'hdr'):
> +g(['khr_compressed_astc-miptree_gl', '-subtest', subtest])
> +g(['khr_compressed_astc-miptree_gles2', '-subtest', subtest])

Won't it auto-run all the subtests on its own? It did that for me, I'm
pretty sure.

>
>  with profile.group_manager(
>   PiglitGLTest,
> diff --git 
> a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c 
> b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> index 20f2415..20d1c79 100644
> --- a/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> +++ b/tests/spec/khr_texture_compression_astc/khr_compressed_astc-miptree.c
> @@ -222,10 +222,9 @@ bool draw_compare_levels(bool check_error, bool 
> check_srgb,
>  enum piglit_result
>  test_miptrees(void* input_type)
>  {
> -   int subtest =  0;
> -   enum test_type * type = (enum test_type*) input_type;
> -   bool is_srgb_test = *type == TEST_TYPE_SRGB;
> -   bool is_hdr_test  = *type == TEST_TYPE_HDR;
> +   const enum test_type subtest = *(enum test_type*) input_type;
> +   const bool is_srgb_test = subtest == TEST_TYPE_SRGB;
> +   const bool is_hdr_test  = subtest == TEST_TYPE_HDR;
>
> static const char * tests[3] = {"hdr", "ldrl", "ldrs"};
> static const char * block_dim_str[14] = {
> @@ -245,62 +244,47 @@ test_miptrees(void* input_type)
> "12x12"
> };
>
> -   bool has_hdr = piglit_is_extension_supported(
> -   "GL_KHR_texture_compression_astc_hdr");
> -
> -   /* If testing sRGB mode, fast-forward to the srgb test. */
> -   if (is_srgb_test) {
> -   subtest =  TEST_TYPE_SRGB;
> -   } else {
> -   /* Skip if on an HDR system not running the HDR test
> -* or if on an LDR system running the HDR test.
> -*/
> -   if (has_hdr != is_hdr_test)
> -   return PIGLIT_SKIP;
> +   if (!is_srgb_test)
> piglit_require_extension("GL_EXT_texture_sRGB_decode");

Which we sadly don't expose in GLES. (Although isn't the functionality
part of GLES3?)

>
> -   }
> -
> GLint pixel_offset_loc = glGetUniformLocation(prog, "pixel_offset");
> GLint level_pixel_size_loc = glGetUniformLocation(prog,
> "level_pixel_size");
>
> -   /* Test each submode */
> -   for (; subtest < ARRAY_SIZE(tests); ++subtest) {
> -
> -   /*  Check for error color if an LDR-only sys reading an HDR
> -*  texture. No need to draw a reference mipmap in this case.
> -*/
> -   int check_error = !has_hdr && subtest == TEST_TYPE_HDR;
> -   int block_dims = 0;
> -   for (; block_dims < ARRAY_SIZE(block_dim_str); ++block_dims) {
> -
> -   /* Texture objects. */
> -   GLuint tex_compressed;
> -   GLuint tex_decompressed;
> -
> -   /* Load texture for current submode and block size */
> -   load_texture("compressed", tests[subtest],
> -   block_dim_str[block_dims],
> -   &tex_compressed);
> -   if (!check_error) {
> -   load_texture("decompressed", tests[subtest],
> -   

Re: [Piglit] [PATCH v3 4/4] framework/test/base.py: use subprocess32 for timeouts.

2015-11-30 Thread Dylan Baker
On Fri, Nov 27, 2015 at 12:04:34PM +, Thomas Wood wrote:
> On 12 November 2015 at 22:52,   wrote:
> > From: Dylan Baker 
> >
> > Subprocess32 provides a backport of (ironically) python 3.3's subprocess
> > module, which has a timeout parameter. When the timeout runs out then an
> > exception is raised, and when that exception is caught we can kill the
> > process.
> >
> > This is fairly similar to the way the current timeout mechanism works,
> > except that the current mechanism is not thread safe. Since one of the
> > major features of piglit is that it offer's processes isolated
> > concurrency of tests, it makes sense to make the effort to provide a
> > timeout mechanism that can be used outside of IGT, which is a
> > non-concurrent test suite.
> >
> > This patch look pretty substantial. It's not as big as it looks, since
> > it adds some fairly big tests.
> >
> > v3: - Wait before terminating process
> > - use os.getgrpid instead of os.getsid
> 
> os.getpgid?
> 
> 
> > - use subprocess32 for accel profile in tox
> > - Add warning when using the dummy version of TimeoutMixin
> 
> TimeoutExpired?
> 

Both of these fixed locally

> 
> > - mark more unittests as slow and add timeouts to them
> > - Remove the use of a Mixin, timeouts are an important enough
> >   feature every test suite should have the option to use them, and
> >   by not using a mixin there is no need for the ugly interface.
> >
> > Signed-off-by: Dylan Baker 
> > ---
> >  framework/results.py  |   2 +
> >  framework/test/base.py| 130 
> > --
> >  framework/tests/base_tests.py | 117 -
> >  tox.ini   |  10 ++--
> >  4 files changed, 171 insertions(+), 88 deletions(-)
> >
> > diff --git a/framework/results.py b/framework/results.py
> > index eeffcb7..ee41126 100644
> > --- a/framework/results.py
> > +++ b/framework/results.py
> > @@ -96,6 +96,8 @@ class StringDescriptor(object):  # pylint: 
> > disable=too-few-public-methods
> >  setattr(instance, self.__name, value.decode('utf-8', 
> > 'replace'))
> >  elif isinstance(value, unicode):
> >  setattr(instance, self.__name, value)
> > +elif value is None:
> > +setattr(instance, self.__name, u'')
> 
> Unrelated change?
> 

I'll have to look to be sure, but I think this actually was required in
the timeout case.

> 
> >  else:
> >  raise TypeError('{} attribute must be a str or unicode 
> > instance, '
> >  'but was {}.'.format(self.__name, type(value)))
> > diff --git a/framework/test/base.py b/framework/test/base.py
> > index 7c7c410..d1b0c35 100644
> > --- a/framework/test/base.py
> > +++ b/framework/test/base.py
> > @@ -25,16 +25,36 @@
> >  from __future__ import print_function, absolute_import
> >  import errno
> >  import os
> > -import subprocess
> >  import time
> >  import sys
> >  import traceback
> > -from datetime import datetime
> > -import threading
> > -import signal
> >  import itertools
> >  import abc
> >  import copy
> > +import signal
> > +import warnings
> > +
> > +try:
> > +import subprocess32 as subprocess
> > +_EXTRA_POPEN_ARGS = {'start_new_session': True}
> > +except ImportError:
> > +import subprocess
> > +
> > +class TimeoutExpired(Exception):
> > +pass
> > +
> > +class Popen(subprocess.Popen):
> > +"""Sublcass of Popen that accepts and ignores a timeout 
> > argument."""
> > +def communicate(self, *args, **kwargs):
> > +if 'timeout' in kwargs:
> > +del kwargs['timeout']
> > +return super(Popen, self).communicate(*args, **kwargs)
> > +
> > +subprocess.TimeoutExpired = TimeoutExpired
> > +subprocess.Popen = Popen
> > +_EXTRA_POPEN_ARGS = {}
> > +
> > +warnings.warn('Timeouts are not available')
> >
> >  from framework import exceptions, options
> >  from framework.results import TestResult
> > @@ -61,56 +81,6 @@ class TestRunError(exceptions.PiglitException):
> >  self.status = status
> >
> >
> > -class ProcessTimeout(threading.Thread):
> > -""" Timeout class for test processes
> > -
> > -This class is for terminating tests that run for longer than a certain
> > -amount of time. Create an instance by passing it a timeout in seconds
> > -and the Popen object that is running your test. Then call the start
> > -method (inherited from Thread) to start the timer. The Popen object is
> > -killed if the timeout is reached and it has not completed. Wait for the
> > -outcome by calling the join() method from the parent.
> > -
> > -"""
> > -
> > -def __init__(self, timeout, proc):
> > -threading.Thread.__init__(self)
> > -self.proc = proc
> > -self.timeout = timeout
> > -self.status = 0
> > -
> > -def run(self):
> > -start_time = datetime.now()
> > - 

Re: [Piglit] [PATCH] astc: avoid deleting a random texture

2015-11-30 Thread Nanley Chery
On Mon, Nov 23, 2015 at 1:28 PM, Ilia Mirkin  wrote:

> On Mon, Nov 23, 2015 at 4:16 PM, Ian Romanick  wrote:
> > On 11/23/2015 12:43 PM, Ilia Mirkin wrote:
> >> On Mon, Nov 23, 2015 at 3:34 PM, Ian Romanick 
> wrote:
> >>> On 11/21/2015 04:08 PM, Ilia Mirkin wrote:
>  In the check_error case, decompressed_tex is completely uninitialized
>  and might point to any texture. This can wreak various havoc.
> >>>
>

Thanks for noticing this issue.


> >>> I might suggest a different approach.  In the check_error case, ensure
> >>> that decompress_texture is zero.  Remove the check_error parameter, and
> >>> s/!check_error/decompress_texture ==
> >>> 0/g;s/check_error/decompress_texture != 0/g.
> >>
> >> Is it legal to delete texture 0? Or can texture 0 be a real thing?
> >> Otherwise I can just initialize it to that below.
> >
> > It's similar to free(NULL) in that respect.  From the glDeleteTextures
> > man page:
> >
> >glDeleteTextures silently ignores 0's and names that do not
> correspond
> >to existing textures.
>
> Ah that's perfect. I should have checked the man page when I was
> futzing with the test.
>
> >
> >> Separately, how would you feel about making the error color decoding
> >> check failing return a warn instead of a fail? I can't get Adreno A420
> >> to spit it out... haven't tried on blob, perhaps I'm missing some bit
> >> of programming. I just get black with the HDR data instead of the
> >> error color.
> >
> > Hmm... I don't know how this particular test is structured.  When we've
> > had cases like this in the past, we've split the test into subcases.
> > One (or several) subcase checks the things that the driver in question
> > can pass, and one, single subcase checks the thing that the driver
> > fails.
> >
> > If this test is conducive that kind of a split (perhaps by running it
> > twice with different data), that would be my preference.
>
> Right now there are 3 subtests (ldr, hdr, srgb), each of which tests
> loading 3 sets of data (ldr, hdr, and ldr srgb). If there's no
> astc_hdr support, the hdr subtest isn't run, and when loading hdr
> data, it checks for the error color. Instead of 3 subtests, I could
> split this up into 9 subtests for the full cross -- does that sound
> reasonable, and then we'd pass the ldr-data ones and fail the hdr data
> ones.
>
> TBH I'm unsure what the diff between the ldr and hdr subtests is...
> they seem identical except that HDR one skips if you don't hdr. But
> otherwise it's identical to the LDR one from what I can tell. Also
> from the looks of it the srgb test doesn't need the full cross either,
> it only runs against the ldrs data. Urgh, I guess this test needs a
> bit of a redo :( Perhaps we can sucker Nanley into fixing it up?
>

I actually have a patch sitting on the list which redoes some of the logic:
http://lists.freedesktop.org/archives/piglit/2015-October/017743.html

The test previously wasn't splitting up into subtests correctly.

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


Re: [Piglit] [PATCH] fbo-depth-array: Fix reporting the piglit_result

2015-11-30 Thread Marek Olšák
Reviewed-by: Marek Olšák 
On Nov 30, 2015 1:19 PM, "Neil Roberts"  wrote:

> test_once was ignoring the return value of draw_and_test_layer so the
> test would report a pass even if it failed.
>
> Reported-by: Marek Olšák 
> ---
>
> Oops, sorry about that. Thanks for reporting it.
>  tests/fbo/fbo-depth-array.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/fbo/fbo-depth-array.c b/tests/fbo/fbo-depth-array.c
> index b0acd5e..45c8b3b 100644
> --- a/tests/fbo/fbo-depth-array.c
> +++ b/tests/fbo/fbo-depth-array.c
> @@ -446,7 +446,7 @@ test_once(void)
> x = 1 + (layer % 3) * (width + 1);
> y = 1 + (layer / 3) * (height + 1);
> }
> -   draw_and_test_layer(x, y, layer);
> +   pass = draw_and_test_layer(x, y, layer) && pass;
>
> if (piglit_use_fbo && !test_single_size && layer <
> layers-1) {
> glClearColor(0.2, 0.1, 0.1, 1.0);
> --
> 1.9.3
>
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


[Piglit] [PATCH] fbo-depth-array: Fix reporting the piglit_result

2015-11-30 Thread Neil Roberts
test_once was ignoring the return value of draw_and_test_layer so the
test would report a pass even if it failed.

Reported-by: Marek Olšák 
---

Oops, sorry about that. Thanks for reporting it.
 
 tests/fbo/fbo-depth-array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fbo/fbo-depth-array.c b/tests/fbo/fbo-depth-array.c
index b0acd5e..45c8b3b 100644
--- a/tests/fbo/fbo-depth-array.c
+++ b/tests/fbo/fbo-depth-array.c
@@ -446,7 +446,7 @@ test_once(void)
x = 1 + (layer % 3) * (width + 1);
y = 1 + (layer / 3) * (height + 1);
}
-   draw_and_test_layer(x, y, layer);
+   pass = draw_and_test_layer(x, y, layer) && pass;
 
if (piglit_use_fbo && !test_single_size && layer < layers-1) {
glClearColor(0.2, 0.1, 0.1, 1.0);
-- 
1.9.3

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


Re: [Piglit] [PATCH] arb_tessellation_shader: Check piglit_link_check_status results.

2015-11-30 Thread Marek Olšák
Reviewed-by. Marek Olšák 

Marek

On Mon, Nov 30, 2015 at 8:46 AM, Vinson Lee  wrote:
> Fixes Coverity "unchecked return value" defects.
>
> Signed-off-by: Vinson Lee 
> ---
>  tests/spec/arb_tessellation_shader/invalid-get-program-params.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/spec/arb_tessellation_shader/invalid-get-program-params.c 
> b/tests/spec/arb_tessellation_shader/invalid-get-program-params.c
> index 4cbe29a..c3347d8 100644
> --- a/tests/spec/arb_tessellation_shader/invalid-get-program-params.c
> +++ b/tests/spec/arb_tessellation_shader/invalid-get-program-params.c
> @@ -78,11 +78,11 @@ piglit_init(int argc, char **argv)
>
> tcs_prog = glCreateShaderProgramv(GL_TESS_CONTROL_SHADER, 1,
>   (const GLchar *const*)&tcs_source);
> -   piglit_link_check_status(tcs_prog);
> +   pass = piglit_link_check_status(tcs_prog) && pass;
>
> tes_prog = glCreateShaderProgramv(GL_TESS_EVALUATION_SHADER, 1,
>   (const GLchar *const*)&tes_source);
> -   piglit_link_check_status(tes_prog);
> +   pass = piglit_link_check_status(tes_prog) && pass;
>
> for (i = 0; i < ARRAY_SIZE(tes_params); ++i ) {
> pass = piglit_check_gl_error(GL_NO_ERROR) && pass;
> --
> 2.6.3
>
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH] arb_separate_shader_objects: test explicit location with a struct

2015-11-30 Thread Tapani Pälli

Reviewed-by: Tapani Pälli 

On 11/30/2015 09:42 AM, Timothy Arceri wrote:

Test results:
Nvidia GeForce 840M - NVIDIA 352.41: pass
i965 - Mesa 11.2-dev: fail
---
  .../execution/layout-location-struct.shader_test   | 51 ++
  1 file changed, 51 insertions(+)
  create mode 100644 
tests/spec/arb_separate_shader_objects/execution/layout-location-struct.shader_test

diff --git 
a/tests/spec/arb_separate_shader_objects/execution/layout-location-struct.shader_test
 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-struct.shader_test
new file mode 100644
index 000..b6b3d1c
--- /dev/null
+++ 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-struct.shader_test
@@ -0,0 +1,51 @@
+// Test that inputs and outputs are assigned the correct location when using
+// a struct and explicit locations.
+
+[require]
+GLSL >= 1.50
+GL_ARB_separate_shader_objects
+
+[vertex shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+in vec4 piglit_vertex;
+
+layout(location = 0) out struct S {
+   vec4 a;
+} s;
+
+layout(location = 2) out struct S2 {
+   vec4 a;
+} s2;
+
+void main()
+{
+   s.a = vec4(1.0, 0.0, 0.0, 1.0);
+   s2.a = vec4(0.0, 1.0, 1.0, 1.0);
+
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+layout(location = 2) in struct S {
+   vec4 a;
+} s;
+
+layout(location = 0) in struct S1 {
+   vec4 a;
+} s2;
+
+out vec4 color;
+
+void main()
+{
+   color = s2.a;
+}
+
+[test]
+draw rect -1 -1 2 2
+probe all rgb 1 0 0


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


Re: [Piglit] [PATCH] arb_separate_shader_objects: test explicit location on interface blocks

2015-11-30 Thread Tapani Pälli

Reviewed-by: Tapani Pälli 

--- 8< ---

BTW these most likely won't pass on AMD since I've noticed that if you 
use SSO and builtin gl_Position, you'll you will need to redeclare 
built-in interface blocks like this:


out gl_PerVertex { vec4 gl_Position; };

I believe this is a bug in AMD drivers since this redeclaration rule 
should only apply on glsl version 140, not for 150 and above.




On 11/30/2015 08:22 AM, Timothy Arceri wrote:

Test results:
Nvidia GeForce 840M - NVIDIA 352.41: pass
i965 - Mesa 11.2-dev: fail
---
  .../execution/layout-location-block.shader_test| 51 ++
  .../layout-location-named-block.shader_test| 51 ++
  2 files changed, 102 insertions(+)
  create mode 100644 
tests/spec/arb_separate_shader_objects/execution/layout-location-block.shader_test
  create mode 100644 
tests/spec/arb_separate_shader_objects/execution/layout-location-named-block.shader_test

diff --git 
a/tests/spec/arb_separate_shader_objects/execution/layout-location-block.shader_test
 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-block.shader_test
new file mode 100644
index 000..76b4cd1
--- /dev/null
+++ 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-block.shader_test
@@ -0,0 +1,51 @@
+// Test that inputs and outputs are assigned the correct location when using
+// interface blocks and explicit locations.
+
+[require]
+GLSL >= 1.50
+GL_ARB_separate_shader_objects
+
+[vertex shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+in vec4 piglit_vertex;
+
+layout(location = 0) out block {
+   vec4 a;
+};
+
+layout(location = 2) out block2 {
+   vec4 b;
+};
+
+void main()
+{
+   a = vec4(1.0, 0.0, 0.0, 1.0);
+   b = vec4(0.0, 1.0, 1.0, 1.0);
+
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+layout(location = 2) in block {
+   vec4 a;
+};
+
+layout(location = 0) in block2 {
+   vec4 b;
+};
+
+out vec4 color;
+
+void main()
+{
+   color = b;
+}
+
+[test]
+draw rect -1 -1 2 2
+probe all rgb 1 0 0
diff --git 
a/tests/spec/arb_separate_shader_objects/execution/layout-location-named-block.shader_test
 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-named-block.shader_test
new file mode 100644
index 000..51bf7c5
--- /dev/null
+++ 
b/tests/spec/arb_separate_shader_objects/execution/layout-location-named-block.shader_test
@@ -0,0 +1,51 @@
+// Test that inputs and outputs are assigned the correct location when using
+// interface blocks and explicit locations.
+
+[require]
+GLSL >= 1.50
+GL_ARB_separate_shader_objects
+
+[vertex shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+in vec4 piglit_vertex;
+
+layout(location = 0) out block {
+   vec4 a;
+} name;
+
+layout(location = 2) out block2 {
+   vec4 a;
+} name2;
+
+void main()
+{
+   name.a = vec4(0.0, 1.0, 1.0, 1.0);
+   name2.a = vec4(1.0, 0.0, 0.0, 1.0);
+
+   gl_Position = piglit_vertex;
+}
+
+[fragment shader]
+#version 150
+#extension GL_ARB_separate_shader_objects: require
+
+layout(location = 2) in block {
+   vec4 a;
+} name;
+
+layout(location = 0) in block2 {
+   vec4 a;
+} name2;
+
+out vec4 color;
+
+void main()
+{
+   color = name.a;
+}
+
+[test]
+draw rect -1 -1 2 2
+probe all rgb 1 0 0


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