argh broken everything

This commit is contained in:
John Cupitt 2010-03-05 14:43:49 +00:00
parent 0023008db7
commit 060b4d4a3d
7 changed files with 260 additions and 211 deletions

29
TODO
View File

@ -1,7 +1,36 @@
- 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
A + B -> C
we create A, we create B, we create C, we call im_add ... C is the parent
of A and B, since parents have many children (see comment in image.h for
GSList *parents)
however, we make A and B before C, and pixels flow from them to C, so they
seem older, argh
change name scheme from parent/child to upstream/downstream, so A and B are
upstream of C, C is downstream of A and B
clearer expression of arrow direction in DAG
- add a dependency on the icc-profiles package - add a dependency on the icc-profiles package
the vips load/save ops need to search the icc-profile dir the vips load/save ops need to search the icc-profile dir
where is it? is there a pkg-config for it?
no, just dumps profiles into /usr/share/color/icc
configure should test for the dir? what's the equivalent dir on Windows? we
also have nip2's dir
- doing im_create_fmask() and friends - doing im_create_fmask() and friends
- how about im_invalidate_area()? we currently repaint the whole window on - how about im_invalidate_area()? we currently repaint the whole window on

View File

@ -120,7 +120,6 @@ typedef struct im__buffer_t {
Rect area; /* Area this pixel buffer covers */ Rect area; /* Area this pixel buffer covers */
gboolean done; /* Calculated and in cache */ gboolean done; /* Calculated and in cache */
im_buffer_cache_t *cache; im_buffer_cache_t *cache;
gboolean invalid; /* Needs to be recalculated */
char *buf; /* Private malloc() area */ char *buf; /* Private malloc() area */
size_t bsize; /* Size of private malloc() */ size_t bsize; /* Size of private malloc() */
} im_buffer_t; } im_buffer_t;

View File

@ -65,6 +65,11 @@ typedef struct _REGION {
/* Ref to the buffer we use for this region, if any. /* Ref to the buffer we use for this region, if any.
*/ */
im_buffer_t *buffer; im_buffer_t *buffer;
/* The image this region is on has changed and caches need to be
* dropped.
*/
gboolean invalid;
} REGION; } REGION;
REGION *im_region_create( IMAGE *im ); REGION *im_region_create( IMAGE *im );

View File

