more sequential improvements

always do everything on first pixel access if there's a @load() method

libvips/foreign/foreign.c

@@ -684,7 +684,27 @@ vips_foreign_load_temp( VipsForeignLoad *load )
 	const guint64 disc_threshold = vips_get_disc_threshold();
 	const guint64 image_size = VIPS_IMAGE_SIZEOF_IMAGE( load->out );
 
-	VipsImage *temp;
+	/* If this is a partial operation, we can open directly.
+	 */
+	if( load->flags & VIPS_FOREIGN_PARTIAL ) {
+#ifdef DEBUG
+		printf( "vips_foreign_load_temp: partial temp\n" );
+#endif /*DEBUG*/
+
+		return( vips_image_new() );
+	}
+
+	/* If it can do sequential access and it's been requested, we can open
+	 * directly.
+	 */
+	if( (load->flags & VIPS_FOREIGN_SEQUENTIAL) && 
+		load->sequential ) {
+#ifdef DEBUG
+		printf( "vips_foreign_load_temp: partial sequential temp\n" );
+#endif /*DEBUG*/
+
+		return( vips_image_new() );
+	}
 
 	/* We open via disc if:
 	 * - 'disc' is set
@@ -696,23 +716,19 @@ vips_foreign_load_temp( VipsForeignLoad *load )
 		disc_threshold && 
 		image_size > disc_threshold ) {
 #ifdef DEBUG
-		printf( "vips_foreign_load_temp: making disc temp\n" );
+		printf( "vips_foreign_load_temp: disc temp\n" );
 #endif /*DEBUG*/
 
-		if( !(temp = vips_image_new_disc_temp( "%s.v" )) )
+		return( vips_image_new_disc_temp( "%s.v" ) );
-			return( NULL );
 	}
-	else {
+
 #ifdef DEBUG
-		printf( "vips_foreign_load_start: making 'p' temp\n" );
+	printf( "vips_foreign_load_temp: memory temp\n" );
 #endif /*DEBUG*/
 
-		/* Otherwise, fall back to a "p".
+	/* Otherwise, fall back to a memory buffer.
-		 */
+	 */
-		temp = vips_image_new();
+	return( vips_image_new_buffer() );
-	}
-
-	return( temp );
 }
 
 /* Check two images for compatibility: their geometries need to match.
@@ -839,48 +855,12 @@ vips_foreign_load_build( VipsObject *object )
 	 * everything. Otherwise, it's just set fields and we must also
 	 * load pixels.
 	 *
-	 * If it's a partial loader, or if it's a sequiential loader and
+	 * Delay the load until the first pixel is requested by doing the work
-	 * sequential load has been requested, we can load here. 
+	 * in the start function of the copy.
-	 *
-	 * Otherwise, it's a whole-image read and we delay until first pixel
-	 * access. 
 	 */
