Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-04-11 Thread Juan A. Suarez Romero
Pushed!


On Wed, 2018-04-11 at 09:45 -0700, Anthony Pesch wrote:
> From: Anthony Pesch 
> 
> Change expected error from INVALID_OPERATION to INVALID_VALUE when querying a
> level which hasn't been explicitly defined. The level is valid, however, the
> level hasn't been explicitly defined so it should have a default width and
> height of 0, making the 8x8 query produce an INVALID_VALUE.
> 
> From the OpenGL 4.6 spec, 8.22 Texture State and Proxy State:
> "Each initial texture image is null. It has zero width, height, and depth,
> internal format RGBA, or R8 for buffer textures, component sizes set to zero 
> and
> component types set to NONE, the compressed flag set to FALSE, a zero 
> compressed
> size, and the bound buffer object name is zero."
> 
> From the GetTextureSubImage errors in 8.11.4:
> "An INVALID_VALUE error is generated if xoffset + width is greater than the
> texture’s width, yoffset + height is greater than the texture’s height, or
> zoffset + depth is greater than the texture’s depth."
> 
> Reviewed-by: Arthur Huillet 
> Reviewed-by: Juan A. Suarez 
> ---
>  tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
> b/tests/spec/arb_get_texture_sub_image/errors.c
> index 34fec4a95..57875fa6a 100644
> --- a/tests/spec/arb_get_texture_sub_image/errors.c
> +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> @@ -200,7 +200,7 @@ test_invalid_values(void)
>8, 8, 1, /* size */
>GL_RGBA, GL_FLOAT,  /* bad enum */
>sizeof(buffer), buffer);
> - if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> + if (!piglit_check_gl_error(GL_INVALID_VALUE))
>   pass = false;
>  
>   /* Test getting invalid offset */
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-04-08 Thread Anthony Pesch
Hey Juan,

I've updated the patch to include the OpenGL spec references:

Author: Anthony Pesch <ape...@nvidia.com>
Date:   Thu Mar 22 11:06:13 2018 -0400

arb_get_texture_sub_image: fix expected error when querying a level which 
hasn't
been explicitly defined

Change expected error from INVALID_OPERATION to INVALID_VALUE when querying 
a
level which hasn't been explicitly defined. The level is valid, however, the
level hasn't been explicitly defined so it should have a default width and
height of 0, making the 8x8 query produce an INVALID_VALUE.

From the OpenGL 4.6 spec, 8.22 Texture State and Proxy State:
"Each initial texture image is null. It has zero width, height, and depth,
internal format RGBA, or R8 for buffer textures, component sizes set to 
zero and
component types set to NONE, the compressed flag set to FALSE, a zero 
compressed
size, and the bound buffer object name is zero."

From the GetTextureSubImage errors in 8.11.4:
"An INVALID_VALUE error is generated if xoffset + width is greater than the
texture’s width, yoffset + height is greater than the texture’s height, or
zoffset + depth is greater than the texture’s depth."

diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
b/tests/spec/arb_get_texture_sub_image/errors.c
index 34fec4a95..57875fa6a 100644
--- a/tests/spec/arb_get_texture_sub_image/errors.c
+++ b/tests/spec/arb_get_texture_sub_image/errors.c
@@ -200,7 +200,7 @@ test_invalid_values(void)
 8, 8, 1, /* size */
 GL_RGBA, GL_FLOAT,  /* bad enum */
 sizeof(buffer), buffer);
-   if (!piglit_check_gl_error(GL_INVALID_OPERATION))
+   if (!piglit_check_gl_error(GL_INVALID_VALUE))
pass = false;
 
/* Test getting invalid offset */

 - Anthony


From: Juan A. Suarez Romero <jasua...@igalia.com>
Sent: Wednesday, April 4, 2018 1:17 PM
To: Anthony Pesch; Anthony Pesch; piglit@lists.freedesktop.org
Subject: Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error 
when querying a level which hasn't been explicitly defined

On Wed, 2018-04-04 at 15:04 +, Anthony Pesch wrote:
> Hey Juan,
>
> The change from INVALID_OPERATION to INVALID_VALUE isn't because the level 
> value is invalid.
>
> The level is valid, however, the level hasn't been explicitly defined so it 
> should have a default width and height of 0, making the 8x8 query invalid.
>
> From the OpenGL 4.6 spec, 8.22 Texture State and Proxy State:
> "Each initial texture image is null. It has zero width, height, and depth,  
> internal format RGBA, or R8 for buffer textures, component sizes set to zero 
> and component types set to NONE, the compressed flag set to FALSE, a zero 
> compressed size, and the bound buffer object name is zero."
>
> From the GetTextureSubImage errors in 8.11.4:
> "An INVALID_VALUE error is generated if xoffset + width is greater than the  
> texture’s  width, yoffset + height is  greater  than  the  texture’s  height, 
>  or zoffset + depth is greater than the texture’s depth."
>


