remove vipspool

vips_object_local_array() is much better ... get embed using this as
well

ChangeLog

@@ -14,7 +14,6 @@
 - VipsMin stops search early if it can
 - C API supports optional output args
 - switch back to int-valued operations
-- fix up VipsPool
 - add the operation cache
 - fallback vips_init()
 - vips_tracked_malloc() tracks allocation size and can report total mem usage
@@ -29,6 +28,7 @@
 - added VipsArea as a public struct
 - added array members and arguments
 - added nary
+- remove VipsPool, vips_object_local_array() is much better
 
 12/10/11 started 7.26.6
 - NOCACHE was not being set correctly on OS X causing performance 

TODO

@@ -1,16 +1,60 @@
+- strange
+
+$ valgrind vips embed Gugg_coloured.jpg x.jpg 100 100 5000 5000 --extend
+mirror
+==32146== Memcheck, a memory error detector
+==32146== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
+==32146== Using Valgrind-3.6.1-Debian and LibVEX; rerun with -h for copyright
+info
+==32146== Command: vips embed Gugg_coloured.jpg x.jpg 100 100 5000 5000
+--extend mirror
+==32146== 
+==32146== Thread 2:
+==32146== Conditional jump or move depends on uninitialised value(s)
+==32146==    at 0x4F8ED7D: vips_sink_base_allocate (sink.c:257)
+==32146==    by 0x4F99BC2: vips_thread_allocate (threadpool.c:450)
+==32146==    by 0x4F99CB5: vips_thread_work_unit (threadpool.c:508)
+==32146==    by 0x4F99DF0: vips_thread_main_loop (threadpool.c:547)
+==32146==    by 0x55852B5: ??? (in
+/lib/x86_64-linux-gnu/libglib-2.0.so.0.3000.0)
+==32146==    by 0x5818EFB: start_thread (pthread_create.c:304)
+==32146==    by 0x5B0F89C: clone (clone.S:112)
+==32146== 
+
+it's the 'stop' member of SinkThreadState
+
+	/* Has work requested early termination?
+	 */
+	if( sstate->stop ) {
+		*stop = TRUE;
+
+		return( 0 );
+	}
+
+
+
 
 
 - don't do vips_image_write() at the end of a pipeline, instead try:
 
-	g_object_set( object, "output", t );
+	g_object_set( object, "out", t, NULL );
 
   ie. replace the output object
 
-  in cache.c, we must not include the hash of the output objects, I guess we
+  this doesn't work, I wonder why ... do we need to add an extra ref to t?
+
+
+
+
+- in cache.c, we must not include the hash of the output objects, I guess we
   don't
 
 
 
+- get rid of vipspool
+
+
+
 
 - vipsimage should be cached too, eg.
 

libvips/conversion/embed.c

@@ -325,84 +325,14 @@ vips_embed_gen( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop )
 	return( 0 );
 }
 
