wb: make sure color calibration / white balance warning is shown when WB disabled

temperature.c: keep the module handle published even when disabled (comment in develop.h already promised 'always available for GUI reports'). commit params nulled it when disabled -> early exit in channelmixerrgb.c _set_trouble_messages -> 'white balance missing' never shown.
channelmixerrgb.c: fix potential null pointer dereference if worker sets chr->temperature to null; minor rename/cleanup

Author: kofa73

src/common/illuminants.h

@@ -343,15 +343,15 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant,           /
 static inline void WB_coeffs_to_illuminant_xy(const float CAM_to_XYZ[4][3], const dt_aligned_pixel_t WB,
                                               float *x, float *y)
 {
-  // Find the illuminant chromaticity x y from RAW WB coeffs and camera input matrice
+  // Find the illuminant chromaticity x y from RAW WB coeffs and camera input matrix
   dt_aligned_pixel_t XYZ, LMS;
   // Simulate white point, aka convert (1, 1, 1) in camera space to XYZ
-  // warning : we multiply the transpose of CAM_to_XYZ  since the pseudoinverse transposes it
+  // warning: we multiply the transpose of CAM_to_XYZ since the pseudoinverse transposes it
   XYZ[0] = CAM_to_XYZ[0][0] / WB[0] + CAM_to_XYZ[1][0] / WB[1] + CAM_to_XYZ[2][0] / WB[2];
   XYZ[1] = CAM_to_XYZ[0][1] / WB[0] + CAM_to_XYZ[1][1] / WB[1] + CAM_to_XYZ[2][1] / WB[2];
   XYZ[2] = CAM_to_XYZ[0][2] / WB[0] + CAM_to_XYZ[1][2] / WB[1] + CAM_to_XYZ[2][2] / WB[2];
 
-  // Matrices white point is D65. We need to convert it for darktable's pipe (D50)
+  // Matrix's white point is D65. We need to convert it for darktable's pipe (D50)
   static const dt_aligned_pixel_t D65 = { 0.941238f, 1.040633f, 1.088932f, 0.f };
   const float p = powf(1.088932f / 0.818155f, 0.0834f);
 

src/iop/channelmixerrgb.c

@@ -2005,20 +2005,27 @@ static void _set_trouble_messages(dt_iop_module_t *self)
   const dt_develop_t *dev = self->dev;
   const dt_dev_chroma_t *chr = &dev->chroma;
 
+  // module handle snapshots: dt_dev_reset_chroma() may NULL them
+  // from the pipe worker thread. single aligned pointer loads can't tear;
+  // the pointed-to modules are only freed on this (main) thread,
+  // so the snapshots stay valid for this call
+  dt_iop_module_t *const temperature = chr->temperature;
+  dt_iop_module_t *const adaptation = chr->adaptation;
+
   dt_print(DT_DEBUG_PIPE | DT_DEBUG_VERBOSE, "trouble message for %s%s : temp=%p adapt=%p",
-    self->op, dt_iop_get_instance_id(self), chr->temperature, chr->adaptation);
+    self->op, dt_iop_get_instance_id(self), temperature, adaptation);
 
   // in temperature module we make sure this is only presented if temperature is enabled
-  if(!chr->temperature)
+  if(!temperature)
   {
-    if(chr->adaptation)
-      dt_iop_set_module_trouble_message(chr->adaptation, NULL, NULL, NULL);
+    if(adaptation)
+      dt_iop_set_module_trouble_message(adaptation, NULL, NULL, NULL);
     return;
   }
 
-  if(!chr->adaptation)
+  if(!adaptation)
   {
-    dt_iop_set_module_trouble_message(chr->temperature, NULL, NULL, NULL);
+    dt_iop_set_module_trouble_message(temperature, NULL, NULL, NULL);
     dt_iop_set_module_trouble_message(self, NULL, NULL, NULL);
     return;
   }
@@ -2028,46 +2035,46 @@ static void _set_trouble_messages(dt_iop_module_t *self)
     && !(p->illuminant == DT_ILLUMINANT_PIPE || p->adaptation == DT_ADAPTATION_RGB)
     && !dt_image_is_monochrome(&dev->image_storage);
 
-  const gboolean temperature_enabled = chr->temperature && chr->temperature->enabled;
-  const gboolean adaptation_enabled = chr->adaptation && chr->adaptation->enabled;
+  const gboolean temperature_enabled = temperature->enabled;
+  const gboolean adaptation_enabled = adaptation->enabled;
 
-  // our first and biggest problem : white balance module is being
-  // clever with WB coeffs
-  const gboolean problem1 = valid
-                            && chr->adaptation == self
+  // this instance does CAT while WB uses an incompatible setup (neither camera reference
+  // D65, nor another mode with late_correction)
+  const gboolean wb_applied_twice = valid
+                            && adaptation == self
                             && temperature_enabled
                             && !_dev_is_D65_chroma(dev);
 
