Skip to content

Three ways to increase pixelpipe UI responsiveness #21274

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jenshannoschwalm wants to merge 3 commits into
base: master
Choose a base branch
from
+159 −66
Open

Three ways to increase pixelpipe UI responsiveness #21274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f981fb3
Fixed _module_pipe_stop() and superfluous cacheline invalidation
jenshannoschwalm Jun 8, 2026
65f80ee
Introduce and use dt_dev_pixelpipe_piece_shutdown()
jenshannoschwalm Jun 3, 2026
3bcc3bf
Introduce pixelpipe zoom knocking
jenshannoschwalm Jun 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/develop/develop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3268,7 +3268,11 @@ void dt_dev_zoom_move(dt_dev_viewport_t *port,
port->pipe->changed |= DT_DEV_PIPE_ZOOMED;

if(port->widget)
{
if(port->pipe->processing)
dt_dev_pixelpipe_set_shutdown(port->pipe, DT_DEV_PIXELPIPE_STOP_ZOOM);
dt_control_queue_redraw_widget(port->widget);
}
if(port == &dev->full)
dt_control_navigation_redraw();
}
Expand Down
31 changes: 23 additions & 8 deletions src/develop/pixelpipe_cache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -388,17 +388,32 @@ void dt_dev_pixelpipe_cache_invalidate_later(dt_dev_pixelpipe_t *pipe,
invalidated++;
}
}
pipe->bcache_hash = DT_INVALID_HASH;

if(invalidated)
dt_print_pipe(DT_DEBUG_PIPE, "pipecache invalidate", pipe, NULL, DT_DEVICE_NONE, NULL, NULL,
"%s%i cachelines after ioporder=%i", info ? info : "", invalidated, order);
}

const gboolean bcache = pipe->bcache_data != NULL && pipe->bcache_hash != DT_INVALID_HASH;
void dt_dev_pixelpipe_cache_invalidate_iop(dt_dev_pixelpipe_t *pipe,
const int32_t order,
const char *info)
{
const dt_dev_pixelpipe_cache_t *cache = &pipe->cache;
int invalidated = 0;
for(int k = DT_PIPECACHE_MIN; k < cache->entries; k++)
{
if((cache->ioporder[k] == order) && (cache->hash[k] != DT_INVALID_HASH))
{
_mark_invalid_cacheline(cache, k);
invalidated++;
}
}
pipe->bcache_hash = DT_INVALID_HASH;

if(invalidated || bcache)
dt_print_pipe(DT_DEBUG_PIPE,
order ? "pipecache invalidate" : "pipecache flush",
pipe, NULL, DT_DEVICE_NONE, NULL, NULL,
"%s%i cachelines after ioporder=%i%s",
info ? info : "",
invalidated, order, bcache ? ", blend cache" : "");
if(invalidated)
dt_print_pipe(DT_DEBUG_PIPE, "pipecache invalidate", pipe, NULL, DT_DEVICE_NONE, NULL, NULL,
"%s%i cachelines for ioporder=%i", info ? info : "", invalidated, order);
}

void dt_dev_pixelpipe_cache_flush(dt_dev_pixelpipe_t *pipe)
Expand Down
3 changes: 3 additions & 0 deletions src/develop/pixelpipe_cache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ void dt_dev_pixelpipe_cache_flush(struct dt_dev_pixelpipe_t *pipe);
/** invalidates all cachelines for modules with at least the same iop_order */
void dt_dev_pixelpipe_cache_invalidate_later(struct dt_dev_pixelpipe_t *pipe, const int32_t order, const char *info);

/** invalidates all cachelines marked with the given iop_order */
void dt_dev_pixelpipe_cache_invalidate_iop(struct dt_dev_pixelpipe_t *pipe, const int32_t order, const char *info);

/** makes this buffer very important after it has been pulled from the cache. */
void dt_dev_pixelpipe_important_cacheline(const struct dt_dev_pixelpipe_t *pipe, const void *data, const size_t size);

Expand Down
173 changes: 118 additions & 55 deletions src/develop/pixelpipe_hb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@

static inline gboolean _pipe_has_shutdown(dt_dev_pixelpipe_t *pipe)
{
return dt_atomic_get_int(&pipe->shutdown) != DT_DEV_PIXELPIPE_STOP_NO;
const dt_dev_pixelpipe_stopper_t stopper = dt_atomic_get_int(&pipe->shutdown);
return stopper != DT_DEV_PIXELPIPE_STOP_NO && stopper != DT_DEV_PIXELPIPE_STOP_ZOOM;
}

// benchmarking and pfm dumps should happen for these pipes if there is no shutdown request
Expand Down Expand Up @@ -108,6 +109,7 @@ const char *dt_dev_pixelpipe_shutdown_to_str(const dt_dev_pixelpipe_stopper_t st
case DT_DEV_PIXELPIPE_STOP_NO: return "DT_DEV_PIXELPIPE_STOP_NO";
case DT_DEV_PIXELPIPE_STOP_NODES: return "DT_DEV_PIXELPIPE_STOP_NODES";
case DT_DEV_PIXELPIPE_STOP_HQ: return "DT_DEV_PIXELPIPE_STOP_HQ";
case DT_DEV_PIXELPIPE_STOP_ZOOM: return "DT_DEV_PIXELPIPE_STOP_ZOOM";
case DT_DEV_PIXELPIPE_STOP_LAST: return "DT_DEV_PIXELPIPE_STOP_LAST";
default: return "DT_DEV_PIXELPIPE_STOP_MODULE";
}
Expand Down Expand Up @@ -321,7 +323,6 @@ size_t dt_get_available_pipe_mem(const dt_dev_pixelpipe_t *pipe)
static void get_output_format(dt_iop_module_t *module,
dt_dev_pixelpipe_t *pipe,
dt_dev_pixelpipe_iop_t *piece,
dt_develop_t *dev,
dt_iop_buffer_dsc_t *dsc)
{
if(module) return module->output_format(module, pipe, piece, dsc);
Expand All @@ -348,7 +349,7 @@ void dt_dev_pixelpipe_set_input(dt_dev_pixelpipe_t *pipe,
pipe->iscale = iscale;
pipe->input = input;
pipe->image = dev->image_storage;
get_output_format(NULL, pipe, NULL, dev, &pipe->dsc);
get_output_format(NULL, pipe, NULL, &pipe->dsc);
}

void dt_dev_pixelpipe_set_icc(dt_dev_pixelpipe_t *pipe,
Expand Down Expand Up @@ -1224,10 +1225,10 @@ static void _collect_histogram_on_CPU(dt_dev_pixelpipe_t *pipe,
As modules might change internal mask visualizing modes not visible via parameters
we clear the blending cache line whenever we invalidate pixelpipe cache lines.
*/
static inline dt_hash_t _piece_process_hash(const dt_dev_pixelpipe_iop_t *piece,
const dt_iop_roi_t *roi,
const dt_iop_module_t *module,
const int position)
static inline dt_hash_t _piece_blend_hash(const dt_dev_pixelpipe_iop_t *piece,
const dt_iop_roi_t *roi,
const dt_iop_module_t *module,
const int position)
{
dt_hash_t phash = dt_dev_pixelpipe_cache_hash(roi, piece->pipe, position -1);
phash = dt_hash(phash, roi, sizeof(dt_iop_roi_t));
Expand All @@ -1237,14 +1238,16 @@ static inline dt_hash_t _piece_process_hash(const dt_dev_pixelpipe_iop_t *piece,
return phash;
}

static inline gboolean _piece_fast_blend(const dt_dev_pixelpipe_iop_t *piece,
const dt_iop_module_t *module)
static inline gboolean _piece_fast_blend(const dt_dev_pixelpipe_iop_t *piece)
{
const dt_iop_module_t *module = piece->module;

return dt_pipe_is_screen(piece->pipe)
&& darktable.pipe_cache
&& module->dev
&& module->dev->gui_attached
&& module == module->dev->gui_module
&& dt_pipe_no_mask_display(piece->pipe)
&& dt_dev_modulegroups_test_activated(darktable.develop)
&& _transform_for_blend(module, piece);
}
Expand All @@ -1261,12 +1264,14 @@ static inline float *_get_fast_blendcache(const size_t nfloats,

/* use _module_pipe_stop to test for the just processed module leaving a flag to shutdown the pipeline
immediately.
This requires extra care as we want pipecache data after the module and it's input data to be invalidated.
All dt_dev_pixelpipe_stopper_t enums must be served.
All dt_dev_pixelpipe_stopper_t enums must be served correctly, this might require
- cacheline invalidation for the module just handled
- or possible writeback of cl_mem to input cacheline.
*/
static inline gboolean _module_pipe_stop(dt_dev_pixelpipe_t *pipe,
const dt_iop_module_t *module,
float *input)
float *input,
void *cl_in)
{
const dt_dev_pixelpipe_stopper_t stopper = dt_atomic_get_int(&pipe->shutdown);
if(stopper <= DT_DEV_PIXELPIPE_STOP_NO)
Expand All @@ -1280,9 +1285,79 @@ static inline gboolean _module_pipe_stop(dt_dev_pixelpipe_t *pipe,
if(stopper == DT_DEV_PIXELPIPE_STOP_NODES || stopper == DT_DEV_PIXELPIPE_STOP_HQ)
return TRUE;

// stopper reflects the iop_order of the stopping mode so we must invalidate.
dt_dev_pixelpipe_invalidate_cacheline(pipe, input);
dt_dev_pixelpipe_cache_invalidate_later(pipe, stopper, "module pipe stop: ");
/* If cl_in has been provided this means for **all** other stoppers
- we might have changed cl_in data (for example by colorspace conversion)
- or there is no valid data in pipecache presented as input
so we write back cl_in to host, in case there is an error we have to invalidate.
*/
#ifdef HAVE_OPENCL
if(cl_in && input && !pipe->nocache)
{
cl_mem cl_img = (cl_mem)cl_in;
const int width = dt_opencl_get_image_width(cl_img);
const int height = dt_opencl_get_image_height(cl_img);
const int el = dt_opencl_get_image_element_size(cl_img);

const dt_dev_pixelpipe_cache_t *cache = &pipe->cache;
size_t lsize = 0;
for(int k = DT_PIPECACHE_MIN; k < cache->entries; k++)
{
if(cache->data[k] == input)
{
lsize = cache->size[k];
break;
}
}

// copy back to input cacheline
if(!lsize
|| (lsize != ((size_t)width * height * el))
|| dt_opencl_copy_image_to_host(pipe->devid, input, cl_img, width, height, el) != CL_SUCCESS)
dt_dev_pixelpipe_invalidate_cacheline(pipe, input);

dt_opencl_release_mem_object(cl_img);
}
#endif

if(stopper == DT_DEV_PIXELPIPE_STOP_ZOOM)
dt_dev_pixelpipe_cache_invalidate_iop(pipe, module->iop_order, "zoomed pipe stop: ");
else
// stopper reflects the iop_order of the stopping mode so we must invalidate output cachelines.
dt_dev_pixelpipe_cache_invalidate_iop(pipe, stopper, "module pipe stop: ");
return TRUE;
}

gboolean dt_dev_pixelpipe_piece_shutdown(dt_dev_pixelpipe_iop_t *piece)
{
dt_dev_pixelpipe_t *pipe = piece->pipe;
const dt_dev_pixelpipe_stopper_t stopper = dt_atomic_get_int(&pipe->shutdown);
if(stopper <= DT_DEV_PIXELPIPE_STOP_NO)
return FALSE;

/* We do this while processing a module so if we stop further processing
the output is certainly incorrect and we can't be sure about it's input data -
at least on CPU pipe code we might have modified it for colorspace.

Thus we change the shutdown mode to the modules's iop_order and use the cleanup
system provided exactly for this situation in _module_pipe_stop() for both modes
that normally don't require any cacheline safety code.

Other valid shutdown modes as in dt_dev_pixelpipe_stopper_t return TRUE but
will be handled in pipeline code.

Usecase note: as this test might impact cache hitrate due to cacheline invalidations
it should only be used in heavy-processing modules based on iterations like diffuse.
*/

if(stopper == DT_DEV_PIXELPIPE_STOP_NODES || stopper == DT_DEV_PIXELPIPE_STOP_HQ)
{
dt_print_pipe(DT_DEBUG_PIPE, "modify piece shutdown",
pipe, piece->module, pipe->devid, NULL, NULL, "%s",
dt_dev_pixelpipe_shutdown_to_str(stopper));

dt_atomic_set_int(&pipe->shutdown, piece->module->iop_order);
}

return TRUE;
}

Expand Down Expand Up @@ -1407,18 +1482,10 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
roi_in->width, roi_in->height,
cst_from, cst_to, &input_format->cst,
work_profile);
/* We just have changed the input data if cst_from != cst_to so
the cacheline cst does not reflect the module input colorspace any more!
Note: in opencl code path this is different as the in-data colorspace conversion
is always done in cl_mem and thus does not affect pipecache.

Possible ways to handle this:
1. for now we just invalidate that cacheline so if the pipe is reprocessed we won't
use wrong data.
2. we should implement code for the pipe cache that modifies the data cst.
/* We just might have in-place changed the input data (cst_from != cst_to)
please note that cacheline dt_dev_pixelpipe_cache_t dsc has been fixed
via &input_format->cst so a cache hit in a following pipe won't concert again.
*/
if(cst_from != cst_to)
dt_dev_pixelpipe_invalidate_cacheline(pipe, input);

if(_pipe_has_shutdown(pipe))
return TRUE;
Expand All @@ -1444,13 +1511,9 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
TRUE, dt_dev_pixelpipe_type_to_str(pipe->type));

const size_t nfloats = bpp * roi_out->width * roi_out->height / sizeof(float);
const gboolean want_bcache = _piece_fast_blend(piece, module);
const dt_hash_t phash = want_bcache
? _piece_process_hash(piece, roi_out, module, position)
: DT_INVALID_HASH;
const gboolean bcaching = want_bcache
? pipe->bcache_data && phash == pipe->bcache_hash && phash != DT_INVALID_HASH
: FALSE;
const gboolean fast_blend = _piece_fast_blend(piece);
const dt_hash_t phash = fast_blend ? _piece_blend_hash(piece, roi_out, module, position) : DT_INVALID_HASH;
const gboolean bcaching = pipe->bcache_data && phash == pipe->bcache_hash && phash != DT_INVALID_HASH;

if(!fitting && _piece_may_tile(piece))
{
Expand All @@ -1468,13 +1531,11 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
else
{
module->process_tiling(module, piece, input, *output, roi_in, roi_out, in_bpp);
if(want_bcache)
if(fast_blend)
{
if(dt_pipe_no_mask_display(pipe))
{
float *cache = _get_fast_blendcache(nfloats, phash, pipe);
if(cache) dt_iop_image_copy(cache, *output, nfloats);
}
float *cache = _get_fast_blendcache(nfloats, phash, pipe);
if(cache)
dt_iop_image_copy(cache, *output, nfloats);
else
pipe->bcache_hash = DT_INVALID_HASH;
}
Expand Down Expand Up @@ -1506,7 +1567,7 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
_cpu_benchmark(pipe, module, piece, input, *output, roi_out, roi_in);

module->process(module, piece, input, *output, roi_in, roi_out);
if(want_bcache)
if(fast_blend)
{
if(dt_pipe_no_mask_display(pipe))
{
Expand All @@ -1523,6 +1584,9 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
| PIXELPIPE_FLOW_PROCESSED_WITH_TILING);
}

if(_module_pipe_stop(pipe, module, input, NULL))
return TRUE;

if(pfm_dump)
{
dt_dump_pipe_pfm(module->op, *output, roi_out->width, roi_out->height, bpp,
Expand Down Expand Up @@ -1772,7 +1836,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,

if(module)
g_strlcpy(module_name, module->op, MIN(sizeof(module_name), sizeof(module->op)));
get_output_format(module, pipe, piece, dev, *out_format);
get_output_format(module, pipe, piece, *out_format);
const size_t bpp = dt_iop_buffer_dsc_to_bpp(*out_format);
const size_t bufsize = (size_t)bpp * roi_out->width * roi_out->height;

Expand Down Expand Up @@ -2222,11 +2286,9 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
}
}

const gboolean want_bcache = _piece_fast_blend(piece, module);
const dt_hash_t phash = want_bcache
? _piece_process_hash(piece, roi_out, module, pos)
: DT_INVALID_HASH;
const gboolean bcaching = want_bcache && pipe->bcache_data && phash == pipe->bcache_hash && phash != DT_INVALID_HASH;
const gboolean fast_blend = _piece_fast_blend(piece);
const dt_hash_t phash = fast_blend ? _piece_blend_hash(piece, roi_out, module, pos) : DT_INVALID_HASH;
const gboolean bcaching = pipe->bcache_data && phash == pipe->bcache_hash && phash != DT_INVALID_HASH;

dt_print_pipe(DT_DEBUG_PIPE,
bcaching ? "from blend cache" : "process",
Expand Down Expand Up @@ -2292,7 +2354,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
}

cl_int err = CL_SUCCESS;
if(bcaching)
if(fast_blend)
{
*cl_mem_output = dt_opencl_copy_host_to_image(pipe->devid, pipe->bcache_data, roi_out->width, roi_out->height, out_bpp);
if(*cl_mem_output == NULL)
Expand All @@ -2312,7 +2374,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
else
err = CL_MEM_OBJECT_ALLOCATION_FAILURE;

if(want_bcache && (err == CL_SUCCESS))
if(fast_blend && (err == CL_SUCCESS))
{
// processing was good
if(dt_pipe_no_mask_display(pipe))
Expand Down Expand Up @@ -2362,9 +2424,9 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
pipe->dsc.cst = module->output_colorspace(module, pipe, piece);
}

if(_module_pipe_stop(pipe, module, input))
if(_module_pipe_stop(pipe, module, input, cl_mem_input))
{
dt_opencl_release_mem_object(cl_mem_input);
cl_mem_input = NULL;
return TRUE;
}

Expand Down Expand Up @@ -2486,9 +2548,10 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
roi_in.width, roi_in.height,
input_format->cst, cst_to, &input_format->cst,
work_profile);
// the cacheline cst does not reflect the module input colorspace any more!
// FIXME let's invalidate for now
dt_dev_pixelpipe_invalidate_cacheline(pipe, input);
/* We just might have in-place changed the input data (cst_from != cst_to)
please note that cacheline dt_dev_pixelpipe_cache_t dsc has been fixed
via &input_format->cst so a cache hit in a following pipe won't concert again.
*/
}

// histogram collection for module
Expand All @@ -2513,7 +2576,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
{
err = module->process_tiling_cl(module, piece, input,
*output, &roi_in, roi_out, in_bpp);
if(want_bcache && (err == CL_SUCCESS))
if(fast_blend && (err == CL_SUCCESS))
{
if(dt_pipe_no_mask_display(pipe))
{
Expand Down Expand Up @@ -2542,7 +2605,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
pipe->dsc.cst = module->output_colorspace(module, pipe, piece);
}

if(_module_pipe_stop(pipe, module, input))
if(_module_pipe_stop(pipe, module, input, NULL))
return TRUE;

dt_iop_colorspace_type_t blend_cst =
Expand Down
Loading
Loading