@ -12,6 +12,9 @@
* 20/2/07 * 20/2/07
* - add im_buffer_cache_list_t and we can avoid some hash ops on * - add im_buffer_cache_list_t and we can avoid some hash ops on
* done/undone * done/undone
* 5/3/10
* - move invalid stuff to region
* - move link maintenace to im_demand_hint
*/ */
/* /*
@ -52,7 +55,6 @@
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <assert.h>
#include <vips/vips.h> #include <vips/vips.h>
#include <vips/internal.h> #include <vips/internal.h>
@ -203,8 +205,8 @@ im_buffer_done( im_buffer_t *buffer )
g_hash_table_insert( cache->hash, im, cache_list ); g_hash_table_insert( cache->hash, im, cache_list );
} }
assert( !g_slist_find( cache_list->buffers, buffer ) ); g_assert( !g_slist_find( cache_list->buffers, buffer ) );
assert( cache_list->thread == cache->thread ); g_assert( cache_list->thread == cache->thread );
cache_list->buffers = cache_list->buffers =
g_slist_prepend( cache_list->buffers, buffer ); g_slist_prepend( cache_list->buffers, buffer );
@ -229,13 +231,13 @@ im_buffer_undone( im_buffer_t *buffer )
g_thread_self(), buffer, cache ); g_thread_self(), buffer, cache );
#endif /*DEBUG*/ #endif /*DEBUG*/
assert( cache->thread == g_thread_self() ); g_assert( cache->thread == g_thread_self() );
cache_list = g_hash_table_lookup( cache->hash, im ); cache_list = g_hash_table_lookup( cache->hash, im );
assert( cache_list ); g_assert( cache_list );
assert( cache_list->thread == cache->thread ); g_assert( cache_list->thread == cache->thread );
assert( g_slist_find( cache_list->buffers, buffer ) ); g_assert( g_slist_find( cache_list->buffers, buffer ) );
cache_list->buffers = cache_list->buffers =
g_slist_remove( cache_list->buffers, buffer ); g_slist_remove( cache_list->buffers, buffer );
@ -260,7 +262,7 @@ im_buffer_unref( im_buffer_t *buffer )
buffer ); buffer );
#endif /*DEBUG*/ #endif /*DEBUG*/
assert( buffer->ref_count > 0 ); g_assert( buffer->ref_count > 0 );
buffer->ref_count -= 1; buffer->ref_count -= 1;
@ -279,7 +281,7 @@ im_buffer_unref( im_buffer_t *buffer )
#ifdef DEBUG #ifdef DEBUG
g_mutex_lock( im__global_lock ); g_mutex_lock( im__global_lock );
assert( g_slist_find( im__buffers_all, buffer ) ); g_assert( g_slist_find( im__buffers_all, buffer ) );
im__buffers_all = g_slist_remove( im__buffers_all, buffer ); im__buffers_all = g_slist_remove( im__buffers_all, buffer );
printf( "%d buffers in vips\n", printf( "%d buffers in vips\n",
g_slist_length( im__buffers_all ) ); g_slist_length( im__buffers_all ) );
@ -303,7 +305,6 @@ buffer_new( IMAGE *im, Rect *area )
buffer->area = *area; buffer->area = *area;
buffer->done = FALSE; buffer->done = FALSE;
buffer->cache = NULL; buffer->cache = NULL;
buffer->invalid = FALSE;
buffer->bsize = (size_t) IM_IMAGE_SIZEOF_PEL( im ) * buffer->bsize = (size_t) IM_IMAGE_SIZEOF_PEL( im ) *
area->width * area->height; area->width * area->height;
if( !(buffer->buf = im_malloc( NULL, buffer->bsize )) ) { if( !(buffer->buf = im_malloc( NULL, buffer->bsize )) ) {
@ -335,12 +336,11 @@ buffer_move( im_buffer_t *buffer, Rect *area )
IMAGE *im = buffer->im; IMAGE *im = buffer->im;
size_t new_bsize; size_t new_bsize;
assert( buffer->ref_count == 1 ); g_assert( buffer->ref_count == 1 );
buffer->area = *area; buffer->area = *area;
im_buffer_undone( buffer ); im_buffer_undone( buffer );
assert( !buffer->done ); g_assert( !buffer->done );
buffer->invalid = FALSE;
new_bsize = (size_t) IM_IMAGE_SIZEOF_PEL( im ) * new_bsize = (size_t) IM_IMAGE_SIZEOF_PEL( im ) *
area->width * area->height; area->width * area->height;
@ -370,6 +370,9 @@ buffer_find( IMAGE *im, Rect *r )
/* This needs to be quick :-( don't use /* This needs to be quick :-( don't use
* im_slist_map2()/im_rect_includesrect(), do the search inline. * im_slist_map2()/im_rect_includesrect(), do the search inline.
*
* FIXME we return the first enclosing buffer, perhaps we should
* search for the largest?
*/ */
for( ; p; p = p->next ) { for( ; p; p = p->next ) {
buffer = (im_buffer_t *) p->data; buffer = (im_buffer_t *) p->data;
@ -416,36 +419,44 @@ im_buffer_ref( IMAGE *im, Rect *area )
return( buffer ); return( buffer );
} }
/* Unref old, ref new, in a single operation. Move the buffer if we can. /* Unref old, ref new, in a single operation. Reuse stuff if we can. The
* buffer we return might or might not be done.
*/ */
im_buffer_t * im_buffer_t *
im_buffer_unref_ref( im_buffer_t *old_buffer, IMAGE *im, Rect *area ) im_buffer_unref_ref( im_buffer_t *old_buffer, IMAGE *im, Rect *area )
{ {
im_buffer_t *buffer; im_buffer_t *buffer;
assert( !old_buffer || old_buffer->im == im ); g_assert( !old_buffer || old_buffer->im == im );
if( (buffer = buffer_find( im, area )) && !buffer->invalid ) { /* Is the current buffer OK?
/* The new area has an OK buffer already: use that. */
*/ if( im_rect_includesrect( &buffer->area, area ) )
return( old_buffer );
/* Does the new area already have a buffer?
*/
if( (buffer = buffer_find( im, area )) ) {
IM_FREEF( im_buffer_unref, old_buffer ); IM_FREEF( im_buffer_unref, old_buffer );
return( buffer );
} }
else if( old_buffer && old_buffer->ref_count == 1 ) {
/* The old buffer is not shared ... we can reuse it. /* Is the current buffer unshared? We can just move it.
*/ */
buffer = old_buffer; if( old_buffer && old_buffer->ref_count == 1 ) {
if( buffer_move( buffer, area ) ) { if( buffer_move( old_buffer, area ) ) {
im_buffer_unref( buffer ); im_buffer_unref( old_buffer );
return( NULL ); return( NULL );
} }
return( old_buffer );
} }
else {
/* Old buffer in use ... need another. /* Fallback ... unref the old one, make a new one.
*/ */
IM_FREEF( im_buffer_unref, old_buffer ); IM_FREEF( im_buffer_unref, old_buffer );
if( !(buffer = buffer_new( im, area )) ) if( !(buffer = buffer_new( im, area )) )
return( NULL ); return( NULL );
}
return( buffer ); return( buffer );
} }
@ -460,179 +471,10 @@ im_buffer_print( im_buffer_t *buffer )
printf( "area.width = %d, ", buffer->area.width ); printf( "area.width = %d, ", buffer->area.width );
printf( "area.height = %d, ", buffer->area.height ); printf( "area.height = %d, ", buffer->area.height );
printf( "done = %d, ", buffer->done ); printf( "done = %d, ", buffer->done );
printf( "invalid = %d, ", buffer->invalid );
printf( "buf = %p, ", buffer->buf ); printf( "buf = %p, ", buffer->buf );
printf( "bsize = %zd\n", buffer->bsize ); printf( "bsize = %zd\n", buffer->bsize );
} }
/* Make a parent/child link. child is one of parent's inputs.
*/
void
im__link_make( IMAGE *parent, IMAGE *child )
{
assert( parent );
assert( child );
parent->children = g_slist_prepend( parent->children, child );
child->parents = g_slist_prepend( child->parents, parent );
/* Propogate the progress indicator.
*/
if( child->progress && !parent->progress )
parent->progress = child->progress;
}
/* Break link. child is one of parent's inputs.
*/
static void *
im__link_break( IMAGE *parent, IMAGE *child )
{
assert( parent );
assert( child );
assert( g_slist_find( parent->children, child ) );
assert( g_slist_find( child->parents, parent ) );
parent->children = g_slist_remove( parent->children, child );
child->parents = g_slist_remove( child->parents, parent );
/* Unlink the progress chain.
*/
if( parent->progress && parent->progress == child->progress )
parent->progress = NULL;
return( NULL );
}
static void *
im__link_break_rev( IMAGE *child, IMAGE *parent )
{
return( im__link_break( parent, child ) );
}
/* An IMAGE is going ... break all links.
*/
void
im__link_break_all( IMAGE *im )
{
im_slist_map2( im->parents,
(VSListMap2Fn) im__link_break, im, NULL );
im_slist_map2( im->children,
(VSListMap2Fn) im__link_break_rev, im, NULL );
}
static void *
im__link_mapp( IMAGE *im, VSListMap2Fn fn, int *serial, void *a, void *b )
{
void *res;
/* Loop?
*/
if( im->serial == *serial )
return( NULL );
im->serial = *serial;
if( (res = fn( im, a, b )) )
return( res );
return( im_slist_map4( im->parents,
(VSListMap4Fn) im__link_mapp, fn, serial, a, b ) );
}
/* Apply a function to an image and all it's parents, direct and indirect.
*/
void *
im__link_map( IMAGE *im, VSListMap2Fn fn, void *a, void *b )
{
static int serial = 0;
serial += 1;
return( im__link_mapp( im, fn, &serial, a, b ) );
}
static void *
im_invalidate_region( REGION *reg )
{
if( reg->buffer )
reg->buffer->invalid = TRUE;
return( NULL );
}
static void *
im_invalidate_image( IMAGE *im, GSList **to_be_invalidated )
{
(void) im_slist_map2( im->regions,
(VSListMap2Fn) im_invalidate_region, NULL, NULL );
*to_be_invalidated = g_slist_prepend( *to_be_invalidated, im );
return( NULL );
}
/* Trigger a callbacks on a list of images, where the callbacks might create
* or destroy the images.
*
* We make a set of temp regions to hold the images open, but when we switch
* to VipsObject we should incr/decr ref count.
*/
static void
im_invalidate_trigger( GSList *images )
{
GSList *regions;
GSList *p;
regions = NULL;
for( p = images; p; p = p->next ) {
IMAGE *im = (IMAGE *) p->data;
regions = g_slist_prepend( regions, im_region_create( im ) );
}
for( p = images; p; p = p->next ) {
IMAGE *im = (IMAGE *) p->data;
(void) im__trigger_callbacks( im->invalidatefns );
}
for( p = regions; p; p = p->next ) {
REGION *r = (REGION *) p->data;
im_region_free( r );
}
g_slist_free( regions );
}
/**
* im_invalidate:
* @im: #IMAGE to invalidate
*
* Invalidate all pixel caches on an #IMAGE and any derived images. The
* "invalidate" callback is triggered for all invalidated images.
*
* See also: im_add_invalidate_callback().
*/
void
im_invalidate( IMAGE *im )
{
GSList *to_be_invalidated;
/* 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
* and trigger them all in one go, checking that they are not
* invalidated.
*/
to_be_invalidated = NULL;
(void) im__link_map( im,
(VSListMap2Fn) im_invalidate_image, &to_be_invalidated, NULL );
im_invalidate_trigger( to_be_invalidated );
g_slist_free( to_be_invalidated );
}
/* Init the buffer cache system. /* Init the buffer cache system.
*/ */
void void

