Skip to content

Commit

Permalink
Merge pull request #18360 from jenshannoschwalm/reverting_pipe_stuff
Browse files Browse the repository at this point in the history
Reverting pipe stuff
  • Loading branch information
TurboGit authored Feb 5, 2025
2 parents 80e5903 + ed4c0d2 commit 2b22e83
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 56 deletions.
12 changes: 6 additions & 6 deletions src/common/iop_order.c
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,7 @@ static gboolean _operation_already_handled(GList *e_list,
}

// returns the nth module's priority being active or not
static int _get_multi_priority(dt_develop_t *dev,
int _get_multi_priority(dt_develop_t *dev,
const char *operation,
const int n,
const gboolean only_disabled)
Expand All @@ -1522,9 +1522,9 @@ static int _get_multi_priority(dt_develop_t *dev,
return INT_MAX;
}

static void _ioppr_update_for_entries(dt_develop_t *dev,
GList *entry_list,
const gboolean append)
void dt_ioppr_update_for_entries(dt_develop_t *dev,
GList *entry_list,
const gboolean append)
{
// for each priority list to be checked
for(GList *e_list = entry_list; e_list; e_list = g_list_next(e_list))
Expand Down Expand Up @@ -1656,7 +1656,7 @@ void dt_ioppr_update_for_style_items(dt_develop_t *dev,

e_list = g_list_reverse(e_list); // list was built in reverse order, so un-reverse it

_ioppr_update_for_entries(dev, e_list, append);
dt_ioppr_update_for_entries(dev, e_list, append);

// write back the multi-priority

Expand Down Expand Up @@ -1703,7 +1703,7 @@ void dt_ioppr_update_for_modules(dt_develop_t *dev,
}
e_list = g_list_reverse(e_list); // list was built in reverse order, so un-reverse it

_ioppr_update_for_entries(dev, e_list, append);
dt_ioppr_update_for_entries(dev, e_list, append);

// write back the multi-priority

Expand Down
8 changes: 5 additions & 3 deletions src/common/styles.c
Original file line number Diff line number Diff line change
Expand Up @@ -866,9 +866,9 @@ void _styles_apply_to_image_ext(const char *name,

dt_ioppr_check_iop_order(dev_dest, newimgid, "dt_styles_apply_to_image 1");

dt_print(DT_DEBUG_IOPORDER,
"[styles_apply_to_image_ext] Apply style on image `%s' id %i, history size %i",
dev_dest->image_storage.filename, newimgid, dev_dest->history_end);
dt_print(DT_DEBUG_IOPORDER | DT_DEBUG_PIPE,
"[styles_apply_to_image_ext] Apply `%s' on ID=%i, history size %i",
name, newimgid, dev_dest->history_end);

// go through all entries in style
// clang-format off
Expand Down Expand Up @@ -1004,6 +1004,8 @@ void dt_styles_apply_to_dev(const char *name, const dt_imgid_t imgid)
{
if(!darktable.develop || !dt_is_valid_imgid(darktable.develop->image_storage.id))
return;
dt_print(DT_DEBUG_IOPORDER | DT_DEBUG_PIPE,
"[dt_styles_apply_to_dev] Apply `%s' on ID=%d", name, imgid);

/* write current history changes so nothing gets lost */
dt_dev_write_history(darktable.develop);
Expand Down
43 changes: 21 additions & 22 deletions src/develop/pixelpipe_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,39 +104,38 @@ void dt_dev_pixelpipe_cache_cleanup(dt_dev_pixelpipe_t *pipe)

static dt_hash_t _dev_pixelpipe_cache_basichash(const dt_imgid_t imgid,
dt_dev_pixelpipe_t *pipe,
const int order)
const int position)
{
/* What do we use for the basic hash
1) imgid as all structures using the hash might possibly contain data from other images
2) pipe->type as we want to keep status of fast mode included
3) pipe->want_detail_mask makes sure old cachelines from before activating details are
2) pipe->type for the cache it's important to keep status of fast mode included here
also, we might use the hash also for different pipe.
3) pipe->want_detail_mask make sure old cachelines from before activating details are
not valid any more.
Do we have to keep the roi of details mask? No, as that is always defined by roi_in
Do we have to keep the roi of details mask? No as that is always defined by roi_in
of the mask writing module (rawprepare or demosaic)
4) The piece->hash of modules within the given limit excluding the skipped
4) Please note that position is not the iop_order but the n-th node position in the pipe
*/
const uint32_t hashing_pipemode[3] = {(uint32_t)imgid,
(uint32_t)pipe->type,
(uint32_t)pipe->want_detail_mask };
dt_hash_t hash = dt_hash(DT_INITHASH, &hashing_pipemode, sizeof(hashing_pipemode));

// go through all modules up to iop_order and compute a hash using the operation and params.
// go through all modules up to position and compute a hash using the operation and params.
GList *pieces = pipe->nodes;
while(pieces)
for(int k = 0; k < position && pieces; k++)
{
const dt_dev_pixelpipe_iop_t *piece = pieces->data;
const dt_iop_module_t *module = piece->module;

if(module->iop_order > order) break;

dt_dev_pixelpipe_iop_t *piece = pieces->data;
// As this runs through all pipe nodes - also the ones not commited -
// we can safely avoid disabled modules/pieces
const gboolean included = piece->module->enabled || piece->enabled;
// don't take skipped modules into account
const gboolean skipped = dt_iop_module_is_skipped(module->dev, module)
&& (pipe->type & DT_DEV_PIXELPIPE_BASIC);

if(!skipped)
const gboolean skipped = dt_iop_module_is_skipped(piece->module->dev, piece->module)
&& (pipe->type & DT_DEV_PIXELPIPE_BASIC);
if(!skipped && included)
{
hash = dt_hash(hash, &piece->hash, sizeof(piece->hash));
if(module->request_color_pick != DT_REQUEST_COLORPICK_OFF)
if(piece->module->request_color_pick != DT_REQUEST_COLORPICK_OFF)
{
if(darktable.lib->proxy.colorpicker.primary_sample->size == DT_LIB_COLORPICKER_SIZE_BOX)
{
Expand All @@ -156,9 +155,9 @@ static dt_hash_t _dev_pixelpipe_cache_basichash(const dt_imgid_t imgid,
dt_hash_t dt_dev_pixelpipe_cache_hash(const dt_imgid_t imgid,
const dt_iop_roi_t *roi,
dt_dev_pixelpipe_t *pipe,
const int order)
const int position)
{
dt_hash_t hash = _dev_pixelpipe_cache_basichash(imgid, pipe, order);
dt_hash_t hash = _dev_pixelpipe_cache_basichash(imgid, pipe, position);
// also include roi data
// FIXME include full roi data in cachelines
hash = dt_hash(hash, roi, sizeof(dt_iop_roi_t));
Expand Down Expand Up @@ -214,7 +213,7 @@ static int _get_oldest_cacheline(dt_dev_pixelpipe_cache_t *cache,
return id;
}

static int __get_cacheline(dt_dev_pixelpipe_cache_t *cache)
static int _get_c_cacheline(dt_dev_pixelpipe_cache_t *cache)
{
int oldest = _get_oldest_cacheline(cache, DT_CACHETEST_INVALID);
if(oldest > 0) return oldest;
Expand All @@ -235,7 +234,7 @@ static int _get_cacheline(dt_dev_pixelpipe_t *pipe)
if((cache->entries == DT_PIPECACHE_MIN) || pipe->mask_display || pipe->nocache)
return cache->calls & 1;

cache->lastline = __get_cacheline(cache);
cache->lastline = _get_c_cacheline(cache);
return cache->lastline;
}

Expand Down Expand Up @@ -287,7 +286,7 @@ gboolean dt_dev_pixelpipe_cache_get(dt_dev_pixelpipe_t *pipe,
const size_t size,
void **data,
dt_iop_buffer_dsc_t **dsc,
dt_iop_module_t *module,
const dt_iop_module_t *module,
const gboolean important)
{
dt_dev_pixelpipe_cache_t *cache = &pipe->cache;
Expand Down
8 changes: 4 additions & 4 deletions src/develop/pixelpipe_cache.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
This file is part of darktable,
Copyright (C) 2009-2024 darktable developers.
Copyright (C) 2009-2025 darktable developers.
darktable is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -65,17 +65,17 @@ typedef enum dt_dev_pixelpipe_cache_test_t
gboolean dt_dev_pixelpipe_cache_init(struct dt_dev_pixelpipe_t *pipe, const int entries, const size_t size, const size_t limit);
void dt_dev_pixelpipe_cache_cleanup(struct dt_dev_pixelpipe_t *pipe);

/** creates a hopefully unique hash from the complete module stack up to the modules iop_order, including current viewport. */
/** creates a hopefully unique hash from the complete module stack up to the module-th, including the roi. */
dt_hash_t dt_dev_pixelpipe_cache_hash(const dt_imgid_t imgid, const struct dt_iop_roi_t *roi,
struct dt_dev_pixelpipe_t *pipe, const int order);
struct dt_dev_pixelpipe_t *pipe, const int position);

/** returns a float data buffer in 'data' for the given hash from the cache, dsc is updated too.
If the hash does not match any cache line, use an old buffer or allocate a fresh one.
The size of the buffer in 'data' will be at least of size bytes.
Returned flag is TRUE for a new buffer
*/
gboolean dt_dev_pixelpipe_cache_get(struct dt_dev_pixelpipe_t *pipe, const dt_hash_t hash,
const size_t size, void **data, struct dt_iop_buffer_dsc_t **dsc, struct dt_iop_module_t *module, const gboolean important);
const size_t size, void **data, struct dt_iop_buffer_dsc_t **dsc, const struct dt_iop_module_t *module, const gboolean important);

/** test availability of a cache line without destroying another, if it is not found. */
gboolean dt_dev_pixelpipe_cache_available(struct dt_dev_pixelpipe_t *pipe, const dt_hash_t hash, const size_t size);
Expand Down
35 changes: 18 additions & 17 deletions src/develop/pixelpipe_hb.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,7 @@ static void _dev_pixelpipe_synch(dt_dev_pixelpipe_t *pipe,
if(active && hist->iop_order == INT_MAX)
{
piece->enabled = FALSE;
dt_print_pipe(DT_DEBUG_PARAMS | DT_DEBUG_PIPE | DT_DEBUG_IOPORDER, "dt_dev_pixelpipe_synch",
dt_print_pipe(DT_DEBUG_PARAMS | DT_DEBUG_PIPE, "dt_dev_pixelpipe_synch",
pipe, piece->module, DT_DEVICE_NONE, NULL, NULL,
"enabled module with iop_order of INT_MAX is disabled");
}
Expand Down Expand Up @@ -1133,9 +1133,10 @@ static void _collect_histogram_on_CPU(dt_dev_pixelpipe_t *pipe,
*/
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 dt_iop_module_t *module,
const int position)
{
dt_hash_t phash = dt_dev_pixelpipe_cache_hash(piece->pipe->image.id, roi, piece->pipe, module->iop_order -1);
dt_hash_t phash = dt_dev_pixelpipe_cache_hash(piece->pipe->image.id, roi, piece->pipe, position -1);
phash = dt_hash(phash, roi, sizeof(dt_iop_roi_t));
phash = dt_hash(phash, &module->so->op, strlen(module->so->op));
phash = dt_hash(phash, &module->instance, sizeof(module->instance));
Expand Down Expand Up @@ -1175,7 +1176,8 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
dt_iop_module_t *module,
dt_dev_pixelpipe_iop_t *piece,
dt_develop_tiling_t *tiling,
dt_pixelpipe_flow_t *pixelpipe_flow)
dt_pixelpipe_flow_t *pixelpipe_flow,
const int position)
{
if(dt_atomic_get_int(&pipe->shutdown))
return TRUE;
Expand Down Expand Up @@ -1248,7 +1250,7 @@ static gboolean _pixelpipe_process_on_CPU(dt_dev_pixelpipe_t *pipe,
TRUE, dt_dev_pixelpipe_type_to_str(pipe->type));

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

Expand Down Expand Up @@ -1498,7 +1500,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
if(dt_atomic_get_int(&pipe->shutdown))
return TRUE;

const dt_hash_t hash = dt_dev_pixelpipe_cache_hash(pipe->image.id, roi_out, pipe, module ? module->iop_order : 0);
dt_hash_t hash = dt_dev_pixelpipe_cache_hash(pipe->image.id, roi_out, pipe, pos);

// we do not want data from the preview pixelpipe cache
// for gamma so we can compute the final scope
Expand Down Expand Up @@ -1638,7 +1640,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
module->modify_roi_in(module, piece, roi_out, &roi_in);
if((darktable.unmuted & DT_DEBUG_PIPE) && memcmp(roi_out, &roi_in, sizeof(dt_iop_roi_t)))
dt_print_pipe(DT_DEBUG_PIPE,
"modify roi IN", pipe, module, DT_DEVICE_NONE, roi_out, &roi_in, "ID=%i",
"modified roi IN", pipe, module, DT_DEVICE_NONE, roi_out, &roi_in, "ID=%i",
pipe->image.id);
// recurse to get actual data of input buffer

Expand Down Expand Up @@ -1945,7 +1947,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
if(success_opencl)
{
const gboolean relevant = _piece_fast_blend(piece, module);
const dt_hash_t phash = relevant ? _piece_process_hash(piece, roi_out, module) : 0;
const dt_hash_t phash = relevant ? _piece_process_hash(piece, roi_out, module, pos) : 0;
const gboolean bcaching = relevant ? pipe->bcache_data && phash == pipe->bcache_hash : FALSE;

dt_print_pipe(DT_DEBUG_PIPE,
Expand Down Expand Up @@ -2225,7 +2227,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
if(success_opencl)
{
const gboolean relevant = _piece_fast_blend(piece, module);
const dt_hash_t phash = relevant ? _piece_process_hash(piece, roi_out, module) : 0;
const dt_hash_t phash = relevant ? _piece_process_hash(piece, roi_out, module, pos) : 0;
const gboolean bcaching = relevant ? pipe->bcache_data && phash == pipe->bcache_hash : FALSE;
dt_print_pipe(DT_DEBUG_PIPE,
bcaching ? "from focus cache" : "process tiled",
Expand Down Expand Up @@ -2375,7 +2377,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
*/
important_cl =
(pipe->mask_display == DT_DEV_PIXELPIPE_DISPLAY_NONE)
&& (pipe->type & DT_DEV_PIXELPIPE_BASIC)
&& pipe->type & DT_DEV_PIXELPIPE_BASIC
&& dev->gui_attached
&& ((module == dt_dev_gui_module())
|| darktable.develop->history_last_module == module
Expand Down Expand Up @@ -2469,7 +2471,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,
}
if(_pixelpipe_process_on_CPU(pipe, dev, input, input_format, &roi_in, output,
out_format,
roi_out, module, piece, &tiling, &pixelpipe_flow))
roi_out, module, piece, &tiling, &pixelpipe_flow, pos))
return TRUE;
}

Expand Down Expand Up @@ -2507,7 +2509,7 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,

if(_pixelpipe_process_on_CPU(pipe, dev, input, input_format, &roi_in,
output, out_format,
roi_out, module, piece, &tiling, &pixelpipe_flow))
roi_out, module, piece, &tiling, &pixelpipe_flow, pos))
return TRUE;
}

Expand All @@ -2522,13 +2524,13 @@ static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,

if(_pixelpipe_process_on_CPU(pipe, dev, input, input_format, &roi_in,
output, out_format, roi_out,
module, piece, &tiling, &pixelpipe_flow))
module, piece, &tiling, &pixelpipe_flow, pos))
return TRUE;
}
#else // HAVE_OPENCL
if(_pixelpipe_process_on_CPU(pipe, dev, input, input_format, &roi_in,
output, out_format, roi_out,
module, piece, &tiling, &pixelpipe_flow))
module, piece, &tiling, &pixelpipe_flow, pos))
return TRUE;
#endif // HAVE_OPENCL

Expand Down Expand Up @@ -2980,7 +2982,7 @@ gboolean dt_dev_pixelpipe_process(dt_dev_pixelpipe_t *pipe,

// terminate
dt_pthread_mutex_lock(&pipe->backbuf_mutex);
pipe->backbuf_hash = dt_dev_pixelpipe_cache_hash(pipe->image.id, &roi, pipe, INT_MAX);
pipe->backbuf_hash = dt_dev_pixelpipe_cache_hash(pipe->image.id, &roi, pipe, pos);

//FIXME lock/release cache line instead of copying
if(pipe->type & DT_DEV_PIXELPIPE_SCREEN)
Expand Down Expand Up @@ -3045,7 +3047,7 @@ void dt_dev_pixelpipe_get_dimensions(dt_dev_pixelpipe_t *pipe,
module->modify_roi_out(module, piece, &roi_out, &roi_in);
if((darktable.unmuted & DT_DEBUG_PIPE) && memcmp(&roi_out, &roi_in, sizeof(dt_iop_roi_t)))
dt_print_pipe(DT_DEBUG_PIPE,
"modify roi OUT", pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out);
"modified roi OUT", pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out);
}
else
{
Expand Down Expand Up @@ -3176,7 +3178,6 @@ float *dt_dev_get_raster_mask(dt_dev_pixelpipe_iop_t *piece,
if(!_skip_piece_on_tags(it_piece))
{
if(it_piece->module->distort_mask
&& it_piece->enabled
// hack against pipes not using finalscale
&& !(dt_iop_module_is(it_piece->module->so, "finalscale")
&& it_piece->processed_roi_in.width == 0
Expand Down
2 changes: 1 addition & 1 deletion src/iop/ashift.c
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,7 @@ void modify_roi_out(struct dt_iop_module_t *self,
if(roi_out->width < 4 || roi_out->height < 4)
{
dt_print_pipe(DT_DEBUG_PIPE,
"insane data", piece->pipe, self, DT_DEVICE_NONE, roi_in, roi_out);
"safety check", piece->pipe, self, DT_DEVICE_NONE, roi_in, roi_out);

roi_out->width = roi_in->width;
roi_out->height = roi_in->height;
Expand Down
2 changes: 1 addition & 1 deletion src/iop/clipping.c
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ void modify_roi_out(dt_iop_module_t *self, dt_dev_pixelpipe_iop_t *piece, dt_iop
if(roi_out->width < 4 || roi_out->height < 4)
{
dt_print_pipe(DT_DEBUG_PIPE,
"insane data", piece->pipe, self, DT_DEVICE_NONE, roi_in, roi_out);
"safety check", piece->pipe, self, DT_DEVICE_NONE, roi_in, roi_out);

roi_out->x = roi_in->x;
roi_out->y = roi_in->y;
Expand Down
11 changes: 10 additions & 1 deletion src/iop/toneequal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1012,7 +1012,16 @@ void toneeq_process(dt_iop_module_t *self,
const size_t num_elem = width * height;

// Get the hash of the upstream pipe to track changes
const int position = self->iop_order;
int position = 0;
GList *pieces = piece->pipe->nodes;
while(pieces)
{
position++;
const dt_dev_pixelpipe_iop_t *node = pieces->data;
if(piece == node) break;

pieces = g_list_next(pieces);
}
const dt_hash_t hash = dt_dev_pixelpipe_cache_hash(piece->pipe->image.id,
roi_out, piece->pipe, position);

Expand Down
5 changes: 4 additions & 1 deletion src/libs/history.c
Original file line number Diff line number Diff line change
Expand Up @@ -1269,8 +1269,11 @@ static gboolean _lib_history_button_clicked_callback(GtkWidget *widget,
Yet - there are modules that require fresh data for internal visualizing.
As there is currently no way to know about that we do a brute-force way and simply
invalidate cachelines.
(we might want an additional iop module flag and keep track of that in pixelpipe cache code ???)
For raws we have at least rawprepare and demosaic
*/
dt_dev_pixelpipe_cache_invalidate_later(darktable.develop->preview_pipe, 0);
const int order = dt_image_is_raw(&darktable.develop->image_storage) ? 2 : 0;
dt_dev_pixelpipe_cache_invalidate_later(darktable.develop->preview_pipe, order);

/* signal history changed */
dt_dev_undo_end_record(darktable.develop);
Expand Down

0 comments on commit 2b22e83

Please sign in to comment.