[Piglit] [PATCH] arb_copy_image: fix a few error check tests

2015-08-31 Thread Brian Paul
Some of the error checks were incorrect before.  Per the spec:

1. GL_TEXTURE_BUFFER and GL_TEXTURE_CUBE_MAP_+/-_XYZ are not legal targets
and should be flagged as invalid enums.

2. GL_INVALID_OPERATION should be generated when trying to copy between
compressed/uncompressed formats whose block/texel size do not match.
---
 tests/spec/arb_copy_image/api_errors.c | 23 +++
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/spec/arb_copy_image/api_errors.c 
b/tests/spec/arb_copy_image/api_errors.c
index 0ef1eda..6f94d06 100644
--- a/tests/spec/arb_copy_image/api_errors.c
+++ b/tests/spec/arb_copy_image/api_errors.c
@@ -140,7 +140,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
glCopyImageSubData(src, targets[i], 0, 0, 0, 0,
   dst, dst_target, 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+if (targets[i] == GL_TEXTURE_BUFFER ||
+   (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
+targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
+   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   }
+   else {
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   }
if (!pass)
return false;
}
@@ -154,7 +161,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
glCopyImageSubData(src, src_target, 0, 0, 0, 0,
   dst, targets[i], 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+if (targets[i] == GL_TEXTURE_BUFFER ||
+   (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
+targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
+   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   }
+   else {
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   }
if (!pass)
return false;
}
@@ -235,14 +249,15 @@ test_compressed_alignment_errors()
glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGB16UI, 32, 32);
glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
   tex[2], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
-   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   pass &= piglit_check_gl_error(GL_INVALID_OPERATION);
 
+/* Check for invalid copy between different compressed formats */
glBindTexture(GL_TEXTURE_2D, tex[3]);
glTexStorage2D(GL_TEXTURE_2D, 1,
   GL_COMPRESSED_RGBA_S3TC_DXT1_EXT, 32, 32);
glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
   tex[3], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
-   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   pass &= piglit_check_gl_error(GL_INVALID_OPERATION);
 
glDeleteTextures(4, tex);
 
-- 
1.9.1

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


[Piglit] [PATCH] arb_copy_image: fix a few error check tests

2015-09-01 Thread Brian Paul
Some of the error checks were incorrect before.  Per the spec:

1. GL_TEXTURE_BUFFER and GL_TEXTURE_CUBE_MAP_+/-_XYZ are not legal targets
and should be flagged as invalid enums.

2. GL_INVALID_OPERATION should be generated when trying to copy between
compressed/uncompressed formats whose block/texel size do not match.

v2: Also quote spec language for the tests being done, per Ian.
And, remove testing invalid target enums.
---
 tests/spec/arb_copy_image/api_errors.c | 59 ++
 1 file changed, 45 insertions(+), 14 deletions(-)

diff --git a/tests/spec/arb_copy_image/api_errors.c 
b/tests/spec/arb_copy_image/api_errors.c
index 0ef1eda..00c97a5 100644
--- a/tests/spec/arb_copy_image/api_errors.c
+++ b/tests/spec/arb_copy_image/api_errors.c
@@ -78,22 +78,16 @@ image_storage(GLenum target, GLuint name, GLenum 
internal_format,
}
 }
 
