From 19009b15a94f74803d1b27e13c7bcf1352ea13b3 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Sat, 3 Sep 2022 15:31:37 +0100 Subject: [PATCH] revise loader demand hints Add a check that header and load methods agree on the demand hint, and make sure all loaders pass. If they disagree, you can get bad performance in some cases, since the pipeline can be built from the header dhint. --- libvips/foreign/analyze2vips.c | 3 +++ libvips/foreign/foreign.c | 10 +++++++--- libvips/foreign/jp2kload.c | 6 ++---- libvips/foreign/matlab.c | 5 +++++ libvips/foreign/niftiload.c | 5 ++++- libvips/foreign/popplerload.c | 10 +++++----- libvips/foreign/svgload.c | 5 ++--- libvips/foreign/vipspng.c | 13 +------------ 8 files changed, 29 insertions(+), 28 deletions(-) diff --git a/libvips/foreign/analyze2vips.c b/libvips/foreign/analyze2vips.c index 1d629e64..45da4972 100644 --- a/libvips/foreign/analyze2vips.c +++ b/libvips/foreign/analyze2vips.c @@ -552,6 +552,9 @@ vips__analyze_read_header( const char *filename, VipsImage *out ) attach_meta( out, d ); + if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_THINSTRIP, NULL ) ) + return( -1 ); + return( 0 ); } diff --git a/libvips/foreign/foreign.c b/libvips/foreign/foreign.c index 1d35c954..31b788f1 100644 --- a/libvips/foreign/foreign.c +++ b/libvips/foreign/foreign.c @@ -932,13 +932,17 @@ vips_foreign_load_temp( VipsForeignLoad *load ) static gboolean vips_foreign_load_iscompat( VipsImage *a, VipsImage *b ) { + /* dhint must match too, or we can get terrible performance with some + * formats. + */ if( a->Xsize != b->Xsize || a->Ysize != b->Ysize || a->Bands != b->Bands || a->Coding != b->Coding || - a->BandFmt != b->BandFmt ) { - vips_error( "VipsForeignLoad", - "%s", _( "images do not match" ) ); + a->BandFmt != b->BandFmt || + a->dhint != b->dhint ) { + vips_error( "VipsForeignLoad", "%s", + _( "images do not match between header and load" ) ); return( FALSE ); } diff --git a/libvips/foreign/jp2kload.c b/libvips/foreign/jp2kload.c index a6dd7d4c..223ffa64 100644 --- a/libvips/foreign/jp2kload.c +++ b/libvips/foreign/jp2kload.c @@ -447,11 +447,9 @@ vips_foreign_load_jp2k_set_header( VipsForeignLoadJp2k *jp2k, VipsImage *out ) return( -1 ); } - /* Even though this is a tiled reader, we hint thinstrip since with - * the cache we are quite happy serving that if anything downstream - * would like it. + /* We use a tilecache on our output. */ - if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_THINSTRIP, NULL ) ) + if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL ) ) return( -1 ); vips_image_init_fields( out, diff --git a/libvips/foreign/matlab.c b/libvips/foreign/matlab.c index b184fe99..8da55e78 100644 --- a/libvips/foreign/matlab.c +++ b/libvips/foreign/matlab.c @@ -220,6 +220,11 @@ mat2vips_get_header( matvar_t *var, VipsImage *im ) format, VIPS_CODING_NONE, interpretation, 1.0, 1.0 ); + /* We read to a huge memory area. + */ + if( vips_image_pipelinev( im, VIPS_DEMAND_STYLE_ANY, NULL ) ) + return( -1 ); + return( 0 ); } diff --git a/libvips/foreign/niftiload.c b/libvips/foreign/niftiload.c index 27eb5679..ded3163c 100644 --- a/libvips/foreign/niftiload.c +++ b/libvips/foreign/niftiload.c @@ -482,7 +482,10 @@ vips_foreign_load_nifti_set_header( VipsForeignLoadNifti *nifti, printf( "get_vips_properties: fmt = %d\n", fmt ); #endif /*DEBUG*/ - vips_image_pipelinev( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL ); + /* We load to memory then write to out, so we'll hint THINSTRIP. + */ + vips_image_pipelinev( out, VIPS_DEMAND_STYLE_THINSTRIP, NULL ); + vips_image_init_fields( out, width, height, bands, fmt, VIPS_CODING_NONE, diff --git a/libvips/foreign/popplerload.c b/libvips/foreign/popplerload.c index 237e82d7..1e450048 100644 --- a/libvips/foreign/popplerload.c +++ b/libvips/foreign/popplerload.c @@ -277,11 +277,6 @@ vips_foreign_load_pdf_set_image( VipsForeignLoadPdf *pdf, VipsImage *out ) printf( "vips_foreign_load_pdf_set_image: %p\n", pdf ); #endif /*DEBUG*/ - /* We render to a linecache, so fat strips work well. - */ - if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_FATSTRIP, NULL ) ) - return( -1 ); - /* Extract and attach metadata. Set the old name too for compat. */ vips_image_set_int( out, "pdf-n_pages", pdf->n_pages ); @@ -308,6 +303,11 @@ vips_foreign_load_pdf_set_image( VipsForeignLoadPdf *pdf, VipsImage *out ) 4, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, res, res ); + /* We render to a tilecache, so it has to be SMALLTILE. + */ + if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL ) ) + return( -1 ); + return( 0 ); } diff --git a/libvips/foreign/svgload.c b/libvips/foreign/svgload.c index 0fe829d7..94072581 100644 --- a/libvips/foreign/svgload.c +++ b/libvips/foreign/svgload.c @@ -543,10 +543,9 @@ vips_foreign_load_svg_parse( VipsForeignLoadSvg *svg, VipsImage *out ) 4, VIPS_FORMAT_UCHAR, VIPS_CODING_NONE, VIPS_INTERPRETATION_sRGB, res, res ); - /* We render to a cache with a couple of rows of tiles, so fat strips - * work well. + /* We use a tilecache, so it's smalltile. */ - if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_FATSTRIP, NULL ) ) + if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL ) ) return( -1 ); return( 0 ); diff --git a/libvips/foreign/vipspng.c b/libvips/foreign/vipspng.c index ca32776a..c9c55a7f 100644 --- a/libvips/foreign/vipspng.c +++ b/libvips/foreign/vipspng.c @@ -393,7 +393,6 @@ png2vips_header( Read *read, VipsImage *out ) png_uint_32 width, height; int bitdepth, color_type; int interlace_type; - VipsDemandStyle hint; png_uint_32 res_x, res_y; int unit_type; @@ -524,17 +523,7 @@ png2vips_header( Read *read, VipsImage *out ) VIPS_SETSTR( out->filename, vips_connection_filename( VIPS_CONNECTION( read->source ) ) ); - /* Uninterlaced images will be read in seq mode. Interlaced images are - * read via a huge memory buffer. - */ - if( interlace_type == PNG_INTERLACE_NONE ) - /* Sequential mode needs thinstrip to work with things like - * vips_shrink(). - */ - hint = VIPS_DEMAND_STYLE_THINSTRIP; - else - hint = VIPS_DEMAND_STYLE_ANY; - if( vips_image_pipelinev( out, hint, NULL ) ) + if( vips_image_pipelinev( out, VIPS_DEMAND_STYLE_THINSTRIP, NULL ) ) return( -1 ); /* Fetch the ICC profile. @name is useless, something like "icc" or