Refactor dt_iop_module_is_skipped()

1. Use only 'piece' as parameter 2. Include/deduplicate pipe->type check 3. Don't use iop_order to test for skipping 4. Some debugging log
darktable-org · Jan 28, 2025 · 46cd9cc · 46cd9cc
1 parent 1757ac5
commit 46cd9cc
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 23 deletions.
diff --git a/src/develop/develop.c b/src/develop/develop.c
@@ -2592,6 +2592,7 @@ static gboolean _dev_distort_backtransform_locked(dt_develop_t *dev,
     dt_dev_pixelpipe_iop_t *piece = pieces->data;
     if(piece->enabled
        && piece->data
+       && !dt_iop_piece_is_skipped(piece)
        && ((transf_direction == DT_DEV_TRANSFORM_DIR_ALL)
            || (transf_direction == DT_DEV_TRANSFORM_DIR_FORW_INCL
                && module->iop_order >= iop_order)
@@ -2600,9 +2601,7 @@ static gboolean _dev_distort_backtransform_locked(dt_develop_t *dev,
            || (transf_direction == DT_DEV_TRANSFORM_DIR_BACK_INCL
                && module->iop_order <= iop_order)
            || (transf_direction == DT_DEV_TRANSFORM_DIR_BACK_EXCL
-               && module->iop_order < iop_order))
-       && !(dt_iop_module_is_skipped(dev, module)
-            && (pipe->type & DT_DEV_PIXELPIPE_BASIC)))
+               && module->iop_order < iop_order)))
     {
       module->distort_backtransform(module, piece, points, points_count);
     }
@@ -2632,6 +2631,7 @@ static gboolean _dev_distort_transform_locked(dt_develop_t *dev,
     dt_dev_pixelpipe_iop_t *piece = pieces->data;
     if(piece->enabled
        && piece->data
+       && !dt_iop_piece_is_skipped(piece)
        && ((transf_direction == DT_DEV_TRANSFORM_DIR_ALL)
            || (transf_direction == DT_DEV_TRANSFORM_DIR_FORW_INCL
                && module->iop_order >= iop_order)
@@ -2640,9 +2640,7 @@ static gboolean _dev_distort_transform_locked(dt_develop_t *dev,
            || (transf_direction == DT_DEV_TRANSFORM_DIR_BACK_INCL
                && module->iop_order <= iop_order)
            || (transf_direction == DT_DEV_TRANSFORM_DIR_BACK_EXCL
-               && module->iop_order < iop_order))
-       && !(dt_iop_module_is_skipped(dev, module)
-            && (pipe->type & DT_DEV_PIXELPIPE_BASIC)))
+               && module->iop_order < iop_order)))
     {
       module->distort_transform(module, piece, points, points_count);
     }

diff --git a/src/develop/imageop.c b/src/develop/imageop.c
@@ -3873,13 +3873,22 @@ void dt_iop_gui_changed(dt_action_t *action, GtkWidget *widget, gpointer data)
   dt_dev_add_history_item_target(darktable.develop, module, TRUE, widget);
 }
 
-gboolean dt_iop_module_is_skipped(const dt_develop_t *dev,
-                                  const dt_iop_module_t *module)
-{
-  return dev->gui_module
-      && dev->gui_module != module
-      && (dev->gui_module->operation_tags_filter() & module->operation_tags())
-      && (dev->gui_module->iop_order < module->iop_order);
+gboolean dt_iop_piece_is_skipped(const dt_dev_pixelpipe_iop_t *piece)
+{
+  const dt_iop_module_t *gui_module = dt_dev_gui_module();
+  const gboolean skipped = gui_module
+                        && gui_module != piece->module
+                        && (gui_module->operation_tags_filter() & piece->module->operation_tags())
+                        && (piece->pipe->type & DT_DEV_PIXELPIPE_BASIC);
+
+  if(skipped || (piece->module->iop_order == INT_MAX))
+    dt_print_pipe(DT_DEBUG_PIPE | DT_DEBUG_IOPORDER,
+      "skip test", piece->pipe, piece->module, piece->pipe->devid, &piece->processed_roi_in, &piece->processed_roi_out,
+      "gui module '%s': %s%s",
+      gui_module ? gui_module->op : "???",
+      skipped ? "skipped " : "",
+      piece->module->iop_order == INT_MAX ? "illegal order" : "");
+  return skipped;
 }
 
 enum

diff --git a/src/develop/imageop.h b/src/develop/imageop.h
@@ -566,7 +566,7 @@ void dt_iop_gui_rename_module(dt_iop_module_t *module);
 
 void dt_iop_gui_changed(dt_action_t *action, GtkWidget *widget, gpointer data);
 
-gboolean dt_iop_module_is_skipped(const struct dt_develop_t *dev, const dt_iop_module_t *module);
+gboolean dt_iop_piece_is_skipped(const struct dt_dev_pixelpipe_iop_t *piece);
 
 // copy the RGB channels of a pixel using nontemporal stores if
 // possible; includes the 'alpha' channel as well if faster due to

diff --git a/src/develop/pixelpipe_cache.c b/src/develop/pixelpipe_cache.c
@@ -130,10 +130,7 @@ static dt_hash_t _dev_pixelpipe_cache_basichash(const dt_imgid_t imgid,
     if(module->iop_order > order) break;
 
     // 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)
+    if(!dt_iop_piece_is_skipped(piece))
     {
       hash = dt_hash(hash, &piece->hash, sizeof(piece->hash));
       if(module->request_color_pick != DT_REQUEST_COLORPICK_OFF)

diff --git a/src/develop/pixelpipe_hb.c b/src/develop/pixelpipe_hb.c
@@ -1433,9 +1433,8 @@ static inline gboolean _skip_piece_on_tags(const dt_dev_pixelpipe_iop_t *piece)
   if(!piece->enabled || piece->module->iop_order == INT_MAX)
     return TRUE;
 
-  return dt_iop_module_is_skipped(piece->module->dev, piece->module)
-          && (piece->pipe->type & DT_DEV_PIXELPIPE_BASIC);
-}
+  return dt_iop_piece_is_skipped(piece);
+ }
 
 // recursive helper for process, returns TRUE in case of unfinished work or error
 static gboolean _dev_pixelpipe_process_rec(dt_dev_pixelpipe_t *pipe,

diff --git a/src/iop/ashift.c b/src/iop/ashift.c
@@ -4072,8 +4072,7 @@ static int call_distort_transform(dt_develop_t *dev,
   if(!piece) return ret;
   if(piece->module == self
       // && *piece->enabled see note below
-      && !(dt_iop_module_is_skipped(dev, piece->module)
-            && (pipe->type & DT_DEV_PIXELPIPE_BASIC)))
+      && !dt_iop_piece_is_skipped(piece))
   {
     ret = piece->module->distort_transform(piece->module, piece, points, points_count);
   }