-  // our second biggest problem : another channelmixerrgb instance is doing CAT
-  // earlier in the pipe and we don't use masking here.
-  const gboolean problem2 = valid
+  // another channelmixerrgb instance is doing CAT
+  // earlier in the pipe, and we don't use masking here
+  const gboolean double_cat = valid
                             && adaptation_enabled
                             && temperature_enabled
-                            && chr->adaptation != self
+                            && adaptation != self
                             && !g->is_blending;
 
-  // our third and minor problem: white balance module is not active but default_enabled
-  // and we do chromatic adaptation in color calibration
-  const gboolean problem3 = valid
+  // white balance is off on an image that needs it (default_enabled), but we rely on
+  // it to pre-process the data (neutralizing coeffs + late_correction, or D65 coeffs)
+  const gboolean wb_missing = valid
                             && adaptation_enabled
                             && !temperature_enabled
-                            && chr->temperature->default_enabled;
+                            && temperature->default_enabled;
 
-  const gboolean anyproblem = problem1 || problem2 || problem3;
+  const gboolean any_problem = wb_applied_twice || double_cat || wb_missing;
 
   const dt_image_t *img = &dev->image_storage;
-  dt_print_pipe(DT_DEBUG_PIPE, anyproblem ? "chroma trouble" : "chroma data",
+  dt_print_pipe(DT_DEBUG_PIPE, any_problem ? "chroma trouble" : "chroma data",
       NULL, self, DT_DEVICE_NONE, NULL, NULL,
       "%s%s%sD65=%s.  D65 %.3f %.3f %.3f, AS-SHOT %.3f %.3f %.3f ID=%i",
-      problem1 ? "white balance applied twice, " : "",
-      problem2 ? "double CAT applied, " : "",
-      problem3 ? "white balance missing, " : "",
+      wb_applied_twice ? "white balance applied twice, " : "",
+      double_cat ? "double CAT applied, " : "",
+      wb_missing ? "white balance missing, " : "",
       STR_YESNO(_dev_is_D65_chroma(dev)),
       chr->D65coeffs[0], chr->D65coeffs[1], chr->D65coeffs[2],
       chr->as_shot[0], chr->as_shot[1], chr->as_shot[2],
       img->id);
 
-  if(problem2)
+  if(double_cat)
   {
     dt_iop_set_module_trouble_message
       (self,
@@ -2080,10 +2087,10 @@ static void _set_trouble_messages(dt_iop_module_t *self)
     return;
   }
 
-  if(problem1)
+  if(wb_applied_twice)
   {
     dt_iop_set_module_trouble_message
-      (chr->temperature,
+      (temperature,
         _("white balance applied twice (<u>details</u>)"),
         _("the color calibration module is enabled and already provides\n"
           "chromatic adaptation.\n"
@@ -2102,10 +2109,10 @@ static void _set_trouble_messages(dt_iop_module_t *self)
     return;
   }
 
-  if(problem3)
+  if(wb_missing)
   {
     dt_iop_set_module_trouble_message
-      (chr->temperature,
+      (temperature,
         _("white balance missing (<u>details</u>)"),
         _("this module is not providing a valid reference illuminant\n"
           "causing chromatic adaptation issues in color calibration.\n"
@@ -2124,9 +2131,9 @@ static void _set_trouble_messages(dt_iop_module_t *self)
     return;
   }
 
-  if(chr->adaptation && chr->adaptation == self)
+  if(adaptation == self)
   {
-    dt_iop_set_module_trouble_message(chr->temperature, NULL, NULL, NULL);
+    dt_iop_set_module_trouble_message(temperature, NULL, NULL, NULL);
     dt_iop_set_module_trouble_message(self, NULL, NULL, NULL);
   }
 }

src/iop/temperature.c

@@ -734,6 +734,8 @@ void commit_params(dt_iop_module_t *self,
   {
     for_four_channels(k)
       chr->wb_coeffs[k] = 1.0f;
+    // keep the module handle available for GUI reports (see below)
+    chr->temperature = self;
     return;
   }
 
@@ -760,13 +762,15 @@ void commit_params(dt_iop_module_t *self,
   // image to D65 via D65coeffs/1.0.
   chr->late_correction = piece->enabled ? effective_late_correction : FALSE;
 
-  /* Make sure the chroma information stuff is valid
-     If piece is disabled we always clear the trouble message and
-     make sure chroma does know there is no temperature module.
+  /* Always publish the module handle for GUI reports, regardless of the
+     enabled state. The enabled state is tracked separately via
+     chr->temperature->enabled (module flag) and pipe->dsc.temperature.enabled
+     (pipe flag). This lets color calibration warn about a disabled white
+     balance module while it is still doing chromatic adaptation. Trouble
+     message state is owned entirely by _set_trouble_messages() in
+     channelmixerrgb, so we no longer clear it here.
   */
-  chr->temperature = piece->enabled ? self : NULL;
-  if(dt_pipe_is_preview(pipe) && !piece->enabled)
-    dt_iop_set_module_trouble_message(self, NULL, NULL, NULL);
+  chr->temperature = self;
 }
 
 void init_pipe(dt_iop_module_t *self,