-GLenum targets[] = {
+/* only testing legal targets below */
+static const GLenum targets[] = {
GL_TEXTURE_1D,
GL_TEXTURE_1D_ARRAY,
GL_TEXTURE_2D,
GL_TEXTURE_RECTANGLE,
-   GL_TEXTURE_BUFFER,
GL_TEXTURE_2D_ARRAY,
GL_TEXTURE_2D_MULTISAMPLE,
GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
GL_TEXTURE_CUBE_MAP,
-   GL_TEXTURE_CUBE_MAP_POSITIVE_X,
-   GL_TEXTURE_CUBE_MAP_NEGATIVE_X,
-   GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
-   GL_TEXTURE_CUBE_MAP_NEGATIVE_Y,
-   GL_TEXTURE_CUBE_MAP_POSITIVE_Z,
-   GL_TEXTURE_CUBE_MAP_NEGATIVE_Z,
GL_TEXTURE_CUBE_MAP_ARRAY,
GL_TEXTURE_3D
 };
@@ -132,29 +126,45 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
/* This is no longer needed */
image_delete(src_target, src2);
 
+   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
+* Profile spec says:
+*
+* "An INVALID_VALUE error is generated if either name does not
+* correspond to a valid renderbuffer or texture object according
+* to the corresponding target parameter."
+*/
if (src_target != GL_RENDERBUFFER_EXT) {
for (i = 0; i < ARRAY_LENGTH(targets); ++i) {
if (targets[i] == src_target)
continue;
 
+   /* here, targets[i] doesn't match src object's target */
glCopyImageSubData(src, targets[i], 0, 0, 0, 0,
   dst, dst_target, 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
if (!pass)
return false;
}
}
 
+   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
+* Profile spec says:
+*
+* "An INVALID_VALUE error is generated if either name does not
+* correspond to a valid renderbuffer or texture object according
+* to the corresponding target parameter."
+*/
if (dst_target != GL_RENDERBUFFER_EXT) {
for (i = 0; i < ARRAY_LENGTH(targets); ++i) {
if (targets[i] == dst_target)
continue;
 
+   /* here, targets[i] doesn't match dst object's target */
glCopyImageSubData(src, src_target, 0, 0, 0, 0,
   dst, targets[i], 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
if (!pass)
return false;
}
@@ -230,19 +240,38 @@ test_compressed_alignment_errors()
   tex[1], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 24, 1);
pass &= piglit_check_gl_error(GL_INVALID_VALUE);
 
-   /* Check for compressed with wrong block size */
+   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
+* Profile spec says:
+*
+* "An INVALID_OPERATION error is generated if the texel size of
+* the uncompressed image is not equal to the block size of the
+* compressed image."
+*/
glBindTexture(GL_TEXTURE_2D, tex[2]);
glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGB16UI, 32, 32);
glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
   tex[2], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
-   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   pass &= piglit_check_gl_error(GL_INVALID_OPERATION);
 
+   /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
+* Profile spec says:
+*
+* "An INVALID_OPERATION error is generated if the formats are
+ * not compatible."
+

Re: [Piglit] [PATCH] arb_copy_image: fix a few error check tests

2015-09-01 Thread Ian Romanick
On 08/31/2015 04:22 PM, Brian Paul wrote:
> Some of the error checks were incorrect before.  Per the spec:
> 
> 1. GL_TEXTURE_BUFFER and GL_TEXTURE_CUBE_MAP_+/-_XYZ are not legal targets
> and should be flagged as invalid enums.

I think we should not check these targets.  Checking for either error
assumes that the implementation checks for the (multiple) error
conditions in a particular order.

> 2. GL_INVALID_OPERATION should be generated when trying to copy between
> compressed/uncompressed formats whose block/texel size do not match.
> ---
>  tests/spec/arb_copy_image/api_errors.c | 23 +++
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/spec/arb_copy_image/api_errors.c 
> b/tests/spec/arb_copy_image/api_errors.c
> index 0ef1eda..6f94d06 100644
> --- a/tests/spec/arb_copy_image/api_errors.c
> +++ b/tests/spec/arb_copy_image/api_errors.c
> @@ -140,7 +140,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
>   glCopyImageSubData(src, targets[i], 0, 0, 0, 0,
>  dst, dst_target, 0, 0, 0, 0,
>  0, 0, 0);
> - pass &= piglit_check_gl_error(GL_INVALID_ENUM);

I'd support a follow-up patch that changes this test to follow the
piglit idiom of

pass = test() && pass;

> +if (targets[i] == GL_TEXTURE_BUFFER ||
> + (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
> +  targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
> + pass &= piglit_check_gl_error(GL_INVALID_ENUM);
> + }
> + else {
> + pass &= piglit_check_gl_error(GL_INVALID_VALUE);
> + }
>   if (!pass)
>   return false;
>   }
> @@ -154,7 +161,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
>   glCopyImageSubData(src, src_target, 0, 0, 0, 0,
>  dst, targets[i], 0, 0, 0, 0,
>  0, 0, 0);
> - pass &= piglit_check_gl_error(GL_INVALID_ENUM);
> +if (targets[i] == GL_TEXTURE_BUFFER ||
> + (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
> +  targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
> + pass &= piglit_check_gl_error(GL_INVALID_ENUM);
> + }
> + else {
> + pass &= piglit_check_gl_error(GL_INVALID_VALUE);
> + }
>   if (!pass)
>   return false;
>   }
> @@ -235,14 +249,15 @@ test_compressed_alignment_errors()

Ugh.  This test is woefully lacking in spec quotations. :(  If you
replace the comment above this line with:

/* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
 * Profile spec says:
 *
 * "An INVALID_OPERATION error is generated if the texel size of
 * the uncompressed image is not equal to the block size of the
 * compressed image."
 */

>   glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGB16UI, 32, 32);
>   glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
>  tex[2], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
> - pass &= piglit_check_gl_error(GL_INVALID_VALUE);
> + pass &= piglit_check_gl_error(GL_INVALID_OPERATION);
>  
> +/* Check for invalid copy between different compressed formats */

and replace this comment with

/* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
 * Profile spec says:
 *
 * "An INVALID_OPERATION error is generated if the formats are
 * not compatible."
 *
 * The definition of compatible refers back to table 8.22 "Compatible
 * internal formats for TextureView."  This table does not contain
 * S3TC formats because they are from an older extension.  Given the
 * different encodings of DXT1 and DXT3 textures, it is reasonable to
 * assume they would not be compatible for texture views, and this
 * matches at least NVIDIA's implementation.
 */

then the last two hunks are

Signed-off-by: Ian Romanick 
Reviewed-by: Ian Romanick 

With the amount of effort that justification required, I almost wonder if we
should change the test to use RGTC formats (which are in table 8.22 instead).

It also looks like this test needs to check that one of the S3TC extensions
is supported. :(  I can submit a patch for that... assuming you don't want
to change the test to use RGTC instead.

>   glBindTexture(GL_TEXTURE_2D, tex[3]);
>   glTexStorage2D(GL_TEXTURE_2D, 1,
>  GL_COMPRESSED_RG

Re: [Piglit] [PATCH] arb_copy_image: fix a few error check tests

2015-09-01 Thread Brian Paul

On 09/01/2015 02:33 PM, Ian Romanick wrote:

On 08/31/2015 04:22 PM, Brian Paul wrote:

Some of the error checks were incorrect before.  Per the spec:

1. GL_TEXTURE_BUFFER and GL_TEXTURE_CUBE_MAP_+/-_XYZ are not legal targets
and should be flagged as invalid enums.


I think we should not check these targets.  Checking for either error
assumes that the implementation checks for the (multiple) error
conditions in a particular order.


2. GL_INVALID_OPERATION should be generated when trying to copy between
compressed/uncompressed formats whose block/texel size do not match.
---
  tests/spec/arb_copy_image/api_errors.c | 23 +++
  1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/tests/spec/arb_copy_image/api_errors.c 
b/tests/spec/arb_copy_image/api_errors.c
index 0ef1eda..6f94d06 100644
--- a/tests/spec/arb_copy_image/api_errors.c
+++ b/tests/spec/arb_copy_image/api_errors.c
@@ -140,7 +140,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
glCopyImageSubData(src, targets[i], 0, 0, 0, 0,
   dst, dst_target, 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);


I'd support a follow-up patch that changes this test to follow the
piglit idiom of

pass = test() && pass;


+if (targets[i] == GL_TEXTURE_BUFFER ||
+   (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
+targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
+   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   }
+   else {
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   }
if (!pass)
return false;
}
@@ -154,7 +161,14 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
glCopyImageSubData(src, src_target, 0, 0, 0, 0,
   dst, targets[i], 0, 0, 0, 0,
   0, 0, 0);
-   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+if (targets[i] == GL_TEXTURE_BUFFER ||
+   (targets[i] >= GL_TEXTURE_CUBE_MAP_POSITIVE_X &&
+targets[i] <= GL_TEXTURE_CUBE_MAP_NEGATIVE_Z)) {
+   pass &= piglit_check_gl_error(GL_INVALID_ENUM);
+   }
+   else {
+   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   }
if (!pass)
return false;
}
@@ -235,14 +249,15 @@ test_compressed_alignment_errors()


Ugh.  This test is woefully lacking in spec quotations. :(  If you
replace the comment above this line with:

/* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
 * Profile spec says:
 *
 * "An INVALID_OPERATION error is generated if the texel size of
 * the uncompressed image is not equal to the block size of the
 * compressed image."
 */


glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGB16UI, 32, 32);
glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
   tex[2], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
-   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
+   pass &= piglit_check_gl_error(GL_INVALID_OPERATION);

+/* Check for invalid copy between different compressed formats */


and replace this comment with

/* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
 * Profile spec says:
 *
 * "An INVALID_OPERATION error is generated if the formats are
  * not compatible."
  *
  * The definition of compatible refers back to table 8.22 "Compatible
 * internal formats for TextureView."  This table does not contain
 * S3TC formats because they are from an older extension.  Given the
 * different encodings of DXT1 and DXT3 textures, it is reasonable to
 * assume they would not be compatible for texture views, and this
 * matches at least NVIDIA's implementation.
 */

then the last two hunks are

Signed-off-by: Ian Romanick 
Reviewed-by: Ian Romanick 

With the amount of effort that justification required, I almost wonder if we
should change the test to use RGTC formats (which are in table 8.22 instead).

It also looks like this test needs to check that one of the S3TC extensions
is supported. :(  I can submit a patch for that... assuming you don't want
to change the test to use RGTC instead.


glBindTexture(GL_TEXTURE_2D, tex[3]);
glTexStorage2D(GL_TEXTURE_2D, 1,
   G

Re: [Piglit] [PATCH] arb_copy_image: fix a few error check tests

2015-09-02 Thread Ian Romanick
On 09/01/2015 02:56 PM, Brian Paul wrote:
> Some of the error checks were incorrect before.  Per the spec:
> 
> 1. GL_TEXTURE_BUFFER and GL_TEXTURE_CUBE_MAP_+/-_XYZ are not legal targets
> and should be flagged as invalid enums.
> 
> 2. GL_INVALID_OPERATION should be generated when trying to copy between
> compressed/uncompressed formats whose block/texel size do not match.
> 
> v2: Also quote spec language for the tests being done, per Ian.
> And, remove testing invalid target enums.
> ---
>  tests/spec/arb_copy_image/api_errors.c | 59 
> ++
>  1 file changed, 45 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/spec/arb_copy_image/api_errors.c 
> b/tests/spec/arb_copy_image/api_errors.c
> index 0ef1eda..00c97a5 100644
> --- a/tests/spec/arb_copy_image/api_errors.c
> +++ b/tests/spec/arb_copy_image/api_errors.c
> @@ -78,22 +78,16 @@ image_storage(GLenum target, GLuint name, GLenum 
> internal_format,
>   }
>  }
>  
> -GLenum targets[] = {
> +/* only testing legal targets below */
> +static const GLenum targets[] = {
>   GL_TEXTURE_1D,
>   GL_TEXTURE_1D_ARRAY,
>   GL_TEXTURE_2D,
>   GL_TEXTURE_RECTANGLE,
> - GL_TEXTURE_BUFFER,
>   GL_TEXTURE_2D_ARRAY,
>   GL_TEXTURE_2D_MULTISAMPLE,
>   GL_TEXTURE_2D_MULTISAMPLE_ARRAY,
>   GL_TEXTURE_CUBE_MAP,
> - GL_TEXTURE_CUBE_MAP_POSITIVE_X,
> - GL_TEXTURE_CUBE_MAP_NEGATIVE_X,
> - GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
> - GL_TEXTURE_CUBE_MAP_NEGATIVE_Y,
> - GL_TEXTURE_CUBE_MAP_POSITIVE_Z,
> - GL_TEXTURE_CUBE_MAP_NEGATIVE_Z,
>   GL_TEXTURE_CUBE_MAP_ARRAY,
>   GL_TEXTURE_3D
>  };
> @@ -132,29 +126,45 @@ test_simple_errors(GLenum src_target, GLenum dst_target)
>   /* This is no longer needed */
>   image_delete(src_target, src2);
>  
> + /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
> +  * Profile spec says:
> +  *
> +  * "An INVALID_VALUE error is generated if either name does not
> +  * correspond to a valid renderbuffer or texture object according
> +  * to the corresponding target parameter."
> +  */
>   if (src_target != GL_RENDERBUFFER_EXT) {
>   for (i = 0; i < ARRAY_LENGTH(targets); ++i) {
>   if (targets[i] == src_target)
>   continue;
>  
> + /* here, targets[i] doesn't match src object's target */
>   glCopyImageSubData(src, targets[i], 0, 0, 0, 0,
>  dst, dst_target, 0, 0, 0, 0,
>  0, 0, 0);
> - pass &= piglit_check_gl_error(GL_INVALID_ENUM);
> + pass &= piglit_check_gl_error(GL_INVALID_VALUE);
>   if (!pass)
>   return false;
>   }
>   }
>  
> + /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
> +  * Profile spec says:
> +  *
> +  * "An INVALID_VALUE error is generated if either name does not
> +  * correspond to a valid renderbuffer or texture object according
> +  * to the corresponding target parameter."
> +  */
>   if (dst_target != GL_RENDERBUFFER_EXT) {
>   for (i = 0; i < ARRAY_LENGTH(targets); ++i) {
>   if (targets[i] == dst_target)
>   continue;
>  
> + /* here, targets[i] doesn't match dst object's target */
>   glCopyImageSubData(src, src_target, 0, 0, 0, 0,
>  dst, targets[i], 0, 0, 0, 0,
>  0, 0, 0);
> - pass &= piglit_check_gl_error(GL_INVALID_ENUM);
> + pass &= piglit_check_gl_error(GL_INVALID_VALUE);
>   if (!pass)
>   return false;
>   }
> @@ -230,19 +240,38 @@ test_compressed_alignment_errors()
>  tex[1], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 24, 1);
>   pass &= piglit_check_gl_error(GL_INVALID_VALUE);
>  
> - /* Check for compressed with wrong block size */
> + /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
> +  * Profile spec says:
> +  *
> +  * "An INVALID_OPERATION error is generated if the texel size of
> +  * the uncompressed image is not equal to the block size of the
> +  * compressed image."
> +  */
>   glBindTexture(GL_TEXTURE_2D, tex[2]);
>   glTexStorage2D(GL_TEXTURE_2D, 1, GL_RGB16UI, 32, 32);
>   glCopyImageSubData(tex[0], GL_TEXTURE_2D, 0, 0, 0, 0,
>  tex[2], GL_TEXTURE_2D, 0, 0, 0, 0, 20, 20, 1);
> - pass &= piglit_check_gl_error(GL_INVALID_VALUE);
> + pass &= piglit_check_gl_error(GL_INVALID_OPERATION);
>  
> + /* Section 18.3.2 (Copying Between Images) of the OpenGL 4.5 Core
> +  * Profile spec says:
> +