Merge pull request #18362 from jenshannoschwalm/reverting_pipe_stuff_501

Reverting pipe stuff 501

src/common/image.c

@@ -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)

src/develop/develop.c

@@ -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
@@ -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;
@@ -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

src/develop/pixelpipe_cache.c

@@ -1,6 +1,6 @@
 /*
     This file is part of darktable,
-    Copyright (C) 2009-2025 darktable developers.
+    Copyright (C) 2009-2024 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
@@ -104,39 +104,37 @@ 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
   */
   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)
         {
@@ -156,9 +154,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));
@@ -191,7 +189,7 @@ gboolean dt_dev_pixelpipe_cache_available(dt_dev_pixelpipe_t *pipe,
 // While looking for the oldest cacheline we always ignore the first two lines as they are used
 // for swapping buffers while in entries==DT_PIPECACHE_MIN or masking mode
 static int _get_oldest_cacheline(dt_dev_pixelpipe_cache_t *cache,
-                                 const dt_dev_pixelpipe_cache_test_t mode)
+                                 dt_dev_pixelpipe_cache_test_t mode)
 {
   // we never want the latest used cacheline! It was <= 0 and the weight has increased just now
   int age = 1;
@@ -241,7 +239,7 @@ static int _get_cacheline(dt_dev_pixelpipe_t *pipe)
 
 // return TRUE in case of a hit
 static gboolean _get_by_hash(dt_dev_pixelpipe_t *pipe,
-                             const dt_iop_module_t *module,
+                             dt_iop_module_t *module,
                              const dt_hash_t hash,
                              const size_t size,
                              void **data,

src/develop/pixelpipe_cache.h

@@ -1,6 +1,6 @@
 /*
     This file is part of darktable,
-    Copyright (C) 2009-2024 darktable developers.
+    Copyright (C) 2009-2022 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
@@ -65,9 +65,9 @@ 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 current viewport. */
 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.

src/develop/pixelpipe_hb.c

@@ -521,7 +521,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");
       }
@@ -633,34 +633,42 @@ 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",
+  dt_print_pipe(DT_DEBUG_PIPE, "dev_pixelpipe_change",
+      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);
+    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);
+    }
   }
   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,
@@ -1413,7 +1421,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
@@ -2256,7 +2264,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
@@ -2859,7 +2867,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)
@@ -2906,6 +2914,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;
@@ -2922,8 +2932,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
     {
@@ -3054,7 +3063,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

src/iop/ashift.c

@@ -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;

src/iop/clipping.c

@@ -908,7 +908,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;

src/iop/toneequal.c

@@ -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);
 

src/libs/history.c

@@ -1305,8 +1305,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);