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

[Emil Velikov]

On 17/06/14 14:41, Brian Paul wrote:
> 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.
>>
Hi Ian,

This patch has been nominated for stable for a couple of months but is jet to
land in master. Has it been super-seeded by another patch or is there anything
else stopping it from going forward ?

Thanks
Emil

>> 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
> 
> 
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev