back to fixed refs, add a pool system instead

floating refs were very confusing and annoying, instead use simple fixed
refs and add a pool system to track refs for you

also fixed a couple of dumb errors in close callbacks

ChangeLog

@@ -65,6 +65,7 @@
 - laplacian generator lost -ve lobes for large sigma
 - added im_aconv(), approximate convolution
 - bumped smalltile to 512x512 for testing
+- added VipsPool, got rid of floating refs again, argh
 
 30/11/10 started 7.24.0
 - bump for new stable

TODO

@@ -1,6 +1,3 @@
-- leak on exit, try:
-
-	vips im_aconv img_0075.jpg x.v gmask_201.con 10
 
 
 - also VipsFormat ... could this replace vips_image_new_from_string()? or
@@ -13,67 +10,6 @@
 
 
 
-- perhaps we should have hard refs everywhere? it's very confusing having a
-  mixture :-( use vips_object_local() to make hard refs autounref
-
-  what would the API look like?
-
-
-	VipsImage *
-	thing( VipsImage *in1, VipsImage *in2 )
-	{
-		VipsImage *t;
-		VipsImage *out;
-
-		if( vips_add( in1, in2, &t, NULL ) )
-			return( NULL );
-		if( vips_add( in1, t, &out, NULL ) ) {
-			g_object_unref( t );
-			return( NULL );
-		}
-		g_object_unref( t );
-
-		return( out );
-	}
-
-  yuk! how can we simplify this? keep refs in an array?
-
-	VipsImage *
-	thing( VipsImage *in1, VipsImage *in2 )
-	{
-		VipsImage *t[5] = { NULL }; 
-		VipsImage *out;
-
-		if( vips_add( in1, in2, &t[0], NULL ) ||
-			vips_add( in1, t[0], &out, NULL ) ) {
-			vips_unref_array( t, 5 );
-			return( NULL );
-		}
-		vips_unref_array( t, 5 );
-
-		return( out );
-	}
-
-  vips_unref_array() just unrefs all non-NULL pointers in the array
-
-  not too bad! and getting rid of floating refs is a win
-
-
-	
-
-
-
-
-
-
-- try out:
-
-	http://incubator.quasimondo.com/processing/stackblur.pde
-
-
-
-
-
 
 - make something for Python as well
 

libvips/convolution/im_aconv.c

@@ -35,6 +35,17 @@
 
  */
 
+/*
+
+  See:
+
+	http://incubator.quasimondo.com/processing/stackblur.pde
+
+  This thing is a little like stackblur, but generalised to any separable 
+  mask.
+
+ */
+
 /* Show sample pixels as they are transformed.
 #define DEBUG_PIXELS
  */

libvips/deprecated/vips7compat.c

@@ -60,11 +60,6 @@ im_open( const char *filename, const char *mode )
 	if( !(image = vips_image_new_mode( filename, mode )) )
 		return( NULL );
 
-	/* We have to refsink since the im_open() result is used like a hard
-	 * reference.
-	 */
-	g_object_ref_sink( image );
-
 	return( image );
 }
 

libvips/include/vips/object.h

