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