View File

@ -64,6 +64,90 @@
*/ */
#define MAX_IMAGES (1000) #define MAX_IMAGES (1000)
/* Make a parent/child link. child is one of parent's inputs.
*/
void
im__link_make( IMAGE *parent, IMAGE *child )
{
g_assert( parent );
g_assert( child );
parent->children = g_slist_prepend( parent->children, child );
child->parents = g_slist_prepend( child->parents, parent );
/* Propogate the progress indicator.
*/
if( child->progress && !parent->progress )
parent->progress = child->progress;
}
/* Break link. child is one of parent's inputs.
*/
static void *
im__link_break( IMAGE *parent, IMAGE *child )
{
g_assert( parent );
g_assert( child );
g_assert( g_slist_find( parent->children, child ) );
g_assert( g_slist_find( child->parents, parent ) );
parent->children = g_slist_remove( parent->children, child );
child->parents = g_slist_remove( child->parents, parent );
/* Unlink the progress chain.
*/
if( parent->progress && parent->progress == child->progress )
parent->progress = NULL;
return( NULL );
}
static void *
im__link_break_rev( IMAGE *child, IMAGE *parent )
{
return( im__link_break( parent, child ) );
}
/* An IMAGE is going ... break all links.
*/
void
im__link_break_all( IMAGE *im )
{
im_slist_map2( im->parents,
(VSListMap2Fn) im__link_break, im, NULL );
im_slist_map2( im->children,
(VSListMap2Fn) im__link_break_rev, im, NULL );
}
static void *
im__link_mapp( IMAGE *im, VSListMap2Fn fn, int *serial, void *a, void *b )
{
void *res;
/* Loop?
*/
if( im->serial == *serial )
return( NULL );
im->serial = *serial;
if( (res = fn( im, a, b )) )
return( res );
return( im_slist_map4( im->parents,
(VSListMap4Fn) im__link_mapp, fn, serial, a, b ) );
}
/* Apply a function to an image and all it's parents, direct and indirect.
*/
void *
im__link_map( IMAGE *im, VSListMap2Fn fn, void *a, void *b )
{
static int serial = 0;
serial += 1;
return( im__link_mapp( im, fn, &serial, a, b ) );
}
/* Given two im_demand_type, return the most restrictive. /* Given two im_demand_type, return the most restrictive.
*/ */
static im_demand_type static im_demand_type