-	if( class->load && 
+	if( class->load ) {
-		((load->flags & VIPS_FOREIGN_PARTIAL) ||
-		 ((load->flags & VIPS_FOREIGN_SEQUENTIAL) && 
-		  load->sequential)) ) { 
 #ifdef DEBUG
-		printf( "vips_foreign_load_build: partial read\n" );
+		printf( "vips_foreign_load_build: delaying read ...\n" );
-#endif /*DEBUG*/
-
-		/* Read the image to @real.
-		 */
-		load->real = vips_image_new();
-		if( class->load( load ) ||
-			vips_image_pio_input( load->real ) ) 
-			return( -1 );
-
-		/* Must match ->header(). 
-		 */
-		if( !vips_foreign_load_iscompat( load->real, load->out ) )
-			return( -1 ); 
-
-		/* ->header() should set the dhint. It'll default to the safe
-		 * SMALLTILE if ->header() did not set it.
-		 */
-		vips_demand_hint( load->out, load->out->dhint, 
-			load->real, NULL );
-
-		if( vips_image_generate( load->out, 
-			vips_start_one, 
-			vips_foreign_load_generate, 
-			vips_stop_one, 
-			load->real, load ) ) 
-			return( -1 );
-	}
-	else if( class->load ) { 
-#ifdef DEBUG
-		printf( "vips_foreign_load_build: whole-image read\n" );
 #endif /*DEBUG*/
 
 		/* ->header() should set the dhint. It'll default to the safe

libvips/foreign/vipspng.c

@@ -112,7 +112,6 @@ typedef struct {
 	png_structp pPng;
 	png_infop pInfo;
 	png_bytep *row_pointer;
-	int interlace_type;
 } Read;
 
 static void
@@ -138,7 +137,6 @@ read_new( const char *name, VipsImage *out )
 	read->pPng = NULL;
 	read->pInfo = NULL;
 	read->row_pointer = NULL;
-	read->interlace_type = PNG_INTERLACE_NONE;
 
 	g_signal_connect( out, "close", 
 		G_CALLBACK( read_destroy ), read ); 
@@ -169,6 +167,7 @@ png2vips_header( Read *read, VipsImage *out )
 {
 	png_uint_32 width, height;
 	int bit_depth, color_type;
+	int interlace_type;
 
 	png_uint_32 res_x, res_y;
 	int unit_type;
@@ -184,7 +183,7 @@ png2vips_header( Read *read, VipsImage *out )
 	png_read_info( read->pPng, read->pInfo );
 	png_get_IHDR( read->pPng, read->pInfo, 
 		&width, &height, &bit_depth, &color_type,
-		&read->interlace_type, NULL, NULL );
+		&interlace_type, NULL, NULL );
 
 	/* png_get_channels() gives us 1 band for palette images ... so look
 	 * at colour_type for output bands.
@@ -341,6 +340,13 @@ png2vips_generate( VipsRegion *or,
 		r->top, r->height );
 #endif /*DEBUG*/
 
+	/* We're inside a tilecache where tiles are the full image width, so
+	 * this should always be true.
+	 */
+	g_assert( r->left == 0 );
+	g_assert( r->width == or->im->Xsize );
+	g_assert( VIPS_RECT_BOTTOM( r ) <= or->im->Ysize );
+
 	if( setjmp( png_jmpbuf( read->pPng ) ) ) 
 		return( -1 );
 
@@ -364,12 +370,11 @@ vips__png_isinterlaced( const char *filename )
 	int interlace_type;
 
 	image = vips_image_new();
-	if( !(read = read_new( filename, image )) ||
+	if( !(read = read_new( filename, image )) ) {
-		png2vips_header( read, image ) ) {
 		g_object_unref( image );
 		return( -1 );
 	}
-	interlace_type = read->interlace_type;
+	interlace_type = png_get_interlace_type( read->pPng, read->pInfo );
 	g_object_unref( image );
 
 	return( interlace_type != PNG_INTERLACE_NONE );
@@ -387,21 +392,24 @@ vips__png_read( const char *name, VipsImage *out )
 	printf( "png2vips: reading \"%s\"\n", name );
 #endif /*DEBUG*/
 
-	t[0] = vips_image_new_buffer();
+	if( !(read = read_new( name, out )) )
-	if( !(read = read_new( name, out )) ||
-		png2vips_header( read, t[0] ) )
 		return( -1 );
 
-	if( read->interlace_type != PNG_INTERLACE_NONE ) { 
+	if( png_get_interlace_type( read->pPng, read->pInfo ) != 
-		/* Arg we have to read to a huge mem buffer, then copy to out.
+		PNG_INTERLACE_NONE ) { 
+		/* Arg awful interlaced image. We have to load to a huge mem 
+		 * buffer, then copy to out.
 		 */
 		t[0] = vips_image_new_buffer();
-		if( png2vips_interlace( read, t[0] ) ||
+		if( png2vips_header( read, t[0] ) ||
+			png2vips_interlace( read, t[0] ) ||
 			vips_image_write( t[0], out ) )
 			return( -1 );
 	}
 	else {
-		if( vips_image_generate( t[0], 
+		t[0] = vips_image_new();
+		if( png2vips_header( read, t[0] ) ||
+			vips_image_generate( t[0], 
 				NULL, png2vips_generate, NULL, 
 				read, NULL ) ||
 			vips_sequential( t[0], &t[1], NULL ) ||
@@ -414,6 +422,10 @@ vips__png_read( const char *name, VipsImage *out )
 			return( -1 );
 	}
 
+#ifdef DEBUG
+	printf( "png2vips: done\n" );
+#endif /*DEBUG*/
+
 	return( 0 );
 }
 

libvips/iofuncs/sinkmemory.c

@@ -31,8 +31,8 @@
  */
 
 /*
-#define VIPS_DEBUG
  */
+#define VIPS_DEBUG
 
 #ifdef HAVE_CONFIG_H
 #include <config.h>
@@ -103,18 +103,6 @@ sink_memory_work( VipsThreadState *state, void *a )
 		&state->pos, state->pos.left, state->pos.top ) )
 		return( -1 );
 
-#ifdef VIPS_DEBUG
-{
-	VipsPel *p = VIPS_REGION_ADDR( state->reg, 
-		state->pos.left, state->pos.top );
-	int i;
-
-	VIPS_DEBUG_MSG( "sink_memory_work: %p\n", memory );
-	for( i = 0; i < VIPS_IMAGE_SIZEOF_PEL( state->reg->im ); i++ )
-		printf( "\t%d) %02x\n", i, p[i] );
-}
-#endif /*VIPS_DEBUG*/
-
 	return( 0 );
 }