Skip to content

Commit 26dea41

Browse files
Improved handling of dt_dev_pixelpipe_change()
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. Some log improvements reducing misleading noise.
1 parent 1f5b211 commit 26dea41

File tree

3 files changed

+41
-29
lines changed

3 files changed

+41
-29
lines changed

src/common/image.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -862,9 +862,9 @@ void dt_image_update_final_size(const dt_imgid_t imgid)
862862
dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
863863
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
864864
DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
865+
dt_print(DT_DEBUG_PIPE, "updated final size for ID=%i to %ix%i", imgid, ww, hh);
865866
}
866867
}
867-
dt_print(DT_DEBUG_PIPE, "[dt_image_update_final_size] for ID=%i, updated to %ix%i", imgid, ww, hh);
868868
}
869869

870870
gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height)

src/develop/develop.c

+8-5
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,9 @@ void dt_dev_process_image_job(dt_develop_t *dev,
346346
dt_dev_pixelpipe_set_input(pipe, dev, (float *)buf.buf, buf.width, buf.height,
347347
port ? 1.0 : buf.iscale);
348348

349+
// We require calculation of pixelpipe dimensions via dt_dev_pixelpipe_change() in these cases
350+
const gboolean initial = pipe->loading || dev->image_force_reload || pipe->input_changed;
351+
349352
if(pipe->loading)
350353
{
351354
// init pixel pipeline
@@ -398,10 +401,10 @@ void dt_dev_process_image_job(dt_develop_t *dev,
398401
if(port == &dev->full)
399402
pipe->input_timestamp = dev->timestamp;
400403

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

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

420423
// determine scale according to new dimensions

src/develop/pixelpipe_hb.c

+32-23
Original file line numberDiff line numberDiff line change
@@ -633,34 +633,42 @@ void dt_dev_pixelpipe_synch_top(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
633633
void dt_dev_pixelpipe_change(dt_dev_pixelpipe_t *pipe, dt_develop_t *dev)
634634
{
635635
dt_pthread_mutex_lock(&dev->history_mutex);
636-
637-
dt_print_pipe(DT_DEBUG_PIPE, "pipe state changing",
638-
pipe, NULL, DT_DEVICE_NONE, NULL, NULL, "%s%s%s%s",
636+
dt_print_pipe(DT_DEBUG_PIPE, "dev_pixelpipe_change",
637+
pipe, NULL, DT_DEVICE_NONE, NULL, NULL, "%s%s%s%s%s",
639638
pipe->changed & DT_DEV_PIPE_ZOOMED ? "zoomed, " : "",
640639
pipe->changed & DT_DEV_PIPE_TOP_CHANGED ? "top changed, " : "",
641640
pipe->changed & DT_DEV_PIPE_SYNCH ? "synch all, " : "",
642-
pipe->changed & DT_DEV_PIPE_REMOVE ? "pipe remove" : "");
643-
// case DT_DEV_PIPE_UNCHANGED: case DT_DEV_PIPE_ZOOMED:
644-
if(pipe->changed & DT_DEV_PIPE_TOP_CHANGED)
645-
{
646-
// only top history item changed.
647-
dt_dev_pixelpipe_synch_top(pipe, dev);
648-
}
649-
if(pipe->changed & DT_DEV_PIPE_SYNCH)
650-
{
651-
// pipeline topology remains intact, only change all params.
652-
dt_dev_pixelpipe_synch_all(pipe, dev);
653-
}
654-
if(pipe->changed & DT_DEV_PIPE_REMOVE)
641+
pipe->changed & DT_DEV_PIPE_REMOVE ? "pipe remove" : "",
642+
pipe->changed == DT_DEV_PIPE_UNCHANGED ? "dimension" : "");
643+
644+
if(pipe->changed & (DT_DEV_PIPE_TOP_CHANGED | DT_DEV_PIPE_SYNCH | DT_DEV_PIPE_REMOVE))
655645
{
656-
// modules have been added in between or removed. need to rebuild
657-
// the whole pipeline.
658-
dt_dev_pixelpipe_cleanup_nodes(pipe);
659-
dt_dev_pixelpipe_create_nodes(pipe, dev);
660-
dt_dev_pixelpipe_synch_all(pipe, dev);
646+
const gboolean sync_all = pipe->changed & (DT_DEV_PIPE_SYNCH | DT_DEV_PIPE_REMOVE);
647+
const gboolean sync_remove = pipe->changed & DT_DEV_PIPE_REMOVE;
648+
649+
if((pipe->changed & DT_DEV_PIPE_TOP_CHANGED) && !sync_all)
650+
{
651+
// only top history item changed. Not required if we synch_all
652+
dt_dev_pixelpipe_synch_top(pipe, dev);
653+
}
654+
655+
if((pipe->changed & DT_DEV_PIPE_SYNCH) && !sync_remove)
656+
{
657+
// pipeline topology remains intact but change all params. Not required if we rebuild all nodes
658+
dt_dev_pixelpipe_synch_all(pipe, dev);
659+
}
660+
661+
if(pipe->changed & DT_DEV_PIPE_REMOVE)
662+
{
663+
// modules have been added in between or removed. need to rebuild the whole pipeline.
664+
dt_dev_pixelpipe_cleanup_nodes(pipe);
665+
dt_dev_pixelpipe_create_nodes(pipe, dev);
666+
dt_dev_pixelpipe_synch_all(pipe, dev);
667+
}
661668
}
662669
pipe->changed = DT_DEV_PIPE_UNCHANGED;
663670
dt_pthread_mutex_unlock(&dev->history_mutex);
671+
664672
dt_dev_pixelpipe_get_dimensions(pipe, dev,
665673
pipe->iwidth, pipe->iheight,
666674
&pipe->processed_width,
@@ -2906,6 +2914,8 @@ void dt_dev_pixelpipe_get_dimensions(dt_dev_pixelpipe_t *pipe,
29062914
{
29072915
dt_pthread_mutex_lock(&pipe->busy_mutex);
29082916
dt_iop_roi_t roi_in = (dt_iop_roi_t){ 0, 0, width_in, height_in, 1.0 };
2917+
dt_print_pipe(DT_DEBUG_PIPE,
2918+
"get dimensions", pipe, NULL, DT_DEVICE_NONE, &roi_in, NULL, "ID=%i", pipe->image.id);
29092919
dt_iop_roi_t roi_out;
29102920
GList *modules = pipe->iop;
29112921
GList *pieces = pipe->nodes;
@@ -2922,8 +2932,7 @@ void dt_dev_pixelpipe_get_dimensions(dt_dev_pixelpipe_t *pipe,
29222932
module->modify_roi_out(module, piece, &roi_out, &roi_in);
29232933
if((darktable.unmuted & DT_DEBUG_PIPE) && memcmp(&roi_out, &roi_in, sizeof(dt_iop_roi_t)))
29242934
dt_print_pipe(DT_DEBUG_PIPE,
2925-
"modify roi OUT", piece->pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out, "ID=%i",
2926-
pipe->image.id);
2935+
"modify roi OUT", pipe, module, DT_DEVICE_NONE, &roi_in, &roi_out);
29272936
}
29282937
else
29292938
{

0 commit comments

Comments
 (0)