From 470aee77f4b52ea8e6f0be65f4ee829fae76d8c7 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 6 Jan 2017 08:43:09 +0000 Subject: [PATCH 1/9] note govips --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index 93bb69cd..4102e4b1 100644 --- a/README.md +++ b/README.md @@ -25,6 +25,8 @@ binding](http://www.vips.ecs.soton.ac.uk/supported/current/doc/html/libvips/usin and a [command-line interface](http://www.vips.ecs.soton.ac.uk/supported/current/doc/html/libvips/using-cli.html). Bindings are available for [Ruby](https://rubygems.org/gems/ruby-vips), +[PHP](https://github.com/jcupitt/php-vips), +[Go](https://github.com/davidbyttow/govips), JavaScript and others. There is full [documentation](http://www.vips.ecs.soton.ac.uk/supported/current/doc/html/libvips/index.html). There are several GUIs as well, see the [VIPS From 8bbba73d73e8cdabd473160a88c20994663eb885 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 6 Jan 2017 19:37:36 +0000 Subject: [PATCH 2/9] try to fix soname.h again now make in install-exec-hook so we are certain libvips.la has been built --- TODO | 4 ---- libvips/Makefile.am | 6 ++---- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/TODO b/TODO index a7bd4ade..715f4c8f 100644 --- a/TODO +++ b/TODO @@ -1,7 +1,3 @@ -- vipsdisp-tiny makes stripes if the image is not tagged correctly - - is one of the colourspace functions not reading format? - - not sure about utf8 error messages on win - strange: diff --git a/libvips/Makefile.am b/libvips/Makefile.am index 9f3f91fd..3823b451 100644 --- a/libvips/Makefile.am +++ b/libvips/Makefile.am @@ -64,13 +64,11 @@ EXTRA_DIST = \ CLEANFILES = -all-local: - while [ ! -f libvips.la ]; do sleep 1; done; sleep 1; \ +install-exec-hook: echo '/* This file is autogenerated, do not edit. */' > soname.h && \ source libvips.la && \ echo "#define VIPS_SONAME \"$$dlname\"" >> soname.h && \ - ( cmp -s soname.h include/vips/soname.h || \ - cp soname.h include/vips ) && \ + cp soname.h $(DESTDIR)$(pkgincludedir) && \ rm soname.h -include $(INTROSPECTION_MAKEFILE) From c5e675f7db4c8f5a8581724742579f4878d89b74 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 10 Jan 2017 14:12:24 +0000 Subject: [PATCH 3/9] fix --vips-cache-max etc. --vips-cache-max, --vips-cache-max-memory and --vips-cache-max-files were not working and probably hadn't been for a while vipsthumbnail.c turns off the operation cache, it's not useful for the same operation repeated across many files --- ChangeLog | 1 + libvips/include/vips/internal.h | 5 ----- libvips/iofuncs/cache.c | 15 --------------- libvips/iofuncs/init.c | 33 ++++++++++++++++++++++++++++++--- libvips/resample/thumbnail.c | 26 ++++++++++++++++++++++++++ tools/vipsthumbnail.c | 4 ++++ 6 files changed, 61 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index b00adb01..32a4f5c7 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,7 @@ VIPS_LIBRARY_AGE - better support for bscale / bzero in fits images - deprecate vips_warn() / vips_info(); use g_warning() / g_info() instead +- fix --vips-cache-max etc. 8/12/16 started 8.4.5 - allow libgsf-1.14.26 to help centos, thanks tdiprima diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 1fa940a1..379d0ad7 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -94,11 +94,6 @@ extern int vips__info; */ extern char *vips__disc_threshold; -/* Cache size settings. - */ -extern char *vips__cache_max; -extern char *vips__cache_max_mem; -extern char *vips__cache_max_files; extern gboolean vips__cache_dump; extern gboolean vips__cache_trace; diff --git a/libvips/iofuncs/cache.c b/libvips/iofuncs/cache.c index c2393e2e..d5810fc3 100644 --- a/libvips/iofuncs/cache.c +++ b/libvips/iofuncs/cache.c @@ -67,9 +67,6 @@ /* Set by GOption from the command line, eg. "12m". */ -char *vips__cache_max = NULL; -char *vips__cache_max_mem = NULL; -char *vips__cache_max_files = NULL; gboolean vips__cache_dump = FALSE; gboolean vips__cache_trace = FALSE; @@ -456,18 +453,6 @@ vips__cache_init( void ) vips_cache_table = g_hash_table_new( (GHashFunc) vips_operation_hash, (GEqualFunc) vips_operation_equal ); - - if( vips__cache_max ) - vips_cache_max = - vips__parse_size( vips__cache_max ); - - if( vips__cache_max_mem ) - vips_cache_max_mem = - vips__parse_size( vips__cache_max_mem ); - - if( vips__cache_max_files ) - vips_cache_max_files = - vips__parse_size( vips__cache_max_files ); } } diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index d8bfd35f..b6c367b2 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -614,6 +614,33 @@ vips_lib_version_cb( const gchar *option_name, const gchar *value, exit( 0 ); } +static gboolean +vips_cache_max_cb( const gchar *option_name, const gchar *value, + gpointer data, GError **error ) +{ + vips_cache_set_max( vips__parse_size( value ) ); + + return( TRUE ); +} + +static gboolean +vips_cache_max_memory_cb( const gchar *option_name, const gchar *value, + gpointer data, GError **error ) +{ + vips_cache_set_max_mem( vips__parse_size( value ) ); + + return( TRUE ); +} + +static gboolean +vips_cache_max_files_cb( const gchar *option_name, const gchar *value, + gpointer data, GError **error ) +{ + vips_cache_set_max_files( vips__parse_size( value ) ); + + return( TRUE ); +} + static GOptionEntry option_entries[] = { { "vips-info", 0, G_OPTION_FLAG_HIDDEN | G_OPTION_FLAG_NO_ARG, G_OPTION_ARG_CALLBACK, (gpointer) &vips_lib_info_cb, @@ -652,13 +679,13 @@ static GOptionEntry option_entries[] = { G_OPTION_ARG_NONE, &vips__vector_enabled, N_( "disable vectorised versions of operations" ), NULL }, { "vips-cache-max", 0, 0, - G_OPTION_ARG_STRING, &vips__cache_max, + G_OPTION_ARG_CALLBACK, (gpointer) &vips_cache_max_cb, N_( "cache at most N operations" ), "N" }, { "vips-cache-max-memory", 0, 0, - G_OPTION_ARG_STRING, &vips__cache_max_mem, + G_OPTION_ARG_CALLBACK, (gpointer) &vips_cache_max_memory_cb, N_( "cache at most N bytes in memory" ), "N" }, { "vips-cache-max-files", 0, 0, - G_OPTION_ARG_STRING, &vips__cache_max_files, + G_OPTION_ARG_CALLBACK, (gpointer) &vips_cache_max_files_cb, N_( "allow at most N open files" ), "N" }, { "vips-cache-trace", 0, 0, G_OPTION_ARG_NONE, &vips__cache_trace, diff --git a/libvips/resample/thumbnail.c b/libvips/resample/thumbnail.c index c90c5610..db6d18c5 100644 --- a/libvips/resample/thumbnail.c +++ b/libvips/resample/thumbnail.c @@ -106,6 +106,30 @@ typedef struct _VipsThumbnailClass { G_DEFINE_ABSTRACT_TYPE( VipsThumbnail, vips_thumbnail, VIPS_TYPE_OPERATION ); +static void +vips_thumbnail_dispose( GObject *gobject ) +{ +#ifdef DEBUG + printf( "vips_thumbnail_dispose: " ); + vips_object_print_name( VIPS_OBJECT( gobject ) ); + printf( "\n" ); +#endif /*DEBUG*/ + + G_OBJECT_CLASS( vips_thumbnail_parent_class )->dispose( gobject ); +} + +static void +vips_thumbnail_finalize( GObject *gobject ) +{ +#ifdef DEBUG + printf( "vips_thumbnail_finalize: " ); + vips_object_print_name( VIPS_OBJECT( gobject ) ); + printf( "\n" ); +#endif /*DEBUG*/ + + G_OBJECT_CLASS( vips_thumbnail_parent_class )->finalize( gobject ); +} + /* Calculate the shrink factor, taking into account auto-rotate, the fit mode, * and so on. */ @@ -470,6 +494,8 @@ vips_thumbnail_class_init( VipsThumbnailClass *class ) GObjectClass *gobject_class = G_OBJECT_CLASS( class ); VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class ); + gobject_class->dispose = vips_thumbnail_dispose; + gobject_class->finalize = vips_thumbnail_finalize; gobject_class->set_property = vips_object_set_property; gobject_class->get_property = vips_object_get_property; diff --git a/tools/vipsthumbnail.c b/tools/vipsthumbnail.c index e6a184e4..f3fa487b 100644 --- a/tools/vipsthumbnail.c +++ b/tools/vipsthumbnail.c @@ -265,6 +265,10 @@ main( int argc, char **argv ) textdomain( GETTEXT_PACKAGE ); setlocale( LC_ALL, "" ); + /* The operation cache is not useful for processing many files. + vips_cache_set_max( 0 ); + */ + /* On Windows, argv is ascii-only .. use this to get a utf-8 version of * the args. */ From d1ef5a68901df9cb71113a7c7ef411a473b20d20 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 11 Jan 2017 14:05:50 +0000 Subject: [PATCH 4/9] compiles, needs more testing --- TODO | 64 ++++++ libvips/arithmetic/arithmetic.c | 7 +- libvips/convolution/conv.c | 3 + libvips/include/vips/image.h | 5 + libvips/include/vips/internal.h | 4 + libvips/iofuncs/Makefile.am | 1 + libvips/iofuncs/generate.c | 3 + libvips/iofuncs/init.c | 4 + libvips/iofuncs/recomp.c | 331 ++++++++++++++++++++++++++++++++ 9 files changed, 418 insertions(+), 4 deletions(-) create mode 100644 libvips/iofuncs/recomp.c diff --git a/TODO b/TODO index 715f4c8f..9af9f705 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,67 @@ +- try with our Python script + +- check sort is going in the right direction + +- use vips_image_prepare_many() elsewhere + +- add vips__recomp_add_margin() elsewhere + +- should add_margin be in the API? + +- docs + + + + +- 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 570c9aa3..6f9aa69d 100644 --- a/libvips/arithmetic/arithmetic.c +++ b/libvips/arithmetic/arithmetic.c @@ -516,11 +516,10 @@ vips_arithmetic_gen( VipsRegion *or, /* Prepare all input regions and make buffer pointers. */ - for( i = 0; ir[i]; i++ ) { - if( vips_region_prepare( ir[i], r ) ) - return( -1 ); + if( vips_image_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 ); - } p[i] = NULL; q = (VipsPel *) VIPS_REGION_ADDR( or, r->left, r->top ); diff --git a/libvips/convolution/conv.c b/libvips/convolution/conv.c index af23b0cc..d91bcfe3 100644 --- a/libvips/convolution/conv.c +++ b/libvips/convolution/conv.c @@ -113,6 +113,9 @@ vips_conv_build( VipsObject *object ) g_assert_not_reached(); } + vips__recomp_add_margin( convolution->out, + VIPS_MAX( convolution->M->Xsize, convolution->M->Ysize ) ); + return( 0 ); } diff --git a/libvips/include/vips/image.h b/libvips/include/vips/image.h index df98217f..a440dec0 100644 --- a/libvips/include/vips/image.h +++ b/libvips/include/vips/image.h @@ -506,6 +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. + */ +int vips_image_prepare_many( VipsImage *image, + struct _VipsRegion **regions, VipsRect *r ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 379d0ad7..9f5fe5f4 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -250,6 +250,10 @@ int vips__foreign_convert_saveable( VipsImage *in, VipsImage **ready, int vips__image_intize( VipsImage *in, VipsImage **out ); +int vips__recomp_set_input( VipsImage *image, VipsImage **in ); +void vips__recomp_init( void ); +void vips__recomp_add_margin( VipsImage *image, int margin ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/iofuncs/Makefile.am b/libvips/iofuncs/Makefile.am index 0a7807dc..a7f22312 100644 --- a/libvips/iofuncs/Makefile.am +++ b/libvips/iofuncs/Makefile.am @@ -1,6 +1,7 @@ noinst_LTLIBRARIES = libiofuncs.la libiofuncs_la_SOURCES = \ + recomp.c \ vipsmarshal.h \ vipsmarshal.c \ type.c \ diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c index 58a9c013..38ae8af6 100644 --- a/libvips/iofuncs/generate.c +++ b/libvips/iofuncs/generate.c @@ -385,6 +385,9 @@ vips_image_pipeline_array( VipsImage *image, vips__image_copy_fields_array( image, in ) ) return( -1 ); + if( vips__recomp_set_input( image, in ) ) + return( -1 ); + return( 0 ); } diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index b6c367b2..5652aab9 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -364,6 +364,10 @@ vips_init( const char *argv0 ) */ vips__cache_init(); + /* Recomp reordering system. + */ + vips__recomp_init(); + /* Start up packages. */ (void) vips_system_get_type(); diff --git a/libvips/iofuncs/recomp.c b/libvips/iofuncs/recomp.c new file mode 100644 index 00000000..36ed9c7b --- /dev/null +++ b/libvips/iofuncs/recomp.c @@ -0,0 +1,331 @@ +/* recomp.c ... manage recomp reordering + * + * 11/1/17 + * - first version + */ + +/* + + This file is part of VIPS. + + VIPS is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301 USA + + */ + +/* + + These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk + + */ + +/* + */ +#define DEBUG + +#ifdef HAVE_CONFIG_H +#include +#endif /*HAVE_CONFIG_H*/ +#include + +#include +#include +#include + +/* Have one of these on every image, identified by a quark. + */ +typedef struct _VipsRecomp { + /* The image we are attached to. + */ + VipsImage *image; + + /* The direct inputs to this image, so a copy of the array that is + * passed to vips_image_pipeline_array(), and in the same order. + * NULL-terminated. + * + * Score is the priority we give to the inputs as we de-dupe the source + * arrays. + * + * The recomp order is the order we prepare regions in ... just sort + * recomp_order by score. + */ + int n_inputs; + VipsImage **input; + int *score; + int *recomp_order; + + /* Source images are images with no input images, so file load, + * vips_black(), etc. NULL-terminated array. + * + * The cumulative margin is the total margin that has been added to + * each source image up to this point in the pipeline. + */ + int n_sources; + VipsImage **source; + int *cumulative_margin; + +} VipsRecomp; + +GQuark vips__image_recomp_quark = 0; + +#ifdef DEBUG +static void +vips_recomp_print( VipsRecomp *recomp ) +{ + int i; + + printf( "vips_recomp_print: " ); + vips_object_print_name( VIPS_OBJECT( recomp->image ) ); + printf( "\n" ); + + printf( "n_inputs = %d\n", recomp->n_inputs ); + printf( " n score order\n" ); + for( i = 0; i < recomp->n_inputs; i++ ) { + printf( "%2d - %8d, %8d, ", + i, recomp->score[i], recomp->recomp_order[i] ); + vips_object_print_name( VIPS_OBJECT( recomp->input[i] ) ); + printf( "\n" ); + } + + printf( "n_sources = %d\n", recomp->n_sources ); + printf( " n margin\n" ); + for( i = 0; i < recomp->n_sources; i++ ) { + printf( "%2d - %8d, ", + i, recomp->cumulative_margin[i] ); + vips_object_print_name( VIPS_OBJECT( recomp->source[i] ) ); + printf( "\n" ); + } + +} +#endif /*DEBUG*/ + +static VipsRecomp * +vips_recomp_get( VipsImage *image ) +{ + VipsRecomp *recomp; + + if( (recomp = g_object_get_qdata( G_OBJECT( image ), + vips__image_recomp_quark )) ) + return( recomp ); + + recomp = VIPS_NEW( image, VipsRecomp ); + recomp->image = image; + recomp->n_inputs = 0; + recomp->input = NULL; + recomp->score = NULL; + recomp->recomp_order = NULL; + recomp->n_sources = 0; + recomp->source = NULL; + recomp->cumulative_margin = NULL; + + g_object_set_qdata( G_OBJECT( image ), vips__image_recomp_quark, + recomp ); + + return( recomp ); +} + +static int +vips_recomp_compare_score( const void *a, const void *b, void *arg ) +{ + int i1 = *((int *) a); + int i2 = *((int *) b); + VipsRecomp *recomp = (VipsRecomp *) arg; + + return( recomp->score[i1] - recomp->score[i2] ); +} + +int +vips__recomp_set_input( VipsImage *image, VipsImage **in ) +{ + VipsRecomp *recomp = vips_recomp_get( image ); + + int i; + int total; + + printf( "vips__recomp_set_input: starting for image %p\n", image ); + + /* We have to support being called more than once on the same image. + * Two cases: + * + * 1. in the first call, no images were set ... we throw away + * everything from the first call and try again. foreign can do this. + * + * 2. warn if the args were different and do nothing. + */ + if( recomp->source ) { + printf( "vips__recomp_set_input: run again\n" ); + + if( recomp->n_inputs == 0 ) { + printf( "vips__recomp_set_input: " + "no args to first call\n" ); + + recomp->n_sources = 0; + } + else { + for( i = 0; in[i]; i++ ) + if( i >= recomp->n_inputs || + in[i] != recomp->input[i] ) { + printf( "vips__recomp_set_input: " + "args differ\n" ); + break; + } + + return( 0 ); + } + } + + /* Make a copy of the input array. + */ + for( i = 0; in[i]; i++ ) + ; + recomp->n_inputs = i; + recomp->input = VIPS_ARRAY( image, recomp->n_inputs + 1, VipsImage * ); + recomp->score = VIPS_ARRAY( image, recomp->n_inputs, int ); + recomp->recomp_order = VIPS_ARRAY( image, recomp->n_inputs, int ); + if( !recomp->input ) + return( -1 ); + if( recomp->n_inputs && + (!recomp->score || + !recomp->recomp_order) ) + return( -1 ); + + for( i = 0; i < recomp->n_inputs; i++ ) { + recomp->input[i] = in[i]; + recomp->score[i] = 0; + recomp->recomp_order[i] = i; + } + recomp->input[i] = NULL; + + /* Find the total number of source images -- this gives an upper bound + * to the size of the unique source image array we will need. + */ + total = 0; + for( i = 0; i < recomp->n_inputs; i++ ) + total += vips_recomp_get( recomp->input[i] )->n_sources; + + /* No source images means this must itself be a source image, so it has + * a source image of itself. + */ + total = VIPS_MAX( 1, total ); + + recomp->source = VIPS_ARRAY( image, total + 1, VipsImage * ); + recomp->cumulative_margin = VIPS_ARRAY( image, total, int ); + if( !recomp->source || + !recomp->cumulative_margin ) + return( -1 ); + + /* Copy source images over, removing duplicates. If we find a + * duplicate, we have a reordering opportunity, and we adjust the + * scores of the two images containing the dupe. + */ + for( i = 0; i < recomp->n_inputs; i++ ) { + VipsRecomp *input = vips_recomp_get( recomp->input[i] ); + + int j; + + for( j = 0; j < input->n_sources; j++ ) { + int k; + + /* Search for dupe. + */ + for( k = 0; k < recomp->n_sources; k++ ) + if( recomp->source[k] == input->source[j] ) + break; + + if( k < recomp->n_sources ) { + /* Found a dupe. Does this new use of + * input->source[j] have a larger or smaller + * margin? Adjust the score to reflect the + * change, note the new max. + */ + recomp->cumulative_margin[k] = VIPS_MAX( + recomp->cumulative_margin[k], + input->cumulative_margin[j] ); + recomp->score[i] += input->cumulative_margin[j] - + recomp->cumulative_margin[k]; + } + else { + /* No dupe, just add to the table. + */ + recomp->source[recomp->n_sources] = + input->source[j]; + recomp->cumulative_margin[recomp->n_sources] = + input->cumulative_margin[j]; + recomp->n_sources += 1; + + vips_recomp_print( recomp ); + } + } + } + + /* Sort recomp_order by score. qsort_r() is a GNU libc thing, don't use + * it. + */ + if( recomp->n_inputs > 1 ) + g_qsort_with_data( recomp->recomp_order, + recomp->n_inputs, + sizeof( int ), + vips_recomp_compare_score, recomp ); + + /* No sources ... make one, us! + */ + if( recomp->n_inputs == 0 ) { + recomp->source[0] = image; + recomp->cumulative_margin[0] = 0; + recomp->n_sources = 1; + } + +#ifdef DEBUG + vips_recomp_print( recomp ); +#endif /*DEBUG*/ + + return( 0 ); +} + +int +vips_image_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) +{ + VipsRecomp *recomp = vips_recomp_get( image ); + + int i; + + for( i = 0; i < recomp->n_inputs; i++ ) { + g_assert( regions[i] ); + + if( vips_region_prepare( regions[recomp->recomp_order[i]], r ) ) + return( -1 ); + } + + return( 0 ); +} + +void +vips__recomp_add_margin( VipsImage *image, int margin ) +{ + VipsRecomp *recomp = vips_recomp_get( image ); + + int i; + + for( i = 0; i < recomp->n_sources; i++ ) + recomp->cumulative_margin[i] += margin; +} + +void +vips__recomp_init( void ) +{ + if( !vips__image_recomp_quark ) + vips__image_recomp_quark = + g_quark_from_static_string( "vips-image-recomp" ); +} From 3d216da8c99b8b4eccb5495c625e9a71068aa56b Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 12 Jan 2017 09:15:10 +0000 Subject: [PATCH 5/9] rename recomp as reorder --- TODO | 4 +- libvips/convolution/conv.c | 4 +- libvips/include/vips/internal.h | 6 +- libvips/iofuncs/Makefile.am | 2 +- libvips/iofuncs/generate.c | 2 +- libvips/iofuncs/init.c | 2 +- libvips/iofuncs/recomp.c | 331 ------------------------------ libvips/iofuncs/reorder.c | 343 ++++++++++++++++++++++++++++++++ 8 files changed, 353 insertions(+), 341 deletions(-) delete mode 100644 libvips/iofuncs/recomp.c create mode 100644 libvips/iofuncs/reorder.c diff --git a/TODO b/TODO index 9af9f705..1500373f 100644 --- a/TODO +++ b/TODO @@ -2,9 +2,9 @@ - check sort is going in the right direction -- use vips_image_prepare_many() elsewhere +- use vips_image_prepare_many() elsewhere ... bandary, -- add vips__recomp_add_margin() elsewhere +- add vips__recomp_add_margin() elsewhere ... morph, local hist, - should add_margin be in the API? diff --git a/libvips/convolution/conv.c b/libvips/convolution/conv.c index d91bcfe3..8e31af89 100644 --- a/libvips/convolution/conv.c +++ b/libvips/convolution/conv.c @@ -113,8 +113,8 @@ vips_conv_build( VipsObject *object ) g_assert_not_reached(); } - vips__recomp_add_margin( convolution->out, - VIPS_MAX( convolution->M->Xsize, convolution->M->Ysize ) ); + vips__reorder_add_margin( convolution->out, + convolution->M->Xsize * convolution->M->Ysize ); return( 0 ); } diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 9f5fe5f4..c8cea58b 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -250,9 +250,9 @@ int vips__foreign_convert_saveable( VipsImage *in, VipsImage **ready, int vips__image_intize( VipsImage *in, VipsImage **out ); -int vips__recomp_set_input( VipsImage *image, VipsImage **in ); -void vips__recomp_init( void ); -void vips__recomp_add_margin( VipsImage *image, int margin ); +int vips__reorder_set_input( VipsImage *image, VipsImage **in ); +void vips__reorder_init( void ); +void vips__reorder_add_margin( VipsImage *image, int margin ); #ifdef __cplusplus } diff --git a/libvips/iofuncs/Makefile.am b/libvips/iofuncs/Makefile.am index a7f22312..cdf244a8 100644 --- a/libvips/iofuncs/Makefile.am +++ b/libvips/iofuncs/Makefile.am @@ -1,7 +1,7 @@ noinst_LTLIBRARIES = libiofuncs.la libiofuncs_la_SOURCES = \ - recomp.c \ + reorder.c \ vipsmarshal.h \ vipsmarshal.c \ type.c \ diff --git a/libvips/iofuncs/generate.c b/libvips/iofuncs/generate.c index 38ae8af6..c81a62b5 100644 --- a/libvips/iofuncs/generate.c +++ b/libvips/iofuncs/generate.c @@ -385,7 +385,7 @@ vips_image_pipeline_array( VipsImage *image, vips__image_copy_fields_array( image, in ) ) return( -1 ); - if( vips__recomp_set_input( image, in ) ) + if( vips__reorder_set_input( image, in ) ) return( -1 ); return( 0 ); diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index 5652aab9..d3d9aa6b 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -366,7 +366,7 @@ vips_init( const char *argv0 ) /* Recomp reordering system. */ - vips__recomp_init(); + vips__reorder_init(); /* Start up packages. */ diff --git a/libvips/iofuncs/recomp.c b/libvips/iofuncs/recomp.c deleted file mode 100644 index 36ed9c7b..00000000 --- a/libvips/iofuncs/recomp.c +++ /dev/null @@ -1,331 +0,0 @@ -/* recomp.c ... manage recomp reordering - * - * 11/1/17 - * - first version - */ - -/* - - This file is part of VIPS. - - VIPS is free software; you can redistribute it and/or modify - it under the terms of the GNU Lesser General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public License - along with this program; if not, write to the Free Software - Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA - 02110-1301 USA - - */ - -/* - - These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk - - */ - -/* - */ -#define DEBUG - -#ifdef HAVE_CONFIG_H -#include -#endif /*HAVE_CONFIG_H*/ -#include - -#include -#include -#include - -/* Have one of these on every image, identified by a quark. - */ -typedef struct _VipsRecomp { - /* The image we are attached to. - */ - VipsImage *image; - - /* The direct inputs to this image, so a copy of the array that is - * passed to vips_image_pipeline_array(), and in the same order. - * NULL-terminated. - * - * Score is the priority we give to the inputs as we de-dupe the source - * arrays. - * - * The recomp order is the order we prepare regions in ... just sort - * recomp_order by score. - */ - int n_inputs; - VipsImage **input; - int *score; - int *recomp_order; - - /* Source images are images with no input images, so file load, - * vips_black(), etc. NULL-terminated array. - * - * The cumulative margin is the total margin that has been added to - * each source image up to this point in the pipeline. - */ - int n_sources; - VipsImage **source; - int *cumulative_margin; - -} VipsRecomp; - -GQuark vips__image_recomp_quark = 0; - -#ifdef DEBUG -static void -vips_recomp_print( VipsRecomp *recomp ) -{ - int i; - - printf( "vips_recomp_print: " ); - vips_object_print_name( VIPS_OBJECT( recomp->image ) ); - printf( "\n" ); - - printf( "n_inputs = %d\n", recomp->n_inputs ); - printf( " n score order\n" ); - for( i = 0; i < recomp->n_inputs; i++ ) { - printf( "%2d - %8d, %8d, ", - i, recomp->score[i], recomp->recomp_order[i] ); - vips_object_print_name( VIPS_OBJECT( recomp->input[i] ) ); - printf( "\n" ); - } - - printf( "n_sources = %d\n", recomp->n_sources ); - printf( " n margin\n" ); - for( i = 0; i < recomp->n_sources; i++ ) { - printf( "%2d - %8d, ", - i, recomp->cumulative_margin[i] ); - vips_object_print_name( VIPS_OBJECT( recomp->source[i] ) ); - printf( "\n" ); - } - -} -#endif /*DEBUG*/ - -static VipsRecomp * -vips_recomp_get( VipsImage *image ) -{ - VipsRecomp *recomp; - - if( (recomp = g_object_get_qdata( G_OBJECT( image ), - vips__image_recomp_quark )) ) - return( recomp ); - - recomp = VIPS_NEW( image, VipsRecomp ); - recomp->image = image; - recomp->n_inputs = 0; - recomp->input = NULL; - recomp->score = NULL; - recomp->recomp_order = NULL; - recomp->n_sources = 0; - recomp->source = NULL; - recomp->cumulative_margin = NULL; - - g_object_set_qdata( G_OBJECT( image ), vips__image_recomp_quark, - recomp ); - - return( recomp ); -} - -static int -vips_recomp_compare_score( const void *a, const void *b, void *arg ) -{ - int i1 = *((int *) a); - int i2 = *((int *) b); - VipsRecomp *recomp = (VipsRecomp *) arg; - - return( recomp->score[i1] - recomp->score[i2] ); -} - -int -vips__recomp_set_input( VipsImage *image, VipsImage **in ) -{ - VipsRecomp *recomp = vips_recomp_get( image ); - - int i; - int total; - - printf( "vips__recomp_set_input: starting for image %p\n", image ); - - /* We have to support being called more than once on the same image. - * Two cases: - * - * 1. in the first call, no images were set ... we throw away - * everything from the first call and try again. foreign can do this. - * - * 2. warn if the args were different and do nothing. - */ - if( recomp->source ) { - printf( "vips__recomp_set_input: run again\n" ); - - if( recomp->n_inputs == 0 ) { - printf( "vips__recomp_set_input: " - "no args to first call\n" ); - - recomp->n_sources = 0; - } - else { - for( i = 0; in[i]; i++ ) - if( i >= recomp->n_inputs || - in[i] != recomp->input[i] ) { - printf( "vips__recomp_set_input: " - "args differ\n" ); - break; - } - - return( 0 ); - } - } - - /* Make a copy of the input array. - */ - for( i = 0; in[i]; i++ ) - ; - recomp->n_inputs = i; - recomp->input = VIPS_ARRAY( image, recomp->n_inputs + 1, VipsImage * ); - recomp->score = VIPS_ARRAY( image, recomp->n_inputs, int ); - recomp->recomp_order = VIPS_ARRAY( image, recomp->n_inputs, int ); - if( !recomp->input ) - return( -1 ); - if( recomp->n_inputs && - (!recomp->score || - !recomp->recomp_order) ) - return( -1 ); - - for( i = 0; i < recomp->n_inputs; i++ ) { - recomp->input[i] = in[i]; - recomp->score[i] = 0; - recomp->recomp_order[i] = i; - } - recomp->input[i] = NULL; - - /* Find the total number of source images -- this gives an upper bound - * to the size of the unique source image array we will need. - */ - total = 0; - for( i = 0; i < recomp->n_inputs; i++ ) - total += vips_recomp_get( recomp->input[i] )->n_sources; - - /* No source images means this must itself be a source image, so it has - * a source image of itself. - */ - total = VIPS_MAX( 1, total ); - - recomp->source = VIPS_ARRAY( image, total + 1, VipsImage * ); - recomp->cumulative_margin = VIPS_ARRAY( image, total, int ); - if( !recomp->source || - !recomp->cumulative_margin ) - return( -1 ); - - /* Copy source images over, removing duplicates. If we find a - * duplicate, we have a reordering opportunity, and we adjust the - * scores of the two images containing the dupe. - */ - for( i = 0; i < recomp->n_inputs; i++ ) { - VipsRecomp *input = vips_recomp_get( recomp->input[i] ); - - int j; - - for( j = 0; j < input->n_sources; j++ ) { - int k; - - /* Search for dupe. - */ - for( k = 0; k < recomp->n_sources; k++ ) - if( recomp->source[k] == input->source[j] ) - break; - - if( k < recomp->n_sources ) { - /* Found a dupe. Does this new use of - * input->source[j] have a larger or smaller - * margin? Adjust the score to reflect the - * change, note the new max. - */ - recomp->cumulative_margin[k] = VIPS_MAX( - recomp->cumulative_margin[k], - input->cumulative_margin[j] ); - recomp->score[i] += input->cumulative_margin[j] - - recomp->cumulative_margin[k]; - } - else { - /* No dupe, just add to the table. - */ - recomp->source[recomp->n_sources] = - input->source[j]; - recomp->cumulative_margin[recomp->n_sources] = - input->cumulative_margin[j]; - recomp->n_sources += 1; - - vips_recomp_print( recomp ); - } - } - } - - /* Sort recomp_order by score. qsort_r() is a GNU libc thing, don't use - * it. - */ - if( recomp->n_inputs > 1 ) - g_qsort_with_data( recomp->recomp_order, - recomp->n_inputs, - sizeof( int ), - vips_recomp_compare_score, recomp ); - - /* No sources ... make one, us! - */ - if( recomp->n_inputs == 0 ) { - recomp->source[0] = image; - recomp->cumulative_margin[0] = 0; - recomp->n_sources = 1; - } - -#ifdef DEBUG - vips_recomp_print( recomp ); -#endif /*DEBUG*/ - - return( 0 ); -} - -int -vips_image_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) -{ - VipsRecomp *recomp = vips_recomp_get( image ); - - int i; - - for( i = 0; i < recomp->n_inputs; i++ ) { - g_assert( regions[i] ); - - if( vips_region_prepare( regions[recomp->recomp_order[i]], r ) ) - return( -1 ); - } - - return( 0 ); -} - -void -vips__recomp_add_margin( VipsImage *image, int margin ) -{ - VipsRecomp *recomp = vips_recomp_get( image ); - - int i; - - for( i = 0; i < recomp->n_sources; i++ ) - recomp->cumulative_margin[i] += margin; -} - -void -vips__recomp_init( void ) -{ - if( !vips__image_recomp_quark ) - vips__image_recomp_quark = - g_quark_from_static_string( "vips-image-recomp" ); -} diff --git a/libvips/iofuncs/reorder.c b/libvips/iofuncs/reorder.c new file mode 100644 index 00000000..f4373a5c --- /dev/null +++ b/libvips/iofuncs/reorder.c @@ -0,0 +1,343 @@ +/* reorder.c ... manage reorder reordering + * + * 11/1/17 + * - first version + */ + +/* + + This file is part of VIPS. + + VIPS is free software; you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + 02110-1301 USA + + */ + +/* + + These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk + + */ + +/* + */ +#define DEBUG + +#ifdef HAVE_CONFIG_H +#include +#endif /*HAVE_CONFIG_H*/ +#include + +#include +#include +#include + +/* Have one of these on every image, identified by a quark. + */ +typedef struct _VipsReorder { + /* The image we are attached to. + */ + VipsImage *image; + + /* The direct inputs to this image, so a copy of the array that is + * passed to vips_image_pipeline_array(), and in the same order. + * NULL-terminated. + * + * Score is the priority we give to the inputs as we de-dupe the source + * arrays. + * + * The recomp order is the order we prepare regions in ... just make a + * range then sort by score. + */ + int n_inputs; + VipsImage **input; + int *score; + int *recomp_order; + + /* Source images are images with no input images, so file load, + * vips_black(), etc. NULL-terminated array. + * + * The cumulative margin is the total margin that has been added to + * each source image up to this point in the pipeline. + */ + int n_sources; + VipsImage **source; + int *cumulative_margin; + +} VipsReorder; + +GQuark vips__image_reorder_quark = 0; + +#ifdef DEBUG +static void +vips_reorder_print( VipsReorder *reorder ) +{ + int i; + + printf( "vips_reorder_print: " ); + vips_object_print_name( VIPS_OBJECT( reorder->image ) ); + printf( "\n" ); + + printf( "n_inputs = %d\n", reorder->n_inputs ); + printf( " n score order\n" ); + for( i = 0; i < reorder->n_inputs; i++ ) { + printf( "%2d - %8d, %8d, ", + i, reorder->score[i], reorder->recomp_order[i] ); + vips_object_print_name( VIPS_OBJECT( reorder->input[i] ) ); + printf( "\n" ); + } + + printf( "n_sources = %d\n", reorder->n_sources ); + printf( " n margin\n" ); + for( i = 0; i < reorder->n_sources; i++ ) { + printf( "%2d - %8d, ", + i, reorder->cumulative_margin[i] ); + vips_object_print_name( VIPS_OBJECT( reorder->source[i] ) ); + printf( "\n" ); + } + +} +#endif /*DEBUG*/ + +static VipsReorder * +vips_reorder_get( VipsImage *image ) +{ + VipsReorder *reorder; + + if( (reorder = g_object_get_qdata( G_OBJECT( image ), + vips__image_reorder_quark )) ) + return( reorder ); + + reorder = VIPS_NEW( image, VipsReorder ); + reorder->image = image; + reorder->n_inputs = 0; + reorder->input = NULL; + reorder->score = NULL; + reorder->recomp_order = NULL; + reorder->n_sources = 0; + reorder->source = NULL; + reorder->cumulative_margin = NULL; + + g_object_set_qdata( G_OBJECT( image ), vips__image_reorder_quark, + reorder ); + + return( reorder ); +} + +static int +vips_reorder_compare_score( const void *a, const void *b, void *arg ) +{ + int i1 = *((int *) a); + int i2 = *((int *) b); + VipsReorder *reorder = (VipsReorder *) arg; + + return( reorder->score[i2] - reorder->score[i1] ); +} + +int +vips__reorder_set_input( VipsImage *image, VipsImage **in ) +{ + VipsReorder *reorder = vips_reorder_get( image ); + + 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: + * + * 1. in the first call, no images were set ... we throw away + * everything from the first call and try again. foreign can do this. + * + * 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" ); + + 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: " + "args differ\n" ); + break; + } + + return( 0 ); + } + } + + /* Make a copy of the input array. + */ + 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 ); + if( !reorder->input ) + return( -1 ); + if( reorder->n_inputs && + (!reorder->score || + !reorder->recomp_order) ) + return( -1 ); + + for( i = 0; i < reorder->n_inputs; i++ ) { + reorder->input[i] = in[i]; + reorder->score[i] = 0; + reorder->recomp_order[i] = i; + } + reorder->input[i] = NULL; + + /* Find the total number of source images -- this gives an upper bound + * to the size of the unique source image array we will need. + */ + total = 0; + for( i = 0; i < reorder->n_inputs; i++ ) + total += vips_reorder_get( reorder->input[i] )->n_sources; + + /* No source images means this must itself be a source image, so it has + * a source image of itself. + */ + total = VIPS_MAX( 1, total ); + + reorder->source = VIPS_ARRAY( image, total + 1, VipsImage * ); + reorder->cumulative_margin = VIPS_ARRAY( image, total, int ); + if( !reorder->source || + !reorder->cumulative_margin ) + return( -1 ); + + /* Copy source images over, removing duplicates. If we find a + * duplicate, we have a reordering opportunity, and we adjust the + * scores of the two images containing the dupe. + */ + for( i = 0; i < reorder->n_inputs; i++ ) { + VipsReorder *input = vips_reorder_get( reorder->input[i] ); + + int j; + + for( j = 0; j < input->n_sources; j++ ) { + int k; + + /* Search for dupe. + */ + for( k = 0; k < reorder->n_sources; k++ ) + if( reorder->source[k] == input->source[j] ) + break; + + if( k < reorder->n_sources ) { + /* Found a dupe. Does this new use of + * input->source[j] have a larger or smaller + * 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->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 { + /* No dupe, just add to the table. + */ + reorder->source[reorder->n_sources] = + input->source[j]; + reorder->cumulative_margin[reorder->n_sources] = + input->cumulative_margin[j]; + reorder->n_sources += 1; + + vips_reorder_print( reorder ); + } + } + } + + /* Sort recomp_order by score. qsort_r() is a GNU libc thing, don't use + * it. + */ + if( reorder->n_inputs > 1 ) + g_qsort_with_data( reorder->recomp_order, + reorder->n_inputs, + sizeof( int ), + vips_reorder_compare_score, reorder ); + + /* No sources ... make one, us! + */ + if( reorder->n_inputs == 0 ) { + reorder->source[0] = image; + reorder->cumulative_margin[0] = 0; + reorder->n_sources = 1; + } + +#ifdef DEBUG + vips_reorder_print( reorder ); +#endif /*DEBUG*/ + + return( 0 ); +} + +int +vips_image_prepare_many( VipsImage *image, VipsRegion **regions, VipsRect *r ) +{ + VipsReorder *reorder = vips_reorder_get( image ); + + int i; + + for( i = 0; i < reorder->n_inputs; i++ ) { + g_assert( regions[i] ); + + if( vips_region_prepare( regions[reorder->recomp_order[i]], r ) ) + return( -1 ); + } + + return( 0 ); +} + +void +vips__reorder_add_margin( VipsImage *image, int margin ) +{ + VipsReorder *reorder = vips_reorder_get( image ); + + int i; + + for( i = 0; i < reorder->n_sources; i++ ) + reorder->cumulative_margin[i] += margin; +} + +void +vips__reorder_init( void ) +{ + if( !vips__image_reorder_quark ) + vips__image_reorder_quark = + g_quark_from_static_string( "vips-image-reorder" ); +} From f302bd6570fef0084f8c8f62921c5d84cd2d0620 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 12 Jan 2017 14:06:54 +0000 Subject: [PATCH 6/9] all working! --- TODO | 61 +-------------------- libvips/arithmetic/arithmetic.c | 2 +- libvips/convolution/conv.c | 2 +- libvips/foreign/vips2webp.c | 2 +- libvips/include/vips/image.h | 5 +- libvips/include/vips/internal.h | 3 +- libvips/iofuncs/generate.c | 8 +++ libvips/iofuncs/reorder.c | 95 +++++++++++++++++++++------------ 8 files changed, 77 insertions(+), 101 deletions(-) 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 ); From 6b325145e4f07049d6ff3d446e5c4ec8eacb9203 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 12 Jan 2017 14:36:44 +0000 Subject: [PATCH 7/9] use new hints and prepare everywhere --- ChangeLog | 3 +++ TODO | 6 ------ libvips/colour/colour.c | 5 ++--- libvips/conversion/bandary.c | 7 +++---- libvips/conversion/ifthenelse.c | 3 +++ libvips/convolution/correlation.c | 3 +++ libvips/convolution/sharpen.c | 3 +-- libvips/deprecated/vips7compat.c | 7 +++---- libvips/histogram/hist_local.c | 2 ++ libvips/histogram/stdif.c | 2 ++ libvips/morphology/morph.c | 3 +++ libvips/morphology/rank.c | 2 ++ libvips/resample/reduceh.cpp | 2 ++ libvips/resample/reducev.cpp | 2 ++ 14 files changed, 31 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 32a4f5c7..b1407fd5 100644 --- a/ChangeLog +++ b/ChangeLog @@ -20,6 +20,9 @@ - better support for bscale / bzero in fits images - deprecate vips_warn() / vips_info(); use g_warning() / g_info() instead - fix --vips-cache-max etc. +- add compute reordering, plus some new API to support it: + vips_reorder_margin_hint() and vips_reorder_prepare_many(), thanks + aferrero2707 8/12/16 started 8.4.5 - allow libgsf-1.14.26 to help centos, thanks tdiprima diff --git a/TODO b/TODO index b30851cb..458f2c59 100644 --- a/TODO +++ b/TODO @@ -1,9 +1,3 @@ -- use vips_reorder_prepare_many() elsewhere ... bandary, - -- add vips_reorder_margin_hint() elsewhere ... morph, local hist, - - - - not sure about utf8 error messages on win diff --git a/libvips/colour/colour.c b/libvips/colour/colour.c index ed5666a1..61988a89 100644 --- a/libvips/colour/colour.c +++ b/libvips/colour/colour.c @@ -234,9 +234,8 @@ vips_colour_gen( VipsRegion *or, int i, y; VipsPel *p[MAX_INPUT_IMAGES], *q; - for( i = 0; ir[i]; i++ ) - if( vips_region_prepare( ir[i], r ) ) - return( -1 ); + if( vips_reorder_prepare_many( or->im, ir, r ) ) + return( -1 ); VIPS_GATE_START( "vips_colour_gen: work" ); diff --git a/libvips/conversion/bandary.c b/libvips/conversion/bandary.c index 66b9ef5f..2f14b494 100644 --- a/libvips/conversion/bandary.c +++ b/libvips/conversion/bandary.c @@ -92,11 +92,10 @@ vips_bandary_gen( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) VipsPel *p[MAX_INPUT_IMAGES], *q; int y, i; - for( i = 0; i < bandary->n; i++ ) { - if( vips_region_prepare( ir[i], r ) ) - return( -1 ); + if( vips_reorder_prepare_many( or->im, ir, r ) ) + return( -1 ); + for( i = 0; i < bandary->n; i++ ) p[i] = VIPS_REGION_ADDR( ir[i], r->left, r->top ); - } p[i] = NULL; q = VIPS_REGION_ADDR( or, r->left, r->top ); diff --git a/libvips/conversion/ifthenelse.c b/libvips/conversion/ifthenelse.c index eb5bb2b7..38a76bfd 100644 --- a/libvips/conversion/ifthenelse.c +++ b/libvips/conversion/ifthenelse.c @@ -283,6 +283,9 @@ vips_blend_gen( VipsRegion *or, void *seq, void *client1, void *client2, else { /* Mix of set and clear ... ask for both then and else parts * and interleave. + * + * We can't use vips_reorder_prepare_many() since we always + * want the c image first. */ if( vips_region_prepare( ir[0], r ) || vips_region_prepare( ir[1], r ) ) diff --git a/libvips/convolution/correlation.c b/libvips/convolution/correlation.c index 2abd96c2..31e4b187 100644 --- a/libvips/convolution/correlation.c +++ b/libvips/convolution/correlation.c @@ -125,6 +125,9 @@ vips_correlation_build( VipsObject *object ) correlation->in_ready, correlation ) ) return( -1 ); + vips_reorder_margin_hint( correlation->out, + correlation->ref->Xsize * correlation->ref->Ysize ); + return( 0 ); } diff --git a/libvips/convolution/sharpen.c b/libvips/convolution/sharpen.c index 6a260fbe..a95242fb 100644 --- a/libvips/convolution/sharpen.c +++ b/libvips/convolution/sharpen.c @@ -121,8 +121,7 @@ vips_sharpen_generate( VipsRegion *or, int x, y; - if( vips_region_prepare( in[0], r ) || - vips_region_prepare( in[1], r ) ) + if( vips_reorder_prepare_many( or->im, in, r ) ) return( -1 ); VIPS_GATE_START( "vips_sharpen_generate: work" ); diff --git a/libvips/deprecated/vips7compat.c b/libvips/deprecated/vips7compat.c index 10eec9d0..a3b6d954 100644 --- a/libvips/deprecated/vips7compat.c +++ b/libvips/deprecated/vips7compat.c @@ -649,12 +649,11 @@ process_region( VipsRegion *or, void *seq, void *a, void *b ) /* Prepare all input regions and make buffer pointers. */ - for( i = 0; ir[i]; i++ ) { - if( vips_region_prepare( ir[i], &or->valid ) ) - return( -1 ); + if( vips_reorder_prepare_many( or->im, ir, &or->valid ) ) + return( -1 ); + for( i = 0; ir[i]; i++ ) p[i] = (PEL *) VIPS_REGION_ADDR( ir[i], or->valid.left, or->valid.top ); - } p[i] = NULL; q = (PEL *) VIPS_REGION_ADDR( or, or->valid.left, or->valid.top ); diff --git a/libvips/histogram/hist_local.c b/libvips/histogram/hist_local.c index 59472ea2..f97cdead 100644 --- a/libvips/histogram/hist_local.c +++ b/libvips/histogram/hist_local.c @@ -280,6 +280,8 @@ vips_hist_local_build( VipsObject *object ) local->out->Xoffset = 0; local->out->Yoffset = 0; + vips_reorder_margin_hint( local->out, local->width * local->height ); + return( 0 ); } diff --git a/libvips/histogram/stdif.c b/libvips/histogram/stdif.c index da701555..92d3bdc2 100644 --- a/libvips/histogram/stdif.c +++ b/libvips/histogram/stdif.c @@ -272,6 +272,8 @@ vips_stdif_build( VipsObject *object ) stdif->out->Xoffset = 0; stdif->out->Yoffset = 0; + vips_reorder_margin_hint( stdif->out, stdif->width * stdif->height ); + return( 0 ); } diff --git a/libvips/morphology/morph.c b/libvips/morphology/morph.c index 47333e18..5b38d00e 100644 --- a/libvips/morphology/morph.c +++ b/libvips/morphology/morph.c @@ -120,6 +120,9 @@ vips_morph_build( VipsObject *object ) g_assert_not_reached(); } + vips_reorder_margin_hint( morph->out, + morph->M->Xsize * morph->M->Ysize ); + return( 0 ); } diff --git a/libvips/morphology/rank.c b/libvips/morphology/rank.c index 09bcebc6..fea8e211 100644 --- a/libvips/morphology/rank.c +++ b/libvips/morphology/rank.c @@ -391,6 +391,8 @@ vips_rank_build( VipsObject *object ) rank->out->Xoffset = 0; rank->out->Yoffset = 0; + vips_reorder_margin_hint( rank->out, rank->width * rank->height ); + return( 0 ); } diff --git a/libvips/resample/reduceh.cpp b/libvips/resample/reduceh.cpp index 52ec7d3d..b57c4fcc 100644 --- a/libvips/resample/reduceh.cpp +++ b/libvips/resample/reduceh.cpp @@ -540,6 +540,8 @@ vips_reduceh_build( VipsObject *object ) in, reduceh ) ) return( -1 ); + vips_reorder_margin_hint( resample->out, reduceh->n_point ); + return( 0 ); } diff --git a/libvips/resample/reducev.cpp b/libvips/resample/reducev.cpp index 63a03c43..3cf3d7f7 100644 --- a/libvips/resample/reducev.cpp +++ b/libvips/resample/reducev.cpp @@ -813,6 +813,8 @@ vips_reducev_raw( VipsReducev *reducev, VipsImage *in ) in, reducev ) ) return( -1 ); + vips_reorder_margin_hint( resample->out, reducev->n_point ); + return( 0 ); } From 2f8a279f2d3bd2ecc10c967080211a9fee69534e Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 12 Jan 2017 21:53:24 +0000 Subject: [PATCH 8/9] ooops, forgot convasep --- libvips/convolution/convasep.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libvips/convolution/convasep.c b/libvips/convolution/convasep.c index 5f5209a6..5534bf52 100644 --- a/libvips/convolution/convasep.c +++ b/libvips/convolution/convasep.c @@ -877,6 +877,9 @@ vips_convasep_build( VipsObject *object ) convolution->out->Xoffset = 0; convolution->out->Yoffset = 0; + vips_reorder_margin_hint( convolution->out, + convolution->M->Xsize * convolution->M->Ysize ); + return( 0 ); } From dafa26435e8c5514433ac9e31459650ef56fe43d Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 13 Jan 2017 12:37:19 +0000 Subject: [PATCH 9/9] only use webp presets in lossy mode it seems lossless and near-lossless modes have a separate preset system see https://github.com/jcupitt/libvips/issues/578 --- libvips/foreign/vips2webp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/libvips/foreign/vips2webp.c b/libvips/foreign/vips2webp.c index 5447c24c..a50d660b 100644 --- a/libvips/foreign/vips2webp.c +++ b/libvips/foreign/vips2webp.c @@ -237,7 +237,18 @@ write_webp( WebPPicture *pic, VipsImage *in, WebPConfig config; webp_import import; - if ( !WebPConfigPreset( &config, get_preset( preset ), Q ) ) { + if( !WebPConfigInit( &config ) ) { + vips_error( "vips2webp", + "%s", _( "config version error" ) ); + return( -1 ); + } + + /* These presets are only for lossy compression. There seems to be + * separate API for lossless or near-lossless, see + * WebPConfigLosslessPreset(). + */ + if( !(lossless || near_lossless) && + !WebPConfigPreset( &config, get_preset( preset ), Q ) ) { vips_error( "vips2webp", "%s", _( "config version error" ) ); return( -1 ); }