Mask indicator: handle that button uniformingly

Monkey business of adding/removing the button depending on states by ways of copy-pasting ugly code led to segfauts in some weird corner cases of bad history poping. Handle that button like any other, set it from a single function.
aurelienpierreeng · Jan 15, 2025 · c036757 · c036757
1 parent 03e9230
commit c036757
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 65 deletions.
diff --git a/src/develop/blend_gui.c b/src/develop/blend_gui.c
@@ -1256,16 +1256,7 @@ static gboolean _blendop_masks_mode_changed(GtkWidget *widget, dt_iop_module_t *
   uint32_t mask_mode = _mask_modes_combobox_to_flags(selected);
   dt_iop_gui_blend_data_t *data = module->blend_data;
   _blendop_masks_mode_callback(mask_mode, data);
-
-  // (un)set the mask indicator, but not for uniform blend
-  add_remove_mask_indicator(module, mask_mode != DEVELOP_MASK_DISABLED);
-
-  ++darktable.gui->reset;
-  if(mask_mode != DEVELOP_MASK_DISABLED)
-      gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(module->mask_indicator),
-                                   gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(data->showmask)));
-  --darktable.gui->reset;
-
+  dt_iop_add_remove_mask_indicator(module);
   return TRUE;
 }
 
@@ -2212,6 +2203,7 @@ void dt_iop_gui_update_blendif(dt_iop_module_t *module)
   }
 
   _blendop_blendif_update_tab(module, bd->tab);
+  dt_iop_add_remove_mask_indicator(module);
 
   --darktable.gui->reset;
 }
@@ -2403,6 +2395,8 @@ void dt_masks_iop_update(dt_iop_module_t *module)
     }
   }
 
+  dt_iop_add_remove_mask_indicator(module);
+
   --darktable.gui->reset;
 }
 
@@ -2718,8 +2712,7 @@ void dt_iop_gui_update_blending(dt_iop_module_t *module)
   dt_bauhaus_combobox_set(bd->mask_mode_combo, _mask_modes_flags_to_combobox(module->blend_params->mask_mode));
 
   // (un)set the mask indicator
-  add_remove_mask_indicator(module, (module->blend_params->mask_mode != DEVELOP_MASK_DISABLED) &&
-                            (module->blend_params->mask_mode != DEVELOP_MASK_ENABLED));
+  dt_iop_add_remove_mask_indicator(module);
 
   // initialization of blending modes
   if(bd->csp != bd->blend_modes_csp)
@@ -2883,9 +2876,6 @@ void dt_iop_gui_update_blending(dt_iop_module_t *module)
       module->request_mask_display = DT_DEV_PIXELPIPE_DISPLAY_NONE;
       dt_iop_set_cache_bypass(module, FALSE);
       gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(bd->showmask), FALSE);
-      // (re)set the header mask indicator too
-      if(module->mask_indicator)
-          gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(module->mask_indicator), FALSE);
       gtk_widget_hide(GTK_WIDGET(bd->showmask));
     }
     else
@@ -2900,9 +2890,6 @@ void dt_iop_gui_update_blending(dt_iop_module_t *module)
     module->request_mask_display = DT_DEV_PIXELPIPE_DISPLAY_NONE;
     dt_iop_set_cache_bypass(module, FALSE);
     gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(bd->showmask), FALSE);
-    // (re)set the header mask indicator too
-    if(module->mask_indicator)
-      gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(module->mask_indicator), FALSE);
     module->suppress_mask = 0;
     gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(bd->suppress), FALSE);
 
@@ -3000,11 +2987,6 @@ void dt_iop_gui_blending_lose_focus(dt_iop_module_t *module)
     module->suppress_mask = 0;
 
     // (re)set the header mask indicator too
-    ++darktable.gui->reset;
-    if(module->mask_indicator)
-      gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(module->mask_indicator), FALSE);
-    --darktable.gui->reset;
-
     if(bd->masks_support)
     {
       // unselect all tools

diff --git a/src/develop/imageop.c b/src/develop/imageop.c
@@ -945,11 +945,6 @@ static void _gui_off_callback(GtkToggleButton *togglebutton, gpointer user_data)
 
       dt_dev_add_history_item(module->dev, module, FALSE);
     }
