[3/3] arb_get_texture_sub_image: update cube map tests to complete the cube map

Submitted by Anthony Pesch on April 8, 2018, 6:35 p.m.

Details

Message ID 1523212511045.87974@nvidia.com
State New
Headers show
Series "Series without cover letter" ( rev: 3 ) in Piglit

Not browsing as part of any series.

Commit Message

Anthony Pesch April 8, 2018, 6:35 p.m.
Hey Juan,

Good catch - I'd only been focusing on the errors test.

I've updated the patch to also fix the errors in arb_get_texture_sub_image-get:

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

    arb_get_texture_sub_image: update cube map tests to make textures cube complete
    
    Update cube map tests to ensure cube map textures are cube complete before querying
    them. Querying a cube map which is not cube complete should set INVALID_OPERATION
    as per the OpenGL 4.6 Core spec:
    
    "An INVALID_OPERATION error is generated if the effective target is
    TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture object
    is not cube complete or cube array complete, respectively."



 - Anthony

Patch hide | download patch | download mbox

diff --git a/tests/spec/arb_get_texture_sub_image/errors.c b/tests/spec/arb_get_texture_sub_image/errors.c
index 1e7b17115..4b99d1cc2 100644
--- a/tests/spec/arb_get_texture_sub_image/errors.c
+++ b/tests/spec/arb_get_texture_sub_image/errors.c
@@ -253,16 +253,20 @@  test_cubemap_faces(void)
                         0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
         }
 
-        /* try to get all six cube faces, should fail */
+        /* try to query incomplete cube map, should fail */
         glGetTextureSubImage(tex, 0,
                              0, 0, 0,
-                             8, 8, 6,
+                             8, 8, 5,
                              GL_RGBA, GL_UNSIGNED_BYTE,
                              sizeof(results), results);
        if (!piglit_check_gl_error(GL_INVALID_OPERATION))
                pass = false;
 
-        /* try to get five cube faces, should pass */
+       /* upload final face */
+       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + 5,
+                    0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
+
+        /* try to query complete cube map, should now pass */
         glGetTextureSubImage(tex, 0,
                              0, 0, 0,
                              8, 8, 5,
diff --git a/tests/spec/arb_get_texture_sub_image/get.c b/tests/spec/arb_get_texture_sub_image/get.c
index 8aa4c92e1..fc8f9f8e1 100644
--- a/tests/spec/arb_get_texture_sub_image/get.c
+++ b/tests/spec/arb_get_texture_sub_image/get.c
@@ -157,12 +157,16 @@  test_getsubimage(GLenum target,
                                     GL_RGBA, GL_UNSIGNED_BYTE, texData);
                        break;
                case GL_TEXTURE_CUBE_MAP:
-                       /* only set +Y face */
-                       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
-                                    level, intFormat,
-                                    mip_width, mip_height, 0,
-                                    GL_RGBA, GL_UNSIGNED_BYTE,
-                                    texData);
+                       /* specify dimensions and format for all faces to make texture cube complete,
+                          but specify data for only the +Y face as it is the only one read back */
+                       for (i = 0; i < 6; i++) {
+                               GLubyte *faceData = i == 2 ? texData : NULL;
+                               glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i,
+                                            level, intFormat,
+                                            mip_width, mip_height, 0,
+                                            GL_RGBA, GL_UNSIGNED_BYTE,
+                                            faceData);
+                       }
                        break;
                }
        }

Comments

For the series,

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


