Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> <div class="gmail_quote">On Feb 13, 2015 3:56 AM, "Iago Toral Quiroga" <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Some old format conversion code in pack.c implemented byte-swapping like this: GLint comps = _mesa_components_in_format(dstFormat); GLint swapSize = _mesa_sizeof_packed_type(dstType); if (swapSize == 2) _mesa_swap2((GLushort *) dstAddr, n * comps); else if (swapSize == 4) _mesa_swap4((GLuint *) dstAddr, n * comps); where n is the pixel count. But this is incorrect for packed formats, where _mesa_sizeof_packed_type is already returning the size of a pixel instead of the size of a single component, so multiplying this by the number of components in the format results in a larger element count for _mesa_swap than we want. Unfortunately, we followed the same implementation for byte-swapping in the rewrite of the format conversion code for texstore, readpixels and texgetimage. This patch computes the correct element counts for _mesa_swap calls by computing the bytes per pixel in the image and dividing that by the swap size to obtain the number of swaps required per pixel. Then multiplies that by the number of pixels in the image to obtain the swap count that we need to use. Also, when handling byte-swapping in texstore_rgba, we were ignoring the image's depth. This patch fixes this too. --- src/mesa/main/readpix.c | 13 ++++++++----- src/mesa/main/texgetimage.c | 13 ++++++++----- src/mesa/main/texstore.c | 14 +++++++++----- 3 files changed, 25 insertions(+), 15 deletions(-) diff --git a/src/mesa/main/readpix.c b/src/mesa/main/readpix.c index 85f900d..ca4b943 100644 --- a/src/mesa/main/readpix.c +++ b/src/mesa/main/readpix.c @@ -605,12 +605,15 @@ read_rgba_pixels( struct gl_context *ctx, done_swap: /* Handle byte swapping if required */ if (packing->SwapBytes) { - int components = _mesa_components_in_format(format); GLint swapSize = _mesa_sizeof_packed_type(type); - if (swapSize == 2) - _mesa_swap2((GLushort *) dst, width * height * components); - else if (swapSize == 4) - _mesa_swap4((GLuint *) dst, width * height * components); + if (swapSize == 2 || swapSize == 4) { + int swapsPerPixel = _mesa_bytes_per_pixel(format, type) / swapSize; + assert(_mesa_bytes_per_pixel(format, type) % swapSize == 0); + if (swapSize == 2) + _mesa_swap2((GLushort *) dst, width * height * swapsPerPixel); + else if (swapSize == 4) + _mesa_swap4((GLuint *) dst, width * height * swapsPerPixel); + } } done_unmap: diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c index ee465e6..405f085 100644 --- a/src/mesa/main/texgetimage.c +++ b/src/mesa/main/texgetimage.c @@ -511,12 +511,15 @@ get_tex_rgba_uncompressed(struct gl_context *ctx, GLuint dimensions, do_swap: /* Handle byte swapping if required */ if (ctx->Pack.SwapBytes) { - int components = _mesa_components_in_format(format); GLint swapSize = _mesa_sizeof_packed_type(type); - if (swapSize == 2) - _mesa_swap2((GLushort *) dest, width * height * components); - else if (swapSize == 4) - _mesa_swap4((GLuint *) dest, width * height * components); + if (swapSize == 2 || swapSize == 4) { + int swapsPerPixel = _mesa_bytes_per_pixel(format, type) / swapSize; + assert(_mesa_bytes_per_pixel(format, type) % swapSize == 0); + if (swapSize == 2) + _mesa_swap2((GLushort *) dest, width * height * swapsPerPixel); + else if (swapSize == 4) + _mesa_swap4((GLuint *) dest, width * height * swapsPerPixel); + } } /* Unmap the src texture buffer */ diff --git a/src/mesa/main/texstore.c b/src/mesa/main/texstore.c index 4d32659..227693d 100644 --- a/src/mesa/main/texstore.c +++ b/src/mesa/main/texstore.c @@ -721,15 +721,19 @@ texstore_rgba(TEXSTORE_PARAMS) */ GLint swapSize = _mesa_sizeof_packed_type(srcType); if (swapSize == 2 || swapSize == 4) { - int components = _mesa_components_in_format(srcFormat); - int elementCount = srcWidth * srcHeight * components; - tempImage = malloc(elementCount * swapSize); + int bytesPerPixel = _mesa_bytes_per_pixel(srcFormat, srcType); + assert(bytesPerPixel % swapSize == 0); + int swapsPerPixel = bytesPerPixel / swapSize; + int elementCount = srcWidth * srcHeight * srcDepth; + tempImage = malloc(elementCount * bytesPerPixel); if (!tempImage) return GL_FALSE; if (swapSize == 2) - _mesa_swap2_copy(tempImage, (GLushort *) srcAddr, elementCount); + _mesa_swap2_copy(tempImage, (GLushort *) srcAddr, + elementCount * swapsPerPixel); else - _mesa_swap4_copy(tempImage, (GLuint *) srcAddr, elementCount); + _mesa_swap4_copy(tempImage, (GLuint *) srcAddr, + elementCount * swapsPerPixel); srcAddr = tempImage; } } -- 1.9.1 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div>