-
-    const gboolean raster = module->blend_params->mask_mode & DEVELOP_MASK_RASTER;
-    // set mask indicator sensitive according to module activation and raster mask
-    if(module->mask_indicator)
-      gtk_widget_set_sensitive(GTK_WIDGET(module->mask_indicator), !raster && module->enabled);
   }
 
   char tooltip[512];
@@ -1010,6 +1005,7 @@ void dt_iop_gui_update_header(dt_iop_module_t *module)
   // set panel name to display correct multi-instance
   _iop_panel_label(module);
   dt_iop_gui_set_enable_button(module);
+  dt_iop_add_remove_mask_indicator(module);
 }
 
 void dt_iop_gui_set_enable_button_icon(GtkWidget *w, dt_iop_module_t *module)
@@ -2072,8 +2068,10 @@ static void _display_mask_indicator_callback(GtkToggleButton *bt, dt_iop_module_
   const gboolean is_active = gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(bt));
   const dt_iop_gui_blend_data_t *bd = (dt_iop_gui_blend_data_t *)module->blend_data;
 
-  module->request_mask_display &= ~DT_DEV_PIXELPIPE_DISPLAY_MASK;
-  module->request_mask_display |= (is_active ? DT_DEV_PIXELPIPE_DISPLAY_MASK : 0);
+  module->request_mask_display &= ~(DT_DEV_PIXELPIPE_DISPLAY_MASK | DT_DEV_PIXELPIPE_DISPLAY_CHANNEL | DT_DEV_PIXELPIPE_DISPLAY_ANY);
+
+  if(is_active)
+    module->request_mask_display |= DT_DEV_PIXELPIPE_DISPLAY_MASK;
 
   dt_iop_set_cache_bypass(module, module->request_mask_display != DT_DEV_PIXELPIPE_DISPLAY_NONE);
 
@@ -2131,47 +2129,28 @@ static gboolean _mask_indicator_tooltip(GtkWidget *treeview, gint x, gint y, gbo
   return res;
 }
 
