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

[Ian Romanick]

Also... please tag the commit with

Cc: "10.5" <mesa-stable at lists.freedesktop.org>

On 02/13/2015 03:56 AM, Iago Toral Quiroga wrote:
> 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;
>        }
>     }
>