View File

@ -66,6 +66,7 @@
#include <vips/vips.h> #include <vips/vips.h>
#include <vips/debug.h> #include <vips/debug.h>
#include <vips/internal.h>
#ifdef WITH_DMALLOC #ifdef WITH_DMALLOC
#include <dmalloc.h> #include <dmalloc.h>
@ -417,3 +418,90 @@ im_prepare_many( REGION **reg, Rect *r )
return( 0 ); return( 0 );
} }
static void *
im_invalidate_region( REGION *reg )
{
reg->invalid = TRUE;
return( NULL );
}
static void *
im_invalidate_image( IMAGE *im, GSList **to_be_invalidated )
{
g_mutex_lock( im->sslock );
(void) im_slist_map2( im->regions,
(VSListMap2Fn) im_invalidate_region, NULL, NULL );
g_mutex_unlock( im->sslock );
*to_be_invalidated = g_slist_prepend( *to_be_invalidated, im );
return( NULL );
}
/* Trigger a callbacks on a list of images, where the callbacks might create
* or destroy the images.
*
* We make a set of temp regions to hold the images open, but when we switch
* to VipsObject we should incr/decr ref count.
*/
static void
im_invalidate_trigger( GSList *images )
{
GSList *regions;
GSList *p;
regions = NULL;
for( p = images; p; p = p->next ) {
IMAGE *im = (IMAGE *) p->data;
regions = g_slist_prepend( regions, im_region_create( im ) );
}
for( p = images; p; p = p->next ) {
IMAGE *im = (IMAGE *) p->data;
(void) im__trigger_callbacks( im->invalidatefns );
}
for( p = regions; p; p = p->next ) {
REGION *r = (REGION *) p->data;
im_region_free( r );
}
g_slist_free( regions );
}
/**
* im_invalidate:
* @im: #IMAGE to invalidate
*
* Invalidate all pixel caches on an #IMAGE and any derived images. The
* "invalidate" callback is triggered for all invalidated images.
*
* See also: im_add_invalidate_callback().
*/
void
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
* and trigger them all in one go, checking that they are not
* invalidated.
*/
to_be_invalidated = NULL;
(void) im__link_map( im,
(VSListMap2Fn) im_invalidate_image, &to_be_invalidated, NULL );
im_invalidate_trigger( to_be_invalidated );
g_slist_free( to_be_invalidated );
}

