Pixelpipe cache respects internal histograms

For an overview about "how the pixelpipe cache works" see darktable-org#14130

There has been an issue related to some modules internally showing histograms (hist-module),
the underlying reasons are

1. those histograms are updated **while** processing the module
2. if we develop an image in darkroom we often do **not** process the whole pixelpipe but take
   data from the pixelpipe cache thus the histogram updating as in (1) is not done.

We have discussed basically two options until now
1. do some sort of pre-processing before executing the pipe
2. introduce some special widget updating code

both seem to be pretty difficult and likely over-designed.

This pr follows a different approach:

1. If we develop an image with one of the hist-modules **expanded** we accept some performance
   penalty and force the pipeline to be processed from the first expanded hist-module.
   As these modules are late in the pipe the penalty is pretty small.
2. A new mask `DT_REQUEST_EXPANDED` in `dt_dev_request_flags_t` takes care of this.
   The modules in question set this flag.
3. In `dt_dev_pixelpipe_t` we have a new gboolean `nocache`, this flag is checked in the pixelpipe
   and it's caching code.
   If **TRUE**, the requested cachelines are marked as invalid.
   (This is like we do for visualizing masks in some ways)
4. While processing each module in the **full** pixelpipe we check for
     a) the status of `DT_REQUEST_EXPANDED`
     b) the module being enabled & expanded
   and possibly unvalidate the current cacheline and set the pixelpipe `nocache` flag.

Fixes darktable-org#11069
Likely Fixes darktable-org#11003

src/develop/pixelpipe.h

@@ -47,7 +47,8 @@ typedef enum dt_dev_request_flags_t
 {
   DT_REQUEST_NONE = 0,
   DT_REQUEST_ON = 1 << 0,
-  DT_REQUEST_ONLY_IN_GUI = 1 << 1
+  DT_REQUEST_ONLY_IN_GUI = 1 << 1,
+  DT_REQUEST_EXPANDED = 1 << 2 // 
 } dt_dev_request_flags_t;
 
 // params to be used to collect histogram

src/develop/pixelpipe_cache.c