-static int
-vips_embed_repeat( VipsPool *pool, VipsImage *in, VipsImage **out,
-	int x, int y, int width, int height )
-{
-	VipsPoolContext *context = vips_pool_context_new( pool );
-
-	/* Clock arithmetic: we want negative x/y to wrap around
-	 * nicely.
-	 */
-	const int nx = x < 0 ?
-		-x % in->Xsize : in->Xsize - x % in->Xsize;
-	const int ny = y < 0 ?
-		-y % in->Ysize : in->Ysize - y % in->Ysize;
-
-	if( 
-		vips_replicate( in, &VIPS_VI( 1 ), 
-			width / in->Xsize + 2, 
-			height / in->Ysize + 2, NULL ) ||
-		vips_extract_area( VIPS_VI( 1 ), out, 
-			nx, ny, width, height, NULL ) ) 
-		return( -1 );
-
-	return( 0 );
-}
-
-static int
-vips_embed_mirror( VipsPool *pool, VipsImage *in, VipsImage **out,
-	int x, int y, int width, int height )
-{
-	VipsPoolContext *context = vips_pool_context_new( pool );
-
-	/* As repeat, but the tiles are twice the size because of
-	 * mirroring.
-	 */
-	const int w2 = in->Xsize * 2;
-	const int h2 = in->Ysize * 2;
-
-	const int nx = x < 0 ?  -x % w2 : w2 - x % w2;
-	const int ny = y < 0 ?  -y % h2 : h2 - y % h2;
-
-	if( 
-		/* Make a 2x2 mirror tile.
-		 */
-		vips_flip( in, &VIPS_VI( 1 ), 
-			VIPS_DIRECTION_HORIZONTAL, NULL ) ||
-		vips_join( in, VIPS_VI( 1 ), &VIPS_VI( 2 ), 
-			VIPS_DIRECTION_HORIZONTAL, NULL ) ||
-		vips_flip( VIPS_VI( 2 ), &VIPS_VI( 3 ), 
-			VIPS_DIRECTION_VERTICAL, NULL ) ||
-		vips_join( VIPS_VI( 2 ), VIPS_VI( 3 ), &VIPS_VI( 4 ), 
-			VIPS_DIRECTION_VERTICAL, NULL ) ||
-
-		/* Repeat, then cut out the centre.
-		 */
-		vips_replicate( VIPS_VI( 4 ), &VIPS_VI( 5 ), 
-			width / VIPS_VI( 4 )->Xsize + 2, 
-			height / VIPS_VI( 4 )->Ysize + 2, NULL ) ||
-		vips_extract_area( VIPS_VI( 5 ), &VIPS_VI( 6 ), 
-			nx, ny, width, height, NULL ) ||
-
-		/* Overwrite the centre with the in, much faster
-		 * for centre pixels.
-		 */
-		vips_insert( VIPS_VI( 6 ), in, out, 
-			x, y, NULL ) )
-			return( -1 );
-
-	return( 0 );
-}
-
 static int
 vips_embed_build( VipsObject *object )
 {
 	VipsConversion *conversion = VIPS_CONVERSION( object );
 	VipsEmbed *embed = (VipsEmbed *) object;
+	VipsImage **t = (VipsImage **) vips_object_local_array( object, 7 );
 
 	VipsRect want;
-	VipsPool *pool;
 
 	if( VIPS_OBJECT_CLASS( vips_embed_parent_class )->build( object ) )
 		return( -1 );
@@ -419,30 +349,72 @@ vips_embed_build( VipsObject *object )
 		vips_image_pio_output( conversion->out ) )
 		return( -1 );
 
