From 3019e5966b076cc890c1ea02169188a25a0010fc Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 8 May 2017 13:28:23 +0100 Subject: [PATCH] vips_conv*() default to float we had INT as the default, but this will cause serious precision loss with many masks ... instead, have float (always correct) as the default and let people turn on int if they cn --- ChangeLog | 3 ++- TODO | 2 ++ libvips/conversion/smartcrop.c | 8 ++++++-- libvips/convolution/compass.c | 7 +++++-- libvips/convolution/conv.c | 20 ++++++++++++-------- libvips/convolution/convsep.c | 7 +++++-- libvips/convolution/sharpen.c | 4 +++- libvips/deprecated/vips7compat.c | 20 ++++++++++++++------ libvips/histogram/hist_ismonotonic.c | 4 +++- libvips/morphology/countlines.c | 8 ++++++-- 10 files changed, 58 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9ca27c60..45886456 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,10 +4,11 @@ constant image - add new_from_image() to Python as well - slight change to cpp new_from_image() to match py/C behaviour +- vips_conv(), vips_compass(), vips_convsep() default to FLOAT precision 23/4/17 started 8.5.5 - doc polishing -- more improvements for tuncated PNG files, thanks juyunsang +- more improvements for truncated PNG files, thanks juyunsang 23/4/17 started 8.5.4 - don't depend on image width when setting n_lines, thanks kleisauke diff --git a/TODO b/TODO index 95698e30..989041b5 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,5 @@ +- vips_compass() needs docs + - cpp bandjoin should use bandjoin_const() where possibel ... currently uses new_from_image diff --git a/libvips/conversion/smartcrop.c b/libvips/conversion/smartcrop.c index be5a67e9..8c04eb76 100644 --- a/libvips/conversion/smartcrop.c +++ b/libvips/conversion/smartcrop.c @@ -193,8 +193,12 @@ vips_smartcrop_attention( VipsSmartcrop *smartcrop, /* Sobel edge-detect on L. */ if( vips_extract_band( t[1], &t[2], 0, NULL ) || - vips_conv( t[2], &t[3], t[21], NULL ) || - vips_conv( t[2], &t[4], t[22], NULL ) || + vips_conv( t[2], &t[3], t[21], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || + vips_conv( t[2], &t[4], t[22], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_abs( t[3], &t[5], NULL ) || vips_abs( t[4], &t[6], NULL ) || vips_add( t[5], t[6], &t[7], NULL ) ) diff --git a/libvips/convolution/compass.c b/libvips/convolution/compass.c index f7f01870..bc140a76 100644 --- a/libvips/convolution/compass.c +++ b/libvips/convolution/compass.c @@ -2,6 +2,9 @@ * * 23/10/13 * - from vips_conv() + * 8/5/17 + * - default to float ... int will often lose precision and should not be + * the default */ /* @@ -172,7 +175,7 @@ vips_compass_class_init( VipsCompassClass *class ) _( "Convolve with this precision" ), VIPS_ARGUMENT_OPTIONAL_INPUT, G_STRUCT_OFFSET( VipsCompass, precision ), - VIPS_TYPE_PRECISION, VIPS_PRECISION_INTEGER ); + VIPS_TYPE_PRECISION, VIPS_PRECISION_FLOAT ); VIPS_ARG_INT( class, "layers", 204, _( "Layers" ), @@ -196,7 +199,7 @@ vips_compass_init( VipsCompass *compass ) compass->times = 2; compass->angle = VIPS_ANGLE45_D90; compass->combine = VIPS_COMBINE_MAX; - compass->precision = VIPS_PRECISION_INTEGER; + compass->precision = VIPS_PRECISION_FLOAT; compass->layers = 5; compass->cluster = 1; } diff --git a/libvips/convolution/conv.c b/libvips/convolution/conv.c index 1a49290c..1be0d46f 100644 --- a/libvips/convolution/conv.c +++ b/libvips/convolution/conv.c @@ -2,6 +2,9 @@ * * 12/8/13 * - from vips_hist_cum() + * 8/5/17 + * - default to float ... int will often lose precision and should not be + * the default */ /* @@ -137,7 +140,7 @@ vips_conv_class_init( VipsConvClass *class ) _( "Convolve with this precision" ), VIPS_ARGUMENT_OPTIONAL_INPUT, G_STRUCT_OFFSET( VipsConv, precision ), - VIPS_TYPE_PRECISION, VIPS_PRECISION_INTEGER ); + VIPS_TYPE_PRECISION, VIPS_PRECISION_FLOAT ); VIPS_ARG_INT( class, "layers", 104, _( "Layers" ), @@ -158,7 +161,7 @@ vips_conv_class_init( VipsConvClass *class ) static void vips_conv_init( VipsConv *conv ) { - conv->precision = VIPS_PRECISION_INTEGER; + conv->precision = VIPS_PRECISION_FLOAT; conv->layers = 5; conv->cluster = 1; } @@ -187,22 +190,23 @@ vips_conv_init( VipsConv *conv ) * * where scale and offset are part of @mask. * + * By default, @precision is + * #VIPS_PRECISION_FLOAT. The output image + * is always #VIPS_FORMAT_FLOAT unless @in is #VIPS_FORMAT_DOUBLE, in which case + * @out is also #VIPS_FORMAT_DOUBLE. + * * If @precision is #VIPS_PRECISION_INTEGER, then * elements of @mask are converted to * integers before convolution, using rint(), * and the output image * always has the same #VipsBandFormat as the input image. * - * For #VIPS_FORMAT_UCHAR images, vips_conv() uses a fast vector path based on + * For #VIPS_FORMAT_UCHAR images and #VIPS_PRECISION_INTEGER @precision, + * vips_conv() uses a fast vector path based on * fixed-point arithmetic. This can produce slightly different results. * Disable the vector path with `--vips-novector` or `VIPS_NOVECTOR` or * vips_vector_set_enabled(). * - * If @precision is #VIPS_PRECISION_FLOAT then the convolution is performed - * with floating-point arithmetic. The output image - * is always #VIPS_FORMAT_FLOAT unless @in is #VIPS_FORMAT_DOUBLE, in which case - * @out is also #VIPS_FORMAT_DOUBLE. - * * If @precision is #VIPS_PRECISION_APPROXIMATE then, like * #VIPS_PRECISION_INTEGER, @mask is converted to int before convolution, and * the output image diff --git a/libvips/convolution/convsep.c b/libvips/convolution/convsep.c index 3ae4fbf1..33f95008 100644 --- a/libvips/convolution/convsep.c +++ b/libvips/convolution/convsep.c @@ -2,6 +2,9 @@ * * 23/10/13 * - from vips_convsep() + * 8/5/17 + * - default to float ... int will often lose precision and should not be + * the default */ /* @@ -128,7 +131,7 @@ vips_convsep_class_init( VipsConvsepClass *class ) _( "Convolve with this precision" ), VIPS_ARGUMENT_OPTIONAL_INPUT, G_STRUCT_OFFSET( VipsConvsep, precision ), - VIPS_TYPE_PRECISION, VIPS_PRECISION_INTEGER ); + VIPS_TYPE_PRECISION, VIPS_PRECISION_FLOAT ); VIPS_ARG_INT( class, "layers", 204, _( "Layers" ), @@ -149,7 +152,7 @@ vips_convsep_class_init( VipsConvsepClass *class ) static void vips_convsep_init( VipsConvsep *convsep ) { - convsep->precision = VIPS_PRECISION_INTEGER; + convsep->precision = VIPS_PRECISION_FLOAT; convsep->layers = 5; convsep->cluster = 1; } diff --git a/libvips/convolution/sharpen.c b/libvips/convolution/sharpen.c index a95242fb..ab4c78ac 100644 --- a/libvips/convolution/sharpen.c +++ b/libvips/convolution/sharpen.c @@ -264,7 +264,9 @@ vips_sharpen_build( VipsObject *object ) */ if( vips_extract_band( in, &args[0], 0, NULL ) || vips_extract_band( in, &t[3], 1, "n", in->Bands - 1, NULL ) || - vips_convsep( args[0], &args[1], t[1], NULL ) ) + vips_convsep( args[0], &args[1], t[1], + "precision", VIPS_PRECISION_INTEGER, + NULL ) ) return( -1 ); /* Set demand hints. FATSTRIP is good for us, as THINSTRIP will cause diff --git a/libvips/deprecated/vips7compat.c b/libvips/deprecated/vips7compat.c index db23ec21..9420a608 100644 --- a/libvips/deprecated/vips7compat.c +++ b/libvips/deprecated/vips7compat.c @@ -2283,6 +2283,7 @@ im_compass( VipsImage *in, VipsImage *out, INTMASK *mask ) if( vips_compass( in, &t2, t1, "times", 8, "angle", VIPS_ANGLE45_D45, + "precision", VIPS_PRECISION_INTEGER, NULL ) ) { g_object_unref( t1 ); return( -1 ); @@ -2308,6 +2309,7 @@ im_lindetect( IMAGE *in, IMAGE *out, INTMASK *mask ) if( vips_compass( in, &t2, t1, "times", 4, "angle", VIPS_ANGLE45_D45, + "precision", VIPS_PRECISION_INTEGER, NULL ) ) { g_object_unref( t1 ); return( -1 ); @@ -2334,6 +2336,7 @@ im_gradient( IMAGE *in, IMAGE *out, INTMASK *mask ) "times", 2, "angle", VIPS_ANGLE45_D90, "combine", VIPS_COMBINE_SUM, + "precision", VIPS_PRECISION_INTEGER, NULL ) ) { g_object_unref( t1 ); return( -1 ); @@ -2364,6 +2367,7 @@ im_convsep( IMAGE *in, IMAGE *out, INTMASK *mask ) im_imask2vips( mask, t1 ) ) return( -1 ); if( vips_convsep( in, &t2, t1, + "precision", VIPS_PRECISION_INTEGER, NULL ) ) { g_object_unref( t1 ); return( -1 ); @@ -2393,9 +2397,7 @@ im_convsep_f( IMAGE *in, IMAGE *out, DOUBLEMASK *mask ) if( !(t1 = vips_image_new()) || im_mask2vips( mask, t1 ) ) return( -1 ); - if( vips_convsep( in, &t2, t1, - "precision", VIPS_PRECISION_FLOAT, - NULL ) ) { + if( vips_convsep( in, &t2, t1, NULL ) ) { g_object_unref( t1 ); return( -1 ); } @@ -2561,12 +2563,18 @@ im_contrast_surface( IMAGE *in, IMAGE *out, int half_win_size, int spacing ) for( x = 0; x < size; x++ ) *VIPS_MATRIX( t[8], x, y ) = 1.0; - if( vips_conv( in, &t[2], t[0], NULL ) || - vips_conv( in, &t[3], t[1], NULL ) || + if( vips_conv( in, &t[2], t[0], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || + vips_conv( in, &t[3], t[1], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_abs( t[2], &t[4], NULL ) || vips_abs( t[3], &t[5], NULL ) || vips_add( t[4], t[5], &t[6], NULL ) || - vips_conv( t[6], &t[7], t[8], NULL ) || + vips_conv( t[6], &t[7], t[8], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_subsample( t[7], &t[9], spacing, spacing, NULL ) || vips_image_write( t[9], out ) ) return( -1 ); diff --git a/libvips/histogram/hist_ismonotonic.c b/libvips/histogram/hist_ismonotonic.c index ba0e735e..9a4b6986 100644 --- a/libvips/histogram/hist_ismonotonic.c +++ b/libvips/histogram/hist_ismonotonic.c @@ -90,7 +90,9 @@ vips_hist_ismonotonic_build( VipsObject *object ) /* We want >=128 everywhere, ie. no -ve transitions. */ - if( vips_conv( ismonotonic->in, &t[1], t[0], NULL ) || + if( vips_conv( ismonotonic->in, &t[1], t[0], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_moreeq_const1( t[1], &t[2], 128, NULL ) || vips_min( t[2], &m, NULL ) ) return( -1 ); diff --git a/libvips/morphology/countlines.c b/libvips/morphology/countlines.c index ec2b092c..7fbdc85b 100644 --- a/libvips/morphology/countlines.c +++ b/libvips/morphology/countlines.c @@ -92,7 +92,9 @@ vips_countlines_build( VipsObject *object ) case VIPS_DIRECTION_HORIZONTAL: if( !(t[0] = vips_image_new_matrixv( 1, 2, -1.0, 1.0 )) || vips_moreeq_const1( in, &t[1], 128, NULL ) || - vips_conv( t[1], &t[2], t[0], NULL ) || + vips_conv( t[1], &t[2], t[0], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_project( t[2], &t[3], &t[4], NULL ) || vips_avg( t[3], &nolines, NULL ) ) return( -1 ); @@ -101,7 +103,9 @@ vips_countlines_build( VipsObject *object ) case VIPS_DIRECTION_VERTICAL: if( !(t[0] = vips_image_new_matrixv( 2, 1, -1.0, 1.0 )) || vips_moreeq_const1( in, &t[1], 128, NULL ) || - vips_conv( t[1], &t[2], t[0], NULL ) || + vips_conv( t[1], &t[2], t[0], + "precision", VIPS_PRECISION_INTEGER, + NULL ) || vips_project( t[2], &t[3], &t[4], NULL ) || vips_avg( t[4], &nolines, NULL ) ) return( -1 );