| Summary: | texstore_argb888 calls swizzel_ubyte with wrong source format | ||
|---|---|---|---|
| Product: | Mesa | Reporter: | Felix Kühling <fxkuehl> |
| Component: | Mesa core | Assignee: | mesa-dev |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | high | ||
| Version: | git | ||
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
I'm afraid this patch won't work. The srcFormat parameter to _mesa_swizzle_ubyte_image() _must_ be the user-provided format parameter since it's used to compute texel addresses in the incoming image. For example, if the user's image format is GL_RGBA and the internalFormat were GL_LUMINANCE, using GL_LUMINANCE instead of GL_RGBA for srcFormat would result in bad calls to _mesa_image_row_stride(), etc. Perhaps you can take a closer look at this. (In reply to comment #1) > I'm afraid this patch won't work. The srcFormat parameter to > _mesa_swizzle_ubyte_image() _must_ be the user-provided format parameter since > it's used to compute texel addresses in the incoming image. For example, if the > user's image format is GL_RGBA and the internalFormat were GL_LUMINANCE, using > GL_LUMINANCE instead of GL_RGBA for srcFormat would result in bad calls to > _mesa_image_row_stride(), etc. > > Perhaps you can take a closer look at this. Ah, I thought the user's image format would be sufficiently specified by srcType, but looking at _mesa_bytes_per_pixel the format does matter with type GL_UNSIGNED_BYTE (and others). :-( Maybe swizzle_ubyte needs an extra parameter for the baseInternalFormat. I'll take a closer look soon. The problem is that _mesa_swizzle_ubyte_image and compute_component_mapping
sometimes have to deal with 3 different texture formats (in parantheses
restrictions that apply to the swizzle-paths):
A) source format (always bytes, varying numbers of components)
B) base internal format
C) destination hardware format (rgba8888 or argb8888)
But compute_component_mapping only considers two formats: source and destination.
Three cases as to the combinations of formats can be distinguished:
A == B: promotion from a user image in the same format as internalFormat to a
RGBA hardware format. _mesa_swizzle_ubyte_image seems to handle this case correctly.
B == C: promotion from non-RGBA user image to internalFormat RGBA.
_mesa_swizzle_ubyte_image seems to handle this case correctly.
A != B && B != C: Two promotions at once: from user image to a different
non-RGBA internalFormat and then further to a RGBA hardware format.
_mesa_swizzle_ubyte_image and compute_component_mapping can't handle this case
properly because they don't know about B. They could be made to, but that would
increase complexity for a questionable benifit. Instead I propose to fall back
to the general path in this case:
--- ./texstore.c.~1.91.~ 2005-02-09 02:44:44.000000000 +0100
+++ ./texstore.c 2005-04-27 22:15:51.000000000 +0200
@@ -1069,6 +1069,8 @@
srcType == GL_UNSIGNED_BYTE &&
dstFormat == &_mesa_texformat_rgba8888 &&
littleEndian &&
+ (srcFormat == baseInternalFormat ||
+ baseInternalFormat == GL_RGBA) &&
can_swizzle(srcFormat)) {
GLubyte dstmap[4];
@@ -1242,6 +1244,8 @@
dstFormat == &_mesa_texformat_argb8888 &&
srcType == GL_UNSIGNED_BYTE &&
littleEndian &&
+ (srcFormat == baseInternalFormat ||
+ baseInternalFormat == GL_RGBA) &&
can_swizzle(srcFormat)) {
GLubyte dstmap[4];
That sounds fine. Feel free to check in that change and close this bug. I committed the last version of the fix + short explanations to CVS. Closing this bug now. Mass version move, cvs -> git |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
In _mesa_texstore_argb8888 _mesa_swizzel_ubyte_image is called with srcFormat which comes directly from the application. This can lead to wrong results when texture formats are promoted. For example demos/texenv can call glTexImage2D with format=GL_LUMINANCE and internalFormat=GL_INTENSITY8. In this case _mesa_texstore_argb8888 will call _mesa_swizzel_ubyte_image with logicalBaseFormat=GL_LUMINANCE, which will lead to alpha=0xff in the resulting argb image. I think it it should use baseInternalFormat instead of srcFormat. Here is a trivial patch that fixes demos/texenv with GL_INTENSITY8 promoted to ARGB8888: --- ./texstore.c.~1.91.~ 2005-02-09 02:44:44.000000000 +0100 +++ ./texstore.c 2005-04-23 01:23:53.000000000 +0200 @@ -1069,7 +1069,7 @@ srcType == GL_UNSIGNED_BYTE && dstFormat == &_mesa_texformat_rgba8888 && littleEndian && - can_swizzle(srcFormat)) { + can_swizzle(baseInternalFormat)) { GLubyte dstmap[4]; /* dstmap - how to swizzle from GL_RGBA to dst format: @@ -1082,7 +1082,7 @@ dstmap[0] = 3; _mesa_swizzle_ubyte_image(ctx, dims, - srcFormat, + baseInternalFormat, dstmap, 4, dstAddr, dstXoffset, dstYoffset, dstZoffset, dstRowStride, dstImageStride, @@ -1242,7 +1242,7 @@ dstFormat == &_mesa_texformat_argb8888 && srcType == GL_UNSIGNED_BYTE && littleEndian && - can_swizzle(srcFormat)) { + can_swizzle(baseInternalFormat)) { GLubyte dstmap[4]; @@ -1254,7 +1254,7 @@ dstmap[0] = 2; /* blue */ _mesa_swizzle_ubyte_image(ctx, dims, - srcFormat, + baseInternalFormat, dstmap, 4, dstAddr, dstXoffset, dstYoffset, dstZoffset, dstRowStride, dstImageStride,