From 42f5403cfa7d01d5fa3b2a205d1ef4a0d28d42a7 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 15 Dec 2015 10:49:02 +0000 Subject: [PATCH 1/3] notes --- ChangeLog | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ChangeLog b/ChangeLog index c363c142..0e17fb59 100644 --- a/ChangeLog +++ b/ChangeLog @@ -18,6 +18,8 @@ - added vips_image_new_from_memory_copy() - added vips_arrayjoin() - Python x.bandjoin(y) is now x.ibandjoin(y), sorry +- oop, removed a DEBUG from buffer.c, vips is 30% faster +- faster and lower-mem TIFF read 7/5/15 started 8.1.1 - oop, vips-8.0 wrapper script should be vips-8.1, thanks Danilo From bf7bca18515983b6547deb3c09359c3a6d0dbdbc Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 16 Dec 2015 16:26:30 +0000 Subject: [PATCH 2/3] correct default overlap value should be 1 for default layouot mode see https://github.com/jcupitt/libvips/issues/357 --- libvips/foreign/dzsave.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libvips/foreign/dzsave.c b/libvips/foreign/dzsave.c index aac907e4..f5b9c929 100644 --- a/libvips/foreign/dzsave.c +++ b/libvips/foreign/dzsave.c @@ -1923,7 +1923,7 @@ vips_foreign_save_dz_class_init( VipsForeignSaveDzClass *class ) _( "Tile overlap in pixels" ), VIPS_ARGUMENT_OPTIONAL_INPUT, G_STRUCT_OFFSET( VipsForeignSaveDz, overlap ), - 0, 8192, 0 ); + 0, 8192, 1 ); VIPS_ARG_INT( class, "tile_size", 11, _( "Tile size" ), From 253cb8e2e317216d41de9c59badf98e4a072e5e8 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Thu, 17 Dec 2015 11:54:38 +0000 Subject: [PATCH 3/3] fix up dzsave overlap handling There was a mixup with the previous fix to dzsave overlap handling, correct it and update the test suite. In the previous revision, dzsave overlapped tiles by overlap and sized them by tile_size. In fact, tiles should be sized as (tile_size + overlap * 2), ie. tile_size refers to the number of unique pixels per tile. See https://github.com/jcupitt/libvips/issues/357 --- libvips/foreign/dzsave.c | 60 ++++++++++++++++++++++----------------- test/test_foreign.py | 61 +++++++++++++++++++--------------------- 2 files changed, 63 insertions(+), 58 deletions(-) diff --git a/libvips/foreign/dzsave.c b/libvips/foreign/dzsave.c index f5b9c929..39f2a1ca 100644 --- a/libvips/foreign/dzsave.c +++ b/libvips/foreign/dzsave.c @@ -57,6 +57,8 @@ * - better overlap handling, thanks robclouth * 25/11/15 * - always strip tile metadata + * 16/12/15 + * - fix overlap handling again, thanks erdmann */ /* @@ -112,6 +114,15 @@ various combinations of odd and even tile-size and overlap need testing too. + Overlap handling + + For deepzoom, tile-size == 256 and overlap == 1 means that edge tiles are + 257 x 257 (though less at the bottom right) and non-edge tiles are 258 x + 258. Tiles are positioned across the image in tile-size steps. This means + (confusingly) that two adjoining tiles will have two pixels in common. + + This has caused bugs in the past. + */ /* @@ -416,11 +427,6 @@ struct _VipsForeignSaveDz { Layer *layer; /* x2 shrink pyr layer */ - /* We step by tile_size - overlap as we move across the image ... - * make a note of it. - */ - int tile_step; - /* Count zoomify tiles we write. */ int tile_count; @@ -511,8 +517,8 @@ pyramid_build( VipsForeignSaveDz *dz, Layer *above, layer->width = width; layer->height = height; - layer->tiles_across = ROUND_UP( width, dz->tile_step ) / dz->tile_step; - layer->tiles_down = ROUND_UP( height, dz->tile_step ) / dz->tile_step; + layer->tiles_across = ROUND_UP( width, dz->tile_size ) / dz->tile_size; + layer->tiles_down = ROUND_UP( height, dz->tile_size ) / dz->tile_size; layer->real_pixels = *real_pixels; @@ -561,7 +567,7 @@ pyramid_build( VipsForeignSaveDz *dz, Layer *above, strip.left = 0; strip.top = 0; strip.width = layer->image->Xsize; - strip.height = dz->tile_size; + strip.height = dz->tile_size + dz->overlap; if( (strip.height & 1) == 1 ) strip.height += 1; if( vips_region_buffer( layer->strip, &strip ) ) { @@ -940,6 +946,7 @@ strip_init( Strip *strip, Layer *layer ) line.top = layer->y; line.width = image.width; line.height = dz->tile_size; + vips_rect_marginadjust( &line, dz->overlap ); vips_rect_intersectrect( &image, &line, &line ); @@ -970,6 +977,19 @@ strip_allocate( VipsThreadState *state, void *a, gboolean *stop ) printf( "strip_allocate\n" ); #endif /*DEBUG_VERBOSE*/ + /* We can't test for allocated area empty, since it might just have + * bits of the left-hand overlap in and no new pixels. Safest to count + * tiles across. + */ + if( strip->x / dz->tile_size >= layer->tiles_across ) { + *stop = TRUE; +#ifdef DEBUG_VERBOSE + printf( "strip_allocate: done\n" ); +#endif /*DEBUG_VERBOSE*/ + + return( 0 ); + } + image.left = 0; image.top = 0; image.width = layer->width; @@ -981,21 +1001,13 @@ strip_allocate( VipsThreadState *state, void *a, gboolean *stop ) state->pos.top = layer->y; state->pos.width = dz->tile_size; state->pos.height = dz->tile_size; + vips_rect_marginadjust( &state->pos, dz->overlap ); vips_rect_intersectrect( &image, &state->pos, &state->pos ); state->x = strip->x; state->y = layer->y; - strip->x += dz->tile_step; - - if( vips_rect_isempty( &state->pos ) ) { - *stop = TRUE; -#ifdef DEBUG_VERBOSE - printf( "strip_allocate: done\n" ); -#endif /*DEBUG_VERBOSE*/ - - return( 0 ); - } + strip->x += dz->tile_size; return( 0 ); } @@ -1159,7 +1171,7 @@ strip_work( VipsThreadState *state, void *a ) g_mutex_lock( vips__global_lock ); out = tile_name( layer, - state->x / dz->tile_step, state->y / dz->tile_step ); + state->x / dz->tile_size, state->y / dz->tile_size ); status = gsf_output_write( out, len, buf ); dz->bytes_written += len; @@ -1393,11 +1405,11 @@ strip_arrived( Layer *layer ) * Expand the strip if necessary to make sure we have an even * number of lines. */ - layer->y += dz->tile_step; + layer->y += dz->tile_size; new_strip.left = 0; - new_strip.top = layer->y; + new_strip.top = layer->y - dz->overlap; new_strip.width = layer->image->Xsize; - new_strip.height = dz->tile_size; + new_strip.height = dz->tile_size + 2 * dz->overlap; image_area.left = 0; image_area.top = 0; @@ -1568,10 +1580,6 @@ vips_foreign_save_dz_build( VipsObject *object ) save->ready = z; } - /* How much we step by as we write tiles. - */ - dz->tile_step = dz->tile_size - dz->overlap; - /* The real pixels we have from our input. This is about to get * expanded with background. */ diff --git a/test/test_foreign.py b/test/test_foreign.py index 41fe8645..14886ee0 100755 --- a/test/test_foreign.py +++ b/test/test_foreign.py @@ -356,26 +356,34 @@ class TestForeign(unittest.TestCase): # test each option separately and hope they all function together # correctly - # default deepzoom layout - self.colour.dzsave("test") + # default deepzoom layout ... we must use png here, since we want to + # test the overlap for equality + self.colour.dzsave("test", suffix = ".png") # test right edge ... default is 256x256 tiles, overlap 1 - x = Vips.Image.new_from_file("test_files/10/3_2.jpeg") - self.assertEqual(x.width, 256) - y = Vips.Image.new_from_file("test_files/10/4_2.jpeg") - self.assertEqual(y.width, - self.colour.width - 255 * int(self.colour.width / 255)) + tiles_across = int(self.colour.width / 256) + tiles_down = int(self.colour.height / 256) + + x = Vips.Image.new_from_file("test_files/10/%d_0.png" % (tiles_across - 2)) + self.assertEqual(x.width, 258) + y = Vips.Image.new_from_file("test_files/10/%d_0.png" % (tiles_across - 1)) + predict_width = self.colour.width - 256 * (tiles_across - 1) + 1 + self.assertEqual(y.width, predict_width) + + # the right two columns of x should equal the left two columns of y + left = x.crop(x.width - 2, 0, 2, x.height) + right = y.crop(0, 0, 2, y.height) + self.assertEqual((left - right).abs().max(), 0) # test bottom edge - x = Vips.Image.new_from_file("test_files/10/3_2.jpeg") - self.assertEqual(x.height, 256) - y = Vips.Image.new_from_file("test_files/10/3_3.jpeg") - self.assertEqual(y.height, - self.colour.height - - 255 * int(self.colour.height / 255)) + x = Vips.Image.new_from_file("test_files/10/0_%d.png" % (tiles_down - 2)) + self.assertEqual(x.height, 258) + y = Vips.Image.new_from_file("test_files/10/0_%d.png" % (tiles_down - 1)) + predict_height = self.colour.height - 256 * (tiles_down - 1) + 1 + self.assertEqual(y.height, predict_height) # there should be a bottom layer - x = Vips.Image.new_from_file("test_files/0/0_0.jpeg") + x = Vips.Image.new_from_file("test_files/0/0_0.png") self.assertEqual(x.width, 1) self.assertEqual(x.height, 1) @@ -420,8 +428,8 @@ class TestForeign(unittest.TestCase): # test suffix self.colour.dzsave("test", suffix = ".png") - x = Vips.Image.new_from_file("test_files/10/3_2.png") - self.assertEqual(x.width, 256) + x = Vips.Image.new_from_file("test_files/10/0_0.png") + self.assertEqual(x.width, 257) shutil.rmtree("test_files") os.unlink("test.dzi") @@ -429,9 +437,8 @@ class TestForeign(unittest.TestCase): # test overlap self.colour.dzsave("test", overlap = 200) - y = Vips.Image.new_from_file("test_files/10/18_6.jpeg") - self.assertEqual(y.width, - self.colour.width - 56 * int(self.colour.width / 56)) + y = Vips.Image.new_from_file("test_files/10/1_1.jpeg") + self.assertEqual(y.width, 256 + 200 * 2) shutil.rmtree("test_files") os.unlink("test.dzi") @@ -439,19 +446,9 @@ class TestForeign(unittest.TestCase): # test tile-size self.colour.dzsave("test", tile_size = 512) - y = Vips.Image.new_from_file("test_files/10/2_1.jpeg") - self.assertEqual(y.width, - self.colour.width - 511 * int(self.colour.width / 511)) - - shutil.rmtree("test_files") - os.unlink("test.dzi") - - # test tile-size - self.colour.dzsave("test", tile_size = 512) - - y = Vips.Image.new_from_file("test_files/10/2_1.jpeg") - self.assertEqual(y.width, - self.colour.width - 511 * int(self.colour.width / 511)) + y = Vips.Image.new_from_file("test_files/10/0_0.jpeg") + self.assertEqual(y.width, 513) + self.assertEqual(y.height, 513) shutil.rmtree("test_files") os.unlink("test.dzi")