[Mesa-dev] Bug in _mesa_meta_GenerateMipmap

[Brian Paul]

On 07/13/2010 11:52 AM, Maciej Cencora wrote:
> Dnia poniedziałek 12 lipca 2010 o 23:32:37 Brian Paul napisał(a):
>> Maciej Cencora wrote:
>>> Dnia poniedziałek 12 lipca 2010 o 20:31:42 Brian Paul napisał(a):
>>>> Maciej Cencora wrote:
>>>>> Hi,
>>>>>
>>>>> while working on failing piglit tests I've stumbled on a problem with
>>>>> _mesa_meta_GenerateMipmap. The function creates new texture images for
>>>>> lower mipmap levels, but during call to glTexImage the format and type
>>>>> are hardcoded to GL_RGBA and GL_UNSIGNED_BYTE respectivily. And that's
>>>>> where the problem arise because base image (the one that other levels
>>>>> are generated from) could have other format and type specified (in
>>>>> failing test texturing/gen-teximage these are GL_RGBA, GL_FLOAT).
>>>>> The result is that for base image MESA_FORMAT_ARGB8888 is chosen and
>>>>> for next levels it's MESA_FORMAT_RGBA8888_REV. Of course such texture
>>>>> with images of different formats is invalid and will be missampled by
>>>>> GPU.
>>>>>
>>>>> As the fix is not straightforward for me (not sure where to get format
>>>>> and type from, since the gl_texture_image doesn't hold them) could
>>>>> someone more familiar with core mesa take a look at this?
>>>>
>>>> Which driver are you using?
>>>>
>>>> I suspect the problem is the driver's ChooseTextureFormat() callback
>>>> is using more than the 'internalFormat' parameter to choose the actual
>>>> hardware format (i.e. it's also using the format/type params).  Later,
>>>> when we're building the mipmap we don't know the format/type params
>>>> used for creating the base image so we just use
>>>> GL_RGBA/GL_UNSIGNED_BYTE.
>>>>
>>>> We could save those original params when glTexImage2D() is called to
>>>> create the base image, but that would be of no use if the base image
>>>> was created with glCopyTexImage2D().
>>>>
>>>> We might just be able to "poke" in the actual format for the new image
>>>> to override whatever was chosen.  I'll try to write a patch later...
>>>>
>>>> -Brian
>>>
>>> I'm using r300 driver. As you suspect the radeonChooseTextureFormat
>>> (radeon_texture.c) function takes into account format and type to avoid
>>> unnecessary conversions in TexImage callback.
>>
>> Here's a patch to try.  Totally untested.
>>
>> -Brian
>
> It works fine. Unfortunately this solution isn't optimal at least for r100 -
> r600. During TexImage call memory for texture images is allocated in seperate
> bo's if they have different format. Later when we want to sample from such
> texture we need to copy data to one bo (hw needs [BaseLeve, MaxLevel] images
> to be stored in one continuous block when sampling from texture).

I don't understand.  Are you saying we're still allocating mipmap 
levels with different formats, but that's "fine"?

-Brian