[Mesa-dev] [PATCH 06/20] mesa: Fix _mesa_swizzle_and_convert integer conversions to clamp properly
Jason Ekstrand
jason at jlekstrand.net
Tue Nov 18 10:34:59 PST 2014
In general, I like this patch. However, if we are going to claim to follow
the GL rule of "Colors are clampped to their representable range, then
there still seem to be quite a few cases missing. For instance, when going
from signed to unsigned 8-bit formats, we should should take a min with 0
and when converting from unsigned to signed, we should take a max with
0x7f. Maybe we need to add integer helper functions like we have for the
normalized ones and use those just to make sure we don't miss anything.
--Jason
On Tue, Nov 18, 2014 at 12:43 AM, Iago Toral Quiroga <itoral at igalia.com>
wrote:
> From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
>
> Fix various conversion paths that involved integer data types of different
> sizes (uint16_t to uint8_t, int16_t to uint8_t, etc) that were not
> being clamped properly.
>
> Also, one of the paths was incorrectly assigning the value 12, instead of
> 1,
> to the constant "one".
>
> Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> ---
> src/mesa/main/format_utils.c | 33 +++++++++++++++++----------------
> 1 file changed, 17 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/main/format_utils.c b/src/mesa/main/format_utils.c
> index b6d0fbc..8040173 100644
> --- a/src/mesa/main/format_utils.c
> +++ b/src/mesa/main/format_utils.c
> @@ -24,6 +24,7 @@
>
> #include "format_utils.h"
> #include "glformats.h"
> +#include "macros.h"
>
> static const uint8_t map_identity[7] = { 0, 1, 2, 3, 4, 5, 6 };
> static const uint8_t map_3210[7] = { 3, 2, 1, 0, 4, 5, 6 };
> @@ -593,28 +594,28 @@ convert_ubyte(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(uint8_t, uint16_t, unorm_to_unorm(src, 16, 8));
> } else {
> - SWIZZLE_CONVERT(uint8_t, uint16_t, src);
> + SWIZZLE_CONVERT(uint8_t, uint16_t, MIN2(src, 0xff));
> }
> break;
> case GL_SHORT:
> if (normalized) {
> SWIZZLE_CONVERT(uint8_t, int16_t, snorm_to_unorm(src, 16, 8));
> } else {
> - SWIZZLE_CONVERT(uint8_t, int16_t, (src < 0) ? 0 : src);
> + SWIZZLE_CONVERT(uint8_t, int16_t, CLAMP(src, 0, 0xff));
> }
> break;
> case GL_UNSIGNED_INT:
> if (normalized) {
> SWIZZLE_CONVERT(uint8_t, uint32_t, unorm_to_unorm(src, 32, 8));
> } else {
> - SWIZZLE_CONVERT(uint8_t, uint32_t, src);
> + SWIZZLE_CONVERT(uint8_t, uint32_t, MIN2(src, 0xff));
> }
> break;
> case GL_INT:
> if (normalized) {
> SWIZZLE_CONVERT(uint8_t, int32_t, snorm_to_unorm(src, 32, 8));
> } else {
> - SWIZZLE_CONVERT(uint8_t, int32_t, (src < 0) ? 0 : src);
> + SWIZZLE_CONVERT(uint8_t, int32_t, CLAMP(src, 0, 0xff));
> }
> break;
> default:
> @@ -649,7 +650,7 @@ convert_byte(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(int8_t, uint8_t, unorm_to_snorm(src, 8, 8));
> } else {
> - SWIZZLE_CONVERT(int8_t, uint8_t, src);
> + SWIZZLE_CONVERT(int8_t, uint8_t, MIN2(src, 0x7f));
> }
> break;
> case GL_BYTE:
> @@ -659,28 +660,28 @@ convert_byte(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(int8_t, uint16_t, unorm_to_snorm(src, 16, 8));
> } else {
> - SWIZZLE_CONVERT(int8_t, uint16_t, src);
> + SWIZZLE_CONVERT(int8_t, uint16_t, MIN2(src, 0x7f));
> }
> break;
> case GL_SHORT:
> if (normalized) {
> SWIZZLE_CONVERT(int8_t, int16_t, snorm_to_snorm(src, 16, 8));
> } else {
> - SWIZZLE_CONVERT(int8_t, int16_t, src);
> + SWIZZLE_CONVERT(int8_t, int16_t, CLAMP(src, -0x80, 0x7f));
> }
> break;
> case GL_UNSIGNED_INT:
> if (normalized) {
> SWIZZLE_CONVERT(int8_t, uint32_t, unorm_to_snorm(src, 32, 8));
> } else {
> - SWIZZLE_CONVERT(int8_t, uint32_t, src);
> + SWIZZLE_CONVERT(int8_t, uint32_t, MIN2(src, 0x7f));
> }
> break;
> case GL_INT:
> if (normalized) {
> SWIZZLE_CONVERT(int8_t, int32_t, snorm_to_snorm(src, 32, 8));
> } else {
> - SWIZZLE_CONVERT(int8_t, int32_t, src);
> + SWIZZLE_CONVERT(int8_t, int32_t, CLAMP(src, -0x80, 0x7f));
> }
> break;
> default:
> @@ -739,14 +740,14 @@ convert_ushort(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(uint16_t, uint32_t, unorm_to_unorm(src, 32, 16));
> } else {
> - SWIZZLE_CONVERT(uint16_t, uint32_t, src);
> + SWIZZLE_CONVERT(uint16_t, uint32_t, MIN2(src, 0xffff));
> }
> break;
> case GL_INT:
> if (normalized) {
> SWIZZLE_CONVERT(uint16_t, int32_t, snorm_to_unorm(src, 32, 16));
> } else {
> - SWIZZLE_CONVERT(uint16_t, int32_t, (src < 0) ? 0 : src);
> + SWIZZLE_CONVERT(uint16_t, int32_t, CLAMP(src, 0, 0xffff));
> }
> break;
> default:
> @@ -795,7 +796,7 @@ convert_short(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(int16_t, uint16_t, unorm_to_snorm(src, 16, 16));
> } else {
> - SWIZZLE_CONVERT(int16_t, uint16_t, src);
> + SWIZZLE_CONVERT(int16_t, uint16_t, (src < 0) ? 0 : src);
> }
> break;
> case GL_SHORT:
> @@ -805,14 +806,14 @@ convert_short(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(int16_t, uint32_t, unorm_to_snorm(src, 32, 16));
> } else {
> - SWIZZLE_CONVERT(int16_t, uint32_t, src);
> + SWIZZLE_CONVERT(int16_t, uint32_t, MIN2(src, 0x7fff));
> }
> break;
> case GL_INT:
> if (normalized) {
> SWIZZLE_CONVERT(int16_t, int32_t, snorm_to_snorm(src, 32, 16));
> } else {
> - SWIZZLE_CONVERT(int16_t, int32_t, src);
> + SWIZZLE_CONVERT(int16_t, int32_t, CLAMP(src, -0x8000, 0x7fff));
> }
> break;
> default:
> @@ -891,7 +892,7 @@ convert_int(void *void_dst, int num_dst_channels,
> const void *void_src, GLenum src_type, int num_src_channels,
> const uint8_t swizzle[4], bool normalized, int count)
> {
> - const int32_t one = normalized ? INT32_MAX : 12;
> + const int32_t one = normalized ? INT32_MAX : 1;
>
Good catch!
>
> switch (src_type) {
> case GL_FLOAT:
> @@ -940,7 +941,7 @@ convert_int(void *void_dst, int num_dst_channels,
> if (normalized) {
> SWIZZLE_CONVERT(int32_t, uint32_t, unorm_to_snorm(src, 32, 32));
> } else {
> - SWIZZLE_CONVERT(int32_t, uint32_t, src);
> + SWIZZLE_CONVERT(int32_t, uint32_t, MIN2(src, 0x7fffffff));
> }
> break;
> case GL_INT:
> --
> 1.9.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141118/f33c7398/attachment.html>
More information about the mesa-dev
mailing list