diff --git a/ChangeLog b/ChangeLog index 5498c33b..7deeeb98 100644 --- a/ChangeLog +++ b/ChangeLog @@ -16,6 +16,8 @@ - better testing for output to target [barryspearce] - ppmload_source was missing is_a [ewelot] - improve webpload rounding and blending behaviour [lovell] +- fix range clip in int32 -> unsigned casts [ewelot] +- fix precision error in clip of float -> int casts [ewelot] 6/9/20 started 8.10.2 - update magicksave/load profile handling [kelilevi] diff --git a/libvips/conversion/cast.c b/libvips/conversion/cast.c index 3efb0918..69591307 100644 --- a/libvips/conversion/cast.c +++ b/libvips/conversion/cast.c @@ -54,6 +54,8 @@ * 14/11/18 * - revise for better uint/int clipping [erdmann] * - remove old overflow/underflow detect + * 8/12/20 + * - fix range clip in int32 -> unsigned casts [ewelot] */ /* @@ -116,21 +118,17 @@ typedef VipsConversionClass VipsCastClass; G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); -/* Various saturated casts. We want to cast int to uchar, for example, and - * clip the range. X is an int. +/* Cast down from an int. */ - #define CAST_UCHAR( X ) VIPS_CLIP( 0, (X), UCHAR_MAX ) #define CAST_CHAR( X ) VIPS_CLIP( SCHAR_MIN, (X), SCHAR_MAX ) #define CAST_USHORT( X ) VIPS_CLIP( 0, (X), USHRT_MAX ) #define CAST_SHORT( X ) VIPS_CLIP( SHRT_MIN, (X), SHRT_MAX ) -/* We know the source cannot be the same as the dest, so we will only use - * CAST_UINT() for an INT source, and vice versa. We don't need to clip to - * INT_MAX, since float->int does that for us. +/* These cast down from gint64 to uint32 or int32. */ -#define CAST_UINT( X ) VIPS_MAX( 0, (X) ) -#define CAST_INT( X ) (X) +#define CAST_UINT( X ) VIPS_CLIP( 0, (X), UINT_MAX ) +#define CAST_INT( X ) VIPS_CLIP( INT_MIN, (X), INT_MAX ) /* Rightshift an integer type, ie. sizeof(ITYPE) > sizeof(OTYPE). */ @@ -146,7 +144,7 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); } /* Leftshift an integer type, ie. sizeof(ITYPE) < sizeof(OTYPE). We need to - * copy the bottom bit up into the fresh new bits + * copy the bottom bit up into the fresh new bits. */ #define SHIFT_LEFT( ITYPE, OTYPE ) { \ ITYPE * restrict p = (ITYPE *) in; \ @@ -172,7 +170,7 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); } /* Cast int types to an int type. We need to pass in the type of the - * intermediate value, either uint or int, or we'll have problems with uint + * intermediate value, either int or int64, or we'll have problems with uint * sources turning -ve. */ #define CAST_INT_INT( ITYPE, OTYPE, TEMP, CAST ) { \ @@ -217,23 +215,29 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); } /* Cast float types to an int type. + * + * We need to do the range clip as double or we'll get errors for int max, + * since that can't be represented as a 32-bit float. */ #define CAST_FLOAT_INT( ITYPE, OTYPE, TEMP, CAST ) { \ ITYPE * restrict p = (ITYPE *) in; \ OTYPE * restrict q = (OTYPE *) out; \ \ for( x = 0; x < sz; x++ ) \ - q[x] = CAST( p[x] ); \ + q[x] = CAST( (double) p[x] ); \ } /* Cast complex types to an int type. Just take the real part. + * + * We need to do the range clip as double or we'll get errors for int max, + * since that can't be represented as a 32-bit float. */ #define CAST_COMPLEX_INT( ITYPE, OTYPE, TEMP, CAST ) { \ ITYPE * restrict p = (ITYPE *) in; \ OTYPE * restrict q = (OTYPE *) out; \ \ for( x = 0; x < sz; x++ ) { \ - q[x] = CAST( p[0] ); \ + q[x] = CAST( (double) p[0] ); \ p += 2; \ } \ } @@ -290,7 +294,7 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); #define BAND_SWITCH_INNER( ITYPE, INT, FLOAT, COMPLEX ) { \ switch( conversion->out->BandFmt ) { \ case VIPS_FORMAT_UCHAR: \ - INT( ITYPE, unsigned char, unsigned int, CAST_UCHAR ); \ + INT( ITYPE, unsigned char, int, CAST_UCHAR ); \ break; \ \ case VIPS_FORMAT_CHAR: \ @@ -298,7 +302,7 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); break; \ \ case VIPS_FORMAT_USHORT: \ - INT( ITYPE, unsigned short, unsigned int, CAST_USHORT ); \ + INT( ITYPE, unsigned short, int, CAST_USHORT ); \ break; \ \ case VIPS_FORMAT_SHORT: \ @@ -306,11 +310,11 @@ G_DEFINE_TYPE( VipsCast, vips_cast, VIPS_TYPE_CONVERSION ); break; \ \ case VIPS_FORMAT_UINT: \ - INT( ITYPE, unsigned int, unsigned int, CAST_UINT ); \ + INT( ITYPE, unsigned int, gint64, CAST_UINT ); \ break; \ \ case VIPS_FORMAT_INT: \ - INT( ITYPE, signed int, int, CAST_INT ); \ + INT( ITYPE, signed int, gint64, CAST_INT ); \ break; \ \ case VIPS_FORMAT_FLOAT: \ diff --git a/test/test-suite/test_conversion.py b/test/test-suite/test_conversion.py index 94b29b90..033f9006 100644 --- a/test/test-suite/test_conversion.py +++ b/test/test-suite/test_conversion.py @@ -60,6 +60,26 @@ class TestConversion: cls.image = None cls.all_images = None + def test_cast(self): + # casting negative pixels to an unsigned format should clip to zero + for signed in signed_formats: + im = (pyvips.Image.black(1, 1) - 10).cast(signed) + for unsigned in unsigned_formats: + im2 = im.cast(unsigned) + assert im2.avg() == 0 + + # casting very positive pixels to a signed format should clip to max + im = (pyvips.Image.black(1, 1) + max_value["uint"]).cast("uint") + assert im.avg() == max_value["uint"] + im2 = im.cast("int") + assert im2.avg() == max_value["int"] + im = (pyvips.Image.black(1, 1) + max_value["ushort"]).cast("ushort") + im2 = im.cast("short") + assert im2.avg() == max_value["short"] + im = (pyvips.Image.black(1, 1) + max_value["uchar"]).cast("uchar") + im2 = im.cast("char") + assert im2.avg() == max_value["char"] + def test_band_and(self): def band_and(x): if isinstance(x, pyvips.Image):