[Mesa-dev] [PATCH] mesa: Fix GL_LUMINANCE handling for compressed textures in glGetTexImage

[Ian Romanick]

On 11/16/2012 01:21 PM, Anuj Phogat wrote:
> We need to rebase colors (ex: set G=B=0) when getting GL_LUMINANCE
> textures in following cases:
> 1. If the luminance texture is actually stored as rgba
> 2. If getting a luminance texture, but returning rgba
> 3. If getting an rgba texture, but returning luminance
>
> A similar fix was pushed by Brian Paul for uncompressed textures
> in commit: f5d0ced.
>
> Fixes intel oglconform pxconv-gettex test:
> https://bugs.freedesktop.org/show_bug.cgi?id=47220
>
> Observed no failures in piglit and ogles2conform due to this fix.
> This patch will cause failures in intel oglconform pxstore-gettex
> and pxtrans-gettex test cases. The cause of a failures is a bug
> in test cases. Expected luminance value is calculted incorrectly:
                                              calculated

> L = R+G+B.

This seems weird.  We fail pxconv-gettex because we convert the 
luminance texture supplied by the application to RGB by replicating the 
L value to each of R, G, and B.  In this case, and this case only, we 
need to read back luminance by just returning R.  If the application 
actually specified an RGB texture, we need to read back luminance by 
R+G+B.  Right?

Does this match what pxconv-gettex and pxconv-trans expect?

This L vs. RGB crap confuses me every time...

> Note: This is a candifate for stable branches.
                   candidate

>
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>   src/mesa/drivers/common/meta.c |   16 +++++++++++++---
>   src/mesa/main/texgetimage.c    |   34 ++++++++++++++++++++++++----------
>   2 files changed, 37 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index 417dbd0..e019140 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -3685,6 +3685,7 @@ decompress_texture_image(struct gl_context *ctx,
>      /* read pixels from renderbuffer */
>      {
>         GLenum baseTexFormat = texImage->_BaseFormat;
> +      GLenum destBaseFormat = _mesa_base_tex_format(ctx, destFormat);
>
>         /* The pixel transfer state will be set to default values at this point
>          * (see MESA_META_PIXEL_TRANSFER) so pixel transfer ops are effectively
> @@ -3693,9 +3694,18 @@ decompress_texture_image(struct gl_context *ctx,
>          * returned as red and two-channel texture values are returned as
>          * red/alpha.
>          */
> -      if (baseTexFormat == GL_LUMINANCE ||
> -          baseTexFormat == GL_LUMINANCE_ALPHA ||
> -          baseTexFormat == GL_INTENSITY) {
> +      if ((baseTexFormat == GL_LUMINANCE ||
> +           baseTexFormat == GL_LUMINANCE_ALPHA ||
> +           baseTexFormat == GL_INTENSITY) ||
> +          /* If we're reading back an RGB(A) texture (using glGetTexImage) as
> +	   * luminance then we need to return L=tex(R).
> +	   */
> +          ((baseTexFormat == GL_RGBA ||
> +            baseTexFormat == GL_RGB) &&
> +          (destBaseFormat == GL_LUMINANCE ||
> +           destBaseFormat == GL_LUMINANCE_ALPHA ||
> +           destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
> +           destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT))) {
>            /* Green and blue must be zero */
>            _mesa_PixelTransferf(GL_GREEN_SCALE, 0.0f);
>            _mesa_PixelTransferf(GL_BLUE_SCALE, 0.0f);
> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
> index 2ccdceb..bd6d481 100644
> --- a/src/mesa/main/texgetimage.c
> +++ b/src/mesa/main/texgetimage.c
> @@ -229,6 +229,8 @@ get_tex_rgba_compressed(struct gl_context *ctx, GLuint dimensions,
>      const gl_format texFormat =
>         _mesa_get_srgb_format_linear(texImage->TexFormat);
>      const GLenum baseFormat = _mesa_get_format_base_format(texFormat);
> +   const GLenum destBaseFormat = _mesa_base_tex_format(ctx, format);
> +   GLenum rebaseFormat = GL_NONE;
>      const GLuint width = texImage->Width;
>      const GLuint height = texImage->Height;
>      const GLuint depth = texImage->Depth;
> @@ -266,9 +268,30 @@ get_tex_rgba_compressed(struct gl_context *ctx, GLuint dimensions,
>      }
>
>      if (baseFormat == GL_LUMINANCE ||
> +       baseFormat == GL_INTENSITY ||
>          baseFormat == GL_LUMINANCE_ALPHA) {
> +      /* If a luminance (or intensity) texture is read back as RGB(A), the
> +       * returned value should be (L,0,0,1), not (L,L,L,1).  Set rebaseFormat
> +       * here to get G=B=0.
> +       */
> +      rebaseFormat = texImage->_BaseFormat;
> +   }
> +   else if ((baseFormat == GL_RGBA ||
> +             baseFormat == GL_RGB) &&
> +            (destBaseFormat == GL_LUMINANCE ||
> +             destBaseFormat == GL_LUMINANCE_ALPHA ||
> +             destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
> +             destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT)) {
> +      /* If we're reading back an RGB(A) texture as luminance then we need
> +       * to return L=tex(R).  Note, that's different from glReadPixels which
> +       * returns L=R+G+B.
> +       */
> +      rebaseFormat = GL_LUMINANCE_ALPHA; /* this covers GL_LUMINANCE too */
> +   }
> +
> +   if (rebaseFormat) {
>         _mesa_rebase_rgba_float(width * height, (GLfloat (*)[4]) tempImage,
> -                              baseFormat);
> +                              rebaseFormat);
>      }
>
>      srcRow = tempImage;
> @@ -419,15 +442,6 @@ get_tex_rgba(struct gl_context *ctx, GLuint dimensions,
>            transferOps |= IMAGE_CLAMP_BIT;
>         }
>      }
> -   /* This applies to RGB, RGBA textures. if the format is either LUMINANCE
> -    * or LUMINANCE ALPHA, luminance (L) is computed as L=R+G+B .we need to
> -    * clamp the sum to [0,1].
> -    */
> -   else if ((format == GL_LUMINANCE ||
> -            format == GL_LUMINANCE_ALPHA) &&
> -            dataType == GL_UNSIGNED_NORMALIZED) {
> -      transferOps |= IMAGE_CLAMP_BIT;
> -   }
>
>      if (_mesa_is_format_compressed(texImage->TexFormat)) {
>         get_tex_rgba_compressed(ctx, dimensions, format, type,
>