View File

@ -321,6 +321,7 @@ im_region_create( IMAGE *im )
reg->thread = NULL; reg->thread = NULL;
reg->window = NULL; reg->window = NULL;
reg->buffer = NULL; reg->buffer = NULL;
reg->invalid = FALSE;
im__region_take_ownership( reg ); im__region_take_ownership( reg );
@ -347,6 +348,7 @@ im_region_reset( REGION *reg )
{ {
IM_FREEF( im_window_unref, reg->window ); IM_FREEF( im_window_unref, reg->window );
IM_FREEF( im_buffer_unref, reg->buffer ); IM_FREEF( im_buffer_unref, reg->buffer );
reg->invalid = FALSE;
} }
/** /**
@ -448,14 +450,6 @@ im_region_buffer( REGION *reg, Rect *r )
return( -1 ); return( -1 );
} }
/* Already have stuff?
*/
if( reg->type == IM_REGION_BUFFER &&
im_rect_includesrect( &reg->valid, &clipped ) &&
reg->buffer &&
!reg->buffer->invalid )
return( 0 );
/* Don't call im_region_reset() ... we combine buffer unref and new /* Don't call im_region_reset() ... we combine buffer unref and new
* buffer ref in one call to reduce malloc/free cycling. * buffer ref in one call to reduce malloc/free cycling.
*/ */
@ -463,6 +457,13 @@ im_region_buffer( REGION *reg, Rect *r )
if( !(reg->buffer = im_buffer_unref_ref( reg->buffer, im, &clipped )) ) if( !(reg->buffer = im_buffer_unref_ref( reg->buffer, im, &clipped )) )
return( -1 ); return( -1 );
/* If we've been asked to drop caches, flag this as undone.
*/
if( reg->invalid ) {
im_buffer_undone( reg->buffer );
reg->invalid = FALSE;
}
/* Init new stuff. /* Init new stuff.
*/ */
reg->valid = reg->buffer->area; reg->valid = reg->buffer->area;
@ -718,12 +719,12 @@ im_region_position( REGION *reg, int x, int y )
req.height = reg->valid.height; req.height = reg->valid.height;
im_rect_intersectrect( &image, &req, &clipped ); im_rect_intersectrect( &image, &req, &clipped );
if( x < 0 || y < 0 || im_rect_isempty( &clipped ) ) { if( x < 0 || y < 0 || im_rect_isempty( &clipped ) ) {
im_error( "im_region_position", im_error( "im_region_position", "%s", _( "bad position" ) );
"%s", _( "bad position" ) );
return( -1 ); return( -1 );
} }
reg->valid = clipped; reg->valid = clipped;
reg->invalid = FALSE;
return( 0 ); return( 0 );
} }
@ -776,6 +777,7 @@ im_region_print( REGION *reg )
printf( "thread = %p, ", reg->thread ); printf( "thread = %p, ", reg->thread );
printf( "window = %p, ", reg->window ); printf( "window = %p, ", reg->window );
printf( "buffer = %p\n", reg->buffer ); printf( "buffer = %p\n", reg->buffer );
printf( "invalid = %d\n", reg->invalid );
} }
/** /**