On 07/11/2010 09:06 AM, 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?
I've worked with Maciej off-line to come up with a solution for this.
Maciej has already tested the attached patch but I thought I'd post it
for review first to see if anyone sees any problems.
This patch changes when the ctx->Driver.ChooseTextureFormat() function
is called.
The idea is when defining mipmap level 'L' and level L-1 exists and
the new level's internalFormat matches level L-1's internalFormat,
then use the same hardware format. Otherwise, do the regular
ctx->Driver.ChooseTextureFormat() call.
The root problem is the ChooseTextureFormat() implementation in some
drivers uses the user's glTexImage format/type parameters in the
choosing heuristic. Later mipmap levels might be generated without
calls to glTexImage2D() (ex: glCopyTexImage()) so we don't always have
format/type info and the driver may choose a different format.
Just using the previous level's format avoids this problem.
-Brian
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index c548e10..dc6e712 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -2570,12 +2570,6 @@ copy_tex_image(GLcontext *ctx, GLuint dims, GLenum
target, GLint level,
return;
}
- if (texImage->TexFormat == MESA_FORMAT_NONE)
- texImage->TexFormat = ctx->Driver.ChooseTextureFormat(ctx,
- internalFormat,
- format,
- type);
-
_mesa_unlock_texture(ctx, texObj); /* need to unlock first */
/*
@@ -2604,6 +2598,9 @@ copy_tex_image(GLcontext *ctx, GLuint dims, GLenum
target, GLint level,
postConvWidth, postConvHeight, 1,
border, internalFormat);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
+
/*
* Store texture data (with pixel transfer ops)
*/
diff --git a/src/mesa/main/teximage.c b/src/mesa/main/teximage.c
index 29b721d..9b7a021 100644
--- a/src/mesa/main/teximage.c
+++ b/src/mesa/main/teximage.c
@@ -2183,6 +2183,45 @@ override_internal_format(GLenum internalFormat, GLint
width, GLint height)
}
+/**
+ * Choose the actual hardware format for a texture image.
+ * Try to use the same format as the previous image level when possible.
+ * Otherwise, ask the driver for the best format.
+ * It's important to try to choose a consistant format for all levels
+ * for efficient texture memory layout/allocation. In particular, this
+ * comes up during automatic mipmap generation.
+ */
+void
+_mesa_choose_texture_format(GLcontext *ctx,
+ struct gl_texture_object *texObj,
+ struct gl_texture_image *texImage,
+ GLenum target, GLint level,
+ GLenum internalFormat, GLenum format, GLenum type)
+{
+ /* see if we've already chosen a format for the previous level */
+ if (level > 0) {
+ struct gl_texture_image *prevImage =
+ _mesa_select_tex_image(ctx, texObj, target, level - 1);
+ /* See if the prev level is defined and has an internal format which
+ * matches the new internal format.
+ */
+ if (prevImage &&
+ prevImage->Width > 0 &&
+ prevImage->InternalFormat == internalFormat) {
+ /* use the same format */
+ texImage->TexFormat = prevImage->TexFormat;
+ return;
+ }
+ }
+
+ /* choose format from scratch */
+ texImage->TexFormat = ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
+ format, type);
+ ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+}
+
+
+
/*
* Called from the API. Note that width includes the border.
*/
@@ -2243,11 +2282,8 @@ _mesa_TexImage1D( GLenum target, GLint level, GLint
internalFormat,
postConvWidth, 1, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- format, type);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
/* Give the texture to the driver. <pixels> may be null. */
ASSERT(ctx->Driver.TexImage1D);
@@ -2282,12 +2318,14 @@ _mesa_TexImage1D( GLenum target, GLint level, GLint
internalFormat,
}
else {
/* no error, set the tex image parameters */
+ struct gl_texture_object *texObj =
+ _mesa_get_current_tex_object(ctx, target);
ASSERT(texImage);
_mesa_init_teximage_fields(ctx, target, texImage,
postConvWidth, 1, 1,
border, internalFormat);
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat, format, type);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
}
}
else {
@@ -2363,11 +2401,8 @@ _mesa_TexImage2D( GLenum target, GLint level, GLint
internalFormat,
postConvWidth, postConvHeight, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- format, type);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
/* Give the texture to the driver. <pixels> may be null. */
ASSERT(ctx->Driver.TexImage2D);
@@ -2409,11 +2444,13 @@ _mesa_TexImage2D( GLenum target, GLint level, GLint
internalFormat,
}
else {
/* no error, set the tex image parameters */
+ struct gl_texture_object *texObj =
+ _mesa_get_current_tex_object(ctx, target);
_mesa_init_teximage_fields(ctx, target, texImage,
postConvWidth, postConvHeight, 1,
border, internalFormat);
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat, format, type);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
}
}
else {
@@ -2479,11 +2516,8 @@ _mesa_TexImage3D( GLenum target, GLint level, GLint
internalFormat,
width, height, depth,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- format, type);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
/* Give the texture to the driver. <pixels> may be null. */
ASSERT(ctx->Driver.TexImage3D);
@@ -2520,10 +2554,12 @@ _mesa_TexImage3D( GLenum target, GLint level, GLint
internalFormat,
}
else {
/* no error, set the tex image parameters */
+ struct gl_texture_object *texObj =
+ _mesa_get_current_tex_object(ctx, target);
_mesa_init_teximage_fields(ctx, target, texImage, width, height,
depth, border, internalFormat);
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat, format, type);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, format, type);
}
}
else {
@@ -2836,11 +2872,8 @@ _mesa_CopyTexImage1D( GLenum target, GLint level,
_mesa_init_teximage_fields(ctx, target, texImage, postConvWidth, 1, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- GL_NONE, GL_NONE);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
ASSERT(ctx->Driver.CopyTexImage1D);
ctx->Driver.CopyTexImage1D(ctx, target, level, internalFormat,
@@ -2917,11 +2950,8 @@ _mesa_CopyTexImage2D( GLenum target, GLint level, GLenum
internalFormat,
postConvWidth, postConvHeight, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- GL_NONE, GL_NONE);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
ASSERT(ctx->Driver.CopyTexImage2D);
ctx->Driver.CopyTexImage2D(ctx, target, level, internalFormat,
@@ -3446,11 +3476,8 @@ _mesa_CompressedTexImage1DARB(GLenum target, GLint level,
_mesa_init_teximage_fields(ctx, target, texImage, width, 1, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- GL_NONE, GL_NONE);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
ASSERT(ctx->Driver.CompressedTexImage1D);
ctx->Driver.CompressedTexImage1D(ctx, target, level,
@@ -3498,6 +3525,8 @@ _mesa_CompressedTexImage1DARB(GLenum target, GLint level,
texImage = _mesa_select_tex_image(ctx, texObj, target, level);
_mesa_init_teximage_fields(ctx, target, texImage, width, 1, 1,
border, internalFormat);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
}
_mesa_unlock_texture(ctx, texObj);
}
@@ -3573,11 +3602,8 @@ _mesa_CompressedTexImage2DARB(GLenum target, GLint level,
_mesa_init_teximage_fields(ctx, target, texImage, width, height, 1,
border, internalFormat);
- /* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- GL_NONE, GL_NONE);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
ASSERT(ctx->Driver.CompressedTexImage2D);
ctx->Driver.CompressedTexImage2D(ctx, target, level,
@@ -3627,6 +3653,8 @@ _mesa_CompressedTexImage2DARB(GLenum target, GLint level,
texImage = _mesa_select_tex_image(ctx, texObj, target, level);
_mesa_init_teximage_fields(ctx, target, texImage, width, height, 1,
border, internalFormat);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
}
_mesa_unlock_texture(ctx, texObj);
}
@@ -3683,10 +3711,8 @@ _mesa_CompressedTexImage3DARB(GLenum target, GLint level,
border, internalFormat);
/* Choose actual texture format */
- texImage->TexFormat =
- ctx->Driver.ChooseTextureFormat(ctx, internalFormat,
- GL_NONE, GL_NONE);
- ASSERT(texImage->TexFormat != MESA_FORMAT_NONE);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
ASSERT(ctx->Driver.CompressedTexImage3D);
ctx->Driver.CompressedTexImage3D(ctx, target, level,
@@ -3735,6 +3761,8 @@ _mesa_CompressedTexImage3DARB(GLenum target, GLint level,
texImage = _mesa_select_tex_image(ctx, texObj, target, level);
_mesa_init_teximage_fields(ctx, target, texImage, width, height,
depth, border, internalFormat);
+ _mesa_choose_texture_format(ctx, texObj, texImage, target, level,
+ internalFormat, GL_NONE, GL_NONE);
}
_mesa_unlock_texture(ctx, texObj);
}
diff --git a/src/mesa/main/teximage.h b/src/mesa/main/teximage.h
index d82cc98..0dcacab 100644
--- a/src/mesa/main/teximage.h
+++ b/src/mesa/main/teximage.h
@@ -73,6 +73,14 @@ _mesa_init_teximage_fields(GLcontext *ctx, GLenum target,
extern void
+_mesa_choose_texture_format(GLcontext *ctx,
+ struct gl_texture_object *texObj,
+ struct gl_texture_image *texImage,
+ GLenum target, GLint level,
+ GLenum internalFormat, GLenum format, GLenum type);
+
+
+extern void
_mesa_clear_texture_image(GLcontext *ctx, struct gl_texture_image *texImage);
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