@@ -201,7 +201,7 @@ void vips_argument_free_all( VipsObject *object );
 	(G_TYPE_INSTANCE_GET_CLASS( (obj), VIPS_TYPE_OBJECT, VipsObjectClass ))
 
 struct _VipsObject {
-	GInitiallyUnowned parent_object;
+	GObject parent_object;
 
 	gboolean constructed;		/* Construct done and checked */
 
@@ -223,7 +223,7 @@ struct _VipsObject {
 };
 
 struct _VipsObjectClass {
-	GInitiallyUnownedClass parent_class;
+	GObjectClass parent_class;
 
 	/* Build the object ... all argument properties have been set,
 	 * now build the thing.

libvips/include/vips/pool.h

@@ -0,0 +1,88 @@
+/* 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;
+} 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.
+ */
+#define VIPS_VAR_IMAGE_REF( N ) \
+	((VipsImage **) vips_pool_context_object( context, (N) ))
+#define VIPS_VAR_IMAGE( N ) \
+	(*((VipsImage **) vips_pool_context_object( context, (N) )))
+
+#ifdef __cplusplus
+}
+#endif /*__cplusplus*/
+
+#endif /*VIPS_POOL_H*/
+
+

libvips/include/vips/vips.h

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

libvips/iofuncs/Makefile.am

@@ -3,6 +3,7 @@ noinst_LTLIBRARIES = libiofuncs.la
 libiofuncs_la_SOURCES = \
 	enumtypes.c \
 	object.c \
+	pool.c \
 	base64.h \
 	base64.c \
 	error.c \

libvips/iofuncs/image.c

@@ -566,8 +566,10 @@ typedef struct {
 } Lazy;
 
 static void
-lazy_free_cb( Lazy *lazy )
+lazy_free_cb( VipsImage *image, Lazy *lazy )
 {
+	VIPS_DEBUG_MSG( "lazy_free: %p \"%s\"\n", lazy, lazy->filename );
+
 	VIPS_FREE( lazy->filename );
 	VIPS_UNREF( lazy->real );
 }
@@ -578,16 +580,15 @@ lazy_new( VipsImage *image,
 {
 	Lazy *lazy;
 
-	VIPS_DEBUG_MSG( "lazy_new: \"%s\"\n", filename );
-
 	if( !(lazy = VIPS_NEW( image, Lazy )) )
 		return( NULL );
+	VIPS_DEBUG_MSG( "lazy_new: %p \"%s\"\n", lazy, filename );
 	lazy->image = image;
 	lazy->format = format;
 	lazy->filename = NULL;
 	lazy->disc = disc;
 	lazy->real = NULL;
-	g_signal_connect( image, "close", G_CALLBACK( lazy_free_cb ), NULL );
+	g_signal_connect( image, "close", G_CALLBACK( lazy_free_cb ), lazy );
 
 	if( !(lazy->filename = vips_strdup( NULL, filename )) )
 		return( NULL );

libvips/iofuncs/object.c

@@ -534,7 +534,7 @@ vips_object_set_object( VipsObject *object, GParamSpec *pspec,
 
 			/* Ref the argument.
 			 */
-			g_object_ref_sink( *member );
+			g_object_ref( *member );
 		}
 		else if( argument_class->flags & VIPS_ARGUMENT_OUTPUT ) {
 #ifdef DEBUG_REF
@@ -548,7 +548,7 @@ vips_object_set_object( VipsObject *object, GParamSpec *pspec,
 
 			/* The argument reffs us.
 			 */
-			g_object_ref_sink( object );
+			g_object_ref( object );
 			argument_instance->close_id =
 				g_signal_connect( *member, "close",
 					G_CALLBACK( vips_object_arg_close ),
@@ -1586,7 +1586,7 @@ vips_type_find( const char *basename, const char *nickname )
 		return( 0 );
 
 	/* FIXME ... we've not supposed to use G_TYPE_FROM_CLASS(), I think. 
-	 * I'm not * sure what the alternative is.
+	 * I'm not sure what the alternative is.
 	 */
 	return( G_TYPE_FROM_CLASS( class ) );
 }

libvips/iofuncs/operation.c

@@ -199,10 +199,6 @@ vips_operation_new( const char *name )
 		return( NULL );
 	operation = VIPS_OPERATION( g_object_new( type, NULL ) );
 
-	/* Clear the initial floating ref, return a real ref.
-	 */
-	g_object_ref_sink( operation );
-
 	return( operation );
 }
 

libvips/iofuncs/pool.c

@@ -0,0 +1,203 @@
+/* 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 DEBUG
+#define VIPS_DEBUG
+#define DEBUG_REF
+ */
+
+#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>
+
+#ifdef WITH_DMALLOC
+#include <dmalloc.h>
+#endif /*WITH_DMALLOC*/
+
+/*
+
+  Here's how to handle ref counts when calling vips operations:
+
+	VipsImage *
+	thing( VipsImage *in1, VipsImage *in2 )
+	{
+		VipsImage *t;
+		VipsImage *out;
+
+		if( vips_add( in1, in2, &t, NULL ) )
+			return( NULL );
+		if( vips_add( in1, t, &out, NULL ) ) {
+			g_object_unref( t );
+			return( NULL );
+		}
+		g_object_unref( t );
+
+		return( out );
+	}
+
+  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:
+
+  	VipsImage *
+	thing( VipsPool *pool, VipsImage *in1, VipsImage *in2 )
+	{
+		VipsPoolContext *context = vips_pool_context_new( pool );
+
+		VipsImage *out;
+
+		if( vips_add( in1, in2, VIPS_VAR_IMAGE_REF( 1 ), NULL ) ||
+			vips_add( in1, VIPS_VAR_IMAGE( 1 ), &out, NULL ) )
+			return( NULL );
+
+		return( out );
+	}
+
+  vips_pool_context_new() creates a new context to hold a set of temporary 
+  objects. You can get a reference to a temporary image object with the 
+  macro VIPS_VAR_IMAGE_REF(), and get the object with VIPS_VAR_IMAGE(). 
+  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_POOL );
+
+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, VipsBuf *buf )
+{
+	vips_buf_appendf( buf, "VipsPoolContext %p, %d objects\n", 
+		context, context->len );
+}
+
+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;
+
+	pool = VIPS_POOL( g_object_new( VIPS_TYPE_POOL, NULL ) );
+
+	g_object_set( pool, "name", name, NULL );
+
+	if( vips_object_build( VIPS_OBJECT( pool ) ) ) {
+		VIPS_UNREF( pool );
+		return( NULL );
+	}
+
+	return( pool );
+}
+
+VipsPoolContext *
+vips_pool_context_new( VipsPool *pool )
+{
+	VipsPoolContext *context;
+
+	context = g_ptr_array_new_with_free_func( g_object_unref );
+	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 ) );
+}
+

libvips/iofuncs/region.c

@@ -442,10 +442,6 @@ vips_region_new( VipsImage *image )
 		return( NULL );
 	}
 
-	/* We return a hard reference, so unfloat it.
-	 */
-	g_object_ref_sink( region );
-
 	return( region ); 
 }
 

libvips/iofuncs/sinkscreen.c

@@ -568,7 +568,7 @@ tile_equal( gconstpointer a, gconstpointer b )
 }
 
 static int
-render_close_cb( Render *render )
+render_close_cb( VipsImage *image, Render *render )
 {
 	VIPS_DEBUG_MSG_AMBER( "render_close_cb\n" );