From e34b6b92e95042e4aefa00d51006e8dbadf17106 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Fri, 9 Dec 2011 14:30:45 +0000 Subject: [PATCH] small cleanups minor changed to openslide.c, use PEL everywhere as the pixel pointer type --- ChangeLog | 1 + TODO | 4 ++ libvips/arithmetic/measure.c | 4 +- libvips/arithmetic/stats.c | 2 +- libvips/conversion/flip.c | 2 +- libvips/conversion/im_subsample.c | 8 +-- libvips/format/openslide.c | 112 ++++++++++++++++++----------- libvips/histograms_lut/im_histnD.c | 2 +- libvips/include/vips/image.h | 6 +- libvips/include/vips/private.h | 6 +- libvips/include/vips/region.h | 4 +- libvips/iofuncs/region.c | 6 +- libvips/iofuncs/window.c | 2 +- 13 files changed, 96 insertions(+), 63 deletions(-) diff --git a/ChangeLog b/ChangeLog index bda9642a..82e03df9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -50,6 +50,7 @@ - unlink temps on rewind on *nix, less likely to leave temps on a crash - added complex conj as a basic operation - rect/polar/conj work o any format, not just complex +- added openslide support (Benjamin Gilbert) 12/10/11 started 7.26.6 - NOCACHE was not being set correctly on OS X causing performance diff --git a/TODO b/TODO index 8ca28490..763e033a 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,7 @@ +- openslide.c:fill_region() should not allocate memory + + + - vipsimage should be cached too, eg. VipsImage *a = vips_image_new_from_file( "poop.jpg" ); diff --git a/libvips/arithmetic/measure.c b/libvips/arithmetic/measure.c index 975e7368..726a5e7b 100644 --- a/libvips/arithmetic/measure.c +++ b/libvips/arithmetic/measure.c @@ -50,8 +50,8 @@ */ /* - */ #define VIPS_DEBUG + */ #ifdef HAVE_CONFIG_H #include @@ -159,7 +159,7 @@ vips_measure_build( VipsObject *object ) * measure on IM_TYPE_LAB images). */ if( dev * 5 > fabs( avg ) && fabs( avg ) > 3 ) - im_warn( "im_measure", + vips_warn( "VipsMeasure", _( "patch %d x %d, band %d: " "avg = %g, sdev = %g" ), i, j, avg, dev ); diff --git a/libvips/arithmetic/stats.c b/libvips/arithmetic/stats.c index eb4357d4..be429d21 100644 --- a/libvips/arithmetic/stats.c +++ b/libvips/arithmetic/stats.c @@ -56,8 +56,8 @@ */ /* - */ #define VIPS_DEBUG + */ #ifdef HAVE_CONFIG_H #include diff --git a/libvips/conversion/flip.c b/libvips/conversion/flip.c index b26181cd..15567f9b 100644 --- a/libvips/conversion/flip.c +++ b/libvips/conversion/flip.c @@ -133,7 +133,7 @@ vips_flip_horizontal_gen( VipsRegion *or, void *seq, void *a, void *b, VipsRegion *ir = (VipsRegion *) seq; VipsRect *r = &or->valid; VipsRect in; - char *p, *q; + PEL *p, *q; int x, y, z; int le = r->left; diff --git a/libvips/conversion/im_subsample.c b/libvips/conversion/im_subsample.c index 42c94deb..ecaac347 100644 --- a/libvips/conversion/im_subsample.c +++ b/libvips/conversion/im_subsample.c @@ -85,8 +85,8 @@ line_shrink_gen( REGION *or, void *seq, void *a, void *b ) /* Loop down the region. */ for( y = to; y < bo; y++ ) { - char *q = IM_REGION_ADDR( or, le, y ); - char *p; + PEL *q = IM_REGION_ADDR( or, le, y ); + PEL *p; /* Loop across the region, in owidth sized pieces. */ @@ -149,8 +149,8 @@ point_shrink_gen( REGION *or, void *seq, void *a, void *b ) /* Loop down the region. */ for( y = to; y < bo; y++ ) { - char *q = IM_REGION_ADDR( or, le, y ); - char *p; + PEL *q = IM_REGION_ADDR( or, le, y ); + PEL *p; /* Loop across the region, in owidth sized pieces. */ diff --git a/libvips/format/openslide.c b/libvips/format/openslide.c index f57b48e1..59fb71eb 100644 --- a/libvips/format/openslide.c +++ b/libvips/format/openslide.c @@ -41,9 +41,14 @@ */ +/* +#define VIPS_DEBUG + */ + #ifdef HAVE_CONFIG_H #include #endif /*HAVE_CONFIG_H*/ +#include #ifdef HAVE_OPENSLIDE @@ -51,10 +56,11 @@ #include #include #include -#include #include -#include +#include + +#include typedef struct { openslide_t *osr; @@ -82,7 +88,10 @@ check_associated_image( openslide_t *osr, const char *name ) *associated != NULL; associated++ ) if( strcmp( *associated, name ) == 0 ) return( 0 ); - vips_error( "im_openslide2vips", _( "invalid associated image name" )); + + vips_error( "im_openslide2vips", + "%s", _( "invalid associated image name" ) ); + return( -1 ); } @@ -99,14 +108,15 @@ readslide_new( const char *filename, VipsImage *out ) char *associated; rslide = VIPS_NEW( out, ReadSlide ); - memset( rslide, 0, sizeof( *rslide )); + memset( rslide, 0, sizeof( *rslide ) ); g_signal_connect( out, "close", G_CALLBACK( readslide_destroy_cb ), rslide ); vips_filename_split( filename, name, mode ); rslide->osr = openslide_open( name ); if( rslide->osr == NULL ) { - vips_error( "im_openslide2vips", _( "failure opening slide" )); + vips_error( "im_openslide2vips", + "%s", _( "failure opening slide" ) ); return( NULL ); } @@ -124,15 +134,16 @@ readslide_new( const char *filename, VipsImage *out ) /* Mode specifies slide layer. */ if( rslide->layer < 0 || rslide->layer >= - openslide_get_layer_count( rslide->osr )) { + openslide_get_layer_count( rslide->osr ) ) { vips_error( "im_openslide2vips", - _( "invalid slide layer" )); + "%s", _( "invalid slide layer" ) ); return( NULL ); } - } else if( *mode != 0 ) { + } + else if( *mode != 0 ) { /* Mode specifies associated image. */ - if ( check_associated_image( rslide->osr, mode )) + if ( check_associated_image( rslide->osr, mode ) ) return( NULL ); rslide->associated = vips_strdup( VIPS_OBJECT( out ), mode ); } @@ -142,7 +153,8 @@ readslide_new( const char *filename, VipsImage *out ) rslide->associated, &w, &h ); vips_image_set_string( out, "slide-associated-image", rslide->associated ); - } else { + } + else { openslide_get_layer_dimensions( rslide->osr, rslide->layer, &w, &h ); rslide->downsample = openslide_get_layer_downsample( @@ -151,12 +163,12 @@ readslide_new( const char *filename, VipsImage *out ) } if( w < 0 || h < 0 || rslide->downsample < 0 ) { vips_error( "im_openslide2vips", _( "getting dimensions: %s" ), - openslide_get_error( rslide->osr )); + openslide_get_error( rslide->osr ) ); return( NULL ); } if( w > INT_MAX || h > INT_MAX ) { vips_error( "im_openslide2vips", - _( "image dimensions overflow int" )); + "%s", _( "image dimensions overflow int" ) ); return( NULL ); } @@ -167,10 +179,10 @@ readslide_new( const char *filename, VipsImage *out ) *properties != NULL; properties++ ) vips_image_set_string( out, *properties, openslide_get_property_value( rslide->osr, - *properties )); + *properties ) ); associated = g_strjoinv( ", ", (char **) - openslide_get_associated_image_names( rslide->osr )); + openslide_get_associated_image_names( rslide->osr ) ); vips_image_set_string( out, "slide-associated-images", associated ); g_free( associated ); @@ -191,7 +203,8 @@ copy_line( ReadSlide *rslide, uint32_t *in, int count, PEL *out ) out[4 * i + 0] = 255 * ((in[i] >> 16) & 255) / a; out[4 * i + 1] = 255 * ((in[i] >> 8) & 255) / a; out[4 * i + 2] = 255 * (in[i] & 255) / a; - } else { + } + else { /* Use background color. */ out[4 * i + 0] = (rslide->background >> 16) & 255; @@ -211,6 +224,14 @@ fill_region( VipsRegion *out, void *seq, void *_rslide, void *unused, const char *error; int y; + VIPS_DEBUG_MSG( "openslide.c:fill_region: %d x %d @ %d x %d\n", + out->valid.width, out->valid.height, + out->valid.left, out->valid.top ); + + /* We should really not alloc in the render thread. But + * openslide_read_region() does a lot of allocing anyway, so we might + * as well too. + */ buf = VIPS_ARRAY( NULL, out->valid.width * out->valid.height, uint32_t ); openslide_read_region( rslide->osr, buf, @@ -219,8 +240,9 @@ fill_region( VipsRegion *out, void *seq, void *_rslide, void *unused, out->valid.width, out->valid.height ); for( y = 0; y < out->valid.height; y++ ) copy_line( rslide, buf + y * out->valid.width, - out->valid.width, VIPS_REGION_ADDR_TOPLEFT( out ) + - y * VIPS_REGION_LSKIP( out )); + out->valid.width, + VIPS_REGION_ADDR_TOPLEFT( out ) + + y * VIPS_REGION_LSKIP( out ) ); vips_free( buf ); error = openslide_get_error( rslide->osr ); @@ -229,6 +251,7 @@ fill_region( VipsRegion *out, void *seq, void *_rslide, void *unused, error ); return( -1 ); } + return( 0 ); } @@ -245,24 +268,19 @@ fill_associated( VipsImage *out, ReadSlide *rslide ) rslide->associated, &w, &h ); if( w == -1 || h == -1 ) { vips_error( "im_openslide2vips", _( "getting dimensions: %s" ), - openslide_get_error( rslide->osr )); + openslide_get_error( rslide->osr ) ); return( -1 ); } - buf = VIPS_ARRAY( NULL, w * h, uint32_t ); - line = VIPS_ARRAY( NULL, VIPS_IMAGE_SIZEOF_LINE( out ), PEL ); + buf = VIPS_ARRAY( out, w * h, uint32_t ); + line = VIPS_ARRAY( out, VIPS_IMAGE_SIZEOF_LINE( out ), PEL ); openslide_read_associated_image( rslide->osr, rslide->associated, buf ); for( y = 0; y < h; y++ ) { copy_line( rslide, buf + y * w, w, line ); - if( vips_image_write_line( out, y, line )) { - vips_free( line ); - vips_free( buf ); + if( vips_image_write_line( out, y, line ) ) return( -1 ); - } } - vips_free( line ); - vips_free( buf ); error = openslide_get_error( rslide->osr ); if( error ) { @@ -270,6 +288,7 @@ fill_associated( VipsImage *out, ReadSlide *rslide ) _( "reading associated image: %s" ), error ); return( -1 ); } + return( 0 ); } @@ -278,9 +297,9 @@ openslide2vips_header( const char *filename, VipsImage *out ) { ReadSlide *rslide; - rslide = readslide_new( filename, out ); - if( rslide == NULL ) + if( !(rslide = readslide_new( filename, out )) ) return( -1 ); + return( 0 ); } @@ -317,19 +336,26 @@ im_openslide2vips( const char *filename, VipsImage *out ) { ReadSlide *rslide; - rslide = readslide_new( filename, out ); - if( rslide == NULL ) + VIPS_DEBUG_MSG( "im_openslide2vips: %s\n", filename ); + + if( !(rslide = readslide_new( filename, out )) ) return( -1 ); + if( rslide->associated ) { - if( vips_image_wio_output( out )) + if( vips_image_wio_output( out ) ) return( -1 ); - return fill_associated( out, rslide ); - } else { - if( vips_image_pio_output( out )) + + VIPS_DEBUG_MSG( "fill_associated:\n" ); + + return( fill_associated( out, rslide ) ); + } + else { + if( vips_image_pio_output( out ) ) return( -1 ); vips_demand_hint( out, VIPS_DEMAND_STYLE_SMALLTILE, NULL ); - return vips_image_generate( out, NULL, fill_region, NULL, - rslide, NULL ); + + return( vips_image_generate( out, + NULL, fill_region, NULL, rslide, NULL ) ); } } @@ -341,7 +367,7 @@ isslide( const char *filename ) int ok; ok = 1; - osr = openslide_open(filename); + osr = openslide_open( filename ); if( osr != NULL ) { /* If this is a generic tiled TIFF image, decline to support * it, since im_tiff2vips can do better. @@ -352,9 +378,12 @@ isslide( const char *filename ) strcmp( vendor, "generic-tiff" ) == 0 ) ok = 0; openslide_close( osr ); - } else { + } + else ok = 0; - } + + VIPS_DEBUG_MSG( "isslide: %s - %d\n", filename, ok ); + return( ok ); } @@ -367,15 +396,14 @@ slide_flags( const char *filename ) vips_filename_split( filename, name, mode ); strtol( mode, &endp, 10 ); - if( *mode == 0 || *endp == 0 ) { + if( *mode == 0 || *endp == 0 ) /* Slide layer or no mode specified. */ return( VIPS_FORMAT_PARTIAL ); - } else { + else /* Associated image specified. */ return( 0 ); - } } static void diff --git a/libvips/histograms_lut/im_histnD.c b/libvips/histograms_lut/im_histnD.c index 4f228364..42728e44 100644 --- a/libvips/histograms_lut/im_histnD.c +++ b/libvips/histograms_lut/im_histnD.c @@ -171,7 +171,7 @@ find_hist( REGION *reg, void *seq, void *a, void *b, gboolean *stop ) /* Accumulate! */ for( y = to; y < bo; y++ ) { - char *line = IM_REGION_ADDR( reg, le, y ); + PEL *line = IM_REGION_ADDR( reg, le, y ); switch( im->BandFmt ) { case IM_BANDFMT_UCHAR: diff --git a/libvips/include/vips/image.h b/libvips/include/vips/image.h index a4ee555c..428daf26 100644 --- a/libvips/include/vips/image.h +++ b/libvips/include/vips/image.h @@ -282,7 +282,7 @@ typedef struct _VipsImage { */ char *Hist; /* don't use, see vips_image_get_history() */ char *filename; /* pointer to copy of filename */ - char *data; /* start of image data for WIO */ + PEL *data; /* start of image data for WIO */ int kill; /* set to non-zero to block eval */ /* Everything below this private and only used internally by @@ -292,7 +292,7 @@ typedef struct _VipsImage { char *mode; /* mode string passed to _new() */ VipsImageType dtype; /* descriptor type */ int fd; /* file descriptor */ - char *baseaddr; /* pointer to the start of an mmap file */ + void *baseaddr; /* pointer to the start of an mmap file */ size_t length; /* size of mmap area */ guint32 magic; /* magic from header, endian-ness of image */ @@ -445,7 +445,7 @@ extern const size_t vips__image_sizeof_bandformat[]; (X), (Y), \ 0, 0, \ (I)->Xsize, \ - (I)->Ysize ), abort(), (char *) NULL) \ + (I)->Ysize ), abort(), (PEL *) NULL) \ ) #else /*!VIPS_DEBUG*/ #define VIPS_IMAGE_ADDR( I, X, Y ) \ diff --git a/libvips/include/vips/private.h b/libvips/include/vips/private.h index d9f2c628..2977cdf1 100644 --- a/libvips/include/vips/private.h +++ b/libvips/include/vips/private.h @@ -66,9 +66,9 @@ typedef struct { int top; /* Area of image we have mapped, in pixels */ int height; - char *data; /* First pixel of line 'top' */ + PEL *data; /* First pixel of line 'top' */ - PEL *baseaddr; /* Base of window */ + void *baseaddr; /* Base of window */ size_t length; /* Size of window */ } VipsWindow; @@ -106,7 +106,7 @@ typedef struct { VipsRect area; /* Area this pixel buffer covers */ gboolean done; /* Calculated and in cache */ VipsBufferCache *cache; - char *buf; /* Private malloc() area */ + PEL *buf; /* Private malloc() area */ size_t bsize; /* Size of private malloc() */ } VipsBuffer; diff --git a/libvips/include/vips/region.h b/libvips/include/vips/region.h index 33580cc2..352be997 100644 --- a/libvips/include/vips/region.h +++ b/libvips/include/vips/region.h @@ -69,7 +69,7 @@ typedef struct _VipsRegion { */ /*< private >*/ RegionType type; /* What kind of attachment */ - char *data; /* Off here to get data */ + PEL *data; /* Off here to get data */ int bpl; /* Bytes-per-line for data */ void *seq; /* Sequence we are using to fill region */ @@ -150,7 +150,7 @@ int vips_region_prepare_many( VipsRegion **reg, VipsRect *r ); (R)->valid.left, \ (R)->valid.top, \ (R)->valid.width, \ - (R)->valid.height ), abort(), (char *) NULL) \ + (R)->valid.height ), abort(), (PEL *) NULL) \ ) #else /*DEBUG*/ #define VIPS_REGION_ADDR( R, X, Y ) \ diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c index 2f90572d..28810312 100644 --- a/libvips/iofuncs/region.c +++ b/libvips/iofuncs/region.c @@ -934,8 +934,8 @@ vips_region_copy( VipsRegion *reg, VipsRegion *dest, VipsRect *r, int x, int y ) { int z; int len = VIPS_IMAGE_SIZEOF_PEL( reg->im ) * r->width; - char *p = VIPS_REGION_ADDR( reg, r->left, r->top ); - char *q = VIPS_REGION_ADDR( dest, x, y ); + PEL *p = VIPS_REGION_ADDR( reg, r->left, r->top ); + PEL *q = VIPS_REGION_ADDR( dest, x, y ); int plsk = VIPS_REGION_LSKIP( reg ); int qlsk = VIPS_REGION_LSKIP( dest ); @@ -1092,7 +1092,7 @@ vips_region_prepare_to_generate( VipsRegion *reg, VipsRegion *dest, VipsRect *r, int x, int y ) { IMAGE *im = reg->im; - char *p; + PEL *p; if( !im->generate_fn ) { vips_error( "vips_region_prepare_to", diff --git a/libvips/iofuncs/window.c b/libvips/iofuncs/window.c index 25ccb47a..8f7bcad1 100644 --- a/libvips/iofuncs/window.c +++ b/libvips/iofuncs/window.c @@ -245,7 +245,7 @@ vips_window_set( VipsWindow *window, int top, int height ) window->baseaddr = baseaddr; window->length = pagelength; - window->data = (char *) baseaddr + (start - pagestart); + window->data = (PEL *) baseaddr + (start - pagestart); window->top = top; window->height = height;