Fix flatten clip (#2432)

* fic gtk-doc typenames in cgif

* fix flatten clipping

flatten could produce out of range values if max_alpha was less than
the limit of the numeric range of the format

https://github.com/libvips/libvips/issues/2431

ChangeLog

@@ -6,6 +6,8 @@
 - add gifsave [lovell]
 - arrayjoin minimises inputs during sequential processing, saving a lot of 
   memory and file descriptors
+- add vips_image_get_format_max()
+- flatten handles out of range alpha and max_alpha correctly
 
 16/8/21 started 8.11.4
 - fix off-by-one error in new rank fast path

libvips/conversion/flatten.c

@@ -9,6 +9,8 @@
  * 	- add max_alpha to match vips_premultiply() etc.
  * 25/5/16
  * 	- max_alpha defaults to 65535 for RGB16/GREY16
+ * 12/9/21
+ * 	- out of range alpha and max_alpha correctly
  */
 
 /*
@@ -177,7 +179,7 @@ vips_flatten_black_gen( VipsRegion *or, void *vseq, void *a, void *b,
 		VipsPel *in = VIPS_REGION_ADDR( ir, r->left, r->top + y ); 
 		VipsPel *out = VIPS_REGION_ADDR( or, r->left, r->top + y ); 
 
-		switch( flatten->in->BandFmt ) { 
+		switch( ir->im->BandFmt ) { 
 		case VIPS_FORMAT_UCHAR: 
 			VIPS_FLATTEN_BLACK_INT( unsigned char ); 
 			break; 
@@ -242,7 +244,7 @@ vips_flatten_gen( VipsRegion *or, void *vseq, void *a, void *b,
 		VipsPel *in = VIPS_REGION_ADDR( ir, r->left, r->top + y ); 
 		VipsPel *out = VIPS_REGION_ADDR( or, r->left, r->top + y ); 
 
-		switch( flatten->in->BandFmt ) { 
+		switch( ir->im->BandFmt ) { 
 		case VIPS_FORMAT_UCHAR: 
 			VIPS_FLATTEN_INT( unsigned char ); 
 			break; 
@@ -285,18 +287,18 @@ vips_flatten_gen( VipsRegion *or, void *vseq, void *a, void *b,
 	return( 0 );
 }
 
-
 static int
 vips_flatten_build( VipsObject *object )
 {
 	VipsObjectClass *class = VIPS_OBJECT_GET_CLASS( object );
 	VipsConversion *conversion = VIPS_CONVERSION( object );
 	VipsFlatten *flatten = (VipsFlatten *) object;
-	VipsImage **t = (VipsImage **) vips_object_local_array( object, 1 );
+	VipsImage **t = (VipsImage **) vips_object_local_array( object, 4 );
 
 	VipsImage *in;
 	int i;
 	gboolean black;
+	VipsBandFormat original_format;
 
 	if( VIPS_OBJECT_CLASS( vips_flatten_parent_class )->build( object ) )
 		return( -1 );
@@ -315,12 +317,6 @@ vips_flatten_build( VipsObject *object )
 	if( vips_check_noncomplex( class->nickname, in ) )
 		return( -1 );
 
-	if( vips_image_pipelinev( conversion->out, 
-		VIPS_DEMAND_STYLE_THINSTRIP, in, NULL ) )
-		return( -1 );
-
-	conversion->out->Bands -= 1;
-
 	/* Is max-alpha unset? Default to the correct value for this
 	 * interpretation.
 	 */
@@ -329,37 +325,72 @@ vips_flatten_build( VipsObject *object )
 			in->Type == VIPS_INTERPRETATION_RGB16 )
 			flatten->max_alpha = 65535;
 
+	/* Is max_alpha less than the numeric range of this image? If it is,
+	 * we can get int overflow. 
+	 *
+	 * This is not a common case, so efficiency is not so important.
+	 * Cast to double, then cast back to the input type right at the end.
+	 */
+	original_format = VIPS_FORMAT_NOTSET;
+	if( vips_band_format_isint( in->BandFmt ) &&
+		flatten->max_alpha < 
+			vips_image_get_format_max( in->BandFmt ) ) {
+		original_format = in->BandFmt;
+		if( vips_cast( in, &t[1], VIPS_FORMAT_DOUBLE, NULL ) )
+			return( -1 );
+		in = t[1];
+	}
+
+	t[2] = vips_image_new();
+	if( vips_image_pipelinev( t[2], 
+		VIPS_DEMAND_STYLE_THINSTRIP, in, NULL ) )
+		return( -1 );
+	t[2]->Bands -= 1;
+
 	/* Is the background black? We have a special path for this.
 	 */
 	black = TRUE;
