[Mesa-dev] [PATCH 1/3] mesa: Generate pname-based errors from glGetTexLevelParameter first

[Brian Paul]

On 06/16/2014 07:34 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Previously, calling
>
>      glGenTextures(1, &t);
>      glBindTexture(GL_TEXTURE_2D, t);
>      glGetTexLevelParameteriv(GL_TEXTURE_2D, 0, 0xDEADBEEF, &value);
>
> would not generate an error.
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>   src/mesa/main/texparam.c | 63 +++++++++++++++++++++++++++++++++++++++---------
>   1 file changed, 52 insertions(+), 11 deletions(-)
>
> diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c
> index dc17ea5..565c1d6 100644
> --- a/src/mesa/main/texparam.c
> +++ b/src/mesa/main/texparam.c
> @@ -1053,6 +1053,58 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>      const struct gl_texture_image *img = NULL;
>      mesa_format texFormat;
>
> +   /* Generate pname errors now.  OpenGL 4.0 requires special behavior if the
> +    * image is undefined, but that does not supersede pname-based errors.
> +    */
> +   switch (pname) {
> +   case GL_TEXTURE_WIDTH:
> +   case GL_TEXTURE_HEIGHT:
> +   case GL_TEXTURE_DEPTH:
> +   case GL_TEXTURE_INTERNAL_FORMAT:
> +   case GL_TEXTURE_BORDER:
> +   case GL_TEXTURE_RED_SIZE:
> +   case GL_TEXTURE_GREEN_SIZE:
> +   case GL_TEXTURE_BLUE_SIZE:
> +   case GL_TEXTURE_ALPHA_SIZE:
> +   case GL_TEXTURE_INTENSITY_SIZE:
> +   case GL_TEXTURE_LUMINANCE_SIZE:
> +      break;
> +   case GL_TEXTURE_DEPTH_SIZE_ARB:
> +      if (!ctx->Extensions.ARB_depth_texture)
> +         goto invalid_pname;
> +      break;
> +   case GL_TEXTURE_STENCIL_SIZE:
> +      break;
> +   case GL_TEXTURE_SHARED_SIZE:
> +      if (ctx->Version < 30 &&
> +          !ctx->Extensions.EXT_texture_shared_exponent)
> +         goto invalid_pname;
> +      break;
> +   case GL_TEXTURE_COMPRESSED_IMAGE_SIZE:
> +   case GL_TEXTURE_COMPRESSED:
> +      break;
> +   case GL_TEXTURE_RED_TYPE_ARB:
> +   case GL_TEXTURE_GREEN_TYPE_ARB:
> +   case GL_TEXTURE_BLUE_TYPE_ARB:
> +   case GL_TEXTURE_ALPHA_TYPE_ARB:
> +   case GL_TEXTURE_LUMINANCE_TYPE_ARB:
> +   case GL_TEXTURE_INTENSITY_TYPE_ARB:
> +   case GL_TEXTURE_DEPTH_TYPE_ARB:
> +      if (!ctx->Extensions.ARB_texture_float)
> +         goto invalid_pname;
> +      break;
> +   case GL_TEXTURE_SAMPLES:
> +      if (!ctx->Extensions.ARB_texture_multisample)
> +         goto invalid_pname;
> +      break;
> +   case GL_TEXTURE_FIXED_SAMPLE_LOCATIONS:
> +      if (!ctx->Extensions.ARB_texture_multisample)
> +         goto invalid_pname;
> +      break;
> +   default:
> +      goto invalid_pname;
> +   }
> +
>      img = _mesa_select_tex_image(ctx, texObj, target, level);
>      if (!img || img->TexFormat == MESA_FORMAT_NONE) {
>         /* In case of undefined texture image return the default values.
> @@ -1135,17 +1187,12 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>            }
>            break;
>         case GL_TEXTURE_DEPTH_SIZE_ARB:
> -         if (!ctx->Extensions.ARB_depth_texture)
> -            goto invalid_pname;
>            *params = _mesa_get_format_bits(texFormat, pname);
>            break;
>         case GL_TEXTURE_STENCIL_SIZE:
>            *params = _mesa_get_format_bits(texFormat, pname);
>            break;
>         case GL_TEXTURE_SHARED_SIZE:
> -         if (ctx->Version < 30 &&
> -             !ctx->Extensions.EXT_texture_shared_exponent)
> -            goto invalid_pname;
>            *params = texFormat == MESA_FORMAT_R9G9B9E5_FLOAT ? 5 : 0;
>            break;
>
> @@ -1173,8 +1220,6 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>         case GL_TEXTURE_LUMINANCE_TYPE_ARB:
>         case GL_TEXTURE_INTENSITY_TYPE_ARB:
>         case GL_TEXTURE_DEPTH_TYPE_ARB:
> -         if (!ctx->Extensions.ARB_texture_float)
> -            goto invalid_pname;
>   	 if (_mesa_base_format_has_channel(img->_BaseFormat, pname))
>   	    *params = _mesa_get_format_datatype(texFormat);
>   	 else
> @@ -1183,14 +1228,10 @@ get_tex_level_parameter_image(struct gl_context *ctx,
>
>         /* GL_ARB_texture_multisample */
>         case GL_TEXTURE_SAMPLES:
> -         if (!ctx->Extensions.ARB_texture_multisample)
> -            goto invalid_pname;
>            *params = img->NumSamples;
>            break;
>
>         case GL_TEXTURE_FIXED_SAMPLE_LOCATIONS:
> -         if (!ctx->Extensions.ARB_texture_multisample)
> -            goto invalid_pname;
>            *params = img->FixedSampleLocations;
>            break;
>
>

I think another way of handling this might be to define/use a dummy 
gl_texture_image to use when img would have been null.  The dummy 
gl_texture_image would be initialized with suitable defaults for all the 
query-able fields (but mostly zeros).

That would save us the extra switch and might be less work if new 
queries are added in the future.

-Brian