On Sun, 2018-04-08 at 18:35 +0000, Anthony Pesch wrote:
> Hey Juan,
> 
> Good catch - I'd only been focusing on the errors test.
> 
> I've updated the patch to also fix the errors in arb_get_texture_sub_image-get:
> 
> Author: Anthony Pesch <apesch@nvidia.com>
> Date:   Thu Mar 22 13:11:22 2018 -0400
> 
>     arb_get_texture_sub_image: update cube map tests to make textures cube complete
>     
>     Update cube map tests to ensure cube map textures are cube complete before querying
>     them. Querying a cube map which is not cube complete should set INVALID_OPERATION
>     as per the OpenGL 4.6 Core spec:
>     
>     "An INVALID_OPERATION error is generated if the effective target is
>     TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture object
>     is not cube complete or cube array complete, respectively."
> 
> diff --git a/tests/spec/arb_get_texture_sub_image/errors.c b/tests/spec/arb_get_texture_sub_image/errors.c
> index 1e7b17115..4b99d1cc2 100644
> --- a/tests/spec/arb_get_texture_sub_image/errors.c
> +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> @@ -253,16 +253,20 @@ test_cubemap_faces(void)
>                          0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
>          }
>  
> -        /* try to get all six cube faces, should fail */
> +        /* try to query incomplete cube map, should fail */
>          glGetTextureSubImage(tex, 0,
>                               0, 0, 0,
> -                             8, 8, 6,
> +                             8, 8, 5,
>                               GL_RGBA, GL_UNSIGNED_BYTE,
>                               sizeof(results), results);
>         if (!piglit_check_gl_error(GL_INVALID_OPERATION))
>                 pass = false;
>  
> -        /* try to get five cube faces, should pass */
> +       /* upload final face */
> +       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + 5,
> +                    0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> +
> +        /* try to query complete cube map, should now pass */
>          glGetTextureSubImage(tex, 0,
>                               0, 0, 0,
>                               8, 8, 5,
> diff --git a/tests/spec/arb_get_texture_sub_image/get.c b/tests/spec/arb_get_texture_sub_image/get.c
> index 8aa4c92e1..fc8f9f8e1 100644
> --- a/tests/spec/arb_get_texture_sub_image/get.c
> +++ b/tests/spec/arb_get_texture_sub_image/get.c
> @@ -157,12 +157,16 @@ test_getsubimage(GLenum target,
>                                      GL_RGBA, GL_UNSIGNED_BYTE, texData);
>                         break;
>                 case GL_TEXTURE_CUBE_MAP:
> -                       /* only set +Y face */
> -                       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
> -                                    level, intFormat,
> -                                    mip_width, mip_height, 0,
> -                                    GL_RGBA, GL_UNSIGNED_BYTE,
> -                                    texData);
> +                       /* specify dimensions and format for all faces to make texture cube complete,
> +                          but specify data for only the +Y face as it is the only one read back */
> +                       for (i = 0; i < 6; i++) {
> +                               GLubyte *faceData = i == 2 ? texData : NULL;
> +                               glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i,
> +                                            level, intFormat,
> +                                            mip_width, mip_height, 0,
> +                                            GL_RGBA, GL_UNSIGNED_BYTE,
> +                                            faceData);
> +                       }
>                         break;
>                 }
>         }
> 
> 
>  - Anthony
> 
> ________________________________________
> From: Piglit <piglit-bounces@lists.freedesktop.org> on behalf of Juan A. Suarez Romero <jasuarez@igalia.com>
> Sent: Thursday, April 5, 2018 4:43 AM
> To: Anthony Pesch; piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH 3/3] arb_get_texture_sub_image: update cube map tests to complete the cube map
> 
> On Thu, 2018-04-05 at 10:19 +0200, Juan A. Suarez Romero wrote:
> > On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> > > From: Anthony Pesch <apesch@nvidia.com>
> > > 
> > > Update cube map tests to complete the cube map before performing the final
> > > query. This final query is expected to succeed, however, querying a cube map
> > > which is not cube complete should set INVALID_OPERATION as per the OpenGL 4.6
> > > Core spec:
> > > 
> > > "An INVALID_OPERATION error is generated if the effective target is
> > > TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture object
> > > is not cube complete or cube array complete, respectively."
> > > ---
> > 
> > 
> > Isn't there a similar problem in tests/spec/arb_get_texture_sub_image/get.c,
> > when invoking test_getsubimage() with a GL_TEXTURE_CUBE_MAP  ?
> > 
> 
> More specifically, the test is only setting one face of the cube, so the cube is
>  incomplete. It should set all the faces.
> 
> This change can be included within this patch.
> 
>         J.A.
> 
> > 
> >       J.A.
> > 
> > >  tests/spec/arb_get_texture_sub_image/errors.c | 10 +++++++---
> > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/tests/spec/arb_get_texture_sub_image/errors.c b/tests/spec/arb_get_texture_sub_image/errors.c
> > > index 1e7b17115..4b99d1cc2 100644
> > > --- a/tests/spec/arb_get_texture_sub_image/errors.c
> > > +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> > > @@ -253,16 +253,20 @@ test_cubemap_faces(void)
> > >                          0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> > >          }
> > > 
> > > -        /* try to get all six cube faces, should fail */
> > > +        /* try to query incomplete cube map, should fail */
> > >          glGetTextureSubImage(tex, 0,
> > >                               0, 0, 0,
> > > -                             8, 8, 6,
> > > +                             8, 8, 5,
> > >                               GL_RGBA, GL_UNSIGNED_BYTE,
> > >                               sizeof(results), results);
> > >     if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> > >             pass = false;
> > > 
> > > -        /* try to get five cube faces, should pass */
> > > +   /* upload final face */
> > > +   glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + 5,
> > > +                0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> > > +
> > > +        /* try to query complete cube map, should now pass */
> > >          glGetTextureSubImage(tex, 0,
> > >                               0, 0, 0,
> > >                               8, 8, 5,
> > 
> > _______________________________________________
> > 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
>
Thanks for taking the time to review.

I forgot to mention this in my original message, but this is my first patch series so I don't have commit access. What should I do to get these pushed?

 - Anthony
On Tue, 2018-04-10 at 15:56 +0000, Anthony Pesch wrote:
> Thanks for taking the time to review.
> 
> I forgot to mention this in my original message, but this is my first patch series so I don't have commit access. What should I do to get these pushed?
> 

I can push on your behalf.

Do you mind to re-submit the patches with git send-email, including the
Reviewed-by tags, so I can just download and push them directly?


Thanks!


	J.A.




>  - Anthony
> ________________________________________
> From: Juan A. Suarez Romero <jasuarez@igalia.com>
> Sent: Monday, April 9, 2018 3:41 AM
> To: Anthony Pesch; Anthony Pesch; piglit@lists.freedesktop.org
> Subject: Re: [Piglit] [PATCH 3/3] arb_get_texture_sub_image: update cube map tests to complete the cube map
> 
> For the series,
> 
> Reviewed-by: Juan A. Suarez <jasuarez@igalia.com>
> 
> 
> On Sun, 2018-04-08 at 18:35 +0000, Anthony Pesch wrote:
> > Hey Juan,
> > 
> > Good catch - I'd only been focusing on the errors test.
> > 
> > I've updated the patch to also fix the errors in arb_get_texture_sub_image-get:
> > 
> > Author: Anthony Pesch <apesch@nvidia.com>
> > Date:   Thu Mar 22 13:11:22 2018 -0400
> > 
> >     arb_get_texture_sub_image: update cube map tests to make textures cube complete
> > 
> >     Update cube map tests to ensure cube map textures are cube complete before querying
> >     them. Querying a cube map which is not cube complete should set INVALID_OPERATION
> >     as per the OpenGL 4.6 Core spec:
> > 
> >     "An INVALID_OPERATION error is generated if the effective target is
> >     TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture object
> >     is not cube complete or cube array complete, respectively."
> > 
> > diff --git a/tests/spec/arb_get_texture_sub_image/errors.c b/tests/spec/arb_get_texture_sub_image/errors.c
> > index 1e7b17115..4b99d1cc2 100644
> > --- a/tests/spec/arb_get_texture_sub_image/errors.c
> > +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> > @@ -253,16 +253,20 @@ test_cubemap_faces(void)
> >                          0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> >          }
> > 
> > -        /* try to get all six cube faces, should fail */
> > +        /* try to query incomplete cube map, should fail */
> >          glGetTextureSubImage(tex, 0,
> >                               0, 0, 0,
> > -                             8, 8, 6,
> > +                             8, 8, 5,
> >                               GL_RGBA, GL_UNSIGNED_BYTE,
> >                               sizeof(results), results);
> >         if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> >                 pass = false;
> > 
> > -        /* try to get five cube faces, should pass */
> > +       /* upload final face */
> > +       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + 5,
> > +                    0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> > +
> > +        /* try to query complete cube map, should now pass */
> >          glGetTextureSubImage(tex, 0,
> >                               0, 0, 0,
> >                               8, 8, 5,
> > diff --git a/tests/spec/arb_get_texture_sub_image/get.c b/tests/spec/arb_get_texture_sub_image/get.c
> > index 8aa4c92e1..fc8f9f8e1 100644
> > --- a/tests/spec/arb_get_texture_sub_image/get.c
> > +++ b/tests/spec/arb_get_texture_sub_image/get.c
> > @@ -157,12 +157,16 @@ test_getsubimage(GLenum target,
> >                                      GL_RGBA, GL_UNSIGNED_BYTE, texData);
> >                         break;
> >                 case GL_TEXTURE_CUBE_MAP:
> > -                       /* only set +Y face */
> > -                       glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_Y,
> > -                                    level, intFormat,
> > -                                    mip_width, mip_height, 0,
> > -                                    GL_RGBA, GL_UNSIGNED_BYTE,
> > -                                    texData);
> > +                       /* specify dimensions and format for all faces to make texture cube complete,
> > +                          but specify data for only the +Y face as it is the only one read back */
> > +                       for (i = 0; i < 6; i++) {
> > +                               GLubyte *faceData = i == 2 ? texData : NULL;
> > +                               glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + i,
> > +                                            level, intFormat,
> > +                                            mip_width, mip_height, 0,
> > +                                            GL_RGBA, GL_UNSIGNED_BYTE,
> > +                                            faceData);
> > +                       }
> >                         break;
> >                 }
> >         }
> > 
> > 
> >  - Anthony
> > 
> > ________________________________________
> > From: Piglit <piglit-bounces@lists.freedesktop.org> on behalf of Juan A. Suarez Romero <jasuarez@igalia.com>
> > Sent: Thursday, April 5, 2018 4:43 AM
> > To: Anthony Pesch; piglit@lists.freedesktop.org
> > Subject: Re: [Piglit] [PATCH 3/3] arb_get_texture_sub_image: update cube map tests to complete the cube map
> > 
> > On Thu, 2018-04-05 at 10:19 +0200, Juan A. Suarez Romero wrote:
> > > On Wed, 2018-03-28 at 11:15 -0400, Anthony Pesch wrote:
> > > > From: Anthony Pesch <apesch@nvidia.com>
> > > > 
> > > > Update cube map tests to complete the cube map before performing the final
> > > > query. This final query is expected to succeed, however, querying a cube map
> > > > which is not cube complete should set INVALID_OPERATION as per the OpenGL 4.6
> > > > Core spec:
> > > > 
> > > > "An INVALID_OPERATION error is generated if the effective target is
> > > > TEXTURE_CUBE_MAP or TEXTURE_CUBE_MAP_ARRAY, and the texture object
> > > > is not cube complete or cube array complete, respectively."
> > > > ---
> > > 
> > > 
> > > Isn't there a similar problem in tests/spec/arb_get_texture_sub_image/get.c,
> > > when invoking test_getsubimage() with a GL_TEXTURE_CUBE_MAP  ?
> > > 
> > 
> > More specifically, the test is only setting one face of the cube, so the cube is
> >  incomplete. It should set all the faces.
> > 
> > This change can be included within this patch.
> > 
> >         J.A.
> > 
> > > 
> > >       J.A.
> > > 
> > > >  tests/spec/arb_get_texture_sub_image/errors.c | 10 +++++++---
> > > >  1 file changed, 7 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/tests/spec/arb_get_texture_sub_image/errors.c b/tests/spec/arb_get_texture_sub_image/errors.c
> > > > index 1e7b17115..4b99d1cc2 100644
> > > > --- a/tests/spec/arb_get_texture_sub_image/errors.c
> > > > +++ b/tests/spec/arb_get_texture_sub_image/errors.c
> > > > @@ -253,16 +253,20 @@ test_cubemap_faces(void)
> > > >                          0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> > > >          }
> > > > 
> > > > -        /* try to get all six cube faces, should fail */
> > > > +        /* try to query incomplete cube map, should fail */
> > > >          glGetTextureSubImage(tex, 0,
> > > >                               0, 0, 0,
> > > > -                             8, 8, 6,
> > > > +                             8, 8, 5,
> > > >                               GL_RGBA, GL_UNSIGNED_BYTE,
> > > >                               sizeof(results), results);
> > > >     if (!piglit_check_gl_error(GL_INVALID_OPERATION))
> > > >             pass = false;
> > > > 
> > > > -        /* try to get five cube faces, should pass */
> > > > +   /* upload final face */
> > > > +   glTexImage2D(GL_TEXTURE_CUBE_MAP_POSITIVE_X + 5,
> > > > +                0, GL_RGBA, 8, 8, 0, GL_RGBA, GL_FLOAT, NULL);
> > > > +
> > > > +        /* try to query complete cube map, should now pass */
> > > >          glGetTextureSubImage(tex, 0,
> > > >                               0, 0, 0,
> > > >                               8, 8, 5,
> > > 
> > > _______________________________________________
> > > 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
> > 
> 
>