From 8b24518422121d47512f2613eac3d4c2b93855b9 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Sat, 6 Mar 2010 17:42:17 +0000
Subject: [PATCH] fix to buffer caching

---
 ChangeLog                    |  1 +
 TODO                         |  4 ----
 libvips/iofuncs/buffer.c     | 12 ++++++------
 libvips/iofuncs/im_prepare.c |  2 --
 libvips/iofuncs/im_render.c  |  3 +++
 libvips/iofuncs/region.c     |  6 +++---
 6 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c11b2e41..e4e3f6bf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,7 @@
 - im_csv2vips(): allow lines that end with EOF rather than \n
 - im_vips2tiff has a bigtiff option
 - oops, im_lineset() needs to ask for WIO of mask and ink
+- move cache invalidation to REGION, fixes a race
 
 15/1/10 started 7.21.1
 - added "written" callbacks, used to implement write to non-vips formats
diff --git a/TODO b/TODO
index 30e90f03..f97f2b64 100644
--- a/TODO
+++ b/TODO
@@ -1,7 +1,3 @@
-- argh, regions are all broken
-
-- what's the difference between private.h and internal.h?
-
 - we use parent/child a lot, but it's confusing
 
   imagine building the pipeline 
diff --git a/libvips/iofuncs/buffer.c b/libvips/iofuncs/buffer.c
index 9f79a0d8..8e98d372 100644
--- a/libvips/iofuncs/buffer.c
+++ b/libvips/iofuncs/buffer.c
@@ -44,8 +44,8 @@
  */
 
 /*
-#define DEBUG
 #define DEBUG_CREATE
+#define DEBUG
  */
 
 #ifdef HAVE_CONFIG_H
@@ -193,9 +193,9 @@ im_buffer_done( im_buffer_t *buffer )
 		im_buffer_cache_list_t *cache_list;
 
 #ifdef DEBUG
-		printf( "im_buffer_done: thread %p adding "
-			"buffer %p to cache %p\n",
-			g_thread_self(), buffer, cache );
+		printf( "im_buffer_done: thread %p adding to cache %p\n",
+			g_thread_self(), cache );
+		im_buffer_print( buffer ); 
 #endif /*DEBUG*/
 
 		/* Look up and update the buffer list. 
@@ -246,7 +246,7 @@ im_buffer_undone( im_buffer_t *buffer )
 
 #ifdef DEBUG
 		printf( "im_buffer_undone: %d buffers left\n",
-			g_slist_length( buffers ) );
+			g_slist_length( cache_list->buffers ) );
 #endif /*DEBUG*/
 	}
 }
@@ -432,7 +432,7 @@ im_buffer_unref_ref( im_buffer_t *old_buffer, IMAGE *im, Rect *area )
 	/* Is the current buffer OK?
 	 */
 	if( old_buffer && 
-		im_rect_includesrect( &old_buffer->area, area ) )
+		im_rect_includesrect( &old_buffer->area, area ) ) 
 		return( old_buffer );
 
 	/* Does the new area already have a buffer?
diff --git a/libvips/iofuncs/im_prepare.c b/libvips/iofuncs/im_prepare.c
index 66fe0f28..9379dcd9 100644
--- a/libvips/iofuncs/im_prepare.c
+++ b/libvips/iofuncs/im_prepare.c
@@ -489,8 +489,6 @@ im_invalidate( IMAGE *im )
 {
 	GSList *to_be_invalidated;
 
-	return;
-
 	/* Invalidate callbacks might do anything, including removing images
 	 * or invalidating other images, so we can't trigger them from within 
 	 * the image loop. Instead we collect a list of image to invalidate 
diff --git a/libvips/iofuncs/im_render.c b/libvips/iofuncs/im_render.c
index e5409ac8..1865cc9a 100644
--- a/libvips/iofuncs/im_render.c
+++ b/libvips/iofuncs/im_render.c
@@ -410,6 +410,9 @@ render_dirty_process( Render *render )
 		 */
 		tile->state = TILE_PAINTED;
 		im__region_no_ownership( tile->region );
+
+		/* Now clients can update.
+		 */
 		if( render->notify ) 
 			render->notify( render->out, 
 				&tile->area, render->client );
diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c
index f4a7aa78..76022923 100644
--- a/libvips/iofuncs/region.c
+++ b/libvips/iofuncs/region.c
@@ -456,7 +456,7 @@ im_region_buffer( REGION *reg, Rect *r )
 	 * buffer ref in one call to reduce malloc/free cycling.
 	 */
 	IM_FREEF( im_window_unref, reg->window );
-	if( !(reg->buffer = im_buffer_unref_ref( reg->buffer, im, &clipped )) )
+	if( !(reg->buffer = im_buffer_unref_ref( reg->buffer, im, &clipped )) ) 
 		return( -1 );
 
 	/* If we've been asked to drop caches, flag this as undone.
@@ -743,8 +743,8 @@ im_region_fill( REGION *reg, Rect *r, im_region_fill_fn fn, void *a )
 		return( -1 );
 
 	/* Evaluate into or, if we've not got calculated pixels.
-	if( !reg->buffer->done ) {
 	 */
+	if( !reg->buffer->done ) {
 		if( fn( reg, a ) )
 			return( -1 );
 
@@ -752,7 +752,7 @@ im_region_fill( REGION *reg, Rect *r, im_region_fill_fn fn, void *a )
 		 */
 		if( reg->buffer )
 			im_buffer_done( reg->buffer );
-	//}
+	}
 
 	return( 0 );
 }