Skip to content

Commit

Permalink
Improved handling of dt_dev_pixelpipe_change()
Browse files Browse the repository at this point in the history
dt_dev_pixelpipe_change() calls the possibly costly dt_dev_pixelpipe_get_dimensions()
in all cases in addition to possibly syncing pipe nodes.

In dt_dev_process_image_job() we check for situations where we *must* call dt_dev_pixelpipe_change()
as we don't have the pipe dimensions available in addition to pipe->changed != DT_DEV_PIPE_UNCHANGED.

This leads to a significant performance gain due to less "pixelpipe-runs" just done to get the dimension.

In dt_dev_pixelpipe_change() we reduce the amount of syncing if possible as there is another active
dt_dev_pixelpipe_change_t flag that includes work to be done. Locking history is also done only if
required as nodes/parameters have been changed.

Some log improvements reducing misleading noise.
  • Loading branch information
jenshannoschwalm committed Jan 22, 2025
1 parent a6b01f7 commit 4cac472
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 31 deletions.
2 changes: 1 addition & 1 deletion src/common/image.c
Original file line number Diff line number Diff line change
Expand Up @@ -862,9 +862,9 @@ void dt_image_update_final_size(const dt_imgid_t imgid)
dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
dt_print(DT_DEBUG_PIPE, "updated final size for ID=%i to %ix%i", imgid, ww, hh);
}
}
dt_print(DT_DEBUG_PIPE, "[dt_image_update_final_size] for ID=%i, updated to %ix%i", imgid, ww, hh);
}

gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height)
Expand Down
13 changes: 8 additions & 5 deletions src/develop/develop.c
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,9 @@ void dt_dev_process_image_job(dt_develop_t *dev,
dt_dev_pixelpipe_set_input(pipe, dev, (float *)buf.buf, buf.width, buf.height,
port ? 1.0 : buf.iscale);

// We require calculation of pixelpipe dimensions via dt_dev_pixelpipe_change() in these cases
const gboolean initial = pipe->loading || dev->image_force_reload || pipe->input_changed;

if(pipe->loading)
{
// init pixel pipeline
Expand Down Expand Up @@ -398,10 +401,10 @@ void dt_dev_process_image_job(dt_develop_t *dev,
if(port == &dev->full)
pipe->input_timestamp = dev->timestamp;

// dt_dev_pixelpipe_change() will clear the changed value
const dt_dev_pixelpipe_change_t pipe_changed = pipe->changed;
// this locks dev->history_mutex.
dt_dev_pixelpipe_change(pipe, dev);
const gboolean pipe_changed = pipe->changed != DT_DEV_PIPE_UNCHANGED;
// dt_dev_pixelpipe_change() locks history mutex while syncing nodes and finally calculates dimensions
if(pipe_changed || initial || (port && port->pipe->loading))
dt_dev_pixelpipe_change(pipe, dev);

float scale = 1.0f;
int window_width = G_MAXINT;
Expand All @@ -414,7 +417,7 @@ void dt_dev_process_image_job(dt_develop_t *dev,
// if just changed to an image with a different aspect ratio or
// altered image orientation, the prior zoom xy could now be beyond
// the image boundary
if(port->pipe->loading || pipe_changed != DT_DEV_PIPE_UNCHANGED)
if(port->pipe->loading || pipe_changed)
dt_dev_zoom_move(port, DT_ZOOM_MOVE, 0.0f, 0, 0.0f, 0.0f, TRUE);

// determine scale according to new dimensions
Expand Down
61 changes: 36 additions & 25 deletions src/develop/pixelpipe_hb.c
Original file line number Diff line number Diff line change
Expand Up @@ -632,39 +632,49 @@ void dt_dev_pixelpipe_synch_top(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)

void dt_dev_pixelpipe_change(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
{
dt_pthread_mutex_lock(&dev->history_mutex);

dt_print_pipe(DT_DEBUG_PIPE, "pipe state changing",
pipe, NULL, DT_DEVICE_NONE, NULL, NULL, "%s%s%s%s",
pipe, NULL, DT_DEVICE_NONE, NULL, NULL, "%s%s%s%s%s",
pipe->changed & DT_DEV_PIPE_ZOOMED ? "zoomed, " : "",
pipe->changed & DT_DEV_PIPE_TOP_CHANGED ? "top changed, " : "",
pipe->changed & DT_DEV_PIPE_SYNCH ? "synch all, " : "",
pipe->changed & DT_DEV_PIPE_REMOVE ? "pipe remove" : "");
// case DT_DEV_PIPE_UNCHANGED: case DT_DEV_PIPE_ZOOMED:
if(pipe->changed & DT_DEV_PIPE_TOP_CHANGED)
{
// only top history item changed.
dt_dev_pixelpipe_synch_top(pipe, dev);
}
if(pipe->changed & DT_DEV_PIPE_SYNCH)
{
// pipeline topology remains intact, only change all params.
dt_dev_pixelpipe_synch_all(pipe, dev);
}
if(pipe->changed & DT_DEV_PIPE_REMOVE)
pipe->changed & DT_DEV_PIPE_REMOVE ? "pipe remove" : "",
pipe->changed == DT_DEV_PIPE_UNCHANGED ? "dimension" : "");

if(pipe->changed & (DT_DEV_PIPE_TOP_CHANGED | DT_DEV_PIPE_SYNCH | DT_DEV_PIPE_REMOVE))
{
// modules have been added in between or removed. need to rebuild
// the whole pipeline.
dt_dev_pixelpipe_cleanup_nodes(pipe);
dt_dev_pixelpipe_create_nodes(pipe, dev);
dt_dev_pixelpipe_synch_all(pipe, dev);
dt_pthread_mutex_lock(&dev->history_mutex);

const gboolean sync_all = pipe->changed & (DT_DEV_PIPE_SYNCH | DT_DEV_PIPE_REMOVE);
const gboolean sync_remove = pipe->changed & DT_DEV_PIPE_REMOVE;

if((pipe->changed & DT_DEV_PIPE_TOP_CHANGED) && !sync_all)
{
// only top history item changed. Not required if we synch_all
dt_dev_pixelpipe_synch_top(pipe, dev);
}

if((pipe->changed & DT_DEV_PIPE_SYNCH) && !sync_remove)
{
// pipeline topology remains intact but change all params. Not required if we rebuild all nodes
dt_dev_pixelpipe_synch_all(pipe, dev);
}

if(pipe->changed & DT_DEV_PIPE_REMOVE)
{
// modules have been added in between or removed. need to rebuild the whole pipeline.
dt_dev_pixelpipe_cleanup_nodes(pipe);
dt_dev_pixelpipe_create_nodes(pipe, dev);
dt_dev_pixelpipe_synch_all(pipe, dev);
}

dt_pthread_mutex_unlock(&dev->history_mutex);
}
pipe->changed = DT_DEV_PIPE_UNCHANGED;
dt_pthread_mutex_unlock(&dev->history_mutex);

dt_dev_pixelpipe_get_dimensions(pipe, dev,
pipe->iwidth, pipe->iheight,
&pipe->processed_width,
&pipe->processed_height);
pipe->changed = DT_DEV_PIPE_UNCHANGED;
}

void dt_dev_pixelpipe_usedetails(dt_dev_pixelpipe_t *pipe)
Expand Down Expand Up @@ -2903,6 +2913,8 @@ void dt_dev_pixelpipe_get_dimensions(dt_dev_pixelpipe_t *pipe,
{
dt_pthread_mutex_lock(&pipe->busy_mutex);
dt_iop_roi_t roi_in = (dt_iop_roi_t){ 0, 0, width_in, height_in, 1.0 };
dt_print_pipe(DT_DEBUG_PIPE,
"get dimensions", pipe, NULL, DT_DEVICE_NONE, &roi_in, NULL, "ID=%i", pipe->image.id);
dt_iop_roi_t roi_out;
GList *modules = pipe->iop;
GList *pieces = pipe->nodes;
Expand All @@ -2919,8 +2931,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", piece->pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out, "ID=%i",
pipe->image.id);
"modify roi OUT", pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out);
}
else
{
Expand Down

0 comments on commit 4cac472

Please sign in to comment.