-void add_remove_mask_indicator(dt_iop_module_t *module, gboolean add)
+void dt_iop_add_remove_mask_indicator(dt_iop_module_t *module)
 {
-  const gboolean raster = module->blend_params->mask_mode & DEVELOP_MASK_RASTER;
+  if(!module || !module->mask_indicator) return;
 
-  if(module->mask_indicator)
+  const gboolean support_blending = (module->flags() & IOP_FLAGS_SUPPORTS_BLENDING) == IOP_FLAGS_SUPPORTS_BLENDING;
+
+  if(!support_blending || !module->blend_params)
   {
-    if(!add)
-    {
-      gtk_widget_destroy(module->mask_indicator);
-      module->mask_indicator = NULL;
-    }
-    else
-      gtk_widget_set_sensitive(GTK_WIDGET(module->mask_indicator), !raster && module->enabled);
+    gtk_widget_set_visible(GTK_WIDGET(module->mask_indicator), FALSE);
+    gtk_widget_set_has_tooltip(GTK_WIDGET(module->mask_indicator), FALSE);
+    gtk_widget_set_sensitive(GTK_WIDGET(module->mask_indicator), FALSE);
   }
-  else if(add)
-  {
-    module->mask_indicator = dtgtk_togglebutton_new(dtgtk_cairo_paint_showmask, 0, NULL);
-    dt_gui_add_class(module->mask_indicator, "dt_transparent_background");
-    g_signal_connect(G_OBJECT(module->mask_indicator), "toggled",
-                     G_CALLBACK(_display_mask_indicator_callback), module);
-    g_signal_connect(G_OBJECT(module->mask_indicator), "query-tooltip",
-                     G_CALLBACK(_mask_indicator_tooltip), module);
-    gtk_widget_set_has_tooltip(module->mask_indicator, TRUE);
-    gtk_widget_set_sensitive(GTK_WIDGET(module->mask_indicator), !raster && module->enabled);
-    gtk_box_pack_end(GTK_BOX(module->header), module->mask_indicator, FALSE, FALSE, 0);
 
-    // in dynamic modes, we need to put the mask indicator after the drawing area
-    GList *children = gtk_container_get_children(GTK_CONTAINER(module->header));
-    GList *child;
+  // Raster masks can't be previewed
+  const gboolean raster = (module->blend_params->mask_mode & DEVELOP_MASK_RASTER) == DEVELOP_MASK_RASTER;
 
-    for(child = g_list_last(children); child && GTK_IS_BUTTON(child->data); child = g_list_previous(child));
+  // Note : DEVELOP_MASK_ENABLED means uniform blending (opacity), not masks
+  const gboolean use_masks = module->blend_params->mask_mode > DEVELOP_MASK_ENABLED;
 
-    if(GTK_IS_DRAWING_AREA(child->data))
-    {
-      GValue position = G_VALUE_INIT;
-      g_value_init (&position, G_TYPE_INT);
-      gtk_container_child_get_property(GTK_CONTAINER(module->header), child->data ,"position", &position);
-      gtk_box_reorder_child(GTK_BOX(module->header), module->mask_indicator, g_value_get_int(&position));
-    }
-    g_list_free(children);
-  }
+  gtk_widget_set_visible(GTK_WIDGET(module->mask_indicator), use_masks);
+  gtk_widget_set_sensitive(GTK_WIDGET(module->mask_indicator), !raster && module->enabled);
+  gtk_widget_set_has_tooltip(GTK_WIDGET(module->mask_indicator), use_masks);
 }
 
 gboolean _iop_tooltip_callback(GtkWidget *widget, gint x, gint y, gboolean keyboard_mode,
@@ -2291,6 +2270,15 @@ void dt_iop_gui_set_expander(dt_iop_module_t *module)
     g_signal_connect(lab, "query-tooltip", G_CALLBACK(_iop_tooltip_callback), module);
   }
 
+  /* add mask preview button */
+  hw[IOP_MODULE_MASK] = dtgtk_togglebutton_new(dtgtk_cairo_paint_showmask, 0, NULL);
+  dt_gui_add_class(hw[IOP_MODULE_MASK], "dt_transparent_background");
+  g_signal_connect(G_OBJECT(hw[IOP_MODULE_MASK]), "toggled",
+                    G_CALLBACK(_display_mask_indicator_callback), module);
+  g_signal_connect(G_OBJECT(hw[IOP_MODULE_MASK]), "query-tooltip",
+                    G_CALLBACK(_mask_indicator_tooltip), module);
+  module->mask_indicator = hw[IOP_MODULE_MASK];
+
   /* add multi instances menu button */
   hw[IOP_MODULE_INSTANCE] = dtgtk_button_new(dtgtk_cairo_paint_multiinstance, 0, NULL);
   module->multimenu_button = GTK_WIDGET(hw[IOP_MODULE_INSTANCE]);

diff --git a/src/develop/imageop.h b/src/develop/imageop.h
@@ -50,6 +50,7 @@ typedef enum dt_iop_module_header_icons_t
   IOP_MODULE_SWITCH = 0,
   IOP_MODULE_ICON,
   IOP_MODULE_LABEL,
+  IOP_MODULE_MASK,
   IOP_MODULE_INSTANCE,
   IOP_MODULE_RESET,
   IOP_MODULE_PRESETS,
@@ -457,7 +458,7 @@ void dt_iop_queue_history_update(dt_iop_module_t *module, gboolean extend_prior)
 void dt_iop_cancel_history_update(dt_iop_module_t *module);
 
 /** add/remove mask indicator to iop module header */
-void add_remove_mask_indicator(dt_iop_module_t *module, gboolean add);
+void dt_iop_add_remove_mask_indicator(dt_iop_module_t *module);
 
 // format modules description going in tooltips
 const char **dt_iop_set_description(dt_iop_module_t *module, const char *main_text,