[Mesa-dev] [PATCH] mesa: Add GL_TEXTURE_CUBE_MAP to _mesa_max_texture_levels().

[Kenneth Graunke]

On 06/05/2012 07:05 AM, Brian Paul wrote:
> On 06/05/2012 01:16 AM, Kenneth Graunke wrote:
>> For cube maps, _mesa_generate_mipmap() calls this with
>> GL_TEXTURE_CUBE_MAP_ARB (the gl_texture_object's Target) rather than one
>> of the faces.  This caused _mesa_max_texture_levels() to return 0, which
>> resulted in maxLevels == -1 and the next line's assertion to fail.
>>
>> This function is called from several places.  It looks like texstorage.c
>> also wants this enum to be handled, while most callers don't care.
>> TexImage, CompressedTexImage, FramebufferTexture, and GetTexImage
>> already appear to filter out illegal targets, so they should be fine.
>>
>> However, GetTexLevelParameter does rely on this function for filtering
>> out invalid targets, and does not accept GL_TEXTURE_CUBE_MAP.  Special
>> case this, since it apparently supports every other target.
> 
> I think glGet[Compressed]TexImage() needs the special case too.  For
> example, getteximage_error_check() also uses max levels to error check
> the target.
> 
> It might be good to beef-up the comment on _mesa_max_texture_levels() to
> clarify that the function is also used to error-check 'target' in
> various places and some care should be taken when adding support for new
> targets (GL_TEXTURE_CUBE_MAP_ARRAY someday).
> 
> -Brian

You're right, I think I missed a few cases.

I'm tempted to make more functions like legal_teximage_target() and
convert the callers to use that for their checking rather than
implicitly relying on _mesa_max_texture_levels().  It feels like we have
one function doing target checking for many different API calls...not
all of which necessarily support the same set of targets.

Thoughts?