[Mesa-dev] [PATCH 09/14] meta: Use _mesa_unpack_format_to_base_format() to handle integer formats

[Iago Toral]

On Tue, 2015-06-16 at 11:15 -0700, Anuj Phogat wrote:
> _mesa_base_tex_format() doesn't handle GL_*_INTEGER formats.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> ---
>  src/mesa/drivers/common/meta.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
> index fceb25d..c9e58d8 100644
> --- a/src/mesa/drivers/common/meta.c
> +++ b/src/mesa/drivers/common/meta.c
> @@ -3123,7 +3123,7 @@ decompress_texture_image(struct gl_context *ctx,
>     /* read pixels from renderbuffer */
>     {
>        GLenum baseTexFormat = texImage->_BaseFormat;
> -      GLenum destBaseFormat = _mesa_base_tex_format(ctx, destFormat);
> +      GLenum destBaseFormat = _mesa_unpack_format_to_base_format(destFormat);

I think this commit is doing a bit more more than what the shortlog
suggests: it is replacing a call to mesa_base_tex_format that handles
internal formats (so things like GL_COMPRESSED_ALPHA,  GL_RGB_S3TC or
GL_RG16_SNORM for example) with a call to the new
_mesa_unpack_format_to_base_format that does not care for internal
formats at all.

That change seems to be fine because decompress_texture_image claims to
receive a destFormat that is "ala glReadPixels", so there is no need to
care for all those internal formats anyway, but I think you should
explain this in the commit log.

With that change,
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

>        /* The pixel transfer state will be set to default values at this point
>         * (see MESA_META_PIXEL_TRANSFER) so pixel transfer ops are effectively
> @@ -3149,9 +3149,7 @@ decompress_texture_image(struct gl_context *ctx,
>              baseTexFormat == GL_RGB  ||
>              baseTexFormat == GL_RG) &&
>            (destBaseFormat == GL_LUMINANCE ||
> -           destBaseFormat == GL_LUMINANCE_ALPHA ||
> -           destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
> -           destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT))) {
> +           destBaseFormat == GL_LUMINANCE_ALPHA))) {
>           /* Green and blue must be zero */
>           _mesa_PixelTransferf(GL_GREEN_SCALE, 0.0f);
>           _mesa_PixelTransferf(GL_BLUE_SCALE, 0.0f);