[Mesa-dev] [PATCH] mesa/teximage: add more targets for CompressedTexImage3D

[Juan A. Suarez Romero]

On Tue, 2017-11-14 at 16:57 -0800, Nanley Chery wrote:
> On Wed, Nov 08, 2017 at 04:52:02PM +0100, Juan A. Suarez Romero wrote:
> > From section 8.7, page 179 of OpenGL ES 3.2 spec:
> > 
> >   An INVALID_OPERATION error is generated by CompressedTexImage3D
> >   if internalformat is one of the the formats in table 8.17 and target
> >   is not TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP_ARRAY or TEXTURE_3D.
> > 
> > So far it was only considering TEXTURE_2D_ARRAY as valid target. Now it
> > must consider also TEXTURE_CUBE_MAP_ARRAY and TEXTURE_3D.
> > 
> 
> Not necessarily. The next sentence after the one you quoted states:
> 
>    An INVALID_OPERATION error is generated by CompressedTexImage3D if
>    internalformat is TEXTURE_CUBE_MAP_ARRAY and the “Cube Map Array”
>    column of table 8.17 is not checked, or if internalformat is
>    TEXTURE_3D and the “3D Tex.” column of table 8.17 is not checked.
> 
> It seems that the OpenGL ES 3.2 spec has moved towards using a table to
> determine valid compressed targets. It hasn't necessarily stated that
> TEXTURE_CUBE_MAP_ARRAY and TEXTURE_3D are now valid. Though, by looking
> at the table, I think we can conclude that TEXTURE_CUBE_MAP_ARRAY is now
> valid.
> 

Right, but as the "Cube Map Array" is checked for all the cases, it is
fine to say that in effect TEXTURE_CUBE_MAP_ARRAY is now valid.

In any case, I can add both paragraphs to improve clarification.


> > This fixes KHR-GLES32.core.texture_cube_map_array.etc2_texture
> > ---
> >  src/mesa/main/teximage.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> > 
> > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > index 4ec6148bf42..2799669d28a 100644
> > --- a/src/mesa/main/teximage.c
> > +++ b/src/mesa/main/teximage.c
> > @@ -1403,26 +1403,21 @@ _mesa_target_can_be_compressed(const struct gl_context *ctx, GLenum target,
> >         * have already been handled by normal ETC2/EAC behavior.
> >         */
> >  
> > -      /* From section 3.8.6, page 146 of OpenGL ES 3.0 spec:
> > +      /* From section 8.7, page 179 of OpenGL ES 3.2 spec:
> >         *
> > -       *    "The ETC2/EAC texture compression algorithm supports only
> > -       *     two-dimensional images. If internalformat is an ETC2/EAC format,
> > -       *     glCompressedTexImage3D will generate an INVALID_OPERATION error if
> > -       *     target is not TEXTURE_2D_ARRAY."
> > +       *    "An INVALID_OPERATION error is generated by CompressedTexImage3D
> > +       *     if internalformat is one of the the formats in table 8.17 and target is
> > +       *     not TEXTURE_2D_ARRAY, TEXTURE_CUBE_MAP_ARRAY or TEXTURE_3D."
> >         *
> > -       * This should also be applicable for glTexStorage3D(). Other available
> > -       * targets for these functions are: TEXTURE_3D and TEXTURE_CUBE_MAP_ARRAY.
> > +       * This should also be applicable for glTexStorage3D().
> >         */
> > -      if (layout == MESA_FORMAT_LAYOUT_ETC2 && _mesa_is_gles3(ctx))
> > -            return write_error(error, GL_INVALID_OPERATION);
> 
> The OpenGL ES 3.2 spec seems to be adding support for compressed
> CUBE_MAP_ARRAYs, and not clarifying what the behavior should have been
> in OpenGL ES 3.0. In this case, I think we should have a additional
> condition and not replace the existing one.

Sounds fine.

> 
> >        target_can_be_compresed = _mesa_has_texture_cube_map_array(ctx);
> >        break;
> >     case GL_TEXTURE_3D:
> >        switch (layout) {
> >        case MESA_FORMAT_LAYOUT_ETC2:
> >           /* See ETC2/EAC comment in case GL_TEXTURE_CUBE_MAP_ARRAY. */
> > -         if (_mesa_is_gles3(ctx))
> > -            return write_error(error, GL_INVALID_OPERATION);
> > +         target_can_be_compresed = GL_TRUE;
> 
> None of the formats listed in table 8.17 have their 3D Tex. entry
> checked.
> 

Uh! Right, this chunk shouldn't be in the patch. I'll remove it.


Thank you!


	J.A.

> -Nanley
> 
> >           break;
> >        case MESA_FORMAT_LAYOUT_BPTC:
> >           target_can_be_compresed = ctx->Extensions.ARB_texture_compression_bptc;
> > -- 
> > 2.13.6
> > 
> 
>