-	for( i = 0; i < VIPS_AREA( flatten->background )->n; i++ )
+	for( i = 0; i < VIPS_AREA( flatten->background )->n; i++ ) {
-		if( vips_array_double_get( flatten->background, NULL )[i] != 
+		const double *background = 
-			0.0 ) {
+			vips_array_double_get( flatten->background, NULL );
+
+		if( background[i] != 0.0 ) {
 			black = FALSE;
 			break;
 		}
+	}
 
 	if( black ) {
-		if( vips_image_generate( conversion->out,
+		if( vips_image_generate( t[2],
 			vips_start_one, vips_flatten_black_gen, vips_stop_one, 
 			in, flatten ) )
 			return( -1 );
+		in = t[2];
 	}
 	else {
 		/* Convert the background to the image's format.
 		 */
-		if( !(flatten->ink = vips__vector_to_ink( class->nickname, 
+		if( !(flatten->ink = vips__vector_to_ink( class->nickname, t[2],
-			conversion->out, 
 			VIPS_AREA( flatten->background )->data, NULL, 
 			VIPS_AREA( flatten->background )->n )) )
 			return( -1 );
 
-		if( vips_image_generate( conversion->out,
+		if( vips_image_generate( t[2],
 			vips_start_one, vips_flatten_gen, vips_stop_one, 
 			in, flatten ) )
 			return( -1 );
+		in = t[2];
 	}
 
+	if( original_format != VIPS_FORMAT_NOTSET ) {
+		if( vips_cast( in, &t[3], original_format, NULL ) )
+			return( -1 );
+		in = t[3];
+	}
+
+	if( vips_image_write( in, conversion->out ) )
+		return( -1 );
+
 	return( 0 );
 }
 

libvips/foreign/cgifsave.c

@@ -418,9 +418,9 @@ vips_foreign_save_cgif_buffer_init( VipsForeignSaveCgifBuffer *buffer )
  *
  * Optional arguments:
  *
- * * @dither: %double, quantisation dithering level
+ * * @dither: %gdouble, quantisation dithering level
- * * @effort: %int, quantisation CPU effort
+ * * @effort: %gint, quantisation CPU effort
- * * @bitdepth: %int, number of bits per pixel
+ * * @bitdepth: %gint, number of bits per pixel
  *
  * Write a VIPS image to a file as GIF.
  *
@@ -459,9 +459,9 @@ vips_gifsave( VipsImage *in, const char *filename, ... )
  *
  * Optional arguments:
  *
- * * @dither: %double, quantisation dithering level
+ * * @dither: %gdouble, quantisation dithering level
- * * @effort: %int, quantisation CPU effort
+ * * @effort: %gint, quantisation CPU effort
- * * @bitdepth: %int, number of bits per pixel
+ * * @bitdepth: %gint, number of bits per pixel
  *
  * As vips_gifsave(), but save to a memory buffer.
  *
@@ -509,9 +509,9 @@ vips_gifsave_buffer( VipsImage *in, void **buf, size_t *len, ... )
  *
  * Optional arguments:
  *
- * * @dither: %double, quantisation dithering level
+ * * @dither: %gdouble, quantisation dithering level
- * * @effort: %int, quantisation CPU effort
+ * * @effort: %gint, quantisation CPU effort
- * * @bitdepth: %int, number of bits per pixel
+ * * @bitdepth: %gint, number of bits per pixel
  *
  * As vips_gifsave(), but save to a target.
  *

libvips/include/vips/header.h

@@ -164,6 +164,7 @@ int vips_image_get_width( const VipsImage *image );
 int vips_image_get_height( const VipsImage *image );
 int vips_image_get_bands( const VipsImage *image );
 VipsBandFormat vips_image_get_format( const VipsImage *image );
+double vips_image_get_format_max( VipsBandFormat format );
 VipsBandFormat vips_image_guess_format( const VipsImage *image );
 VipsCoding vips_image_get_coding( const VipsImage *image );
 VipsInterpretation vips_image_get_interpretation( const VipsImage *image );

libvips/iofuncs/header.c

@@ -414,6 +414,47 @@ vips_image_get_format( const VipsImage *image )
 	return( image->BandFmt );
 }
 
+/**
+ * vips_image_get_format_max: (method)
+ * @image: image to get from
+ *
+ * Returns: the maximum numeric value possible for this format.
+ */
+double
+vips_image_get_format_max( VipsBandFormat format )
+{
+	switch( format ) {
+	case VIPS_FORMAT_UCHAR:
+		return( UCHAR_MAX );
+
+	case VIPS_FORMAT_CHAR:
+		return( SCHAR_MAX );
+
+	case VIPS_FORMAT_USHORT:
+		return( USHRT_MAX );
+
+	case VIPS_FORMAT_SHORT:
+		return( SHRT_MAX );
+
+	case VIPS_FORMAT_UINT:
+		return( UINT_MAX );
+
+	case VIPS_FORMAT_INT:
+		return( INT_MAX );
+
+	case VIPS_FORMAT_FLOAT:
+	case VIPS_FORMAT_COMPLEX:
+		return( FLT_MAX );
+
+	case VIPS_FORMAT_DOUBLE:
+	case VIPS_FORMAT_DPCOMPLEX:
+		return( DBL_MAX );
+
+	default:
+		return( -1 );
+	}
+}
+
 /**
  * vips_image_guess_format: (method)
  * @image: image to guess for

test/test-suite/test_conversion.py

@@ -8,7 +8,7 @@ import tempfile
 import shutil
 
 import pyvips
-from helpers import IMAGES, JPEG_FILE, unsigned_formats, \
+from helpers import IMAGES, JPEG_FILE, RGBA_FILE, unsigned_formats, \
     signed_formats, float_formats, int_formats, \
     noncomplex_formats, all_formats, max_value, \
     sizeof_format, rot45_angles, rot45_angle_bonds, \
@@ -378,6 +378,21 @@ class TestConversion:
             for x, y in zip(pixel, predict):
                 assert abs(x - y) < 2
 
+        # if the image has max_alpha less than the numeric range of the
+        # format, we can get out of range values ... check they are clipped
+        # correctly
+        rgba = pyvips.Image.new_from_file(RGBA_FILE)
+
+        im = rgba * 256
+        im = im.cast("ushort")
+        im = im.flatten()
+
+        im2 = rgba * 256
+        im2 = im2.flatten()
+        im2 = im2.cast("ushort")
+
+        assert(abs(im - im2).max() == 0)
+
     def test_premultiply(self):
         for fmt in unsigned_formats + [pyvips.BandFormat.SHORT,
                                        pyvips.BandFormat.INT] + float_formats: