diff --git a/TODO b/TODO index 1500373f..b30851cb 100644 --- a/TODO +++ b/TODO @@ -1,67 +1,10 @@ -- try with our Python script +- use vips_reorder_prepare_many() elsewhere ... bandary, -- check sort is going in the right direction - -- use vips_image_prepare_many() elsewhere ... bandary, - -- add vips__recomp_add_margin() elsewhere ... morph, local hist, - -- should add_margin be in the API? - -- docs +- add vips_reorder_margin_hint() elsewhere ... morph, local hist, -- new plan - - images have extra fields: - - 1. an array of original source images, unique by pointer - - 2. an int array giving for each of 1., a cumulative margin - - 3. an array of direct input images, a copy of the array - passed to vips_image_pipeline_array() - - 4. an array of int, the same size as 3., giving the prepare - order - - really, 2 x array(image, int) - - have a quark and use g_object_set_qdata_full() to attach - - vips_image_pipeline_array() - - - if there are no input images, make a new original image with - a margin of 0 - - - create source image array, give each one a priority of 0 - - - create empty original image array - - for each source image - for each original image in this original image array - if not in new original image array, add it - if already there, don't add, but compare the margin - if this margin is greater, - note the bigger margin - increment the priority of this source image - - - sort source image array by priority, highest first - - vips_region_prepare_many() - - - prepare regions in source image array priority - - - needs another arg: the image for which these regions are - being prepared - - - add new func vips_image_prepare_many() - - vips_conv() - - - add N to the margin on all original images - - not sure about utf8 error messages on win - strange: diff --git a/libvips/arithmetic/arithmetic.c b/libvips/arithmetic/arithmetic.c index 6f9aa69d..df6ec224 100644 --- a/libvips/arithmetic/arithmetic.c +++ b/libvips/arithmetic/arithmetic.c @@ -516,7 +516,7 @@ vips_arithmetic_gen( VipsRegion *or, /* Prepare all input regions and make buffer pointers. */ - if( vips_image_prepare_many( or->im, ir, r ) ) + if( vips_reorder_prepare_many( or->im, ir, r ) ) return( -1 ); for( i = 0; ir[i]; i++ ) p[i] = (VipsPel *) VIPS_REGION_ADDR( ir[i], r->left, r->top ); diff --git a/libvips/convolution/conv.c b/libvips/convolution/conv.c index 8e31af89..1a49290c 100644 --- a/libvips/convolution/conv.c +++ b/libvips/convolution/conv.c @@ -113,7 +113,7 @@ vips_conv_build( VipsObject *object ) g_assert_not_reached(); } - vips__reorder_add_margin( convolution->out, + vips_reorder_margin_hint( convolution->out, convolution->M->Xsize * convolution->M->Ysize ); return( 0 ); diff --git a/libvips/foreign/vips2webp.c b/libvips/foreign/vips2webp.c index 5447c24c..a38dfb0f 100644 --- a/libvips/foreign/vips2webp.c +++ b/libvips/foreign/vips2webp.c @@ -160,7 +160,7 @@ vips_webp_writer_appendle( VipsWebPWriter *writer, uint32_t val, int n ) unsigned char buf[4]; int i; - g_assert( n < 4 ); + g_assert( n <= 4 ); for( i = 0; i < n; i++ ) { buf[i] = (unsigned char) (val & 0xff); diff --git a/libvips/include/vips/image.h b/libvips/include/vips/image.h index a440dec0..3f9fe380 100644 --- a/libvips/include/vips/image.h +++ b/libvips/include/vips/image.h @@ -506,10 +506,11 @@ VipsImage **vips_array_image_get( VipsArrayImage *array, int *n ); VipsImage **vips_value_get_array_image( const GValue *value, int *n ); void vips_value_set_array_image( GValue *value, int n ); -/* Defined in recomp.c, but really a function on image. +/* Defined in reorder.c, but really a function on image. */ -int vips_image_prepare_many( VipsImage *image, +int vips_reorder_prepare_many( VipsImage *image, struct _VipsRegion **regions, VipsRect *r ); +void vips_reorder_margin_hint( VipsImage *image, int margin ); #ifdef __cplusplus } diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index c8cea58b..8329cf87 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -250,9 +250,8 @@ int vips__foreign_convert_saveable( VipsImage *in, VipsImage **ready, int vips__image_intize( VipsImage *in, VipsImage **out ); -int vips__reorder_set_input( VipsImage *image, VipsImage **in ); void vips__reorder_init( void ); -void vips__reorder_add_margin( VipsImage *image, int margin ); +int vips__reorder_set_input( VipsImage *image, VipsImage **in ); #ifdef __cplusplus } diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c index c81a62b5..de025d64 100644 --- a/libvips/iofuncs/generate.c +++ b/libvips/iofuncs/generate.c @@ -379,6 +379,14 @@ int vips_image_pipeline_array( VipsImage *image, VipsDemandStyle hint, VipsImage **in ) { + /* This function can be called more than once per output image. For + * example, jpeg header load will call this once on ->out to set the + * default hint, then later call it again to connect the output image + * up to the real image. + * + * It's only ever called first time with in[0] == NULL and second time + * with a real value for @in. + */ vips__demand_hint_array( image, hint, in ); if( in[0] && diff --git a/libvips/iofuncs/reorder.c b/libvips/iofuncs/reorder.c index f4373a5c..2333359a 100644 --- a/libvips/iofuncs/reorder.c +++ b/libvips/iofuncs/reorder.c @@ -32,8 +32,8 @@ */ /* - */ #define DEBUG + */ #ifdef HAVE_CONFIG_H #include @@ -111,6 +111,17 @@ vips_reorder_print( VipsReorder *reorder ) } #endif /*DEBUG*/ +static void +vips_reorder_destroy( VipsReorder *reorder ) +{ + VIPS_FREE( reorder->input ); + VIPS_FREE( reorder->score ); + VIPS_FREE( reorder->recomp_order ); + VIPS_FREE( reorder->source ); + VIPS_FREE( reorder->cumulative_margin ); + VIPS_FREE( reorder ); +} + static VipsReorder * vips_reorder_get( VipsImage *image ) { @@ -120,7 +131,7 @@ vips_reorder_get( VipsImage *image ) vips__image_reorder_quark )) ) return( reorder ); - reorder = VIPS_NEW( image, VipsReorder ); + reorder = VIPS_NEW( NULL, VipsReorder ); reorder->image = image; reorder->n_inputs = 0; reorder->input = NULL; @@ -130,8 +141,8 @@ vips_reorder_get( VipsImage *image ) reorder->source = NULL; reorder->cumulative_margin = NULL; - g_object_set_qdata( G_OBJECT( image ), vips__image_reorder_quark, - reorder ); + g_object_set_qdata_full( G_OBJECT( image ), vips__image_reorder_quark, + reorder, (GDestroyNotify) vips_reorder_destroy ); return( reorder ); } @@ -154,8 +165,6 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) int i; int total; - printf( "vips__reorder_set_input: starting for image %p\n", image ); - /* We have to support being called more than once on the same image. * Two cases: * @@ -165,19 +174,15 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) * 2. warn if the args were different and do nothing. */ if( reorder->source ) { - printf( "vips__reorder_set_input: run again\n" ); - - if( reorder->n_inputs == 0 ) { - printf( "vips__reorder_set_input: " - "no args to first call\n" ); - + if( reorder->n_inputs == 0 ) reorder->n_sources = 0; - } else { for( i = 0; in[i]; i++ ) if( i >= reorder->n_inputs || in[i] != reorder->input[i] ) { - printf( "vips__reorder_set_input: " + /* Should never happen. + */ + g_warning( "vips__reorder_set_input: " "args differ\n" ); break; } @@ -191,9 +196,9 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) for( i = 0; in[i]; i++ ) ; reorder->n_inputs = i; - reorder->input = VIPS_ARRAY( image, reorder->n_inputs + 1, VipsImage * ); - reorder->score = VIPS_ARRAY( image, reorder->n_inputs, int ); - reorder->recomp_order = VIPS_ARRAY( image, reorder->n_inputs, int ); + reorder->input = VIPS_ARRAY( NULL, reorder->n_inputs + 1, VipsImage * ); + reorder->score = VIPS_ARRAY( NULL, reorder->n_inputs, int ); + reorder->recomp_order = VIPS_ARRAY( NULL, reorder->n_inputs, int ); if( !reorder->input ) return( -1 ); if( reorder->n_inputs && @@ -220,8 +225,8 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) */ total = VIPS_MAX( 1, total ); - reorder->source = VIPS_ARRAY( image, total + 1, VipsImage * ); - reorder->cumulative_margin = VIPS_ARRAY( image, total, int ); + reorder->source = VIPS_ARRAY( NULL, total + 1, VipsImage * ); + reorder->cumulative_margin = VIPS_ARRAY( NULL, total, int ); if( !reorder->source || !reorder->cumulative_margin ) return( -1 ); @@ -250,22 +255,13 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) * margin? Adjust the score to reflect the * change, note the new max. */ - - printf( "found a dupe\n" ); - printf( "margin on first one %d\n", - reorder->cumulative_margin[k] ); - printf( "margin on new one %d\n", - input->cumulative_margin[j] ); - - printf( "setting score on %d += %d\n", - i, input->cumulative_margin[j] - - reorder->cumulative_margin[k] ); + reorder->score[i] += + input->cumulative_margin[j] - + reorder->cumulative_margin[k]; reorder->cumulative_margin[k] = VIPS_MAX( reorder->cumulative_margin[k], input->cumulative_margin[j] ); - reorder->score[i] += input->cumulative_margin[j] - - reorder->cumulative_margin[k]; } else { @@ -276,8 +272,6 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) reorder->cumulative_margin[reorder->n_sources] = input->cumulative_margin[j]; reorder->n_sources += 1; - - vips_reorder_print( reorder ); } } } @@ -306,8 +300,24 @@ vips__reorder_set_input( VipsImage *image, VipsImage **in ) return( 0 ); } +/** + * vips_reorder_prepare_many: + * @image: the image that's being written + * @regions: the set of regions to prepare + * @r: the #VipsRect to prepare on each region + * + * vips_reorder_prepare_many() runs vips_region_prepare() on each region in + * @regions, requesting the pixels in @r. + * + * It tries to request the regions in the order which will cause least + * recomputation. This can give a large speedup, in some cases. + * + * See also: vips_region_prepare(), vips_reorder_margin_hint(). + * + * Returns: 0 on success, or -1 on error. + */ int -vips_image_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) +vips_reorder_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) { VipsReorder *reorder = vips_reorder_get( image ); @@ -323,8 +333,23 @@ vips_image_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) return( 0 ); } +/** + * vips_reorder_margin_hint: + * @image: the image to hint on + * @margin: the size of the margin this operation has added + * + * vips_reorder_margin_hint() sets a hint that @image contains a margin, that + * is, that each vips_region_prepare() on @image will request a slightly larger + * region from it's inputs. A good value for @margin is (width * height) for + * the window the operation uses. + * + * This information is used by vips_image_prepare_many() to attempt to reorder + * computations to minimise recomputation. + * + * See also: vips_image_prepare_many(). + */ void -vips__reorder_add_margin( VipsImage *image, int margin ) +vips_reorder_margin_hint( VipsImage *image, int margin ) { VipsReorder *reorder = vips_reorder_get( image );