[Mesa-dev] Bug in _mesa_meta_GenerateMipmap

[Maciej Cencora]

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'd propose this patch as a candidate for 7.8 branch.

Regards,
Maciej