<div dir="ltr"><div>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. </div>--Jason <div><div><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 18, 2014 at 12:43 AM, Iago Toral Quiroga <<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">From: Samuel Iglesias Gonsalvez <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> 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 <<a href="mailto:siglesias@igalia.com">siglesias@igalia.com</a>> --- 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; </blockquote><div> </div><div>Good catch! </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <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> </div></div></div></div>