[Mesa-dev] [PATCH] mesa: Don't override S3TC internalFormat if data is pre-compressed.

[Ian Romanick]

On 09/19/2012 09:01 PM, Kenneth Graunke wrote:
> Commit 42723d88d intended to override an S3TC internalFormat to a
> generic compressed format when the application requested online
> compression of uncompressed data.  Unfortunately, it also broke
> pre-compressed textures when libtxc_dxtn isn't installed but the
> extensions are forced on.
>
> Both glCompressedTexImage2D() and glTexImage2D() call teximage(), which
> calls _mesa_choose_texture_format(), hitting this override code.  If we
> have actual S3TC source data, we can't treat it as any other format, and
> need to avoid the override.
>
> Since glCompressedTexImage2D() passes in a format of GL_NONE (which is
> illegal for glTexImage), we can use that to detect the pre-compressed
> case and avoid the overrides.

Blarg.  That's subtle.  Is there some sort of a unit test we could make 
that would document this bit of the internal API and catch future errors 
of this nature?

> Fixes a regression since 42723d88d370a7599398cc1c2349aeb951ba1c57.
>
> NOTE: This is a candidate for the 9.0 branch.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Ian Romanick <idr at freedesktop.org>

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

> Cc: Jordan Justen <jljusten at gmail.com>
> ---
>   src/mesa/main/teximage.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
> index da8fbcd..dff50ca 100644
> --- a/src/mesa/main/teximage.c
> +++ b/src/mesa/main/teximage.c
> @@ -2806,7 +2806,7 @@ _mesa_choose_texture_format(struct gl_context *ctx,
>      /* If the application requested compression to an S3TC format but we don't
>       * have the DTXn library, force a generic compressed format instead.
>       */
> -   if (internalFormat != format) {
> +   if (internalFormat != format && format != GL_NONE) {
>         const GLenum before = internalFormat;
>
>         switch (internalFormat) {
>