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,
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.