-	pool = vips_pool_new( "VipsEmbed" );
-	vips_object_local( object, pool );
-
 	switch( embed->extend ) {
 	case VIPS_EXTEND_REPEAT:
 {
-		VipsPoolContext *context = vips_pool_context_new( pool );
+		/* Clock arithmetic: we want negative x/y to wrap around
+		 * nicely.
+		 */
+		const int nx = embed->x < 0 ?
+			-embed->x % embed->in->Xsize : 
+			embed->in->Xsize - embed->x % embed->in->Xsize;
+		const int ny = embed->y < 0 ?
+			-embed->y % embed->in->Ysize : 
+			embed->in->Ysize - embed->y % embed->in->Ysize;
 
-		if( vips_embed_repeat( pool, embed->in, &VIPS_VI( 1 ),
+		if( vips_replicate( embed->in, &t[0],
-			embed->x, embed->y, embed->width, embed->height ) ||
+			embed->width / embed->in->Xsize + 2, 
-			vips_image_write( VIPS_VI( 1 ), conversion->out ) )
+			embed->height / embed->in->Ysize + 2, NULL ) ||
+			vips_extract_area( t[0], &t[1], 
+				nx, ny, embed->width, embed->height, NULL ) || 
+			vips_image_write( t[1], conversion->out ) )
 			return( -1 );
-}
 
+}
 		break;
 
 	case VIPS_EXTEND_MIRROR:
 {
-		VipsPoolContext *context = vips_pool_context_new( pool );
+		/* As repeat, but the tiles are twice the size because of
+		 * mirroring.
+		 */
+		const int w2 = embed->in->Xsize * 2;
+		const int h2 = embed->in->Ysize * 2;
 
-		if( vips_embed_mirror( pool, embed->in, &VIPS_VI( 1 ),
+		const int nx = embed->x < 0 ?  
-			embed->x, embed->y, embed->width, embed->height ) ||
+			-embed->x % w2 : w2 - embed->x % w2;
-			vips_image_write( VIPS_VI( 1 ), conversion->out ) )
+		const int ny = embed->y < 0 ?  
-			return( -1 );
+			-embed->y % h2 : h2 - embed->y % h2;
+
+
+		if( 
+			/* Make a 2x2 mirror tile.
+			 */
+			vips_flip( embed->in, &t[0], 
+				VIPS_DIRECTION_HORIZONTAL, NULL ) ||
+			vips_join( embed->in, t[0], &t[1], 
+				VIPS_DIRECTION_HORIZONTAL, NULL ) ||
+			vips_flip( t[1], &t[2], 
+				VIPS_DIRECTION_VERTICAL, NULL ) ||
+			vips_join( t[1], t[2], &t[3], 
+				VIPS_DIRECTION_VERTICAL, NULL ) ||
+
+			/* Repeat, then cut out the centre.
+			 */
+			vips_replicate( t[3], &t[4], 
+				embed->width / t[3]->Xsize + 2, 
+				embed->height / t[3]->Ysize + 2, NULL ) ||
+			vips_extract_area( t[4], &t[5], 
+				nx, ny, embed->width, embed->height, NULL ) ||
+
+			/* Overwrite the centre with the in, much faster
+			 * for centre pixels.
+			 */
+			vips_insert( t[5], embed->in, &t[6], 
+				embed->x, embed->y, NULL ) ||
+
+			vips_image_write( t[6], conversion->out ) )
+				return( -1 );
 }
 		break;
 

libvips/include/vips/Makefile.am

@@ -24,7 +24,6 @@ pkginclude_HEADERS = \
 	interpolate.h \
 	intl.h \
 	mask.h \
-	pool.h \
 	memory.h \
 	morphology.h \
 	mosaicing.h \

libvips/include/vips/pool.h

@@ -1,92 +0,0 @@
-/* manage pools of objects for unreffing
- */
-
-/*
-
-    Copyright (C) 1991-2003 The National Gallery
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with this program; if not, write to the Free Software
-    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
-
- */
-
-/*
-
-    These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk
-
- */
-
-#ifndef VIPS_POOL_H
-#define VIPS_POOL_H
-
-#ifdef __cplusplus
-extern "C" {
-#endif /*__cplusplus*/
-
-/* A pool context ... just an array of GObject pointers.
- */
-typedef GPtrArray VipsPoolContext;
-
-/* A VipsPool is really just a hash table, but we make it a VipsObject so we
- * can put pools into other pools.
- */
-
-#define VIPS_TYPE_POOL (vips_pool_get_type())
-#define VIPS_POOL( obj ) \
-	(G_TYPE_CHECK_INSTANCE_CAST( (obj), VIPS_TYPE_POOL, VipsPool ))
-#define VIPS_POOL_CLASS( klass ) \
-	(G_TYPE_CHECK_CLASS_CAST( (klass), VIPS_TYPE_POOL, VipsPoolClass))
-#define VIPS_IS_POOL( obj ) \
-	(G_TYPE_CHECK_INSTANCE_TYPE( (obj), VIPS_TYPE_POOL ))
-#define VIPS_IS_POOL_CLASS( klass ) \
-	(G_TYPE_CHECK_CLASS_TYPE( (klass), VIPS_TYPE_POOL ))
-#define VIPS_POOL_GET_CLASS( obj ) \
-	(G_TYPE_INSTANCE_GET_CLASS( (obj), VIPS_TYPE_POOL, VipsPoolClass ))
-
-typedef struct _VipsPool {
-	VipsObject parent_object;
-	
-	/* A table of all the contexts we've seen.
-	 */
-	GHashTable *contexts;
-
-	/* Track a name for debugging.
-	 */
-	const char *name;
-} VipsPool;
-
-typedef struct _VipsPoolClass {
-	VipsObjectClass parent_class;
-} VipsPoolClass;
-
-GType vips_pool_get_type( void );
-
-VipsPool *vips_pool_new( const char *name );
-VipsPoolContext *vips_pool_context_new( VipsPool *pool );
-GObject **vips_pool_context_object( VipsPoolContext *context, int n );
-
-/* Save some typing. This assumes you have a (VipsPoolContext *) called
- * "context" in scope.
- */
-#define VIPS_VI( N ) \
-        (*((VipsImage **) vips_pool_context_object( context, (N) )))
-
-
-#ifdef __cplusplus
-}
-#endif /*__cplusplus*/
-
-#endif /*VIPS_POOL_H*/
-
-

