[Mesa-dev] [PATCH] mesa: Fix element count for byte-swaps in texstore, readpix and texgetimage

[Iago Toral Quiroga]

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