@@ -215,7 +215,7 @@ gboolean dt_dev_pixelpipe_cache_available(
            const uint64_t hash,
            const size_t size)
 {
-  if(pipe->mask_display)
+  if(pipe->mask_display || pipe->nocache)
     return FALSE;
 
   dt_dev_pixelpipe_cache_t *cache = &(pipe->cache);
@@ -295,7 +295,7 @@ static int _get_cacheline(struct dt_dev_pixelpipe_t *pipe)
   dt_dev_pixelpipe_cache_t *cache = &(pipe->cache);
   // Simplest case is some pipes having only two cachelines or we are in masking mode so we
   // can just toggle between them.
-  if((cache->entries == DT_PIPECACHE_MIN) || pipe->mask_display)
+  if((cache->entries == DT_PIPECACHE_MIN) || pipe->mask_display || pipe->nocache)
     return cache->queries & 1;
 
   const int old_free = _get_oldest_free_cacheline(cache);
@@ -331,7 +331,7 @@ static gboolean _get_by_hash(
            won't be taken in this pixelpipe process.
            We do so by setting cache->used[k] to something very high.
       */
-      if((cache->size[k] != size) || pipe->mask_display)
+      if((cache->size[k] != size) || pipe->mask_display || pipe->nocache)
       {
         cache->hash[k] = cache->basichash[k] = -1;
         cache->used[k] = 8 * VERY_OLD_CACHE_WEIGHT;

src/develop/pixelpipe_hb.c

@@ -1118,7 +1118,6 @@ static void _collect_histogram_on_CPU(dt_dev_pixelpipe_t *pipe,
         dt_control_queue_redraw_widget(module->widget);
     }
   }
-  return;
 }
 
 static gboolean _pixelpipe_process_on_CPU(
@@ -1423,7 +1422,8 @@ static gboolean _dev_pixelpipe_process_rec(
 
   // we also never want any cached data is in masking mode
   if(!gamma_preview
-      && (pipe->mask_display == DT_DEV_PIXELPIPE_DISPLAY_NONE))
+      && (pipe->mask_display == DT_DEV_PIXELPIPE_DISPLAY_NONE)
+      && !pipe->nocache)
   {
     dt_dev_pixelpipe_cache_fullhash(pipe->image.id, roi_out, pipe, pos, &basichash, &hash);
     // dt_dev_pixelpipe_cache_available() tests for masking mode and returns FALSE in that case 
@@ -2381,7 +2381,17 @@ static gboolean _dev_pixelpipe_process_rec(
   // in case we get this buffer from the cache in the future, cache some stuff:
   **out_format = piece->dsc_out = pipe->dsc;
 
-  if(pipe->mask_display == DT_DEV_PIXELPIPE_DISPLAY_NONE)
+  const gboolean needs_histo = module
+     && darktable.develop->gui_attached
+     && module->expanded
+     && module->enabled
+     && (pipe->type & DT_DEV_PIXELPIPE_FULL)
+     && (module->request_histogram & DT_REQUEST_EXPANDED);
+
+  if(needs_histo)
+    dt_print_pipe(DT_DEBUG_PIPE, "internal histogram", pipe, module->so->op, NULL, NULL, "\n");
+
+  if((pipe->mask_display == DT_DEV_PIXELPIPE_DISPLAY_NONE) && !needs_histo)
   {
     if(module && module == darktable.develop->gui_module)
     {
@@ -2395,6 +2405,12 @@ static gboolean _dev_pixelpipe_process_rec(
     pipe->next_important_module = _check_module_next_important(pipe, module);
   }
 
+  if(needs_histo)
+  {
+    pipe->nocache = TRUE;
+    dt_dev_pixelpipe_cache_unweight(pipe, *output);
+  }
+
   // warn on NaN or infinity
 #ifndef _DEBUG
   if((darktable.unmuted & DT_DEBUG_NAN)
@@ -2646,6 +2662,7 @@ gboolean dt_dev_pixelpipe_process(
            const float scale)
 {
   pipe->processing = TRUE;
+  pipe->nocache = FALSE;
   pipe->opencl_enabled = dt_opencl_running();
   pipe->devid = (pipe->opencl_enabled) ? dt_opencl_lock_device(pipe->type)
                                        : -1; // try to get/lock opencl resource

src/develop/pixelpipe_hb.h

@@ -154,6 +154,8 @@ typedef struct dt_dev_pixelpipe_t
 
   // we have to keep track of the next processing module to use an iop cacheline with high priority
   gboolean next_important_module;
+  // avoid cached data for processed module
+  gboolean nocache;
 
   dt_imgid_t output_imgid;
   // working?

src/iop/colorzones.c

@@ -2558,9 +2558,9 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
   dt_iop_colorzones_gui_data_t *g = (dt_iop_colorzones_gui_data_t *)self->gui_data;
 
   if(pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
-    piece->request_histogram |= (DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
   else
-    piece->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram &= ~DT_REQUEST_ON;
 
 #if 0 // print new preset
   printf("p.channel = %d;\n", p->channel);
@@ -2684,7 +2684,7 @@ void init(dt_iop_module_t *module)
   module->default_enabled = FALSE;
   module->params_size = sizeof(dt_iop_colorzones_params_t);
   module->gui_data = NULL;
-  module->request_histogram |= (DT_REQUEST_ON);
+  module->request_histogram |= DT_REQUEST_ON;
 
   _reset_parameters(module->default_params, DT_IOP_COLORZONES_h, DT_IOP_COLORZONES_SPLINES_V2);
 }

src/iop/levels.c

@@ -470,22 +470,22 @@ void commit_params(dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pixelpipe_
   dt_iop_levels_params_t *p = (dt_iop_levels_params_t *)p1;
 
   if(pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
-    piece->request_histogram |= (DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
   else
-    piece->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram &= ~DT_REQUEST_ON;
 
-  piece->request_histogram |= (DT_REQUEST_ONLY_IN_GUI);
+  piece->request_histogram |= DT_REQUEST_ONLY_IN_GUI;
 
   piece->histogram_params.bins_count = 256;
 
   if(p->mode == LEVELS_MODE_AUTOMATIC)
   {
     d->mode = LEVELS_MODE_AUTOMATIC;
 
-    piece->request_histogram |= (DT_REQUEST_ON);
-    self->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
+    self->request_histogram &= ~DT_REQUEST_ON;
 
-    if(!self->dev->gui_attached) piece->request_histogram &= ~(DT_REQUEST_ONLY_IN_GUI);
+    if(!self->dev->gui_attached) piece->request_histogram &= ~DT_REQUEST_ONLY_IN_GUI;
 
     piece->histogram_params.bins_count = 16384;
 
@@ -511,7 +511,7 @@ void commit_params(dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pixelpipe_
   {
     d->mode = LEVELS_MODE_MANUAL;
 
-    self->request_histogram |= (DT_REQUEST_ON);
+    self->request_histogram |= DT_REQUEST_ON;
 
     d->levels[0] = p->levels[0];
     d->levels[1] = p->levels[1];
@@ -569,7 +569,7 @@ void init(dt_iop_module_t *module)
 {
   dt_iop_default_init(module);
 
-  module->request_histogram |= (DT_REQUEST_ON);
+  module->request_histogram |= DT_REQUEST_ON;
 
   dt_iop_levels_params_t *d = module->default_params;
 

src/iop/rgbcurve.c

@@ -1488,7 +1488,7 @@ void init(dt_iop_module_t *module)
 {
   dt_iop_default_init(module);
 
-  module->request_histogram |= (DT_REQUEST_ON);
+  module->request_histogram |= (DT_REQUEST_ON | DT_REQUEST_EXPANDED);
 
   dt_iop_rgbcurve_params_t *d = module->default_params;
 
@@ -1592,12 +1592,12 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
 
   if(pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
   {
-    piece->request_histogram |= (DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
     self->histogram_middle_grey = p->compensate_middle_grey;
   }
   else
   {
-    piece->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram &= ~DT_REQUEST_ON;
   }
 
   for(int ch = 0; ch < DT_IOP_RGBCURVE_MAX_CHANNELS; ch++)

src/iop/rgblevels.c

@@ -824,9 +824,11 @@ void commit_params(dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pixelpipe_
   dt_iop_rgblevels_params_t *p = (dt_iop_rgblevels_params_t *)p1;
 
   if(pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
-    piece->request_histogram |= (DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
   else
-    piece->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram &= ~DT_REQUEST_ON;
+
+  piece->request_histogram |= DT_REQUEST_EXPANDED;
 
   memcpy(&(d->params), p, sizeof(dt_iop_rgblevels_params_t));
 
@@ -889,7 +891,7 @@ void init(dt_iop_module_t *self)
 {
   dt_iop_default_init(self);
 
-  self->request_histogram |= (DT_REQUEST_ON);
+  self->request_histogram |= (DT_REQUEST_ON | DT_REQUEST_EXPANDED);
 
   dt_iop_rgblevels_params_t *d = self->default_params;
 

src/iop/tonecurve.c

@@ -646,9 +646,9 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
   dt_iop_tonecurve_params_t *p = (dt_iop_tonecurve_params_t *)p1;
 
   if(pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
-    piece->request_histogram |= (DT_REQUEST_ON);
+    piece->request_histogram |= DT_REQUEST_ON;
   else
-    piece->request_histogram &= ~(DT_REQUEST_ON);
+    piece->request_histogram &= ~DT_REQUEST_ON;
 
   for(int ch = 0; ch < ch_max; ch++)
   {
@@ -820,7 +820,7 @@ void init(dt_iop_module_t *module)
 {
   dt_iop_default_init(module);
 
-  module->request_histogram |= (DT_REQUEST_ON);
+  module->request_histogram |=  (DT_REQUEST_ON | DT_REQUEST_EXPANDED);
 
   dt_iop_tonecurve_params_t *d = module->default_params;