libvips/include/vips/vips.h

@@ -98,7 +98,6 @@ extern "C" {
 #include <vips/util.h>
 #include <vips/object.h>
 #include <vips/type.h>
-#include <vips/pool.h>
 
 #include <vips/version.h>
 #include <vips/rect.h>

libvips/iofuncs/Makefile.am

@@ -4,7 +4,6 @@ libiofuncs_la_SOURCES = \
 	type.c \
 	enumtypes.c \
 	object.c \
-	pool.c \
 	base64.h \
 	base64.c \
 	error.c \

libvips/iofuncs/pool.c

@@ -1,219 +0,0 @@
-/* manage pools of objects for unreffing
- */
-
-/*
-
-    Copyright (C) 1991-2003 The National Gallery
-
-    This program is free software; you can redistribute it and/or modify
-    it under the terms of the GNU General Public License as published by
-    the Free Software Foundation; either version 2 of the License, or
-    (at your option) any later version.
-
-    This program is distributed in the hope that it will be useful,
-    but WITHOUT ANY WARRANTY; without even the implied warranty of
-    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-    GNU General Public License for more details.
-
-    You should have received a copy of the GNU General Public License
-    along with this program; if not, write to the Free Software
-    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
-
- */
-
-/*
-
-    These files are distributed with VIPS - http://www.vips.ecs.soton.ac.uk
-
- */
-
-/*
-#define VIPS_DEBUG
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif /*HAVE_CONFIG_H*/
-#include <vips/intl.h>
-
-#include <stdio.h>
-#include <string.h>
-
-#include <vips/vips.h>
-#include <vips/internal.h>
-#include <vips/debug.h>
-
-/*
-
-  Here's how to handle ref counts when calling vips operations:
-
-	int
-	thing( VipsImage *in1, VipsImage *in2, VipsImage **out )
-	{
-		VipsImage *t;
-
-		if( vips_add( in1, in2, &t, NULL ) )
-			return( -1 );
-		if( vips_add( in1, t, out, NULL ) ) {
-			g_object_unref( t );
-			return( -1 );
-		}
-		g_object_unref( t );
-
-		return( 0 );
-	}
-
-  The first vips_add() call returns (via the reference argument) a new 
-  VipsImage in variable t. The second vips_add() uses this as an input and 
-  takes the ref count up to two. After calling the second vips_add() we have 
-  to drop t to avoid leaks. We also have to drop t if the second vips_add() 
-  fails.
-
-  VipsPool provides a nicer way to track the objects that you create and free 
-  them safely. The above function would become:
-
-  	int
-	thing( VipsPool *pool, VipsImage *in1, VipsImage *in2, VipsImage **out )
-	{
-		VipsPoolContext *context = vips_pool_context_new( pool );
-
-		if( vips_add( in1, in2, &VIPS_VI( 1 ), NULL ) ||
-			vips_add( in1, VIPS_VI( 1 ), out, NULL ) )
-			return( -1 );
-
-		return( 0 );
-	}
-
-  vips_pool_context_new() creates a new context to hold a set of temporary 
-  objects. You can get a pointer to a temporary image object with the 
-  macro VIPS_VI() (this assumes there is a variable called "context" in scope).
-  Temporary objects are numbered from zero.
-
-  Our caller will (eventually) call g_object_unref() on the pool and this 
-  will in turn unref all objects in the pool.
-
- */
-
-G_DEFINE_TYPE( VipsPool, vips_pool, VIPS_TYPE_OBJECT );
-
-static void
-vips_pool_dispose( GObject *gobject )
-{
-	VipsPool *pool = VIPS_POOL( gobject );
-
-#ifdef VIPS_DEBUG
-	VIPS_DEBUG_MSG( "vips_pool_dispose: " );
-	vips_object_print( VIPS_OBJECT( gobject ) );
-#endif /*VIPS_DEBUG*/
-
-	VIPS_FREEF( g_hash_table_unref, pool->contexts );
-
-	G_OBJECT_CLASS( vips_pool_parent_class )->dispose( gobject );
-}
-
-static void
-vips_pool_context_print( VipsPoolContext *context, VipsPoolContext *value, 
-	VipsBuf *buf )
-{
-	int i, n;
-
-	n = 0;
-	for( i = 0; i < context->len; i++ )
-		if( g_ptr_array_index( context, i ) ) 
-			n += 1;
-
-	vips_buf_appendf( buf, "VipsPoolContext %p, size %d, %d objects\n", 
-		context, context->len, n );
-
-	for( i = 0; i < context->len; i++ )
-		if( g_ptr_array_index( context, i ) ) 
-			vips_buf_appendf( buf, "\t%p\n", 
-				g_ptr_array_index( context, i ) ) ;
-}
-
-static void
-vips_pool_print( VipsObject *object, VipsBuf *buf )
-{
-	VipsPool *pool = VIPS_POOL( object );
-
-	if( pool->contexts ) {
-		int size = g_hash_table_size( pool->contexts );
-
-		vips_buf_appendf( buf, "%d contexts\n", size ); 
-		if( size > 0 )  
-			g_hash_table_foreach( pool->contexts, 
-				(GHFunc) vips_pool_context_print, buf );
-	}
-
-	VIPS_OBJECT_CLASS( vips_pool_parent_class )->print( object, buf );
-}
-
-static void
-vips_pool_class_init( VipsPoolClass *class )
-{
-	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
-	VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class );
-
-	gobject_class->dispose = vips_pool_dispose;
-
-	vobject_class->print = vips_pool_print;
-}
-
-static void
-vips_pool_init( VipsPool *pool )
-{
-	pool->contexts = g_hash_table_new_full( g_direct_hash, g_direct_equal, 
-		NULL, (GDestroyNotify) g_ptr_array_unref );
-}
-
-VipsPool *
-vips_pool_new( const char *name )
-{
-	VipsPool *pool;
-
-	vips_check_init();
-
-	pool = VIPS_POOL( g_object_new( VIPS_TYPE_POOL, NULL ) );
-
-	pool->name = name;
-
-	if( vips_object_build( VIPS_OBJECT( pool ) ) ) {
-		VIPS_UNREF( pool );
-		return( NULL );
-	}
-
-	return( pool );
-}
-
-static void
-vips_pool_context_element_free( GObject *object )
-{
-	if( object )
-		g_object_unref( object );
-}
-
-VipsPoolContext *
-vips_pool_context_new( VipsPool *pool )
-{
-	VipsPoolContext *context;
-
-	/* g_object_unref() hates unreffing NULL.
-	 */
-	context = g_ptr_array_new_with_free_func( 
-		(GDestroyNotify) vips_pool_context_element_free );
-	g_hash_table_insert( pool->contexts, context, context );
-
-	return( context );
-}
-
-GObject **
-vips_pool_context_object( VipsPoolContext *context, int n )
-{
-	g_assert( n >= 0 );
-
-	if( n >= context->len )
-		g_ptr_array_set_size( context, n + 10 );
-
-	return( (GObject **) &g_ptr_array_index( context, n ) );
-}
-