[Piglit] [PATCH] arb_copy_image: fix a few error check tests
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
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
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
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
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: > +