Thanks for the info!

I think your assumption is correct: all non-defined levels have an empty
initialized empty.

Do you mind to include such OpenGL reference in the commit message?

With that,

Reviewed-by: Juan A. Suarez <jasua...@igalia.com>

>  - Anthony
>
> 
> From: Piglit <piglit-boun...@lists.freedesktop.org> on behalf of Juan A. 
> Suarez Romero <jasua...@igalia.com>
> Sent: Wednesday, April 4, 2018 7:12 AM
> To: Anthony Pesch; piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected 
> error when querying a level which hasn't been explicitly defined
>
> On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> > From: Anthony Pesch <ape...@nvidia.com>
> >
> > Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
> > a level which hasn't been explicitly defined. This is a valid operation, the
> > error set should be due to the requested width and height being greater than
> > the default width and height of zero.
>
> I have some doubts with this patch. I've been checking OpenGL 4.5 spec, 
> section
> 8.11.4 Texture Image Queries, and this is what I found:
>
> "An INVALID_VALUE error is generated if level is negative or larger than the
> maximum allowable level."
>
>
> In this case, level is 4, so we aren't in this case, as the level is not
> negative, and it is smaller than the maximum allowable level (15, the
> implementation I use).
>
>
> So really don't know what should be the expected error,

Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-04-04 Thread Juan A. Suarez Romero
On Wed, 2018-04-04 at 15:04 +, Anthony Pesch wrote:
> Hey Juan,
> 
> The change from INVALID_OPERATION to INVALID_VALUE isn't because the level 
> value is invalid.
> 
> The level is valid, however, the level hasn't been explicitly defined so it 
> should have a default width and height of 0, making the 8x8 query invalid.
> 
> From the OpenGL 4.6 spec, 8.22 Texture State and Proxy State:
> "Each initial texture image is null. It has zero width, height, and depth,  
> internal format RGBA, or R8 for buffer textures, component sizes set to zero 
> and component types set to NONE, the compressed flag set to FALSE, a zero 
> compressed size, and the bound buffer object name is zero."
> 
> From the GetTextureSubImage errors in 8.11.4:
> "An INVALID_VALUE error is generated if xoffset + width is greater than the  
> texture’s  width, yoffset + height is  greater  than  the  texture’s  height, 
>  or zoffset + depth is greater than the texture’s depth."
> 


Thanks for the info!

I think your assumption is correct: all non-defined levels have an empty
initialized empty.

Do you mind to include such OpenGL reference in the commit message?

With that,

Reviewed-by: Juan A. Suarez <jasua...@igalia.com>

>  - Anthony
> 
> 
> From: Piglit <piglit-boun...@lists.freedesktop.org> on behalf of Juan A. 
> Suarez Romero <jasua...@igalia.com>
> Sent: Wednesday, April 4, 2018 7:12 AM
> To: Anthony Pesch; piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected 
> error when querying a level which hasn't been explicitly defined
> 
> On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> > From: Anthony Pesch <ape...@nvidia.com>
> > 
> > Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
> > a level which hasn't been explicitly defined. This is a valid operation, the
> > error set should be due to the requested width and height being greater than
> > the default width and height of zero.
> 
> I have some doubts with this patch. I've been checking OpenGL 4.5 spec, 
> section
> 8.11.4 Texture Image Queries, and this is what I found:
> 
> "An INVALID_VALUE error is generated if level is negative or larger than the
> maximum allowable level."
> 
> 
> In this case, level is 4, so we aren't in this case, as the level is not
> negative, and it is smaller than the maximum allowable level (15, the
> implementation I use).
> 
> 
> So really don't know what should be the expected error, INVALID_OPERATION or
> INVALID_VALUE.
> 
> 
> J.A.
> 
> 
> 
> >  tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
> > b/tests/spec/arb_get_texture_sub_image/errors.c
> > index 34fec4a95..57875fa6a 100644
> > --- a/tests/spec/arb_get_texture_sub_image/errors.c
> > +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> > @@ -200,7 +200,7 @@ test_invalid_values(void)
> >8, 8, 1, /* size */
> >GL_RGBA, GL_FLOAT,  /* bad enum */
> >sizeof(buffer), buffer);
> > - if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> > + if (!piglit_check_gl_error(GL_INVALID_VALUE))
> >   pass = false;
> > 
> >   /* Test getting invalid offset */
> 
> ___
> Piglit mailing list
> Piglit@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/piglit
> 
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-04-04 Thread Anthony Pesch
Hey Juan,

