From f9dc3177c738922997bbae6056b67a9b5569a603 Mon Sep 17 00:00:00 2001 From: Lovell Fuller Date: Fri, 4 Dec 2020 11:50:58 +0000 Subject: [PATCH 1/3] webpload: ensure first frame is not blended --- libvips/foreign/webp2vips.c | 1 + 1 file changed, 1 insertion(+) diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index 309130a8..c9518eec 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -692,6 +692,7 @@ read_next_frame( Read *read ) */ vips_image_paint_image( read->frame, frame, area.left, area.top, + read->iter.frame_num > 1 && read->iter.blend_method == WEBP_MUX_BLEND ); g_object_unref( frame ); From 6eaf1eda30cedb11be31a6f82401da0ebb019c68 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 4 Dec 2020 13:53:24 +0000 Subject: [PATCH 2/3] make webp frame blend do doround to nearest see https://github.com/libvips/libvips/pull/1918 --- ChangeLog | 1 + libvips/foreign/webp2vips.c | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 6d55c0cd..5498c33b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -15,6 +15,7 @@ - force binary mode on win for connection read / write [Alreiber] - better testing for output to target [barryspearce] - ppmload_source was missing is_a [ewelot] +- improve webpload rounding and blending behaviour [lovell] 6/9/20 started 8.10.2 - update magicksave/load profile handling [kelilevi] diff --git a/libvips/foreign/webp2vips.c b/libvips/foreign/webp2vips.c index c9518eec..e0e91e99 100644 --- a/libvips/foreign/webp2vips.c +++ b/libvips/foreign/webp2vips.c @@ -232,7 +232,7 @@ vips_image_paint_area( VipsImage *image, const VipsRect *r, const VipsPel *ink ) /* Blend two guint8. */ #define BLEND( X, aX, Y, aY, scale ) \ - ((X * aX + Y * aY) * scale >> 24) + (((X * aX + Y * aY) * scale + (1 << 12)) >> 24) /* Extract R, G, B, A, assuming little-endian. */ @@ -260,7 +260,7 @@ blend_pixel( guint32 A, guint32 B ) guint8 aB = getA( B ); - guint8 fac = (aB * (256 - aA)) >> 8; + guint8 fac = (aB * (256 - aA) + 128) >> 8; guint8 aR = aA + fac; int scale = (1 << 24) / aR; @@ -693,7 +693,7 @@ read_next_frame( Read *read ) vips_image_paint_image( read->frame, frame, area.left, area.top, read->iter.frame_num > 1 && - read->iter.blend_method == WEBP_MUX_BLEND ); + read->iter.blend_method == WEBP_MUX_BLEND ); g_object_unref( frame ); From 3996f3279bd72f30ea837afbb41c660e4ce18334 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 9 Dec 2020 15:14:59 +0000 Subject: [PATCH 3/3] 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 | 2 ++ libvips/conversion/cast.c | 36 +++++++++++++++++------------- test/test-suite/test_conversion.py | 20 +++++++++++++++++ 3 files changed, 42 insertions(+), 16 deletions(-) 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):