Re: [Mesa-dev] [PATCH v2] mesa: enable ASTC/ETC1 compressed 3D textures

[Juan A. Suarez Romero]

On Thu, 2018-02-01 at 12:45 +0100, Juan A. Suarez Romero wrote:
> On Mon, 2018-01-29 at 16:25 -0800, Eric Anholt wrote:
> > "Juan A. Suarez Romero"  writes:
> > 
> > > Enable these kind of 3D texture when proper extensions are available.
> > > 
> > > Fixes KHR-GLES2.texture_3d.* with these textures.
> > > 
> > > v2: add better clarification (Eric)
> > > ---
> > >  src/mesa/main/teximage.c | 31 ---
> > >  1 file changed, 24 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > > index e5f8bb0718f..56235898c66 100644
> > > --- a/src/mesa/main/teximage.c
> > > +++ b/src/mesa/main/teximage.c
> > > @@ -1487,6 +1487,9 @@ _mesa_target_can_be_compressed(const struct 
> > > gl_context *ctx, GLenum target,
> > >break;
> > > case GL_TEXTURE_3D:
> > >switch (layout) {
> > > +  case MESA_FORMAT_LAYOUT_ETC1:
> > > + target_can_be_compresed = 
> > > ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
> > > + break;
> > >case MESA_FORMAT_LAYOUT_ETC2:
> > >   /* See ETC2/EAC comment in case GL_TEXTURE_CUBE_MAP_ARRAY. */
> > 
> > OK, I took a look at the ETC2/EAC comment above, this time, and it looks
> > like 3D textures are supposed to be not supported with ETC2 or ASTC
> > according to table 8.17 in GLES 3.2.
> > 
> > This is looking more like a testcase bug to me.
> 
> 
> Yes, you're right. This is similar to another patch I sent for review
> and that turned out it was wrong for the same reason (both patches are
> related with the same test).
> 
> So I withdraw this patch, and I'll send a fix for the testcase instead.
> 
> Thanks.
> 

After re-reading again more carefully, I think the patch is partially
correct, though it needs changes.

According to spec KHR_texture_compression_astc_{hdr,ldr} if hdr is
supported then the 3D textures column must be checked for all ASTC
textures.

If only LDR is supported, then it is kept unchecked, unless
KHR_texture_compression_astc_sliced_3d is also supported, which again mark as 
checked all ASTC textures for 3D Textures.

This is already correctly implemented in Mesa for CompressedTexImage3D.

But the last part of the patch was doing the check for
CompressedTexSubImage3D (incorrectly, which needs to be fixed). 

According to KHR_texture_compression_astc_{hdr,ldr}:


  An INVALID_OPERATION error is generated by CompressedTexSubImage3D if
   is TEXTURE_CUBE_MAP_ARRAY and the "Cube Map Array" column of
  table 8.19 is *not* checked, or if  is TEXTURE_3D and the "3D
  Tex." column of table 8.19 is *not* checked"


My understanding is that if HDR is implemented, or LDR and
astc_sliced_3d are implemented, then the 3D Tex. column is checked and
thus it is correct to call either CompressedTexImage3D a
CompressedTexSubImage3D.

I'll send a new patch.

> KHR_texture_compression_astc_sliced_3d
>   J.A.
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] mesa: enable ASTC/ETC1 compressed 3D textures

[Juan A. Suarez Romero]

On Mon, 2018-01-29 at 16:25 -0800, Eric Anholt wrote:
> "Juan A. Suarez Romero"  writes:
> 
> > Enable these kind of 3D texture when proper extensions are available.
> > 
> > Fixes KHR-GLES2.texture_3d.* with these textures.
> > 
> > v2: add better clarification (Eric)
> > ---
> >  src/mesa/main/teximage.c | 31 ---
> >  1 file changed, 24 insertions(+), 7 deletions(-)
> > 
> > diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> > index e5f8bb0718f..56235898c66 100644
> > --- a/src/mesa/main/teximage.c
> > +++ b/src/mesa/main/teximage.c
> > @@ -1487,6 +1487,9 @@ _mesa_target_can_be_compressed(const struct 
> > gl_context *ctx, GLenum target,
> >break;
> > case GL_TEXTURE_3D:
> >switch (layout) {
> > +  case MESA_FORMAT_LAYOUT_ETC1:
> > + target_can_be_compresed = 
> > ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
> > + break;
> >case MESA_FORMAT_LAYOUT_ETC2:
> >   /* See ETC2/EAC comment in case GL_TEXTURE_CUBE_MAP_ARRAY. */
> 
> OK, I took a look at the ETC2/EAC comment above, this time, and it looks
> like 3D textures are supposed to be not supported with ETC2 or ASTC
> according to table 8.17 in GLES 3.2.
> 
> This is looking more like a testcase bug to me.


Yes, you're right. This is similar to another patch I sent for review
and that turned out it was wrong for the same reason (both patches are
related with the same test).

So I withdraw this patch, and I'll send a fix for the testcase instead.

Thanks.


J.A.

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] mesa: enable ASTC/ETC1 compressed 3D textures

[Eric Anholt]

"Juan A. Suarez Romero"  writes:

> Enable these kind of 3D texture when proper extensions are available.
>
> Fixes KHR-GLES2.texture_3d.* with these textures.
>
> v2: add better clarification (Eric)
> ---
>  src/mesa/main/teximage.c | 31 ---
>  1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index e5f8bb0718f..56235898c66 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -1487,6 +1487,9 @@ _mesa_target_can_be_compressed(const struct gl_context 
> *ctx, GLenum target,
>break;
> case GL_TEXTURE_3D:
>switch (layout) {
> +  case MESA_FORMAT_LAYOUT_ETC1:
> + target_can_be_compresed = 
> ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
> + break;
>case MESA_FORMAT_LAYOUT_ETC2:
>   /* See ETC2/EAC comment in case GL_TEXTURE_CUBE_MAP_ARRAY. */

OK, I took a look at the ETC2/EAC comment above, this time, and it looks
like 3D textures are supposed to be not supported with ETC2 or ASTC
according to table 8.17 in GLES 3.2.

This is looking more like a testcase bug to me.


signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH v2] mesa: enable ASTC/ETC1 compressed 3D textures

[Juan A. Suarez Romero]

Enable these kind of 3D texture when proper extensions are available.

Fixes KHR-GLES2.texture_3d.* with these textures.

v2: add better clarification (Eric)
---
 src/mesa/main/teximage.c | 31 ---
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index e5f8bb0718f..56235898c66 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -1487,6 +1487,9 @@ _mesa_target_can_be_compressed(const struct gl_context 
*ctx, GLenum target,
   break;
case GL_TEXTURE_3D:
   switch (layout) {
+  case MESA_FORMAT_LAYOUT_ETC1:
+ target_can_be_compresed = 
ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
+ break;
   case MESA_FORMAT_LAYOUT_ETC2:
  /* See ETC2/EAC comment in case GL_TEXTURE_CUBE_MAP_ARRAY. */
  if (_mesa_is_gles3(ctx))
@@ -4648,10 +4651,12 @@ out:
  */
 static GLboolean
 compressed_subtexture_target_check(struct gl_context *ctx, GLenum target,
-   GLint dims, GLenum format, bool dsa,
+   GLint dims, GLenum intFormat, bool dsa,
const char *caller)
 {
GLboolean targetOK;
+   mesa_format format;
+   enum mesa_format_layout layout;
 
if (dsa && target == GL_TEXTURE_RECTANGLE) {
   _mesa_error(ctx, GL_INVALID_OPERATION, "%s(invalid target %s)", caller,
@@ -4712,21 +4717,33 @@ compressed_subtexture_target_check(struct gl_context 
*ctx, GLenum target,
   * which is (at this time) only bptc. Otherwise we'd say s3tc (and
   * more) are valid here, which they are not, but of course not
   * mentioned by core spec.
+  *
+  * Besides that, the OES_texture_3d spec doesn't specify any list of
+  * supported formats for CompressedTex*Image3D functions, and
+  * OES_texture_compressed_ETC1_RGB8_texture mentions only 2D for a
+  * function supporting it. However, given a KHR conformance test
+  * expects that the two work together in the obvious way, follow the
+  * tests instead of the specs.
   */
- switch (format) {
+ format = _mesa_glenum_to_compressed_format(intFormat);
+ layout = _mesa_get_format_layout(format);
+ switch (layout) {
  /* These are the only 3D compression formats supported at this time */
- case GL_COMPRESSED_RGBA_BPTC_UNORM:
- case GL_COMPRESSED_SRGB_ALPHA_BPTC_UNORM:
- case GL_COMPRESSED_RGB_BPTC_SIGNED_FLOAT:
- case GL_COMPRESSED_RGB_BPTC_UNSIGNED_FLOAT:
+ case MESA_FORMAT_LAYOUT_BPTC:
 /* valid format */
 break;
+ case MESA_FORMAT_LAYOUT_ASTC:
+targetOK = ctx->Extensions.KHR_texture_compression_astc_ldr;
+break;
+ case MESA_FORMAT_LAYOUT_ETC1:
+targetOK = ctx->Extensions.OES_compressed_ETC1_RGB8_texture;
+break;
  default:
 /* invalid format */
 _mesa_error(ctx, GL_INVALID_OPERATION,
 "%s(invalid target %s for format %s)", caller,
 _mesa_enum_to_string(target),
-_mesa_enum_to_string(format));
+_mesa_enum_to_string(intFormat));
 return GL_TRUE;
  }
  break;
-- 
2.14.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev