[Mesa-dev] [PATCH] mesa/teximage: Fix ASTC-caused S3TC regression

[Ian Romanick]

On 10/28/2015 09:52 AM, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.chery at intel.com>
> 
> The ASTC spec forbids other compressed formats from being used against
> the targets: TEXTURE_CUBE_MAP_ARRAY and TEXTURE_3D. Because other

I apologize for not paying very much attention to this previously.

I think we're taking too literal of a reading of the spec.  The ASTC
extension extends the core OpenGL ES and desktop OpenGL specifications.
 It does not interact with other extensions that are not part of the
core.  I see no interactions in the ASTC specification with any S3TC
spec, GL_3DFX_texture_compression_FXT1, GL_EXT_texture_compression_latc,
or GL_ATI_texture_compression_3dc.  As a result, the existence of ASTC
should have *no* affect on what is allowed with those compression formats.

Judging from table 8.17 in the OpenGL ES 3.2 spec, I think the only
formats that should ever be disallowed for cubemap arrays is the ETC
formats.  From reading the errors in section 8.5.3 (Texture Image
Structure) of the OpenGL 4.5 spec, even that limitation should only
apply to OpenGL ES.

> compressed formats were previously supported for these targets in
> desktop GL, it's considered to be a spec bug and Mesa will diverge from
> this behavior for desktop GL.
> 
> Fixes the following Piglit tests on Gen9:
> piglit.spec.arb_direct_state_access.getcompressedtextureimage
> piglit.spec.arb_get_texture_sub_image.arb_get_texture_sub_image-getcompressed
> piglit.spec.arb_texture_cube_map_array.fbo-generatemipmap-cubemap array s3tc_dxt1
> piglit.spec.ext_texture_compression_s3tc.getteximage-targets cube_array s3tc
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91927
> Suggested-by: Neil Roberts <neil at linux.intel.com>
> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> ---
>  src/mesa/main/teximage.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index d9453e3..eccf009 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -1373,7 +1373,8 @@ _mesa_target_can_be_compressed(const struct gl_context *ctx, GLenum target,
>        /* Throw an INVALID_OPERATION error if the target is
>         * TEXTURE_CUBE_MAP_ARRAY and the format is not ASTC.
>         */
> -      if (target_can_be_compresed &&
> +      /* FIXME: Correct the spec to restrict this behavior to GLES as well. */
> +      if (target_can_be_compresed && _mesa_is_gles(ctx) &&
>            ctx->Extensions.KHR_texture_compression_astc_ldr &&
>            layout != MESA_FORMAT_LAYOUT_ASTC)
>           return write_error(error, GL_INVALID_OPERATION);
> @@ -1405,7 +1406,9 @@ _mesa_target_can_be_compressed(const struct gl_context *ctx, GLenum target,
>            * the format is not ASTC.
>            * See comment in switch case GL_TEXTURE_CUBE_MAP_ARRAY for more info.

I'm a bit confused about the comment above and the strangeness with the
error codes.  I'll dig a bit into the specs.

>            */
> -         if (ctx->Extensions.KHR_texture_compression_astc_ldr)
> +         /* FIXME: Correct the spec to restrict this behavior to GLES as well. */
> +         if (_mesa_is_gles(ctx) &&
> +             ctx->Extensions.KHR_texture_compression_astc_ldr)
>              return write_error(error, GL_INVALID_OPERATION);
>           break;
>        }
>