From bd5c4757e8d0fc43c96f34575cc822d692f5f092 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 6 Jun 2013 12:08:26 +0100 Subject: [PATCH] fix vips_shrink() seq again perhaps properly this time --- ChangeLog | 3 +- TODO | 55 --------------------------------- libvips/conversion/sequential.c | 8 ++--- libvips/conversion/tilecache.c | 3 +- libvips/resample/shrink.c | 50 +++++++++++++++++------------- 5 files changed, 35 insertions(+), 84 deletions(-) diff --git a/ChangeLog b/ChangeLog index 25f09a3e..90fbdee0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,13 +1,14 @@ 12/3/13 started 7.33.0 - vipsthumbnail lets you specify the sharpening mask - turn off caching for im_copy()/vips_copy(), we use copy to stop sharing, and -it's cheap so caching doesn't help anyway + it's cheap so caching doesn't help anyway - auto rshift down to 8 bits on save, if necessary - im_gaussnoise(), im_copy_file(), im_grid(), im_scale(), im_scaleps(), im_wrap(), im_rotquad(), im_zoom(), im_subsample(), im_msb(), im_text(), im_system(), im_system_image() redone as classes - add --angle option to dzsave +- another vips_shrink() fix argh 14/5/13 started 7.32.4 - icc import and export could segv on very wide images diff --git a/TODO b/TODO index ddacd869..e0166c40 100644 --- a/TODO +++ b/TODO @@ -1,58 +1,3 @@ -- try - - $ vips shrink Chicago.png x.v 230 230 - ** VIPS:ERROR:vipspng.c:432:png2vips_generate: assertion failed: (r->top == read->y_pos) - Aborted (core dumped) - -$ vips shrink Chicago.png x.v 230 230 --vips-progress -vips_sequential_class_init -vips x.v: 127 x 18 pixels, 4 threads, 127 x 1 tiles, 1280 lines in buffer -vips_sequential_build - -thread 0x24a7cf0 request for 230 lines, start line 0 -thread 0x24a7cf0 has lock ... -thread 0x24a7cf0 reading ... - -thread 0x24a7ca0 request for 230 lines, start line 230 - -thread 0x24a7d40 request for 230 lines, start line 460 - -thread 0x24a7d90 request for 230 lines, start line 690 - -thread 0x24a7cf0 updating y_pos to 230 and waking stalled -thread 0x24a7cf0 unlocking ... - -thread 0x24a7ca0 has lock ... -thread 0x24a7ca0 reading ... - -thread 0x24a7cf0 request for 230 lines, start line 0 - -thread 0x24a7ca0 updating y_pos to 460 and waking stalled -thread 0x24a7ca0 unlocking ... - -thread 0x24a7d40 has lock ... -thread 0x24a7d40 reading ... - -thread 0x24a7ca0 request for 230 lines, start line 230 - -thread 0x24a7d40 updating y_pos to 690 and waking stalled -thread 0x24a7d40 unlocking ... - -thread 0x24a7d90 has lock ... -thread 0x24a7d90 reading ... - -thread 0x24a7d40 request for 230 lines, start line 460 - -thread 0x24a7d90 updating y_pos to 920 and waking stalled -thread 0x24a7d90 unlocking ... - -thread 0x24a7cf0 has lock ... -thread 0x24a7cf0 reading ... -** -VIPS:ERROR:vipspng.c:432:png2vips_generate: assertion failed: (r->top == -read->y_pos) -Aborted (core dumped) - - move other/im_makexy.c into copnversion? should sit with bvips_black()? diff --git a/libvips/conversion/sequential.c b/libvips/conversion/sequential.c index 150c6ba2..ce12f765 100644 --- a/libvips/conversion/sequential.c +++ b/libvips/conversion/sequential.c @@ -119,13 +119,13 @@ vips_sequential_generate( VipsRegion *or, VipsRect *r = &or->valid; VipsRegion *ir = (VipsRegion *) seq; - VIPS_DEBUG_MSG( "thread %p request for %d lines, start line %d\n", - g_thread_self(), r->height, r->top ); + VIPS_DEBUG_MSG( "thread %p request for line %d, height %d\n", + g_thread_self(), r->top, r->height ); if( sequential->trace ) vips_diag( class->nickname, - "request for %d lines, starting at line %d", - r->height, r->top ); + "request for line %d, height %d", + r->top, r->height ); g_mutex_lock( sequential->lock ); diff --git a/libvips/conversion/tilecache.c b/libvips/conversion/tilecache.c index dce182eb..61c7675f 100644 --- a/libvips/conversion/tilecache.c +++ b/libvips/conversion/tilecache.c @@ -863,10 +863,9 @@ vips_line_cache_gen( VipsRegion *or, /* We size up the cache to the largest request. */ if( or->valid.height > - block_cache->max_tiles * block_cache->tile_height ) { + block_cache->max_tiles * block_cache->tile_height ) block_cache->max_tiles = 1 + (or->valid.height / block_cache->tile_height); - } g_mutex_unlock( block_cache->lock ); diff --git a/libvips/resample/shrink.c b/libvips/resample/shrink.c index 22f984a0..c1b03139 100644 --- a/libvips/resample/shrink.c +++ b/libvips/resample/shrink.c @@ -38,6 +38,9 @@ * - don't change xres/yres, see comment below * 8/4/13 * - oops demand_hint was incorrect, thanks Jan + * 6/6/13 + * - don't chunk horizontally, fixes seq problems with large shrink + * factors */ /* @@ -255,43 +258,46 @@ vips_shrink_gen( VipsRegion *or, void *vseq, void *a, void *b, gboolean *stop ) * pixels, so we walk *r in chunks which map to the tile size. * * Make sure we can't ask for a zero step. + * + * We don't chunk horizontally. We want "vips shrink x.jpg b.jpg 100 + * 100" to run sequentially. If we chunk horizontally, we will fetch + * 100x100 lines from the top of the image, then 100x100 100 lines + * down, etc. for each thread, then when they've finished, fetch + * 100x100, 100 pixels across from the top of the image. This will + * break sequentiality. */ - int xstep = shrink->mw > VIPS__TILE_WIDTH ? - 1 : VIPS__TILE_WIDTH / shrink->mw; int ystep = shrink->mh > VIPS__TILE_HEIGHT ? 1 : VIPS__TILE_HEIGHT / shrink->mh; - int x, y; + int y; #ifdef DEBUG printf( "vips_shrink_gen: generating %d x %d at %d x %d\n", r->width, r->height, r->left, r->top ); #endif /*DEBUG*/ - for( y = 0; y < r->height; y += ystep ) - for( x = 0; x < r->width; x += xstep ) { - /* Clip the this rect against the demand size. - */ - int width = VIPS_MIN( xstep, r->width - x ); - int height = VIPS_MIN( ystep, r->height - y ); + for( y = 0; y < r->height; y += ystep ) { + /* Clip the this rect against the demand size. + */ + int height = VIPS_MIN( ystep, r->height - y ); - VipsRect s; + VipsRect s; - s.left = (r->left + x) * shrink->xshrink; - s.top = (r->top + y) * shrink->yshrink; - s.width = ceil( width * shrink->xshrink ); - s.height = ceil( height * shrink->yshrink ); + s.left = r->left * shrink->xshrink; + s.top = (r->top + y) * shrink->yshrink; + s.width = ceil( r->width * shrink->xshrink ); + s.height = ceil( height * shrink->yshrink ); #ifdef DEBUG - printf( "shrink_gen: requesting %d x %d at %d x %d\n", - s.width, s.height, s.left, s.top ); + printf( "shrink_gen: requesting %d x %d at %d x %d\n", + s.width, s.height, s.left, s.top ); #endif /*DEBUG*/ - if( vips_region_prepare( ir, &s ) ) - return( -1 ); + if( vips_region_prepare( ir, &s ) ) + return( -1 ); - vips_shrink_gen2( shrink, seq, - or, ir, - r->left + x, r->top + y, width, height ); - } + vips_shrink_gen2( shrink, seq, + or, ir, + r->left, r->top + y, r->width, height ); + } return( 0 ); }