The change from INVALID_OPERATION to INVALID_VALUE isn't because the level 
value is invalid.

The level is valid, however, the level hasn't been explicitly defined so it 
should have a default width and height of 0, making the 8x8 query invalid.

From the OpenGL 4.6 spec, 8.22 Texture State and Proxy State:
"Each initial texture image is null. It has zero width, height, and depth,  
internal format RGBA, or R8 for buffer textures, component sizes set to zero 
and component types set to NONE, the compressed flag set to FALSE, a zero 
compressed size, and the bound buffer object name is zero."

From the GetTextureSubImage errors in 8.11.4:
"An INVALID_VALUE error is generated if xoffset + width is greater than the  
texture’s  width, yoffset + height is  greater  than  the  texture’s  height,  
or zoffset + depth is greater than the texture’s depth."

 - Anthony


From: Piglit <piglit-boun...@lists.freedesktop.org> on behalf of Juan A. Suarez 
Romero <jasua...@igalia.com>
Sent: Wednesday, April 4, 2018 7:12 AM
To: Anthony Pesch; piglit@lists.freedesktop.org
Subject: Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error 
when querying a level which hasn't been explicitly defined

On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> From: Anthony Pesch <ape...@nvidia.com>
>
> Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
> a level which hasn't been explicitly defined. This is a valid operation, the
> error set should be due to the requested width and height being greater than
> the default width and height of zero.

I have some doubts with this patch. I've been checking OpenGL 4.5 spec, section
8.11.4 Texture Image Queries, and this is what I found:

"An INVALID_VALUE error is generated if level is negative or larger than the
maximum allowable level."


In this case, level is 4, so we aren't in this case, as the level is not
negative, and it is smaller than the maximum allowable level (15, the
implementation I use).


So really don't know what should be the expected error, INVALID_OPERATION or
INVALID_VALUE.


J.A.



>  tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
> b/tests/spec/arb_get_texture_sub_image/errors.c
> index 34fec4a95..57875fa6a 100644
> --- a/tests/spec/arb_get_texture_sub_image/errors.c
> +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> @@ -200,7 +200,7 @@ test_invalid_values(void)
>8, 8, 1, /* size */
>GL_RGBA, GL_FLOAT,  /* bad enum */
>sizeof(buffer), buffer);
> - if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> + if (!piglit_check_gl_error(GL_INVALID_VALUE))
>   pass = false;
>
>   /* Test getting invalid offset */
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-04-04 Thread Juan A. Suarez Romero
On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> From: Anthony Pesch 
> 
> Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
> a level which hasn't been explicitly defined. This is a valid operation, the
> error set should be due to the requested width and height being greater than
> the default width and height of zero.

I have some doubts with this patch. I've been checking OpenGL 4.5 spec, section
8.11.4 Texture Image Queries, and this is what I found:

"An INVALID_VALUE error is generated if level is negative or larger than the
maximum allowable level."


In this case, level is 4, so we aren't in this case, as the level is not
negative, and it is smaller than the maximum allowable level (15, the
implementation I use).


So really don't know what should be the expected error, INVALID_OPERATION or
INVALID_VALUE.


J.A.



>  tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
> b/tests/spec/arb_get_texture_sub_image/errors.c
> index 34fec4a95..57875fa6a 100644
> --- a/tests/spec/arb_get_texture_sub_image/errors.c
> +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> @@ -200,7 +200,7 @@ test_invalid_values(void)
>8, 8, 1, /* size */
>GL_RGBA, GL_FLOAT,  /* bad enum */
>sizeof(buffer), buffer);
> - if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> + if (!piglit_check_gl_error(GL_INVALID_VALUE))
>   pass = false;
>  
>   /* Test getting invalid offset */
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-03-28 Thread Anthony Pesch
I think the bad enum is some copy and paste from an above test case. I feel
it should be changed as well, right now the test is relying on the level /
size being validated before the format.

 - Anthony

P.S. Sorry for the spam on these changes, I had a problem with my filters
and thought the changes weren't being sent, but well... they were.

On Wed, Mar 28, 2018 at 11:41 AM, Arthur Huillet 
wrote:

> I'm confused why the test is passing a bad enum when its purpose seems to
> be to test a non-existent level.
> In any case, your change looks correct to me.
>
> Reviewed-by: Arthur Huillet 
>
>
> On 28.03.2018 17:15, Anthony Pesch wrote:
>
>> From: Anthony Pesch 
>>
>> Change expected error from INVALID_OPERATION to INVALID_VALUE when
>> querying
>> a level which hasn't been explicitly defined. This is a valid operation,
>> the
>> error set should be due to the requested width and height being greater
>> than
>> the default width and height of zero.
>> ---
>>  tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/spec/arb_get_texture_sub_image/errors.c
>> b/tests/spec/arb_get_texture_sub_image/errors.c
>> index 34fec4a95..57875fa6a 100644
>> --- a/tests/spec/arb_get_texture_sub_image/errors.c
>> +++ b/tests/spec/arb_get_texture_sub_image/errors.c
>> @@ -200,7 +200,7 @@ test_invalid_values(void)
>>  8, 8, 1, /* size */
>>  GL_RGBA, GL_FLOAT,  /* bad enum */
>>  sizeof(buffer), buffer);
>> -   if (!piglit_check_gl_error(GL_INVALID_OPERATION))
>> +   if (!piglit_check_gl_error(GL_INVALID_VALUE))
>> pass = false;
>>
>> /* Test getting invalid offset */
>>
>
> --
> A. Huillet
>
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit


Re: [Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-03-28 Thread Arthur Huillet
I'm confused why the test is passing a bad enum when its purpose seems 
to be to test a non-existent level.

In any case, your change looks correct to me.

Reviewed-by: Arthur Huillet 


On 28.03.2018 17:15, Anthony Pesch wrote:

From: Anthony Pesch 

Change expected error from INVALID_OPERATION to INVALID_VALUE when 
querying
a level which hasn't been explicitly defined. This is a valid 
operation, the
error set should be due to the requested width and height being greater 
than

the default width and height of zero.
---
 tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/spec/arb_get_texture_sub_image/errors.c
b/tests/spec/arb_get_texture_sub_image/errors.c
index 34fec4a95..57875fa6a 100644
--- a/tests/spec/arb_get_texture_sub_image/errors.c
+++ b/tests/spec/arb_get_texture_sub_image/errors.c
@@ -200,7 +200,7 @@ test_invalid_values(void)
 8, 8, 1, /* size */
 GL_RGBA, GL_FLOAT,  /* bad enum */
 sizeof(buffer), buffer);
-   if (!piglit_check_gl_error(GL_INVALID_OPERATION))
+   if (!piglit_check_gl_error(GL_INVALID_VALUE))
pass = false;

/* Test getting invalid offset */


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


[Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-03-28 Thread Anthony Pesch
From: Anthony Pesch 

Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
a level which hasn't been explicitly defined. This is a valid operation, the
error set should be due to the requested width and height being greater than
the default width and height of zero.
---
 tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
b/tests/spec/arb_get_texture_sub_image/errors.c
index 34fec4a95..57875fa6a 100644
--- a/tests/spec/arb_get_texture_sub_image/errors.c
+++ b/tests/spec/arb_get_texture_sub_image/errors.c
@@ -200,7 +200,7 @@ test_invalid_values(void)
 8, 8, 1, /* size */
 GL_RGBA, GL_FLOAT,  /* bad enum */
 sizeof(buffer), buffer);
-   if (!piglit_check_gl_error(GL_INVALID_OPERATION))
+   if (!piglit_check_gl_error(GL_INVALID_VALUE))
pass = false;
 
/* Test getting invalid offset */
-- 
2.13.6

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


[Piglit] [PATCH 1/3] arb_get_texture_sub_image: fix expected error when querying a level which hasn't been explicitly defined

2018-03-27 Thread inolen
From: Anthony Pesch 

Change expected error from INVALID_OPERATION to INVALID_VALUE when querying
a level which hasn't been explicitly defined. This is a valid operation, the
error set should be due to the requested width and height being greater than
the default width and height of zero.
---
 tests/spec/arb_get_texture_sub_image/errors.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/spec/arb_get_texture_sub_image/errors.c 
b/tests/spec/arb_get_texture_sub_image/errors.c
index 34fec4a95..57875fa6a 100644
--- a/tests/spec/arb_get_texture_sub_image/errors.c
+++ b/tests/spec/arb_get_texture_sub_image/errors.c
@@ -200,7 +200,7 @@ test_invalid_values(void)
 8, 8, 1, /* size */
 GL_RGBA, GL_FLOAT,  /* bad enum */
 sizeof(buffer), buffer);
-   if (!piglit_check_gl_error(GL_INVALID_OPERATION))
+   if (!piglit_check_gl_error(GL_INVALID_VALUE))
pass = false;
 
/* Test getting invalid offset */
-- 
2.13.6

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