[Mesa-dev] [PATCH v2] mesa/teximage: add TEXTURE_CUBE_MAP_ARRAY target for CompressedTexImage3D

[Nanley Chery]

On Fri, Nov 17, 2017 at 09:42:40AM +0100, Juan A. Suarez Romero wrote:
> On Wed, 2017-11-15 at 17:56 -0500, Ilia Mirkin wrote:
> > On Wed, Nov 15, 2017 at 5:49 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> > > On Wed, Nov 15, 2017 at 12:08:58PM -0500, Ilia Mirkin wrote:
> > > > On Wed, Nov 15, 2017 at 11:54 AM, Juan A. Suarez Romero
> > > > <jasuarez at igalia.com> 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.
> > > > > 
> > > > >   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.
> > > > > 
> > > > > So far it was only considering TEXTURE_2D_ARRAY as valid target. But as
> > > > > "Cube Map Array" column is checked for all the cases, in practice we can
> > > > > consider also TEXTURE_CUBE_MAP_ARRAY.
> > > > > 
> > > > > This fixes KHR-GLES32.core.texture_cube_map_array.etc2_texture
> > > > > ---
> > > > >  src/mesa/main/teximage.c | 25 +++++++++++++++++--------
> > > > >  1 file changed, 17 insertions(+), 8 deletions(-)
> > > > > 
> > > > > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > > > > index 4ec6148bf42..174731d0d8a 100644
> > > > > --- a/src/mesa/main/teximage.c
> > > > > +++ b/src/mesa/main/teximage.c
> > > > > @@ -1403,17 +1403,26 @@ _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.
> > > > > +       *      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.
> > > > > +       *
> > > > > +       * The instances of <internalformat> above should say <target>.
> > > > > +       *
> > > > > +       * This should also be applicable for glTexStorage3D().
> > > > > +       *
> > > > > +       * Such table 8.17 has checked "Cube Map Array" column for all the
> > > > > +       * cases. So in practice, TEXTURE_CUBE_MAP_ARRAY is now valid.
> > > > >         */
> > > 
> > > This deletes the comment describing the old OpenGL ES 3.0 behavior.
> > > 
> > > > > -      if (layout == MESA_FORMAT_LAYOUT_ETC2 && _mesa_is_gles3(ctx))
> > > > > +      if (layout == MESA_FORMAT_LAYOUT_ETC2 && _mesa_is_gles3(ctx) && !_mesa_is_gles32(ctx))
> > > 
> > > Some of these lines look longer than 80 chars.
> > > 
> > > > 
> > > > Perhaps this should be
> > > > 
> > > > !_mesa_has_OES_texture_cube_map_array(ctx)?
> > > > 
> > > 
> > > I think you're right. It looks like OES_texture_cube_map_array was folded
> > > into ES 3.2. I haven't done the comparison myself to confirm this
> > > however.
> > > 
> > > > It's unclear from that spec though... perhaps ES 3.2 goes further and
> > > > is required to be supported.
> > > > 
> > > 
> > > If the OpenGL ES Wikipedia page is reliable, we should probably be checking for
> > > this extension before exposing ES 3.2.
> > 
> > https://cgit.freedesktop.org/mesa/mesa/tree/src/mesa/main/version.c#n547
> > 
> > Anyways, the question is whether OES_texture_cube_map_array is
> > sufficient, or if ES 3.2 is needed for this change to apply. Not sure.
> > I didn't dig in deeply enough.
> > 
> 
> 
> I think it is not enough. I've checked OES_texture_cube_map_array spec
> and it modifies nothing in the OpenGL ES 3.1 spec regarding compressed
> textures.
> 

It does. In the New Tokens section, it adds 

TEXTURE_CUBE_MAP_ARRAY_OES                      0x9009

as an acceptable token for CompressedTexImage3D, and
CompressedTexSubImage3D. The value (0x9009) is equivalent to
TEXTURE_CUBE_MAP_ARRAY.

> So I think ES 3.2 is needed in this change.
> 

Just FYI, I looked into this issue further and found enough evidence to
believe that the extension was folded into 3.2.

-Nanley

> 
> 	J.A.
>