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

[Nanley Chery]

On Wed, Nov 15, 2017 at 05:56:00PM -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.
> >

Hi Juan,

Feel free to send out a v3 that only fixes the issues mentioned above.
That would be strictly better than what we have now.

-Nanley

> >>
> >> 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.
> 
>   -ilia