fix range clips for casts to and from int

Fix two bugs:

- clip in casts from int32 and uint32 could overflow -- do these as gint64 now

- clip in casts from float to int could overflow since float32 can't
  represent the full range of int32 without losing precision -- do these
  as double

And add some more tests.

Thanks ewelot.

see https://github.com/libvips/libvips/issues/1922

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]

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
+/* Cast down from an int.
- * clip the range. X is 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
+/* These cast down from gint64 to uint32 or int32. 
- * 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.
  */
-#define CAST_UINT( X ) VIPS_MAX( 0, (X) )
+#define CAST_UINT( X ) VIPS_CLIP( 0, (X), UINT_MAX )
-#define CAST_INT( X ) (X)
+#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: \

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):