Skip to content

Commit 2656d73

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

1 file changed

Lines changed: 76 additions & 0 deletions

File tree

src/develop/pixelpipe_hb.c

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -635,6 +635,70 @@ static void _dev_pixelpipe_synch(dt_dev_pixelpipe_t *pipe,
635635
}
636636
}
637637

638+
/** remove stale entries (deleted, disabled or de-synced consumers) from a
639+
raster mask source's users table, so it doesn't keep
640+
publishing/invalidating forever */
641+
static void _iop_prune_stale_raster_users(dt_iop_module_t *module)
642+
{
643+
GHashTable *users = module->raster_mask.source.users;
644+
if(!module->dev || !users || g_hash_table_size(users) == 0)
645+
return;
646+
647+
/* A consumer can leak into a source's users table when it is deleted (its
648+
cleanup never removes it from other modules' tables) or when a full resync
649+
nulls its sink.source before the de-register could fire. Such a phantom user
650+
keeps dt_iop_is_raster_mask_used() TRUE, so the source republishes its raster
651+
mask -- and invalidates every downstream cacheline -- on every pipe run.
652+
Drop entries that are provably stale, without ever dereferencing a possibly
653+
dangling (freed) consumer pointer. */
654+
GList *iop = module->dev->iop;
655+
GHashTableIter iter;
656+
gpointer key, value;
657+
g_hash_table_iter_init(&iter, users);
658+
while(g_hash_table_iter_next(&iter, &key, &value))
659+
{
660+
dt_iop_module_t *sink = key;
661+
// pointer-only membership test -- never dereferences a deleted module
662+
if(g_list_find(iop, sink) == NULL)
663+
{
664+
g_hash_table_iter_remove(&iter);
665+
dt_print_pipe(DT_DEBUG_PIPE | DT_DEBUG_MASKS,
666+
"prune stale raster user",
667+
NULL,
668+
module,
669+
DT_DEVICE_NONE,
670+
NULL,
671+
NULL,
672+
"dropped deleted consumer");
673+
continue;
674+
}
675+
// alive: a real consumer must still point back at us, be enabled, and
676+
// actually have its blending in raster-mask mode. A module that named us as
677+
// raster source but is then disabled (or switched its mask to drawn/parametric)
678+
// leaves a phantom entry that would otherwise keep us publishing -- and
679+
// invalidating every downstream cacheline -- on every pipe run.
680+
const gboolean consumes = sink->raster_mask.sink.source == module && sink->enabled &&
681+
(sink->blend_params->mask_mode & DEVELOP_MASK_RASTER);
682+
if(!consumes)
683+
{
684+
g_hash_table_iter_remove(&iter);
685+
dt_print_pipe(DT_DEBUG_PIPE | DT_DEBUG_MASKS,
686+
"prune stale raster user",
687+
NULL,
688+
module,
689+
DT_DEVICE_NONE,
690+
NULL,
691+
NULL,
692+
"dropped '%s%s' (%s)",
693+
sink->op,
694+
dt_iop_get_instance_id(sink),
695+
sink->raster_mask.sink.source != module ? "de-synced"
696+
: !sink->enabled ? "disabled"
697+
: "not in raster mode");
698+
}
699+
}
700+
}
701+
638702
void dt_dev_pixelpipe_synch_all(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
639703
{
640704
dt_pthread_mutex_lock(&pipe->busy_mutex);
@@ -675,6 +739,12 @@ void dt_dev_pixelpipe_synch_all(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
675739
_dev_pixelpipe_synch(pipe, dev, history);
676740
history = g_list_next(history);
677741
}
742+
743+
// history has been (re)applied, so real raster consumers have re-registered;
744+
// drop any phantom users left behind by deleted or de-synced consumers
745+
for(GList *nodes = pipe->nodes; nodes; nodes = g_list_next(nodes))
746+
_iop_prune_stale_raster_users(((dt_dev_pixelpipe_iop_t *)nodes->data)->module);
747+
678748
dt_print_pipe(DT_DEBUG_PARAMS,
679749
"synch all modules done",
680750
pipe, NULL, DT_DEVICE_NONE, NULL, NULL,
@@ -699,6 +769,12 @@ void dt_dev_pixelpipe_synch_top(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
699769
dt_print_pipe(DT_DEBUG_PARAMS, "synch top history module missing!",
700770
pipe, NULL, DT_DEVICE_NONE, NULL, NULL);
701771
}
772+
773+
// clear any phantom raster users (deleted/de-synced consumers) so a source
774+
// doesn't keep republishing its mask and invalidating downstream every run
775+
for(GList *nodes = pipe->nodes; nodes; nodes = g_list_next(nodes))
776+
_iop_prune_stale_raster_users(((dt_dev_pixelpipe_iop_t *)nodes->data)->module);
777+
702778
dt_pthread_mutex_unlock(&pipe->busy_mutex);
703779
}
704780

0 commit comments

Comments
 (0)