[Mesa-dev] [PATCH 1/2] mesa: Don't report types for 0-sized components of textures.

[Brian Paul]

On 11/18/2011 07:10 PM, Eric Anholt wrote:
> The GL_TEXTURE_WHATEVER_SIZE entrypoints were checking if the
> specified base type of the texture allowed that channel to be present
> before reporting the size of the channel, so that GL_RGB didn't end up
> with an alpha size if the hardware driver had to store it that way.
>
> The GL_TEXTURE_WHATEVER_TYPE entrypoints weren't checking it, so you
> would end up with strange responses from the GL involving 0-bit
> floating-point alpha components in GL_RGB32F, even though it says
> GL_NONE as expected for other 0-sized channels.
>
> Make _TYPE check _BaseFormat the same as _SIZE, which results in
> fixing most of the GL_RGB* testcases of gl-3.0-required-sized-formats
> pass on i965.
> ---
>   src/mesa/main/texparam.c |  115 ++++++++++++++++++++++++++--------------------
>   1 files changed, 65 insertions(+), 50 deletions(-)
>
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index 17eac5f..89a8fbb 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -884,6 +884,66 @@ _mesa_TexParameterIuiv(GLenum target, GLenum pname, const GLuint *params)
>   }
>
>
> +static GLboolean
> +base_format_has_channel(GLenum base_format, GLenum pname)
> +{
> +   switch (pname) {
> +   case GL_TEXTURE_RED_SIZE:
> +   case GL_TEXTURE_RED_TYPE:
> +      if (base_format == GL_RED ||
> +	  base_format == GL_RG ||
> +	  base_format == GL_RGB ||
> +	  base_format == GL_RGBA) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_GREEN_SIZE:
> +   case GL_TEXTURE_GREEN_TYPE:
> +      if (base_format == GL_RG ||
> +	  base_format == GL_RGB ||
> +	  base_format == GL_RGBA) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_BLUE_SIZE:
> +   case GL_TEXTURE_BLUE_TYPE:
> +      if (base_format == GL_RGB ||
> +	  base_format == GL_RGBA) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_ALPHA_SIZE:
> +   case GL_TEXTURE_ALPHA_TYPE:
> +      if (base_format == GL_RGBA ||
> +	  base_format == GL_ALPHA ||
> +	  base_format == GL_LUMINANCE_ALPHA) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_LUMINANCE_SIZE:
> +   case GL_TEXTURE_LUMINANCE_TYPE:
> +      if (base_format == GL_LUMINANCE ||
> +	  base_format == GL_LUMINANCE_ALPHA) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_INTENSITY_SIZE:
> +   case GL_TEXTURE_INTENSITY_TYPE:
> +      if (base_format == GL_INTENSITY) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;
> +   case GL_TEXTURE_DEPTH_SIZE:
> +   case GL_TEXTURE_DEPTH_TYPE:
> +      if (base_format == GL_DEPTH_STENCIL ||
> +	  base_format == GL_DEPTH_COMPONENT) {
> +	 return GL_TRUE;
> +      }
> +      return GL_FALSE;

Maybe add a default case with _mesa_warning() to catch unexpected 
cases in the future?


> +   }
> +
> +   return GL_FALSE;
> +}
>
>
>   void GLAPIENTRY
> @@ -981,27 +1041,10 @@ _mesa_GetTexLevelParameteriv( GLenum target, GLint level,
>            *params = img->Border;
>            break;
>         case GL_TEXTURE_RED_SIZE:
> -         if (img->_BaseFormat == GL_RED) {
> -            *params = _mesa_get_format_bits(texFormat, pname);
> -	    break;
> -	 }
> -	 /* FALLTHROUGH */
>         case GL_TEXTURE_GREEN_SIZE:
> -         if (img->_BaseFormat == GL_RG) {
> -            *params = _mesa_get_format_bits(texFormat, pname);
> -	    break;
> -	 }
> -	 /* FALLTHROUGH */
>         case GL_TEXTURE_BLUE_SIZE:
> -         if (img->_BaseFormat == GL_RGB || img->_BaseFormat == GL_RGBA)
> -            *params = _mesa_get_format_bits(texFormat, pname);
> -         else
> -            *params = 0;
> -         break;
>         case GL_TEXTURE_ALPHA_SIZE:
> -         if (img->_BaseFormat == GL_ALPHA ||
> -             img->_BaseFormat == GL_LUMINANCE_ALPHA ||
> -             img->_BaseFormat == GL_RGBA)
> +         if (base_format_has_channel(img->_BaseFormat, pname))
>               *params = _mesa_get_format_bits(texFormat, pname);
>            else
>               *params = 0;
> @@ -1067,46 +1110,18 @@ _mesa_GetTexLevelParameteriv( GLenum target, GLint level,
>
>         /* GL_ARB_texture_float */
>         case GL_TEXTURE_RED_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_RED_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_GREEN_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_GREEN_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_BLUE_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_BLUE_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_ALPHA_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_ALPHA_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_LUMINANCE_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_LUMINANCE_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_INTENSITY_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_INTENSITY_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> -         break;
>         case GL_TEXTURE_DEPTH_TYPE_ARB:
>            if (!ctx->Extensions.ARB_texture_float)
>               goto invalid_pname;
> -         *params = _mesa_get_format_bits(texFormat, GL_TEXTURE_DEPTH_SIZE) ?
> -            _mesa_get_format_datatype(texFormat) : GL_NONE;
> +	 if (base_format_has_channel(img->_BaseFormat, pname))
> +	    *params = _mesa_get_format_datatype(texFormat);
> +	 else
> +	    *params = GL_NONE;
>            break;
>
>         default:


Reviewed-by: Brian Paul <brianp at vmware.com>

The get_component_bits() function in fbobject.c could also make use of 
base_format_has_channel() if we added switch cases for 
GL_RENDERBUFFER_RED_SIZE_EXT and GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE, 
etc.  That could be done in a subsequent patch.

-Brian