diff --git a/src/develop/develop.c b/src/develop/develop.c index eef9b34d3596..e941dddac704 100644 --- a/src/develop/develop.c +++ b/src/develop/develop.c @@ -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(); } diff --git a/src/develop/pixelpipe_cache.c b/src/develop/pixelpipe_cache.c index 80923dd4473c..beee9b5f131d 100644 --- a/src/develop/pixelpipe_cache.c +++ b/src/develop/pixelpipe_cache.c @@ -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) diff --git a/src/develop/pixelpipe_cache.h b/src/develop/pixelpipe_cache.h index d91ed4c8d2c8..5cf5a0974a76 100644 --- a/src/develop/pixelpipe_cache.h +++ b/src/develop/pixelpipe_cache.h @@ -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); diff --git a/src/develop/pixelpipe_hb.c b/src/develop/pixelpipe_hb.c index 599b0a4fc84f..6fff59020cc1 100644 --- a/src/develop/pixelpipe_hb.c +++ b/src/develop/pixelpipe_hb.c @@ -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 @@ -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"; } @@ -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); @@ -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, @@ -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)); @@ -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); } @@ -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) @@ -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; } @@ -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; @@ -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)) { @@ -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; } @@ -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)) { @@ -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, @@ -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; @@ -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", @@ -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) @@ -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)) @@ -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; } @@ -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 @@ -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)) { @@ -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 = diff --git a/src/develop/pixelpipe_hb.h b/src/develop/pixelpipe_hb.h index 0c0acea08819..0ab7e00709ce 100644 --- a/src/develop/pixelpipe_hb.h +++ b/src/develop/pixelpipe_hb.h @@ -31,7 +31,7 @@ G_BEGIN_DECLS #define DT_PIPECACHE_MIN 2 -// #define DT_PIPE_CAS_SHUTDOWN +#define DT_PIPE_CAS_SHUTDOWN /** cached distorted mask at a geometric module's output boundary. * used to avoid re-distorting masks from scratch when multiple @@ -126,6 +126,10 @@ typedef enum dt_dev_pixelpipe_status_t Used to switch between darkroom HQ modes. Requires a restart of the pipe but pixelpipe cache can stay. + DT_DEV_PIXELPIPE_STOP_ZOOM + As we stopped inside a module the piece input data and all pipe cachelines with this + iop_order will be invalidated. + DT_DEV_PIXELPIPE_STOP_LAST If the shutdown value is >= DT_DEV_PIXELPIPE_STOP_LAST it is understood as the iop_order of a module. @@ -139,6 +143,7 @@ typedef enum dt_dev_pixelpipe_stopper_t DT_DEV_PIXELPIPE_STOP_NO = 0, DT_DEV_PIXELPIPE_STOP_NODES, DT_DEV_PIXELPIPE_STOP_HQ, + DT_DEV_PIXELPIPE_STOP_ZOOM, DT_DEV_PIXELPIPE_STOP_LAST, } dt_dev_pixelpipe_stopper_t; @@ -422,6 +427,9 @@ void dt_dev_pixelpipe_disable_after(dt_dev_pixelpipe_t *pipe, const char *op); // disable given op and all that comes before it in the pipe: void dt_dev_pixelpipe_disable_before(dt_dev_pixelpipe_t *pipe, const char *op); +// pieces can test for a pipe shutdown request within module->process(). +gboolean dt_dev_pixelpipe_piece_shutdown(dt_dev_pixelpipe_iop_t *piece); + // helper function to pass a raster mask through a (so far) processed pipe float *dt_dev_get_raster_mask(dt_dev_pixelpipe_iop_t *piece, const struct dt_iop_module_t *raster_mask_source, diff --git a/src/iop/diffuse.c b/src/iop/diffuse.c index ab2f94010b5e..1a04c6c0c8d7 100644 --- a/src/iop/diffuse.c +++ b/src/iop/diffuse.c @@ -1414,7 +1414,7 @@ void process(dt_iop_module_t *self, in = temp1; } - for(int it = 0; it < iterations; it++) + for(int it = 0; it < iterations && !dt_dev_pixelpipe_piece_shutdown(piece); it++) { if(it == 0) { @@ -1680,7 +1680,7 @@ int process_cl(dt_iop_module_t *self, in = temp1; } - for(int it = 0; it < iterations && err == CL_SUCCESS; it++) + for(int it = 0; it < iterations && err == CL_SUCCESS && !dt_dev_pixelpipe_piece_shutdown(piece); it++) { if(it == 0) {