[Mesa-dev] [PATCH] mesa: do more teximage error checking for generic compressed formats

[Brian Paul]

On 08/20/2012 03:34 PM, Anuj Phogat wrote:
> On Fri, Aug 17, 2012 at 8:28 PM, Ian Romanick<idr at freedesktop.org>  wrote:
>> On 08/16/2012 02:29 PM, Anuj Phogat wrote:
>>>
>>> On Thu, Aug 16, 2012 at 10:23 AM, Brian Paul<brianp at vmware.com>  wrote:
>>>>
>>>> On 08/15/2012 02:31 PM, Anuj Phogat wrote:
>>>>>
>>>>>
>>>>> On Tue, May 1, 2012 at 2:07 PM, Brian Paul<brianp at vmware.com
>>>>> <mailto:brianp at vmware.com>>  wrote:
>>>>>
>>>>>       When glTexImage or glCopyTexImage is called with internalFormat
>>>>>       being a
>>>>>       generic compressed format (like GL_COMPRESSED_RGB) we need to do
>>>>>       the same
>>>>>       error checks as for specific compressed formats.  In particular,
>>>>>       check if
>>>>>       the texture target is compatible with the format.  None of the
>>>>> texture
>>>>>       compression formats we support so far work with GL_TEXTURE_1D, for
>>>>>       example.
>>>>>
>>>>>       See also https://bugs.freedesktop.org/show_bug.cgi?id=49124
>>>>>
>>>>>
>>>>> Brian, generic texture compression formats with GL_TEXTURE_1D seem to
>>>>> work fine
>>>>> on i965 drivers.
>>>>
>>>>
>>>> Does that wind up using one of the DXT formats?
>>>
>>> It uses RGTC for GL_COMPRESSED_RED&  GL_COMPRESSED_RG
>>> FXT for GL_COMPRESSED_RGB&  GL_COMPRESSED_RGBA
>>
>>
>> I wonder if that produces correct results.  There are lots of limitations
>> with those formats...
>>
> I noticed failures in case of GL_TEXTURE_3D with GL_COMPRESSED_RGBA
> format. Failures disappeared when I made changes to pick an uncompressed
> format.
>>>>    I verified this by allowing generic texture
>>>>>
>>>>>
>>>>> compression formats for
>>>>> GL_TEXTURE_1D in piglit copyteximage test case and reverting the
>>>>> changes due to
>>>>> this patch on mesa. Is this an issue only on swrast?
>>>>
>>>>
>>>>
>>>> That's what the bug reported, don't recall testing other drivers.
>>>>
>>>>
>>>>
>>>>> Returning
>>>>> GL_INVALID_ENUM
>>>>> error for generic texture compression formats in glTexImage1D() and
>>>>> glCopyTexImage1D() doesn't seem to follow the OpenGL
>>>>> specification. Spec does allow
>>>>> GL_INVALID_ENUM error for a similar scenario in case
>>>>> of glCompressedTexImage1D().
>>>>> Please correct me if I'm missing something.
>>>>
>>>>
>>>>
>>>> I guess I'd like to check what either NVIDIA or AMD do in some of these
>>>> cases.
>>>>
>>>> AFAIK, the various DXT, LATC, etc compressed formats are only spec'd to
>>>> work
>>>> with 2D textures, not 1D.  Since 1D textures are generally pretty small
>>>> to
>>>> start with, there's not a lot of value in compressed 1D textures.  Plus,
>>>> if
>>>> a compressed 1D texture uses 4 or 8 bytes to store a 4x1 block of texels,
>>>> there's only 50% or 0% memory savings with compression.
>>
>>
>> Most of the specific formats are specified to only work with 2D textures.
>>
>>  From EXT_texture_compression_s3tc (note that 1D variants of TexImage,
>> CopyTexImage, etc are missing):
>>
>>      Accepted by the<internalformat>  parameter of TexImage2D,
>> CopyTexImage2D,
>>      and CompressedTexImage2DARB and the<format>  parameter of
>>      CompressedTexSubImage2DARB:
>>
>>          COMPRESSED_RGB_S3TC_DXT1_EXT                   0x83F0
>>          COMPRESSED_RGBA_S3TC_DXT1_EXT                  0x83F1
>>          COMPRESSED_RGBA_S3TC_DXT3_EXT                  0x83F2
>>          COMPRESSED_RGBA_S3TC_DXT5_EXT                  0x83F3
>>
>> Same for 3DFX_texture_compression_FXT1:
>>
>>      Accepted by the<internalformat>  parameter of TexImage2D,
>>      CopyTexImage2D, TexImage3D, CopyTexImage3D, and by the
>>      <internalformat>  and<format>  parameters of
>>      CompressedTexImage2D_ARB, CompressedTexSubImage2D_ARB,
>>      CompressedTexImage3D_ARB, CompressedTexSubImage3D_ARB:
>>
>>          COMPRESSED_RGB_FXT1_3DFX                          0x86B0
>>          COMPRESSED_RGBA_FXT1_3DFX                         0x86B1
>>
>> However, the generic formats should be accepted.  From
>> ARB_texture_compression:
>>
>>      Accepted by the<internalformat>  parameter of TexImage1D, TexImage2D,
>>      TexImage3D, CopyTexImage1D, and CopyTexImage2D:
>>
>>          COMPRESSED_ALPHA_ARB                            0x84E9
>>          COMPRESSED_LUMINANCE_ARB                        0x84EA
>>          COMPRESSED_LUMINANCE_ALPHA_ARB                  0x84EB
>>          COMPRESSED_INTENSITY_ARB                        0x84EC
>>          COMPRESSED_RGB_ARB                              0x84ED
>>          COMPRESSED_RGBA_ARB                             0x84EE
>>
>> When the application specifies a generic format, the driver is always free
>> to pick an uncompressed format.  Many Mesa drivers (primarily the ones we
>> removed a couple years ago) have done that for ages. :)
>>
> ok. I will make changes to allow these formats for 1D/3D textures as well and
> and pick a matching uncompressed format internally.
> Do we need any special treatment for 2D textures? I mean picking a matching
> compressed format as they are well supported on 2D targets.

Feel free to revert a36581ccc06693231011c3fe136207e73191b1ce.

I think we'll ultimately want to add a texture target parameter to the 
ctx->Driver.ChooseTextureFormat() function so that the chooser code 
can do the right thing when internalFormat is a generic compressed format.

BTW, I've just found some other glTexImage error checking mistakes. 
Patch to follow...

-Brian