Skip to content

Commit 1ad409b

Browse files
committed
Fixes raster masks not being properly released
1 parent 5e75065 commit 1ad409b

3 files changed

Lines changed: 65 additions & 0 deletions

File tree

src/develop/imageop.c

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3587,6 +3587,56 @@ void dt_iop_update_multi_name(dt_iop_module_t *module,
35873587
g_free(l_name);
35883588
}
35893589

3590+
void dt_iop_prune_stale_raster_users(dt_iop_module_t *module)
3591+
{
3592+
GHashTable *users = module->raster_mask.source.users;
3593+
if(!module->dev || !users || g_hash_table_size(users) == 0)
3594+
return;
3595+
3596+
/* A consumer can leak into a source's users table when it is deleted (its
3597+
cleanup never removes it from other modules' tables) or when a full resync
3598+
nulls its sink.source before the de-register could fire. Such a phantom user
3599+
keeps dt_iop_is_raster_mask_used() TRUE, so the source republishes its raster
3600+
mask -- and invalidates every downstream cacheline -- on every pipe run.
3601+
Drop entries that are provably stale, without ever dereferencing a possibly
3602+
dangling (freed) consumer pointer. */
3603+
GList *iop = module->dev->iop;
3604+
GHashTableIter iter;
3605+
gpointer key, value;
3606+
g_hash_table_iter_init(&iter, users);
3607+
while(g_hash_table_iter_next(&iter, &key, &value))
3608+
{
3609+
dt_iop_module_t *sink = key;
3610+
// pointer-only membership test -- never dereferences a deleted module
3611+
if(g_list_find(iop, sink) == NULL)
3612+
{
3613+
g_hash_table_iter_remove(&iter);
3614+
dt_print_pipe(DT_DEBUG_PIPE | DT_DEBUG_MASKS,
3615+
"prune stale raster user", NULL, module, DT_DEVICE_NONE, NULL, NULL,
3616+
"dropped deleted consumer");
3617+
continue;
3618+
}
3619+
// alive: a real consumer must still point back at us, be enabled, and
3620+
// actually have its blending in raster-mask mode. A module that named us as
3621+
// raster source but is then disabled (or switched its mask to drawn/parametric)
3622+
// leaves a phantom entry that would otherwise keep us publishing -- and
3623+
// invalidating every downstream cacheline -- on every pipe run.
3624+
const gboolean consumes =
3625+
sink->raster_mask.sink.source == module
3626+
&& sink->enabled
3627+
&& (sink->blend_params->mask_mode & DEVELOP_MASK_RASTER);
3628+
if(!consumes)
3629+
{
3630+
g_hash_table_iter_remove(&iter);
3631+
dt_print_pipe(DT_DEBUG_PIPE | DT_DEBUG_MASKS,
3632+
"prune stale raster user", NULL, module, DT_DEVICE_NONE, NULL, NULL,
3633+
"dropped '%s%s' (%s)", sink->op, dt_iop_get_instance_id(sink),
3634+
sink->raster_mask.sink.source != module ? "de-synced"
3635+
: !sink->enabled ? "disabled" : "not in raster mode");
3636+
}
3637+
}
3638+
}
3639+
35903640
gboolean dt_iop_is_raster_mask_used(const dt_iop_module_t *module, const dt_mask_id_t id)
35913641
{
35923642
GHashTableIter iter;

src/develop/imageop.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,9 @@ void dt_iop_update_multi_name(dt_iop_module_t *module,
502502
const gboolean enable,
503503
const gboolean force);
504504

505+
/** remove stale entries (deleted or de-synced consumers) from a raster mask
506+
source's users table, so it doesn't keep publishing/invalidating forever */
507+
void dt_iop_prune_stale_raster_users(dt_iop_module_t *module);
505508
/** iterates over the users hash table and checks if a specific mask is being used */
506509
gboolean dt_iop_is_raster_mask_used(const dt_iop_module_t *module, const dt_mask_id_t id);
507510
/** checks dt_iop_is_raster_mask_used() or writing for exports */

src/develop/pixelpipe_hb.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,12 @@ void dt_dev_pixelpipe_synch_all(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
675675
_dev_pixelpipe_synch(pipe, dev, history);
676676
history = g_list_next(history);
677677
}
678+
679+
// history has been (re)applied, so real raster consumers have re-registered;
680+
// drop any phantom users left behind by deleted or de-synced consumers
681+
for(GList *nodes = pipe->nodes; nodes; nodes = g_list_next(nodes))
682+
dt_iop_prune_stale_raster_users(((dt_dev_pixelpipe_iop_t *)nodes->data)->module);
683+
678684
dt_print_pipe(DT_DEBUG_PARAMS,
679685
"synch all modules done",
680686
pipe, NULL, DT_DEVICE_NONE, NULL, NULL,
@@ -699,6 +705,12 @@ void dt_dev_pixelpipe_synch_top(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
699705
dt_print_pipe(DT_DEBUG_PARAMS, "synch top history module missing!",
700706
pipe, NULL, DT_DEVICE_NONE, NULL, NULL);
701707
}
708+
709+
// clear any phantom raster users (deleted/de-synced consumers) so a source
710+
// doesn't keep republishing its mask and invalidating downstream every run
711+
for(GList *nodes = pipe->nodes; nodes; nodes = g_list_next(nodes))
712+
dt_iop_prune_stale_raster_users(((dt_dev_pixelpipe_iop_t *)nodes->data)->module);
713+
702714
dt_pthread_mutex_unlock(&pipe->busy_mutex);
703715
}
704716

0 commit comments

Comments
 (0)