From b08b928cf78f5615affbbe880a6a8f2f4586bc8a Mon Sep 17 00:00:00 2001 From: Kofa Date: Wed, 10 Dec 2025 19:25:38 +0100 Subject: [PATCH 01/18] wb: added new param late_correction to temperature.c, exposed as a checkbox; populate dev->chroma->late_correction based on checkbox-driven late_correction flag for 'legacy' modes (DT_IOP_TEMP_AS_SHOT, DT_IOP_TEMP_SPOT, DT_IOP_TEMP_USER) --- src/iop/temperature.c | 97 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 92 insertions(+), 5 deletions(-) diff --git a/src/iop/temperature.c b/src/iop/temperature.c index eee5d23ab7d8..42ead6f214ae 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -43,7 +43,7 @@ #include "common/colorspaces.h" #include "external/cie_colorimetric_tables.c" -DT_MODULE_INTROSPECTION(4, dt_iop_temperature_params_t) +DT_MODULE_INTROSPECTION(5, dt_iop_temperature_params_t) #define INITIALBLACKBODYTEMPERATURE 4000 @@ -72,6 +72,7 @@ typedef struct dt_iop_temperature_params_t float blue; // $MIN: 0.0 $MAX: 8.0 float various; // $MIN: 0.0 $MAX: 8.0 int preset; + gboolean late_correction; // $DEFAULT: FALSE $DESCRIPTION: "prepare data for color calibration" } dt_iop_temperature_params_t; typedef struct dt_iop_temperature_gui_data_t @@ -87,6 +88,7 @@ typedef struct dt_iop_temperature_gui_data_t GtkWidget *btn_d65_late; GtkWidget *temp_label; GtkWidget *balance_label; + GtkWidget *check_late_correction; int preset_cnt; int preset_num[54]; double mod_coeff[4]; @@ -142,6 +144,16 @@ int legacy_params(dt_iop_module_t *self, int preset; } dt_iop_temperature_params_v4_t; + typedef struct dt_iop_temperature_params_v5_t + { + float red; + float green; + float blue; + float various; + int preset; + gboolean late_correction; + } dt_iop_temperature_params_v5_t; + if(old_version == 2) { typedef struct dt_iop_temperature_params_v2_t @@ -179,6 +191,22 @@ int legacy_params(dt_iop_module_t *self, *new_version = 4; return 0; } + if(old_version == 4) + { + const dt_iop_temperature_params_v4_t *o = (dt_iop_temperature_params_v4_t *)old_params; + dt_iop_temperature_params_v5_t *n = malloc(sizeof(dt_iop_temperature_params_v5_t)); + + n->red = o->red; + n->green = o->green; + n->blue = o->blue; + n->various = NAN; + n->preset = DT_IOP_TEMP_UNKNOWN; + n->late_correction = FALSE; + *new_params = n; + *new_params_size = sizeof(dt_iop_temperature_params_v5_t); + *new_version = 5; + return 0; + } return 1; } @@ -721,7 +749,13 @@ void commit_params(dt_iop_module_t *self, If piece is disabled we always clear the trouble message and make sure chroma does know there is no temperature module. */ - chr->late_correction = p->preset == DT_IOP_TEMP_D65_LATE; + if(p->preset == DT_IOP_TEMP_D65_LATE) + chr->late_correction = TRUE; + else if(p->preset == DT_IOP_TEMP_D65) + chr->late_correction = FALSE; + else + chr->late_correction = p->late_correction; + chr->temperature = piece->enabled ? self : NULL; if(dt_pipe_is_preview(pipe) && !piece->enabled) dt_iop_set_module_trouble_message(self, NULL, NULL, NULL); @@ -1163,9 +1197,44 @@ static void _update_preset(dt_iop_module_t *self, int mode) { dt_iop_temperature_params_t *p = self->params; dt_dev_chroma_t *chr = &self->dev->chroma; + dt_iop_temperature_gui_data_t *g = self->gui_data; + + const gboolean is_current_reference = (p->preset == DT_IOP_TEMP_D65_LATE) || + (p->preset == DT_IOP_TEMP_D65); + const gboolean is_new_mode_manual = (mode != DT_IOP_TEMP_D65_LATE) && + (mode != DT_IOP_TEMP_D65); + + if (is_current_reference && is_new_mode_manual) + { + // set iff color calibration active in adaptation mode + p->late_correction = (chr->adaptation != NULL); + } p->preset = mode; - chr->late_correction = mode == DT_IOP_TEMP_D65_LATE; + + if (mode == DT_IOP_TEMP_D65_LATE) + { + chr->late_correction = TRUE; + } + else if (mode == DT_IOP_TEMP_D65) + { + chr->late_correction = FALSE; + } + else + { + // For manual modes, use the parameter + chr->late_correction = p->late_correction; + } + + if (g && g->check_late_correction) + { + // Hide checkbox in reference modes (logic is enforced hardcoded) + // Show in manual modes (user has control) + gtk_widget_set_visible(g->check_late_correction, is_new_mode_manual); + + gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->check_late_correction), + p->late_correction); + } } void gui_update(dt_iop_module_t *self) @@ -1516,6 +1585,7 @@ void reload_defaults(dt_iop_module_t *self) dt_iop_temperature_params_t *p = self->params; d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; + d->late_correction = dt_is_scene_referred(); float *dcoeffs = (float *)d; for_four_channels(k) @@ -1741,8 +1811,10 @@ void gui_changed(dt_iop_module_t *self, GtkWidget *w, void *previous) _mul2temp(self, p, &g->mod_temp, &g->mod_tint); - dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_USER); - _update_preset(self, DT_IOP_TEMP_USER); + if(w != g->check_late_correction) { + dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_USER); + _update_preset(self, DT_IOP_TEMP_USER); + } } static gboolean _btn_toggled(GtkWidget *togglebutton, @@ -2164,12 +2236,27 @@ void gui_init(dt_iop_module_t *self) (g->scale_tint, _("color tint of the image, from magenta (value < 1) to green (value > 1)")); + g->check_late_correction = dt_bauhaus_toggle_from_params(self, "late_correction"); + + g_object_ref(g->check_late_correction); // Prevent destruction + GtkWidget *temp_parent = gtk_widget_get_parent(g->check_late_correction); + if(temp_parent) + gtk_container_remove(GTK_CONTAINER(temp_parent), g->check_late_correction); + + gtk_widget_set_tooltip_text(g->check_late_correction, + _("ensures the white balance coefficients are treated as a reference for the color calibration module.\n" + "keep this checked if you use the modern scene-referred workflow but want to manually adjust white balance.")); + GtkWidget *box_enabled = dt_gui_vbox(g->buttonbar, + g->check_late_correction, g->presets, g->finetune, temp_label_box, g->scale_k, g->scale_tint); + + g_object_unref(g->check_late_correction); + dt_gui_new_collapsible_section (&g->cs, "plugins/darkroom/temperature/expand_coefficients", From f7b48131c3086e805374054acc25c4af60915c30 Mon Sep 17 00:00:00 2001 From: Kofa Date: Tue, 16 Dec 2025 12:25:27 +0100 Subject: [PATCH 02/18] wb: Updated the rest of the files, but channelmixerrgb is broken, everything is purple. --- src/iop/channelmixerrgb.c | 44 +++++++++++++++++++++++++------- src/iop/colorin.c | 26 +++++++++++++------ src/iop/highlights.c | 12 ++++----- src/iop/hlreconstruct/opposed.c | 24 +++++++++++------ src/iop/hlreconstruct/segbased.c | 12 ++++++--- 5 files changed, 83 insertions(+), 35 deletions(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 87e8c003fa05..c7f780784edf 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -593,9 +593,12 @@ void init_presets(dt_iop_module_so_t *self) static gboolean _dev_is_D65_chroma(const dt_develop_t *dev) { const dt_dev_chroma_t *chr = &dev->chroma; - return chr->late_correction - ? dt_dev_equal_chroma(chr->wb_coeffs, chr->as_shot) - : dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); + + // if late correction is active, colorin guarantees the pipe is D65-aligned + // regardless of the specific user coefficients. + if (chr->late_correction) return TRUE; + + return dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); } static gboolean _area_mapping_active(const dt_iop_channelmixer_rgb_gui_data_t *g) @@ -2209,7 +2212,7 @@ void process(dt_iop_module_t *self, // re-run the detection at runtime… float x, y; dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) { @@ -2320,7 +2323,7 @@ int process_cl(dt_iop_module_t *self, // re-run the detection at runtime… float x, y; dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) { @@ -3111,7 +3114,10 @@ void commit_params(dt_iop_module_t *self, float x = p->x; float y = p->y; dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + if(p->illuminant == DT_ILLUMINANT_CAMERA) + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + else + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); @@ -3556,7 +3562,10 @@ static gboolean _illuminant_color_draw(GtkWidget *widget, float y = p->y; dt_aligned_pixel_t RGB = { 0 }; dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + if(p->illuminant == DT_ILLUMINANT_CAMERA) + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + else + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); illuminant_xy_to_RGB(x, y, RGB); @@ -3659,7 +3668,10 @@ static void _update_approx_cct(const dt_iop_module_t *self) float x = p->x; float y = p->y; dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + if(p->illuminant == DT_ILLUMINANT_CAMERA) + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + else + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); @@ -3896,7 +3908,21 @@ void reload_defaults(dt_iop_module_t *self) d->adaptation = DT_ADAPTATION_CAT16; dt_aligned_pixel_t custom_wb; - if(!_get_white_balance_coeff(self, custom_wb)) + gboolean wb_retrieved = FALSE; + + // In modern mode, we want the actual coefficients (User/AsShot) to determine the CCT. + // In legacy mode, we rely on the helper which calculates a D65 ratio. + if (self->dev->chroma.late_correction && dt_image_is_matrix_correction_supported(img)) + { + copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + wb_retrieved = TRUE; + } + else + { + wb_retrieved = !_get_white_balance_coeff(self, custom_wb); + } + + if(wb_retrieved) { if(find_temperature_from_raw_coeffs(img, custom_wb, &(d->x), &(d->y))) d->illuminant = DT_ILLUMINANT_CAMERA; diff --git a/src/iop/colorin.c b/src/iop/colorin.c index b1214f67910c..302af4a0afe2 100644 --- a/src/iop/colorin.c +++ b/src/iop/colorin.c @@ -650,10 +650,15 @@ int process_cl(dt_iop_module_t *self, const dt_dev_chroma_t *chr = &self->dev->chroma; const gboolean corrected = chr->late_correction; - dt_aligned_pixel_t coeffs = { corrected ? chr->D65coeffs[0] / chr->as_shot[0] : 1.0f, - corrected ? chr->D65coeffs[1] / chr->as_shot[1] : 1.0f, - corrected ? chr->D65coeffs[2] / chr->as_shot[2] : 1.0f, - corrected ? chr->D65coeffs[3] / chr->as_shot[3] : 1.0f }; + dt_aligned_pixel_t coeffs; + for_four_channels(k) + { + if(corrected && chr->wb_coeffs[k] > 1e-6f) + coeffs[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + coeffs[k] = 1.0f; + } + if(corrected) { for_four_channels(k) @@ -1196,10 +1201,15 @@ void process(dt_iop_module_t *self, const dt_dev_chroma_t *chr = &self->dev->chroma; const dt_iop_colorin_data_t *const d = piece->data; const gboolean corrected = chr->late_correction && d->type != DT_COLORSPACE_LAB; - const dt_aligned_pixel_t coeffs = { corrected ? chr->D65coeffs[0] / chr->as_shot[0] : 1.0f, - corrected ? chr->D65coeffs[1] / chr->as_shot[1] : 1.0f, - corrected ? chr->D65coeffs[2] / chr->as_shot[2] : 1.0f, - corrected ? chr->D65coeffs[3] / chr->as_shot[3] : 1.0f }; + dt_aligned_pixel_t coeffs; + for_four_channels(k) + { + if(corrected && chr->wb_coeffs[k] > 1e-6f) + coeffs[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + coeffs[k] = 1.0f; + } + dt_dev_pixelpipe_t *pipe = piece->pipe; if(corrected) { diff --git a/src/iop/highlights.c b/src/iop/highlights.c index 01ab23ad8bf0..aabd112fc8ad 100644 --- a/src/iop/highlights.c +++ b/src/iop/highlights.c @@ -652,9 +652,9 @@ int process_cl(dt_iop_module_t *self, dt_aligned_pixel_t clips = { clipper, clipper, clipper, clipper}; if(pipe->dsc.temperature.enabled && chr->late_correction) { - clips[0] *= chr->as_shot[0] / chr->D65coeffs[0]; - clips[1] *= chr->as_shot[1] / chr->D65coeffs[1]; - clips[2] *= chr->as_shot[2] / chr->D65coeffs[2]; + clips[0] *= chr->wb_coeffs[0] / chr->D65coeffs[0]; + clips[1] *= chr->wb_coeffs[1] / chr->D65coeffs[1]; + clips[2] *= chr->wb_coeffs[2] / chr->D65coeffs[2]; } dev_xtrans = dt_opencl_copy_host_to_device_constant(devid, sizeof(piece->xtrans), piece->xtrans); @@ -731,9 +731,9 @@ static void process_clip(dt_iop_module_t *self, dt_aligned_pixel_t clips = { clip, clip, clip, clip}; if(piece->pipe->dsc.temperature.enabled && chr->late_correction) { - clips[0] *= chr->as_shot[0] / chr->D65coeffs[0]; - clips[1] *= chr->as_shot[1] / chr->D65coeffs[1]; - clips[2] *= chr->as_shot[2] / chr->D65coeffs[2]; + clips[0] *= chr->wb_coeffs[0] / chr->D65coeffs[0]; + clips[1] *= chr->wb_coeffs[1] / chr->D65coeffs[1]; + clips[2] *= chr->wb_coeffs[2] / chr->D65coeffs[2]; } for(int row = 0; row < roi_out->height; row++) { diff --git a/src/iop/hlreconstruct/opposed.c b/src/iop/hlreconstruct/opposed.c index 224f3246da66..fb198a830db8 100644 --- a/src/iop/hlreconstruct/opposed.c +++ b/src/iop/hlreconstruct/opposed.c @@ -236,10 +236,14 @@ static float *_process_opposed(dt_iop_module_t *self, const dt_dev_chroma_t *chr = &self->dev->chroma; const gboolean late = chr->late_correction; - const dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && chr->wb_coeffs[k] > 1e-6f) + correction[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + correction[k] = 1.0f; + } const size_t mwidth = roi_in->width / 3; const size_t mheight = roi_in->height / 3; @@ -434,10 +438,14 @@ static cl_int process_opposed_cl(dt_iop_module_t *self, const dt_dev_chroma_t *chr = &self->dev->chroma; const gboolean late = chr->late_correction; - dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && chr->wb_coeffs[k] > 1e-6f) + correction[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + correction[k] = 1.0f; + } cl_int err = CL_MEM_OBJECT_ALLOCATION_FAILURE; cl_mem dev_chrominance = NULL; diff --git a/src/iop/hlreconstruct/segbased.c b/src/iop/hlreconstruct/segbased.c index 62545f631cb7..20d33e117d3b 100644 --- a/src/iop/hlreconstruct/segbased.c +++ b/src/iop/hlreconstruct/segbased.c @@ -466,10 +466,14 @@ static void _process_segmentation(dt_dev_pixelpipe_iop_t *piece, const dt_dev_chroma_t *chr = &piece->module->dev->chroma; const gboolean late = chr->late_correction; - const dt_aligned_pixel_t correction = { late ? (float)(chr->D65coeffs[0] / chr->as_shot[0]) : 1.0f, - late ? (float)(chr->D65coeffs[1] / chr->as_shot[1]) : 1.0f, - late ? (float)(chr->D65coeffs[2] / chr->as_shot[2]) : 1.0f, - 1.0f }; + dt_aligned_pixel_t correction; + for_four_channels(k) + { + if(late && chr->wb_coeffs[k] > 1e-6f) + correction[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + correction[k] = 1.0f; + } const int recovery_mode = d->recovery; const float strength = d->strength; From f2786ae64fbfd2fc7bd9166fd75c2d943ab07386 Mon Sep 17 00:00:00 2001 From: Kofa Date: Tue, 16 Dec 2025 13:50:20 +0100 Subject: [PATCH 03/18] wb: Undid changes to channelmixerrgb.c, will go carefully next time --- src/iop/channelmixerrgb.c | 44 ++++++++------------------------------- 1 file changed, 9 insertions(+), 35 deletions(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index c7f780784edf..87e8c003fa05 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -593,12 +593,9 @@ void init_presets(dt_iop_module_so_t *self) static gboolean _dev_is_D65_chroma(const dt_develop_t *dev) { const dt_dev_chroma_t *chr = &dev->chroma; - - // if late correction is active, colorin guarantees the pipe is D65-aligned - // regardless of the specific user coefficients. - if (chr->late_correction) return TRUE; - - return dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); + return chr->late_correction + ? dt_dev_equal_chroma(chr->wb_coeffs, chr->as_shot) + : dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); } static gboolean _area_mapping_active(const dt_iop_channelmixer_rgb_gui_data_t *g) @@ -2212,7 +2209,7 @@ void process(dt_iop_module_t *self, // re-run the detection at runtime… float x, y; dt_aligned_pixel_t custom_wb; - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + _get_white_balance_coeff(self, custom_wb); if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) { @@ -2323,7 +2320,7 @@ int process_cl(dt_iop_module_t *self, // re-run the detection at runtime… float x, y; dt_aligned_pixel_t custom_wb; - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); + _get_white_balance_coeff(self, custom_wb); if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) { @@ -3114,10 +3111,7 @@ void commit_params(dt_iop_module_t *self, float x = p->x; float y = p->y; dt_aligned_pixel_t custom_wb; - if(p->illuminant == DT_ILLUMINANT_CAMERA) - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); - else - _get_white_balance_coeff(self, custom_wb); + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); @@ -3562,10 +3556,7 @@ static gboolean _illuminant_color_draw(GtkWidget *widget, float y = p->y; dt_aligned_pixel_t RGB = { 0 }; dt_aligned_pixel_t custom_wb; - if(p->illuminant == DT_ILLUMINANT_CAMERA) - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); - else - _get_white_balance_coeff(self, custom_wb); + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); illuminant_xy_to_RGB(x, y, RGB); @@ -3668,10 +3659,7 @@ static void _update_approx_cct(const dt_iop_module_t *self) float x = p->x; float y = p->y; dt_aligned_pixel_t custom_wb; - if(p->illuminant == DT_ILLUMINANT_CAMERA) - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); - else - _get_white_balance_coeff(self, custom_wb); + _get_white_balance_coeff(self, custom_wb); illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); @@ -3908,21 +3896,7 @@ void reload_defaults(dt_iop_module_t *self) d->adaptation = DT_ADAPTATION_CAT16; dt_aligned_pixel_t custom_wb; - gboolean wb_retrieved = FALSE; - - // In modern mode, we want the actual coefficients (User/AsShot) to determine the CCT. - // In legacy mode, we rely on the helper which calculates a D65 ratio. - if (self->dev->chroma.late_correction && dt_image_is_matrix_correction_supported(img)) - { - copy_pixel(custom_wb, self->dev->chroma.wb_coeffs); - wb_retrieved = TRUE; - } - else - { - wb_retrieved = !_get_white_balance_coeff(self, custom_wb); - } - - if(wb_retrieved) + if(!_get_white_balance_coeff(self, custom_wb)) { if(find_temperature_from_raw_coeffs(img, custom_wb, &(d->x), &(d->y))) d->illuminant = DT_ILLUMINANT_CAMERA; From 25dcb6f90af6cf0c7f41a240543fadae2a76c121 Mon Sep 17 00:00:00 2001 From: Kofa Date: Wed, 17 Dec 2025 10:50:12 +0100 Subject: [PATCH 04/18] wb: Fixed publishing of late_correction in temperature.c --- src/iop/temperature.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/iop/temperature.c b/src/iop/temperature.c index 42ead6f214ae..10ba42b6f4b6 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -104,6 +104,7 @@ typedef struct dt_iop_temperature_data_t { float coeffs[4]; int preset; + gboolean late_correction; } dt_iop_temperature_data_t; typedef struct dt_iop_temperature_global_data_t @@ -554,7 +555,7 @@ static inline void _publish_chroma(dt_dev_pixelpipe_iop_t *piece) d->coeffs[k] * piece->pipe->dsc.processed_maximum[k]; chr->wb_coeffs[k] = d->coeffs[k]; } - chr->late_correction = (d->preset == DT_IOP_TEMP_D65_LATE); + chr->late_correction = d->late_correction; } void process(dt_iop_module_t *self, @@ -745,6 +746,20 @@ void commit_params(dt_iop_module_t *self, d->preset = p->preset; + gboolean effective_late_correction = FALSE; + + if (p->preset == DT_IOP_TEMP_D65_LATE) + effective_late_correction = TRUE; + else if (p->preset == DT_IOP_TEMP_D65) + effective_late_correction = FALSE; + else + effective_late_correction = p->late_correction; + + d->late_correction = effective_late_correction; + + chr->late_correction = effective_late_correction; + + chr->temperature = piece->enabled ? self : NULL; /* 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. From 1e7cc362f8bfd160debf355f8ec2b49558e302c9 Mon Sep 17 00:00:00 2001 From: Kofa Date: Wed, 17 Dec 2025 10:53:06 +0100 Subject: [PATCH 05/18] wb: added DT_ILLUMINANT_FROM_WB, separated find_temperature_from_raw_coeffs into find_temperature_from_wb_coeffs, find_temperature_from_as_shot_coeffs; added handling to channelmixerrgb.c + altered logic for 'late correction' check --- src/common/illuminants.h | 108 ++++++++++------ src/develop/develop.h | 6 +- src/iop/channelmixerrgb.c | 252 +++++++++++++++++++++++++++++--------- src/iop/temperature.c | 25 ++-- 4 files changed, 273 insertions(+), 118 deletions(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index 5f9530677c86..32f593d66a54 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -34,6 +34,7 @@ typedef enum dt_illuminant_t DT_ILLUMINANT_BB = 6, // $DESCRIPTION: "Planckian (black body)" general black body radiator - not CIE standard DT_ILLUMINANT_CUSTOM = 7, // $DESCRIPTION: "custom" input x and y directly - bypass search DT_ILLUMINANT_CAMERA = 10,// $DESCRIPTION: "as shot in camera" read RAW EXIF for WB + DT_ILLUMINANT_FROM_WB = 11,// $DESCRIPTION: "as set in white balance module" read coefficients from the white balance module DT_ILLUMINANT_LAST, DT_ILLUMINANT_DETECT_SURFACES = 8, DT_ILLUMINANT_DETECT_EDGES = 9, @@ -215,19 +216,22 @@ static inline void illuminant_CCT_to_RGB(const float t, dt_aligned_pixel_t RGB) illuminant_xy_to_RGB(x, y, RGB); } +static inline gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, + float *chroma_x, float *chroma_y); // Fetch image from pipeline and read EXIF for camera RAW WB coeffs -static inline gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt_aligned_pixel_t custom_wb, +static inline gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, float *chroma_x, float *chroma_y); -static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary type of illuminant - const dt_image_t *img, // image container - const dt_aligned_pixel_t custom_wb, // optional user-set WB coeffs - float *x_out, float *y_out, // chromaticity output - const float t, // temperature in K, if needed - const dt_illuminant_fluo_t fluo, // sub-type of fluorescent illuminant, if needed - const dt_illuminant_led_t iled) // sub-type of led illuminant, if needed +static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary type of illuminant + const dt_image_t *img, // image container + const dt_aligned_pixel_t correction_ratios, // optional D65 correction ratios derived from user-set coefficients + const dt_aligned_pixel_t wb_coeffs, // optional user-set WB coeffs (absolute) + float *x_out, float *y_out, // chromaticity output + const float t, // temperature in K, if needed + const dt_illuminant_fluo_t fluo, // sub-type of fluorescent illuminant, if needed + const dt_illuminant_led_t iled) // sub-type of led illuminant, if needed { /** * Compute the x and y chromaticity coordinates in Yxy spaces for standard illuminants @@ -298,9 +302,15 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, // primary } case DT_ILLUMINANT_CAMERA: { - // Detect WB from RAW EXIF + // Detect WB from RAW EXIF, correcting with D65/wb_coeff ratios if(img) - if(find_temperature_from_raw_coeffs(img, custom_wb, &x, &y)) break; + if(find_temperature_from_as_shot_coeffs(img, correction_ratios, &x, &y)) break; + } + case DT_ILLUMINANT_FROM_WB: + { + // Detect WB from user-provided coefficients + if(img) + if(find_temperature_from_wb_coeffs(img, wb_coeffs, &x, &y)) break; } case DT_ILLUMINANT_CUSTOM: // leave x and y as-is case DT_ILLUMINANT_DETECT_EDGES: @@ -385,31 +395,11 @@ static inline void matrice_pseudoinverse(float (*in)[3], float (*out)[3], int si } } - -static gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt_aligned_pixel_t custom_wb, - float *chroma_x, float *chroma_y) +// returns TRUE is OK, FALSE if failed +static gboolean get_CAM_to_XYZ(const dt_image_t * img, float(* CAM_to_XYZ)[3]) { - if(img == NULL) return FALSE; - if(!dt_image_is_matrix_correction_supported(img)) return FALSE; - - gboolean has_valid_coeffs = TRUE; - const int num_coeffs = (img->flags & DT_IMAGE_4BAYER) ? 4 : 3; - - // Check coeffs - for(int k = 0; has_valid_coeffs && k < num_coeffs; k++) - if(!dt_isnormal(img->wb_coeffs[k]) || img->wb_coeffs[k] == 0.0f) has_valid_coeffs = FALSE; - - if(!has_valid_coeffs) return FALSE; - - // Get white balance camera factors - dt_aligned_pixel_t WB = { img->wb_coeffs[0], img->wb_coeffs[1], img->wb_coeffs[2], img->wb_coeffs[3] }; - - // Adapt the camera coeffs with custom white balance if provided - // this can deal with WB coeffs that don't use the input matrix reference - if(custom_wb) - for(size_t k = 0; k < 4; k++) WB[k] *= custom_wb[k]; - - // Get the camera input profile (matrice of primaries) + if(img == NULL || CAM_to_XYZ == NULL) return FALSE; + // Get the camera input profile (matrice of primaries) - embedded or raw library DB float XYZ_to_CAM[4][3]; dt_mark_colormatrix_invalid(&XYZ_to_CAM[0][0]); @@ -438,21 +428,61 @@ static gboolean find_temperature_from_raw_coeffs(const dt_image_t *img, const dt if(!dt_is_valid_colormatrix(XYZ_to_CAM[0][0])) return FALSE; - // Bloody input matrices define XYZ -> CAM transform, as if we often needed camera profiles to output - // So we need to invert them. Here go your CPU cycles again. - float CAM_to_XYZ[4][3]; dt_mark_colormatrix_invalid(&CAM_to_XYZ[0][0]); matrice_pseudoinverse(XYZ_to_CAM, CAM_to_XYZ, 3); - if(!dt_is_valid_colormatrix(CAM_to_XYZ[0][0])) return FALSE; + return dt_is_valid_colormatrix(CAM_to_XYZ[0][0]); +} + +// returns FALSE if failed; TRUE if successful +static gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, + float *chroma_x, float *chroma_y) +{ + if(img == NULL || wb_coeffs == NULL) return FALSE; + if(!dt_image_is_matrix_correction_supported(img)) return FALSE; + + float CAM_to_XYZ[4][3]; + if(!get_CAM_to_XYZ(img, CAM_to_XYZ)) { + return FALSE; + } float x, y; - WB_coeffs_to_illuminant_xy(CAM_to_XYZ, WB, &x, &y); + WB_coeffs_to_illuminant_xy(CAM_to_XYZ, wb_coeffs, &x, &y); *chroma_x = x; *chroma_y = y; return TRUE; } +// returns FALSE if failed; TRUE if successful +static gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, + float *chroma_x, float *chroma_y) +{ + if(img == NULL) return FALSE; + + gboolean has_valid_coeffs = TRUE; + const int num_coeffs = (img->flags & DT_IMAGE_4BAYER) ? 4 : 3; + + // Check coeffs + for(int k = 0; has_valid_coeffs && k < num_coeffs; k++) + if(!dt_isnormal(img->wb_coeffs[k]) || img->wb_coeffs[k] == 0.0f) has_valid_coeffs = FALSE; + + if(!has_valid_coeffs) return FALSE; + + // Get as-shot white balance camera factors (from raw) + // component wise raw-RGB * wb_coeffs should provide R=G=B for a neutral patch under + // the scene illuminant + dt_aligned_pixel_t WB = { img->wb_coeffs[0], img->wb_coeffs[1], img->wb_coeffs[2], img->wb_coeffs[3] }; + + // Adapt the camera coeffs with custom D65 coefficients if provided ('caveats' workaround) + // this can deal with WB coeffs that don't use the input matrix reference + // adaptation_ratios[k] = chr->D65coeffs[k] / chr->wb_coeffs[k] + if(correction_ratios) + for(size_t k = 0; k < 4; k++) WB[k] *= correction_ratios[k]; + // for a neutral surface, raw RGB * img->wb_coeffs would produce neutral R=G=B + + return find_temperature_from_wb_coeffs(img, WB, chroma_x, chroma_y); +} + DT_OMP_DECLARE_SIMD() static inline float planckian_normal(const float x, const float t) diff --git a/src/develop/develop.h b/src/develop/develop.h index 99f5d610f634..731fbd0f58c0 100644 --- a/src/develop/develop.h +++ b/src/develop/develop.h @@ -147,9 +147,9 @@ typedef struct dt_dev_viewport_t - the currently used wb_coeffs in temperature module - D65coeffs and as_shot are read from exif data f) - late_correction set by temperature if we want to process data as following - If we use the new DT_IOP_TEMP_D65_LATE mode in temperature.c and don#t have - any temp parameters changes later we can calc correction coeffs to modify - as_shot rgb data to D65 + If we use the new DT_IOP_TEMP_D65_LATE mode or enable this in user modes of temperature.c + and don't have any temp parameters changes later we can calc correction ratios + to take white-balanced (neutralized) data to D65 */ typedef struct dt_dev_chroma_t { diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 87e8c003fa05..be702c09d412 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -422,7 +422,7 @@ void init_presets(dt_iop_module_so_t *self) p.illum_fluo = DT_ILLUMINANT_FLUO_F3; p.illum_led = DT_ILLUMINANT_LED_B5; p.temperature = 5003.f; - illuminant_to_xy(DT_ILLUMINANT_PIPE, NULL, NULL, &p.x, &p.y, p.temperature, + illuminant_to_xy(DT_ILLUMINANT_PIPE, NULL, NULL, NULL, &p.x, &p.y, p.temperature, DT_ILLUMINANT_FLUO_LAST, DT_ILLUMINANT_LED_LAST); p.red[0] = 1.f; @@ -593,9 +593,7 @@ void init_presets(dt_iop_module_so_t *self) static gboolean _dev_is_D65_chroma(const dt_develop_t *dev) { const dt_dev_chroma_t *chr = &dev->chroma; - return chr->late_correction - ? dt_dev_equal_chroma(chr->wb_coeffs, chr->as_shot) - : dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); + return chr->late_correction || dt_dev_equal_chroma(chr->wb_coeffs, chr->D65coeffs); } static gboolean _area_mapping_active(const dt_iop_channelmixer_rgb_gui_data_t *g) @@ -611,14 +609,14 @@ static const char *_area_mapping_section_text(const dt_iop_channelmixer_rgb_gui_ return _area_mapping_active(g) ? _("area color mapping (active)") : _("area color mapping"); } -static gboolean _get_white_balance_coeff(const dt_iop_module_t *self, - dt_aligned_pixel_t custom_wb) +static gboolean _get_d65_correction_ratios(const dt_iop_module_t *self, + dt_aligned_pixel_t out_correction_ratios) { const dt_dev_chroma_t *chr = &self->dev->chroma; // Init output with a no-op for_four_channels(k) - custom_wb[k] = 1.0f; + out_correction_ratios[k] = 1.0f; if(!dt_image_is_matrix_correction_supported(&self->dev->image_storage)) return TRUE; @@ -638,7 +636,7 @@ static gboolean _get_white_balance_coeff(const dt_iop_module_t *self, if(valid_chroma && changed_chroma) { for_four_channels(k) - custom_wb[k] = (float)chr->D65coeffs[k] / chr->wb_coeffs[k]; + out_correction_ratios[k] = (float)chr->D65coeffs[k] / chr->wb_coeffs[k]; } return FALSE; } @@ -1246,7 +1244,7 @@ static void _check_if_close_to_daylight(const float x, float uv_test[2]; // Compute the test chromaticity from the daylight model - illuminant_to_xy(DT_ILLUMINANT_D, NULL, NULL, &xy_test[0], &xy_test[1], t, + illuminant_to_xy(DT_ILLUMINANT_D, NULL, NULL, NULL, &xy_test[0], &xy_test[1], t, DT_ILLUMINANT_FLUO_LAST, DT_ILLUMINANT_LED_LAST); xy_to_uv(xy_test, uv_test); @@ -1255,7 +1253,7 @@ static void _check_if_close_to_daylight(const float x, const float delta_daylight = dt_fast_hypotf(uv_test[0] - uv_ref[0], uv_test[1] - uv_ref[1]); // Compute the test chromaticity from the blackbody model - illuminant_to_xy(DT_ILLUMINANT_BB, NULL, NULL, &xy_test[0], &xy_test[1], t, + illuminant_to_xy(DT_ILLUMINANT_BB, NULL, NULL, NULL, &xy_test[0], &xy_test[1], t, DT_ILLUMINANT_FLUO_LAST, DT_ILLUMINANT_LED_LAST); xy_to_uv(xy_test, uv_test); @@ -2208,10 +2206,25 @@ void process(dt_iop_module_t *self, // change here, so we can't update the defaults. So we need to // re-run the detection at runtime… float x, y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); - if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) + if(find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + { + // Convert illuminant from xyY to XYZ + dt_aligned_pixel_t XYZ; + illuminant_xy_to_XYZ(x, y, XYZ); + + // Convert illuminant from XYZ to Bradford modified LMS + convert_any_XYZ_to_LMS(XYZ, data->illuminant, data->adaptation); + data->illuminant[3] = 0.f; + } + } + else if(data->illuminant_type == DT_ILLUMINANT_FROM_WB) + { + float x, y; + + if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2319,10 +2332,33 @@ int process_cl(dt_iop_module_t *self, // change here, so we can't update the defaults. So we need to // re-run the detection at runtime… float x, y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); - if(find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, &(x), &(y))) + if(find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + { + // Convert illuminant from xyY to XYZ + dt_aligned_pixel_t XYZ; + illuminant_xy_to_XYZ(x, y, XYZ); + + // Convert illuminant from XYZ to Bradford modified LMS + convert_any_XYZ_to_LMS(XYZ, d->illuminant, d->adaptation); + d->illuminant[3] = 0.f; + } + } + else if(d->illuminant_type == DT_ILLUMINANT_FROM_WB) + { + // The camera illuminant is a behaviour rather than a preset of + // values: it uses whatever is in the RAW EXIF. But it depends on + // what temperature.c is doing and needs to be updated + // accordingly, to give a consistent result. We initialise the + // CAT defaults using the temperature coeffs at startup, but if + // temperature is changed later, we get no notification of the + // change here, so we can't update the defaults. So we need to + // re-run the detection at runtime… + float x, y; + + if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -3040,6 +3076,38 @@ static void _preview_pipe_finished_callback(gpointer instance, dt_iop_module_t * dt_iop_gui_enter_critical_section(self); gtk_label_set_markup(GTK_LABEL(g->label_delta_E), g->delta_E_label_text); + + dt_iop_channelmixer_rgb_params_t *p = self->params; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + // The WB coefficients might have changed in the temperature module. + // We need to re-calculate the chromaticity (x, y) to update the GUI feedback. + float x = p->x; + float y = p->y; + + if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) + { + p->x = x; + p->y = y; + + darktable.gui->reset++; + // Update derived GUI widgets (CCT label, color patch) + _update_approx_cct(self); + _update_illuminant_color(self); + + // Update Hue/Chroma sliders to match the new coordinates + dt_aligned_pixel_t xyY = { p->x, p->y, 1.f }; + dt_aligned_pixel_t Lch; + dt_xyY_to_Lch(xyY, Lch); + + if(Lch[1] > 0) + dt_bauhaus_slider_set(g->illum_x, rad2degf(Lch[2])); + dt_bauhaus_slider_set(g->illum_y, Lch[1]); + + darktable.gui->reset--; + } + } + dt_iop_gui_leave_critical_section(self); _set_trouble_messages(self); } @@ -3110,16 +3178,22 @@ void commit_params(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + dt_aligned_pixel_t wb_coeffs = {0.f}; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + for(int k = 0; k < 4; k++) + wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; + } + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + wb_coeffs, &x, &y, p->temperature, p->illum_fluo, + p->illum_led); - // if illuminant is set as camera, x and y are set on-the-fly at + // if illuminant is from camera or user WB coefficients, x and y are set on-the-fly at // commit time, so we need to set adaptation too - if(p->illuminant == DT_ILLUMINANT_CAMERA) - _check_if_close_to_daylight(x, y, NULL, NULL, &(d->adaptation)); - + if(p->illuminant == DT_ILLUMINANT_CAMERA || p->illuminant == DT_ILLUMINANT_FROM_WB) + _check_if_close_to_daylight(x, y, NULL, NULL, &(d->adaptation)); d->illuminant_type = p->illuminant; // Convert illuminant from xyY to XYZ @@ -3262,6 +3336,7 @@ static void _update_illuminants(const dt_iop_module_t *self) break; } case DT_ILLUMINANT_CAMERA: + case DT_ILLUMINANT_FROM_WB: { gtk_widget_set_visible(g->adaptation, TRUE); gtk_widget_set_visible(g->temperature, FALSE); @@ -3554,11 +3629,18 @@ static gboolean _illuminant_color_draw(GtkWidget *widget, // camera RAW is chosen float x = p->x; float y = p->y; - dt_aligned_pixel_t RGB = { 0 }; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), custom_wb, - &x, &y, p->temperature, p->illum_fluo, p->illum_led); + dt_aligned_pixel_t RGB = { 0.f }; + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + dt_aligned_pixel_t wb_coeffs = { 0.f }; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + for(int k = 0; k < 4; k++) + wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; + } + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + wb_coeffs, &x, &y, p->temperature, p->illum_fluo, + p->illum_led); illuminant_xy_to_RGB(x, y, RGB); cairo_set_source_rgb(cr, RGB[0], RGB[1], RGB[2]); cairo_rectangle(cr, INNER_PADDING, margin, width, height); @@ -3658,10 +3740,17 @@ static void _update_approx_cct(const dt_iop_module_t *self) float x = p->x; float y = p->y; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + dt_aligned_pixel_t wb_coeffs = { 0.f }; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + for(int k = 0; k < 4; k++) + wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; + } + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + wb_coeffs, &x, &y, p->temperature, p->illum_fluo, + p->illum_led); dt_illuminant_t test_illuminant; float t = 5000.f; @@ -3859,6 +3948,24 @@ void init(dt_iop_module_t *self) d->red[0] = d->green[1] = d->blue[2] = 1.0; } +static void _add_or_remove_illuminant(const dt_iop_module_t *self, + const int illuminant, + const gboolean use_illuminant) +{ + // we only call this if g is not NULL + dt_iop_channelmixer_rgb_gui_data_t *g = self->gui_data; + const int pos = dt_bauhaus_combobox_get_from_value(g->illuminant, illuminant); + if(use_illuminant) + { + if(pos == -1) + dt_bauhaus_combobox_add_introspection(g->illuminant, NULL, + self->so->get_f("illuminant")->Enum.values, + illuminant, illuminant); + } + else if (pos != -1) + dt_bauhaus_combobox_remove_at(g->illuminant, pos); +} + void reload_defaults(dt_iop_module_t *self) { dt_iop_channelmixer_rgb_params_t *d = self->default_params; @@ -3895,10 +4002,10 @@ void reload_defaults(dt_iop_module_t *self) { d->adaptation = DT_ADAPTATION_CAT16; - dt_aligned_pixel_t custom_wb; - if(!_get_white_balance_coeff(self, custom_wb)) + dt_aligned_pixel_t correction_ratios; + if(!_get_d65_correction_ratios(self, correction_ratios)) { - if(find_temperature_from_raw_coeffs(img, custom_wb, &(d->x), &(d->y))) + if(find_temperature_from_as_shot_coeffs(img, correction_ratios, &(d->x), &(d->y))) d->illuminant = DT_ILLUMINANT_CAMERA; _check_if_close_to_daylight(d->x, d->y, &(d->temperature), &(d->illuminant), &(d->adaptation)); @@ -3926,16 +4033,9 @@ void reload_defaults(dt_iop_module_t *self) g->delta_E_label_text = NULL; } - const int pos = dt_bauhaus_combobox_get_from_value(g->illuminant, DT_ILLUMINANT_CAMERA); - if(dt_image_is_matrix_correction_supported(img) && !dt_image_is_monochrome(img)) - { - if(pos == -1) - dt_bauhaus_combobox_add_introspection(g->illuminant, NULL, - self->so->get_f("illuminant")->Enum.values, - DT_ILLUMINANT_CAMERA, DT_ILLUMINANT_CAMERA); - } - else - dt_bauhaus_combobox_remove_at(g->illuminant, pos); + const gboolean has_color_matrix = dt_image_is_matrix_correction_supported(img) && !dt_image_is_monochrome(img); + _add_or_remove_illuminant(self, DT_ILLUMINANT_CAMERA, has_color_matrix); + _add_or_remove_illuminant(self, DT_ILLUMINANT_FROM_WB, has_color_matrix); gui_changed(self, NULL, NULL); } @@ -3999,9 +4099,22 @@ void gui_changed(dt_iop_module_t *self, // illuminant = "as set in camera", temperature and // chromaticity are inited with the preset content when // illuminant is changed. - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - find_temperature_from_raw_coeffs(&(self->dev->image_storage), custom_wb, + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, + &(p->x), &(p->y)); + _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); + } + if(*prev_illuminant == DT_ILLUMINANT_FROM_WB) + { + // If illuminant was previously set with "as set in camera", + // when changing it, we need to ensure the temperature and + // chromaticity are inited with the correct values taken from + // camera EXIF. Otherwise, if using a preset defining + // illuminant = "as set in camera", temperature and + // chromaticity are inited with the preset content when + // illuminant is changed. + find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); } @@ -4017,14 +4130,23 @@ void gui_changed(dt_iop_module_t *self, { // Get camera WB and update illuminant dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - const gboolean found = find_temperature_from_raw_coeffs(&(self->dev->image_storage), + _get_d65_correction_ratios(self, custom_wb); + const gboolean found = find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), custom_wb, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); if(found) dt_control_log(_("white balance successfully extracted from raw image")); } + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + const gboolean found = find_temperature_from_wb_coeffs(&(self->dev->image_storage), + self->dev->chroma.wb_coeffs, &(p->x), &(p->y)); + _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); + + if(found) + dt_control_log(_("white balance successfully extracted from white balance module")); + } #ifdef AI_ACTIVATED else if(p->illuminant == DT_ILLUMINANT_DETECT_EDGES || p->illuminant == DT_ILLUMINANT_DETECT_SURFACES) @@ -4052,17 +4174,19 @@ void gui_changed(dt_iop_module_t *self, // illuminant to allow swapping modes if(p->illuminant != DT_ILLUMINANT_CUSTOM - && p->illuminant != DT_ILLUMINANT_CAMERA) + && p->illuminant != DT_ILLUMINANT_CAMERA + && p->illuminant != DT_ILLUMINANT_FROM_WB) { // We are in any mode defining (x, y) indirectly from an // interface, so commit (x, y) explicitly - illuminant_to_xy(p->illuminant, NULL, NULL, &(p->x), &(p->y), + illuminant_to_xy(p->illuminant, NULL, NULL, NULL, &(p->x), &(p->y), p->temperature, p->illum_fluo, p->illum_led); } if(p->illuminant != DT_ILLUMINANT_D && p->illuminant != DT_ILLUMINANT_BB - && p->illuminant != DT_ILLUMINANT_CAMERA) + && p->illuminant != DT_ILLUMINANT_CAMERA + && p->illuminant != DT_ILLUMINANT_FROM_WB) { // We are in any mode not defining explicitly a temperature, so // find the the closest CCT and commit it @@ -4146,7 +4270,7 @@ void gui_changed(dt_iop_module_t *self, // If "as shot in camera" illuminant is used, CAT space is forced automatically // therefore, make the control insensitive - gtk_widget_set_sensitive(g->adaptation, p->illuminant != DT_ILLUMINANT_CAMERA); + gtk_widget_set_sensitive(g->adaptation, p->illuminant != DT_ILLUMINANT_CAMERA && p->illuminant != DT_ILLUMINANT_FROM_WB); _declare_cat_on_pipe(self, FALSE); @@ -4223,15 +4347,23 @@ static void _auto_set_illuminant(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + dt_aligned_pixel_t wb_coeffs = { 0.f }; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + for(int k = 0; k < 4; k++) + wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; + } + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + wb_coeffs, &x, &y, p->temperature, p->illum_fluo, + p->illum_led); + dt_adaptation_t adaptation = p->adaptation; - dt_aligned_pixel_t custom_wb; - _get_white_balance_coeff(self, custom_wb); - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), - custom_wb, &x, &y, p->temperature, p->illum_fluo, p->illum_led); // if illuminant is set as camera, x and y are set on-the-fly at // commit time, so we need to set adaptation too - if(p->illuminant == DT_ILLUMINANT_CAMERA) + if(p->illuminant == DT_ILLUMINANT_CAMERA || p->illuminant == DT_ILLUMINANT_FROM_WB) _check_if_close_to_daylight(x, y, NULL, NULL, &adaptation); // Convert illuminant from xyY to XYZ diff --git a/src/iop/temperature.c b/src/iop/temperature.c index 10ba42b6f4b6..35145a1146cd 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -197,11 +197,7 @@ int legacy_params(dt_iop_module_t *self, const dt_iop_temperature_params_v4_t *o = (dt_iop_temperature_params_v4_t *)old_params; dt_iop_temperature_params_v5_t *n = malloc(sizeof(dt_iop_temperature_params_v5_t)); - n->red = o->red; - n->green = o->green; - n->blue = o->blue; - n->various = NAN; - n->preset = DT_IOP_TEMP_UNKNOWN; + memcpy(n, o, sizeof(dt_iop_temperature_params_v4_t)); n->late_correction = FALSE; *new_params = n; *new_params_size = sizeof(dt_iop_temperature_params_v5_t); @@ -748,15 +744,14 @@ void commit_params(dt_iop_module_t *self, gboolean effective_late_correction = FALSE; - if (p->preset == DT_IOP_TEMP_D65_LATE) + if(p->preset == DT_IOP_TEMP_D65_LATE) effective_late_correction = TRUE; - else if (p->preset == DT_IOP_TEMP_D65) + else if(p->preset == DT_IOP_TEMP_D65) effective_late_correction = FALSE; else effective_late_correction = p->late_correction; d->late_correction = effective_late_correction; - chr->late_correction = effective_late_correction; chr->temperature = piece->enabled ? self : NULL; @@ -1219,7 +1214,7 @@ static void _update_preset(dt_iop_module_t *self, int mode) const gboolean is_new_mode_manual = (mode != DT_IOP_TEMP_D65_LATE) && (mode != DT_IOP_TEMP_D65); - if (is_current_reference && is_new_mode_manual) + if(is_current_reference && is_new_mode_manual) { // set iff color calibration active in adaptation mode p->late_correction = (chr->adaptation != NULL); @@ -1227,11 +1222,11 @@ static void _update_preset(dt_iop_module_t *self, int mode) p->preset = mode; - if (mode == DT_IOP_TEMP_D65_LATE) + if(mode == DT_IOP_TEMP_D65_LATE) { chr->late_correction = TRUE; } - else if (mode == DT_IOP_TEMP_D65) + else if(mode == DT_IOP_TEMP_D65) { chr->late_correction = FALSE; } @@ -1241,10 +1236,9 @@ static void _update_preset(dt_iop_module_t *self, int mode) chr->late_correction = p->late_correction; } - if (g && g->check_late_correction) + if(g && g->check_late_correction) { - // Hide checkbox in reference modes (logic is enforced hardcoded) - // Show in manual modes (user has control) + // show the checkbox only in modes where user can adjust multipliers gtk_widget_set_visible(g->check_late_correction, is_new_mode_manual); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->check_late_correction), @@ -2252,8 +2246,7 @@ void gui_init(dt_iop_module_t *self) _("color tint of the image, from magenta (value < 1) to green (value > 1)")); g->check_late_correction = dt_bauhaus_toggle_from_params(self, "late_correction"); - - g_object_ref(g->check_late_correction); // Prevent destruction + g_object_ref(g->check_late_correction); // prevent destruction GtkWidget *temp_parent = gtk_widget_get_parent(g->check_late_correction); if(temp_parent) gtk_container_remove(GTK_CONTAINER(temp_parent), g->check_late_correction); From c25ec3a6696d5a552136436b191f787ea523a8b2 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sat, 28 Feb 2026 14:23:02 +0100 Subject: [PATCH 06/18] wb: fix switch-case fall-through in illuminants.h#illuminant_to_xy; validate wb_coeffs for find_temperature_from_wb_coeffs as well as in find_temperature_from_as_shot_coeffs; fix copy-pasted comment in channelmixerrgb.c#gui_changed --- src/common/illuminants.h | 36 +++++++++++++++++++++++------------- src/iop/channelmixerrgb.c | 17 +++++++---------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index 32f593d66a54..8300b2c7110a 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -303,14 +303,20 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, / case DT_ILLUMINANT_CAMERA: { // Detect WB from RAW EXIF, correcting with D65/wb_coeff ratios - if(img) - if(find_temperature_from_as_shot_coeffs(img, correction_ratios, &x, &y)) break; + if(img && find_temperature_from_as_shot_coeffs(img, correction_ratios, &x, &y)) + break; + + // xy calculation failed + return FALSE; } case DT_ILLUMINANT_FROM_WB: { // Detect WB from user-provided coefficients - if(img) - if(find_temperature_from_wb_coeffs(img, wb_coeffs, &x, &y)) break; + if(img && find_temperature_from_wb_coeffs(img, wb_coeffs, &x, &y)) + break; + + // xy calculation failed + return FALSE; } case DT_ILLUMINANT_CUSTOM: // leave x and y as-is case DT_ILLUMINANT_DETECT_EDGES: @@ -395,7 +401,7 @@ static inline void matrice_pseudoinverse(float (*in)[3], float (*out)[3], int si } } -// returns TRUE is OK, FALSE if failed +// returns TRUE if OK, FALSE if failed static gboolean get_CAM_to_XYZ(const dt_image_t * img, float(* CAM_to_XYZ)[3]) { if(img == NULL || CAM_to_XYZ == NULL) return FALSE; @@ -433,12 +439,22 @@ static gboolean get_CAM_to_XYZ(const dt_image_t * img, float(* CAM_to_XYZ)[3]) return dt_is_valid_colormatrix(CAM_to_XYZ[0][0]); } +static inline gboolean _wb_coeffs_invalid(const dt_aligned_pixel_t wb_coeffs, const int num_coeffs) +{ + for(int k = 0; k < num_coeffs; k++) + if(!dt_isnormal(wb_coeffs[k]) || wb_coeffs[k] == 0.0f) return TRUE; + + return FALSE; +} + // returns FALSE if failed; TRUE if successful static gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, float *chroma_x, float *chroma_y) { if(img == NULL || wb_coeffs == NULL) return FALSE; if(!dt_image_is_matrix_correction_supported(img)) return FALSE; + // it's enough to check the first 3 here, as WB_coeffs_to_illuminant_xy only uses indices 0 to 2 + if(_wb_coeffs_invalid(wb_coeffs, 3)) return FALSE; float CAM_to_XYZ[4][3]; if(!get_CAM_to_XYZ(img, CAM_to_XYZ)) { @@ -458,15 +474,9 @@ static gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, cons float *chroma_x, float *chroma_y) { if(img == NULL) return FALSE; - - gboolean has_valid_coeffs = TRUE; const int num_coeffs = (img->flags & DT_IMAGE_4BAYER) ? 4 : 3; - // Check coeffs - for(int k = 0; has_valid_coeffs && k < num_coeffs; k++) - if(!dt_isnormal(img->wb_coeffs[k]) || img->wb_coeffs[k] == 0.0f) has_valid_coeffs = FALSE; - - if(!has_valid_coeffs) return FALSE; + if(_wb_coeffs_invalid(img->wb_coeffs, num_coeffs)) return FALSE; // Get as-shot white balance camera factors (from raw) // component wise raw-RGB * wb_coeffs should provide R=G=B for a neutral patch under @@ -475,7 +485,7 @@ static gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, cons // Adapt the camera coeffs with custom D65 coefficients if provided ('caveats' workaround) // this can deal with WB coeffs that don't use the input matrix reference - // adaptation_ratios[k] = chr->D65coeffs[k] / chr->wb_coeffs[k] + // correction_ratios[k] = chr->D65coeffs[k] / chr->wb_coeffs[k] if(correction_ratios) for(size_t k = 0; k < 4; k++) WB[k] *= correction_ratios[k]; // for a neutral surface, raw RGB * img->wb_coeffs would produce neutral R=G=B diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index be702c09d412..ab7fda00efcc 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -4107,13 +4107,10 @@ void gui_changed(dt_iop_module_t *self, } if(*prev_illuminant == DT_ILLUMINANT_FROM_WB) { - // If illuminant was previously set with "as set in camera", - // when changing it, we need to ensure the temperature and - // chromaticity are inited with the correct values taken from - // camera EXIF. Otherwise, if using a preset defining - // illuminant = "as set in camera", temperature and - // chromaticity are inited with the preset content when - // illuminant is changed. + // If illuminant was previously "as set in white balance module", + // (x, y) were computed at runtime from the WB module's coefficients + // and p->x, p->y may hold stale values. Recompute them now so + // the new illuminant mode starts from the correct chromaticity. find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); @@ -4129,10 +4126,10 @@ void gui_changed(dt_iop_module_t *self, if(p->illuminant == DT_ILLUMINANT_CAMERA) { // Get camera WB and update illuminant - dt_aligned_pixel_t custom_wb; - _get_d65_correction_ratios(self, custom_wb); + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); const gboolean found = find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), - custom_wb, &(p->x), &(p->y)); + correction_ratios, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); if(found) From 19e361cf50b806f9dc1778ce0115bf80c384ece9 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sat, 28 Feb 2026 14:48:17 +0100 Subject: [PATCH 07/18] wb: safe division in _get_d65_correction_ratios --- src/iop/channelmixerrgb.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index ab7fda00efcc..3d56385b3df6 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -636,7 +636,12 @@ static gboolean _get_d65_correction_ratios(const dt_iop_module_t *self, if(valid_chroma && changed_chroma) { for_four_channels(k) - out_correction_ratios[k] = (float)chr->D65coeffs[k] / chr->wb_coeffs[k]; + { + if(chr->wb_coeffs[k] > 1e-6f) + out_correction_ratios[k] = chr->D65coeffs[k] / chr->wb_coeffs[k]; + else + out_correction_ratios[k] = 1.0f; + } } return FALSE; } From 11d9d3166057a27376cfa86ef2529fc7f8655589 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sat, 28 Feb 2026 15:08:24 +0100 Subject: [PATCH 08/18] wb: added _get_corrected_illuminant_xy to deduplicate repeated code --- src/iop/channelmixerrgb.c | 65 +++++++++++++-------------------------- 1 file changed, 21 insertions(+), 44 deletions(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 3d56385b3df6..dab3b029243b 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -646,6 +646,23 @@ static gboolean _get_d65_correction_ratios(const dt_iop_module_t *self, return FALSE; } +static void _get_corrected_illuminant_xy(const dt_iop_module_t *self, + const dt_iop_channelmixer_rgb_params_t *p, + float *x, float *y) +{ + dt_aligned_pixel_t correction_ratios; + _get_d65_correction_ratios(self, correction_ratios); + dt_aligned_pixel_t wb_coeffs = { 0.f }; + if(p->illuminant == DT_ILLUMINANT_FROM_WB) + { + for(int k = 0; k < 4; k++) + wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; + } + illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, + wb_coeffs, x, y, p->temperature, p->illum_fluo, + p->illum_led); +} + DT_OMP_DECLARE_SIMD(aligned(input, output:16) uniform(compression, clip)) static inline void _gamut_mapping(const dt_aligned_pixel_t input, @@ -3183,17 +3200,7 @@ void commit_params(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; - dt_aligned_pixel_t correction_ratios; - _get_d65_correction_ratios(self, correction_ratios); - dt_aligned_pixel_t wb_coeffs = {0.f}; - if(p->illuminant == DT_ILLUMINANT_FROM_WB) - { - for(int k = 0; k < 4; k++) - wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; - } - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, - wb_coeffs, &x, &y, p->temperature, p->illum_fluo, - p->illum_led); + _get_corrected_illuminant_xy(self, p, &x, &y); // if illuminant is from camera or user WB coefficients, x and y are set on-the-fly at // commit time, so we need to set adaptation too @@ -3635,17 +3642,7 @@ static gboolean _illuminant_color_draw(GtkWidget *widget, float x = p->x; float y = p->y; dt_aligned_pixel_t RGB = { 0.f }; - dt_aligned_pixel_t correction_ratios; - _get_d65_correction_ratios(self, correction_ratios); - dt_aligned_pixel_t wb_coeffs = { 0.f }; - if(p->illuminant == DT_ILLUMINANT_FROM_WB) - { - for(int k = 0; k < 4; k++) - wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; - } - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, - wb_coeffs, &x, &y, p->temperature, p->illum_fluo, - p->illum_led); + _get_corrected_illuminant_xy(self, p, &x, &y); illuminant_xy_to_RGB(x, y, RGB); cairo_set_source_rgb(cr, RGB[0], RGB[1], RGB[2]); cairo_rectangle(cr, INNER_PADDING, margin, width, height); @@ -3745,17 +3742,7 @@ static void _update_approx_cct(const dt_iop_module_t *self) float x = p->x; float y = p->y; - dt_aligned_pixel_t correction_ratios; - _get_d65_correction_ratios(self, correction_ratios); - dt_aligned_pixel_t wb_coeffs = { 0.f }; - if(p->illuminant == DT_ILLUMINANT_FROM_WB) - { - for(int k = 0; k < 4; k++) - wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; - } - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, - wb_coeffs, &x, &y, p->temperature, p->illum_fluo, - p->illum_led); + _get_corrected_illuminant_xy(self, p, &x, &y); dt_illuminant_t test_illuminant; float t = 5000.f; @@ -4349,17 +4336,7 @@ static void _auto_set_illuminant(dt_iop_module_t *self, // find x y coordinates of illuminant for CIE 1931 2° observer float x = p->x; float y = p->y; - dt_aligned_pixel_t correction_ratios; - _get_d65_correction_ratios(self, correction_ratios); - dt_aligned_pixel_t wb_coeffs = { 0.f }; - if(p->illuminant == DT_ILLUMINANT_FROM_WB) - { - for(int k = 0; k < 4; k++) - wb_coeffs[k] = self->dev->chroma.wb_coeffs[k]; - } - illuminant_to_xy(p->illuminant, &(self->dev->image_storage), correction_ratios, - wb_coeffs, &x, &y, p->temperature, p->illum_fluo, - p->illum_led); + _get_corrected_illuminant_xy(self, p, &x, &y); dt_adaptation_t adaptation = p->adaptation; From dd450a42486ad2e6f7e0a83857b41dfb5a3eccc8 Mon Sep 17 00:00:00 2001 From: Kofa Date: Fri, 6 Mar 2026 19:16:58 +0100 Subject: [PATCH 09/18] wb: fix illuminants.h switch-case fall-through introduced when DT_ILLUMINANT_CAMERA was added --- src/common/illuminants.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index 8300b2c7110a..99a89343ae4f 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -298,7 +298,8 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, / // Model valid for T in [1667 ; 25000] K CCT_to_xy_blackbody(t, &x, &y); if(y != 0.f && x != 0.f) break; - // else t is out of bounds -> use custom/original values (next case) + // else t is out of bounds -> use custom/original values + return FALSE; } case DT_ILLUMINANT_CAMERA: { From 5f8a0c8ffb7f964619d74f7be907739400b3f644 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 5 Apr 2026 09:00:42 +0200 Subject: [PATCH 10/18] wb: renamed find_temperature_from_... to find_illuminant_xy_from_...; comment maintenance --- src/common/illuminants.h | 15 ++++++++------- src/iop/channelmixerrgb.c | 34 +++++++++++++++------------------- 2 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index 99a89343ae4f..fc74b0e1710b 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -216,11 +216,12 @@ static inline void illuminant_CCT_to_RGB(const float t, dt_aligned_pixel_t RGB) illuminant_xy_to_RGB(x, y, RGB); } -static inline gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, +// Derive illuminant chromaticity from WB module coefficients +static inline gboolean find_illuminant_xy_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, float *chroma_x, float *chroma_y); // Fetch image from pipeline and read EXIF for camera RAW WB coeffs -static inline gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, +static inline gboolean find_illuminant_xy_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, float *chroma_x, float *chroma_y); @@ -304,7 +305,7 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, / case DT_ILLUMINANT_CAMERA: { // Detect WB from RAW EXIF, correcting with D65/wb_coeff ratios - if(img && find_temperature_from_as_shot_coeffs(img, correction_ratios, &x, &y)) + if(img && find_illuminant_xy_from_as_shot_coeffs(img, correction_ratios, &x, &y)) break; // xy calculation failed @@ -313,7 +314,7 @@ static inline int illuminant_to_xy(const dt_illuminant_t illuminant, / case DT_ILLUMINANT_FROM_WB: { // Detect WB from user-provided coefficients - if(img && find_temperature_from_wb_coeffs(img, wb_coeffs, &x, &y)) + if(img && find_illuminant_xy_from_wb_coeffs(img, wb_coeffs, &x, &y)) break; // xy calculation failed @@ -449,7 +450,7 @@ static inline gboolean _wb_coeffs_invalid(const dt_aligned_pixel_t wb_coeffs, co } // returns FALSE if failed; TRUE if successful -static gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, +static gboolean find_illuminant_xy_from_wb_coeffs(const dt_image_t *img, const dt_aligned_pixel_t wb_coeffs, float *chroma_x, float *chroma_y) { if(img == NULL || wb_coeffs == NULL) return FALSE; @@ -471,7 +472,7 @@ static gboolean find_temperature_from_wb_coeffs(const dt_image_t *img, const dt_ } // returns FALSE if failed; TRUE if successful -static gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, +static gboolean find_illuminant_xy_from_as_shot_coeffs(const dt_image_t *img, const dt_aligned_pixel_t correction_ratios, float *chroma_x, float *chroma_y) { if(img == NULL) return FALSE; @@ -491,7 +492,7 @@ static gboolean find_temperature_from_as_shot_coeffs(const dt_image_t *img, cons for(size_t k = 0; k < 4; k++) WB[k] *= correction_ratios[k]; // for a neutral surface, raw RGB * img->wb_coeffs would produce neutral R=G=B - return find_temperature_from_wb_coeffs(img, WB, chroma_x, chroma_y); + return find_illuminant_xy_from_wb_coeffs(img, WB, chroma_x, chroma_y); } diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index dab3b029243b..1c1b1f6a5c8e 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -2231,7 +2231,7 @@ void process(dt_iop_module_t *self, dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - if(find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2244,9 +2244,11 @@ void process(dt_iop_module_t *self, } else if(data->illuminant_type == DT_ILLUMINANT_FROM_WB) { + // Same logic as above (we depend on the coefficients from white balance, + // which don't come from the EXIF this time). float x, y; - if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) + if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2357,7 +2359,7 @@ int process_cl(dt_iop_module_t *self, dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - if(find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2370,17 +2372,11 @@ int process_cl(dt_iop_module_t *self, } else if(d->illuminant_type == DT_ILLUMINANT_FROM_WB) { - // The camera illuminant is a behaviour rather than a preset of - // values: it uses whatever is in the RAW EXIF. But it depends on - // what temperature.c is doing and needs to be updated - // accordingly, to give a consistent result. We initialise the - // CAT defaults using the temperature coeffs at startup, but if - // temperature is changed later, we get no notification of the - // change here, so we can't update the defaults. So we need to - // re-run the detection at runtime… + // Same logic as above (we depend on the coefficients from white balance, + // which don't come from the EXIF this time). float x, y; - if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) + if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -3107,7 +3103,7 @@ static void _preview_pipe_finished_callback(gpointer instance, dt_iop_module_t * float x = p->x; float y = p->y; - if(find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) + if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) { p->x = x; p->y = y; @@ -3997,7 +3993,7 @@ void reload_defaults(dt_iop_module_t *self) dt_aligned_pixel_t correction_ratios; if(!_get_d65_correction_ratios(self, correction_ratios)) { - if(find_temperature_from_as_shot_coeffs(img, correction_ratios, &(d->x), &(d->y))) + if(find_illuminant_xy_from_as_shot_coeffs(img, correction_ratios, &(d->x), &(d->y))) d->illuminant = DT_ILLUMINANT_CAMERA; _check_if_close_to_daylight(d->x, d->y, &(d->temperature), &(d->illuminant), &(d->adaptation)); @@ -4093,7 +4089,7 @@ void gui_changed(dt_iop_module_t *self, // illuminant is changed. dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, + find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); } @@ -4103,7 +4099,7 @@ void gui_changed(dt_iop_module_t *self, // (x, y) were computed at runtime from the WB module's coefficients // and p->x, p->y may hold stale values. Recompute them now so // the new illuminant mode starts from the correct chromaticity. - find_temperature_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, + find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); } @@ -4120,16 +4116,16 @@ void gui_changed(dt_iop_module_t *self, // Get camera WB and update illuminant dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - const gboolean found = find_temperature_from_as_shot_coeffs(&(self->dev->image_storage), + const gboolean found = find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); if(found) dt_control_log(_("white balance successfully extracted from raw image")); } - if(p->illuminant == DT_ILLUMINANT_FROM_WB) + else if(p->illuminant == DT_ILLUMINANT_FROM_WB) { - const gboolean found = find_temperature_from_wb_coeffs(&(self->dev->image_storage), + const gboolean found = find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(p->x), &(p->y)); _check_if_close_to_daylight(p->x, p->y, &(p->temperature), NULL, &(p->adaptation)); From 6981e62dd97c44b0cb55ed5a0d082d0700c7055d Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 31 May 2026 12:27:51 +0200 Subject: [PATCH 11/18] Doc updates --- dev-doc/IOP_Module_API.md | 29 ++- dev-doc/Module_Lifecycle.md | 504 ++++++++++++++++++++++++++++++++++++ 2 files changed, 532 insertions(+), 1 deletion(-) create mode 100644 dev-doc/Module_Lifecycle.md diff --git a/dev-doc/IOP_Module_API.md b/dev-doc/IOP_Module_API.md index 3972e2b5b988..1594f8159822 100644 --- a/dev-doc/IOP_Module_API.md +++ b/dev-doc/IOP_Module_API.md @@ -360,7 +360,13 @@ Access in `process_cl()` via `self->global_data`. ### `reload_defaults()` - Per-Image Defaults -Called when switching images. Update defaults based on image properties: +Called when switching images (and on first load). Its purpose is to make `self->default_params` image-specific — most modules have defaults that depend on whether the image is RAW, HDR, monochrome, etc., and those cannot be compile-time constants. + +When invoked through the wrapper `dt_iop_reload_defaults()`, the harness calls `dt_iop_load_default_params()` after the module's own `reload_defaults()` returns, copying `default_params` into `self->params`. So on the wrapper path, writing to `default_params` inside `reload_defaults()` immediately affects `self->params` as well. Most callers use the wrapper (e.g. `_dt_dev_load_pipeline_defaults()`, the per-module Reset button, instance duplication). + +**Caveat — direct calls bypass only the framework copy, not the module body.** A few sites invoke `module->reload_defaults(module)` directly instead of through the wrapper (for example `develop.c` calls `temperature->reload_defaults(temperature)` to refresh WB side-effects). Those callers do **not** get the automatic `dt_iop_load_default_params()`, so the wholesale `default_params → self->params` copy does not happen. This does **not** mean `self->params` is left untouched: the module's own `reload_defaults()` body may still write specific `self->params` fields directly. `temperature.reload_defaults()` does exactly this — it writes `p->preset` and `p->late_correction` (via `p = self->params`) in lockstep with `default_params`, while the WB coefficient array is written to `default_params` only. On a direct call, the result is therefore a *partial* update: the fields the body touches change in `self->params`, the rest do not, and no history entry is recorded. If you add such a call, prefer `dt_iop_reload_defaults()`, or audit exactly which `self->params` fields the body mutates and sync the rest yourself. + +#### Job 1 (all modules that implement this): compute image-specific default params ```c void reload_defaults(dt_iop_module_t *self) @@ -373,8 +379,29 @@ void reload_defaults(dt_iop_module_t *self) } ``` +Example: `exposure.c` disables itself for non-RAW images by setting `self->default_enabled = FALSE`. The exposure value in `default_params` stays at its static value because exposure is image-type-agnostic beyond that flag. + Common checks: `dt_image_is_raw()`, `dt_image_is_hdr()`, `dt_image_is_ldr()`, `dt_image_is_monochrome()`, `dt_image_is_bayerRGB()`. +#### Job 2 (some modules): write shared inter-module data as a side effect + +Some modules use `reload_defaults()` to populate shared state in `dev->chroma` (or similar structures) that other modules depend on. This happens regardless of whether the module is enabled or has a history entry. + +Example: `temperature.c` always computes the camera's as-shot WB coefficients and D65 reference coefficients from the image's EXIF data and writes them into `dev->chroma.as_shot[]` and `dev->chroma.D65coeffs[]`. `channelmixerrgb.c` reads these values during its own `commit_params()` to perform chromatic adaptation. If `reload_defaults()` did not run unconditionally, `channelmixerrgb` would have no access to the camera's native WB — even when the white balance module itself is disabled or absent from the history. + +#### Where `default_params` is consumed + +**Initial image load.** Inside `dt_dev_read_history_ext()`, `_dt_dev_load_pipeline_defaults()` calls `reload_defaults()` for every module (in reverse pipe order) and copies each `default_params` into `params`. The function then adds the workflow default modules and any auto-presets, and builds `dev->history` **directly from the database rows** — it does *not* call `dt_dev_pop_history_items`. A module with a history row runs with the saved params from that row; a module without one keeps the image-specific `default_params` just loaded. This is true on both the first open of an image and the reopen of an edited one; the only difference is whether the database has any rows for that module. + +**Undo / redo / history-panel navigation.** This is the separate path that uses `dt_dev_pop_history_items_ext()`, which does two passes: + +1. Resets all modules: `module->params = module->default_params` — so modules absent from the replayed range start from their image-specific default, not garbage. +2. Replays history entries: `module->params = hist->params` for each entry up to the target position. + +So `default_params` is consumed in two ways: as the starting point for modules with no (or not-yet-replayed) history, and as the value the per-module "Reset to defaults" button restores. It also sets the visual "default" markers on GUI sliders (via `dt_bauhaus_slider_set_default()`). + +For the full sequence — load order, the `dev->chroma` side effects, and the reverse-iteration hazard — see `Module_Lifecycle.md`. + ### Pipe Lifecycle Functions Each pixelpipe has its own copy of every module's data via `dt_dev_pixelpipe_iop_t` ("piece"). diff --git a/dev-doc/Module_Lifecycle.md b/dev-doc/Module_Lifecycle.md new file mode 100644 index 000000000000..7337e1844db6 --- /dev/null +++ b/dev-doc/Module_Lifecycle.md @@ -0,0 +1,504 @@ +# Module Lifecycle: What Happens When the User Interacts + +`IOP_Module_API.md` documents the callback API: what each function signature means and what +a module must implement. This document is its companion. It describes *when* those callbacks +fire and *what else* the system does around them — pipe change flags, cache invalidation, +history replay, and cross-module data flow. + +After reading the API doc you can implement a module. After reading this one you should also +have a mental model of what darktable does when the user loads an image, drags a slider, +toggles a module, resets it, or undoes an edit. + +References point to code by **file, function, and the relevant block** (an `if`, `for`, or +`while`), and use short snippets where that is clearer. They avoid line numbers, which drift +as the source changes. + +--- + +## 1. Background concepts + +These are the moving parts the later sections rely on. Each one gets a short definition, not +a full reference. + +### History stack (`dt_dev_history_item_t`) + +An ordered list of per-module parameter snapshots on `dt_develop_t`. `dev->history_end` is +the active cursor: only the entries in the range `[0, history_end)` are "live". Note that a +history item's **`enabled` flag is stored separately from `params`** — it is not part of the +params buffer. Toggling a module on or off and editing its parameters are therefore two +different kinds of history change. + +### Pipe types + +Three pixelpipes run the same module chain independently, at different resolutions: + +- **full** — the main darkroom image +- **preview** — the low-resolution thumbnail (used for the histogram and navigation) +- **preview2** — an optional second-window preview + +Each is marked dirty and re-run on its own, often in parallel on separate threads. "Mark all +pipes dirty" below means all of the pipes that exist for the current session. + +### Pipe change flags + +These are set on `pipe->changed`. They tell `dt_dev_pixelpipe_change()` (pixelpipe_hb.c) +which resync strategy to use when the pipeline next runs. The constants are defined in +pixelpipe_hb.h. + +| Flag | Meaning | Strategy | +|---|---|---| +| `DT_DEV_PIPE_TOP_CHANGED` | Only the topmost history item's params changed | `synch_top`: re-commit that one module; upstream piece hashes stay untouched | +| `DT_DEV_PIPE_SYNCH` | All items need a param resync, topology unchanged | `synch_all`: re-commit every history item in order | +| `DT_DEV_PIPE_REMOVE` | A module was added or removed; topology must be rebuilt | full `cleanup_nodes` + `create_nodes` + `synch_all` | +| `DT_DEV_PIPE_ZOOMED` | Zoom or pan only | skip the param sync; only the `modify_roi_in`/`modify_roi_out` chain re-runs | + +`synch_all` re-commits every module, but a module whose hash did not change still produces a +cache hit, so its `process()` is skipped. In other words, the flag controls *committing*, not +*reprocessing*. Reprocessing is decided per module by the cache (see §6). + +### Pipe status flags + +`DIRTY` (needs reprocessing), `RUNNING` (a thread is processing it now), `VALID` (output is +current), `INVALID` (unusable, for example during teardown). + +### `focus_hash` + +A hash of the currently focused module widget, updated by `dt_iop_request_focus()`. Inside +`_dev_add_history_item_ext()` (develop.c), the relevant test is roughly: + +```c +if(dev->focus_hash != hist->focus_hash) { /* force a new history entry */ } +``` + +If the user moved focus since the last commit, a **new** history entry is forced even when +the same module is edited again. The practical effect is that the edit then takes the `SYNCH` +path instead of the cheaper `TOP_CHANGED` path. If you write deferred or batched parameter +changes, keep this in mind, because it changes how much of the pipe re-commits. + +### Pixelpipe cache + +The cache is hash-based with lazy invalidation. The key for a module's output is the +**cumulative hash** of every upstream `piece->hash` value (each computed in +`dt_iop_commit_params()` from the op name, instance, params, and blend_params), combined with +the relevant color-profile information. It is *not* a hash of one module's data on its own. +The hashing happens in the basic-hash helper in pixelpipe_cache.c, which seeds the hash from +the image id and pipe profiles and then walks all modules up to the requested position. + +When the key matches a cached entry, the output is reused and `process()` is skipped. Stale +entries are not actively deleted on a param change; they simply stop being looked up, and the +fixed-size LRU evicts them later. + +### `dev->chroma` (shared WB/CAT state) + +A struct on `dt_develop_t` used by white balance (`temperature.c`, the producer) and color +calibration (`channelmixerrgb.c`, the consumer) to communicate. There is **no signal**; +synchronization is implicit, through the order in which `commit_params()` runs (see §4 and +§5). Temperature has two distinct write sites: + +- `reload_defaults()` writes `as_shot[]` and `D65coeffs[]`. This happens once per image load, + **even if temperature is disabled or absent from the history**, so channelmixerrgb always + has the camera's native WB reference available. +- `commit_params()` writes `wb_coeffs[]`, `chr->temperature`, and `chr->late_correction`, on + every parameter change. + +`channelmixerrgb.commit_params()` reads these values. + +### `module->params` vs `module->default_params` + +`default_params` is **image-specific**: it is set by `reload_defaults()` (see +`IOP_Module_API.md`). `params` is the **live editing state**. +`dt_dev_pop_history_items_ext()` resets every module to its `default_params` and then replays +the history entries on top. As a result, a module with no history entry runs with its +image-specific defaults, not with compile-time constants. + +--- + +## 2. Image loading + +A pristine (never edited) image and one with saved history both go through the **same** +`dt_dev_load_image()` → `dt_dev_read_history_ext()` chain. The "no history" versus "has +history" distinction is handled **inside** `dt_dev_read_history_ext()`, not by different +callers. + +### Load sequence (`dt_dev_load_image()`, develop.c) + +1. **`_dt_dev_load_raw()`** triggers the rawspeed decode through the mipmap cache (this + blocks), then reads the decoded image struct into `dev->image_storage`. The large mipmap + buffer is released right after the decode; `image_storage` keeps the metadata. +2. Mark all pipes `DIRTY` and set `loading = TRUE`. +3. **`dt_iop_load_modules()`** builds the IOP module list (`dev->iop`). +4. **`dt_dev_read_history_ext()`** does everything else — defaults, presets, and history + replay. See below. + +`dt_dev_load_image()` itself does not touch `dev->chroma` and does not load defaults. All of +that happens inside `dt_dev_read_history_ext()`. + +### Inside `dt_dev_read_history_ext()` (develop.c) + +This function applies saved history by **building `dev->history` directly from the database**. +It does *not* call `pop_history_items`. The reset-then-replay `pop` path is the undo/redo +path (§3f), not the initial-load path. + +**For every image** — this is the `if(!no_image)` block, and it runs whether or not the image +has saved history: + +- Clear the scratch `memory.history` table. +- **`dt_dev_reset_chroma()`** clears part of the shared WB state (§5 lists exactly which + fields): `temperature` and `adaptation` become `NULL`, and `wb_coeffs[]` is reset to 1.0. +- **`_dt_dev_load_pipeline_defaults()`** calls `dt_iop_reload_defaults()` on every module in + **reverse** pipe order (§5 explains the direction). This sets each module's image-specific + `default_params`. Because it goes through the wrapper, it also copies them into `params` + (via `dt_iop_load_default_params()`). Temperature additionally populates + `dev->chroma.as_shot[]` and `D65coeffs[]` as a side effect. +- **`_dev_add_default_modules()`** prepends the workflow-mandated modules into the in-memory + history. +- **`_dev_auto_apply_presets()`** applies auto-presets. It is gated on the + `DT_IMAGE_AUTO_PRESETS_APPLIED` image flag, **not** on `change_timestamp == -1` (that test + guards only the separate legacy pre-3.0 WB recovery branch). It is a no-op for an image + whose presets were already applied, so only a first import actually gains preset entries + here. +- **`_dev_merge_history()`** merges the in-memory default and preset history into + `main.history`, so the database read below picks it up. + +**Then, for all images,** the same database-read loop runs. For each `main.history` row it +allocates a `hist` item, copies the params (falling back to the module's `default_params` +when the row has none), sets `hist->enabled`, appends to `dev->history`, and increments +`history_end`: + +```c +memcpy(hist->params, hist->module->default_params, hist->module->params_size); +hist->enabled = TRUE; +... +dev->history = g_list_append(dev->history, hist); +dev->history_end++; +``` + +Modules with no database row keep the image-specific `default_params` set earlier by +`_dt_dev_load_pipeline_defaults()`. The `enabled` flag is carried per entry, separately from +params. + +After the loop, if both `temperature` and `channelmixerrgb` are present, the function calls +`temperature->reload_defaults(temperature)` **directly**, to resync the WB/CAT shared state: + +```c +if(temperature && channelmixerrgb) + temperature->reload_defaults(temperature); +``` + +This direct call is a deliberate wrapper bypass: it skips the framework's wholesale +`default_params → params` copy (`dt_iop_load_default_params()`). It still refreshes +`default_params` and the `dev->chroma` reference data, and temperature's own body also writes +a few `self->params` fields directly (`preset`, `late_correction`). So `params` is partially +updated, not fully resynced — see the `reload_defaults` caveat in `IOP_Module_API.md`. + +The function then re-reads `history_end` from `main.images`, and — when the GUI is attached — +calls `dt_dev_pipe_synch_all()` and `dt_dev_invalidate_all()` so the pipes will commit and +reprocess. The pipes stay `DIRTY`, and the pipeline threads run on the next redraw (§6). + +--- + +## 3. Editing operations + +### 3a. Adjust a slider — the common `TOP_CHANGED` path + +1. The bauhaus slider fires its value-change handler (`bauhaus.c`), which calls + `dt_iop_gui_changed(module, widget, &prev)` directly. +2. The module's `gui_changed()` runs (module-specific; it may update dependent widgets). +3. `dt_dev_add_history_item_target()` reaches `_dev_add_history_item_ext()` (develop.c), which + chooses one of two branches: + - **`TOP_CHANGED`**: the same module is already at the top of the history, + `dev->focus_hash == hist->focus_hash`, and only the params differ. The params are + updated in place and `pipe->changed |= DT_DEV_PIPE_TOP_CHANGED`. + - **`SYNCH`**: anything else (a different module, or focus changed since the last commit). + A new history entry is pushed and `pipe->changed |= DT_DEV_PIPE_SYNCH`. +4. `dt_dev_invalidate_all()` marks all pipes `DIRTY` and increments `dev->timestamp`. +5. `DT_SIGNAL_DEVELOP_HISTORY_CHANGE` is emitted **only when `need_end_record` is true**, not + on every slider tick. It is tied to the undo-record start/end pairing, so one logical edit + emits the signal once. +6. On the pipeline thread, `dt_dev_pixelpipe_change()` dispatches on the flag: + - `TOP_CHANGED` → `dt_dev_pixelpipe_synch_top()`: runs `commit_params()` for that one + module only; upstream piece hashes are not recomputed. + - `SYNCH` → `dt_dev_pixelpipe_synch_all()`: runs `commit_params()` for every history item + in order. Upstream modules re-commit, but an unchanged hash still hits the cache. +7. The pipeline runs: modules with unchanged hashes reuse their cached output; the changed + module and everything downstream of it reprocess. + +### 3b. Enable / disable a module + +1. The toggle button fires its `toggled` signal, which reaches `_gui_off_callback()` + (imageop.c). +2. The callback sets `module->enabled` and calls `dt_dev_add_history_item()`. The + `hist->enabled` flag is recorded **separately from params**. +3. `pipe->changed |= DT_DEV_PIPE_SYNCH`, because an enable/disable propagates downstream. +4. A full `synch_all` runs on the next pipeline pass. + +### 3c. Reset to defaults + +1. The reset button fires `button-press-event`, which reaches `_gui_reset_callback()` + (imageop.c). +2. `dt_iop_reload_defaults(module)` recomputes the image-specific `default_params` and copies + them into `module->params` (this is the wrapper path, so the copy does happen). +3. `dt_iop_gui_update(module)` syncs the widgets to the new params. +4. `dt_dev_add_history_item(module->dev, module, TRUE)` adds or updates a **single** history + entry. It does not rebuild the whole stack. +5. `pipe->changed |= DT_DEV_PIPE_SYNCH`. + +### 3d. Add a module instance (+ button) + +1. `dt_iop_gui_duplicate()` (imageop.c) starts the duplication. +2. It records the base module's state: `dt_dev_add_history_item(base->dev, base, FALSE)`. +3. `dt_dev_module_duplicate()` creates the new instance in `dev->iop`. +4. `dt_iop_reload_defaults(new_module)` sets the image-specific defaults for the new instance. +5. `dt_dev_add_history_item(new_module->dev, new_module, TRUE)` adds a new history entry. +6. `dt_dev_pixelpipe_rebuild()` sets `pipe->changed |= DT_DEV_PIPE_REMOVE`, which forces a + full topology rebuild — the node graph changed, so it is rebuilt from scratch. + +### 3e. Remove a module instance (− button) + +`dt_dev_module_remove()` (develop.c) does the teardown. Inside its `if(dev->gui_attached)` +block — true in the interactive darkroom — it **prunes the history**. It walks `dev->history` +and, for each entry whose module is the one being removed, frees the item, unlinks it, and +decrements `history_end`: + +```c +if(module == hist->module) +{ + dt_dev_free_history_item(hist); + dev->history = g_list_delete_link(dev->history, elem); + dev->history_end--; +} +``` + +It then removes the module from `dev->iop`. A `REMOVE` flag triggers a full topology rebuild +(`cleanup_nodes` + `create_nodes`). + +So a removed instance's history entries are **actively deleted**, not merely skipped. In a +headless context, where `gui_attached` is false, this pruning block does not run — but +interactive removal is the case this document covers. + +### 3f. Undo / redo / history-panel navigation + +This is a **distinct path** from the initial image load. During an active editing session it +is the main way `module->params` are rewritten from saved entries. + +1. The user clicks a history entry, or presses Ctrl+Z / Ctrl+Y. +2. `dt_dev_reload_history_items()` (develop.c) sets `dev->history_end` to the target position. +3. `dt_dev_pop_history_items(dev, 0)` resets all modules to their `default_params`. +4. The history is re-read (from memory or the database). +5. `dt_dev_pop_history_items(dev, history_end)` replays up to the target position. +6. `dt_dev_invalidate_all()` marks the pipes dirty, which leads to a full pipeline rerun. + +The two `pop_history_items` calls live **here**, in `dt_dev_reload_history_items()`, not in +`dt_dev_read_history_ext()`. Apart from the initial image load, this is the only path that +rewrites `module->params` from saved history. The `SYNCH` flag ensures every module +re-commits after the replay. + +--- + +## 4. Cross-module interactions + +### 4a. Temperature (WB) → ChannelMixerRGB (through `dev->chroma`) + +There is no signal; synchronization is implicit and ordered by iop_order: + +- During `synch_all` or `synch_top`, `dt_iop_commit_params()` runs in pipe order. +- `temperature.commit_params()` writes `dev->chroma.wb_coeffs[]`, `chr->late_correction`, and + the `chr->temperature` pointer. +- `channelmixerrgb.commit_params()` runs **after** it and reads `wb_coeffs[]` to build its + chromatic-adaptation matrix. +- As a once-per-image-load side effect, `temperature.reload_defaults()` writes + `dev->chroma.as_shot[]` and `D65coeffs[]` **unconditionally** — even when temperature is + disabled or absent from the history — because channelmixerrgb needs the camera's native WB + reference to compute its default illuminant. +- For late correction, temperature sets `chr->late_correction` and channelmixerrgb reads it. + A WB-mode switch raises `SYNCH`, so both re-commit in order. + +### 4b. Color-profile change (colorin / colorout) + +This follows the same flow as a slider adjust: `TOP_CHANGED` or `SYNCH`, depending on +`focus_hash` and on whether it is an in-place update. There is no `REMOVE`, because the +topology does not change. `commit_params()` rebuilds the color transforms; no cross-module +signal is needed. Because the transform changes the pixels, all downstream cache hashes change +and everything downstream reprocesses. + +--- + +## 5. Pipeline ordering asymmetry + +Two operations walk the module list in **opposite** directions, which is a common source of +confusion. + +- **`commit_params()` (through `synch_all`)** iterates the history in history-list order. For + auto-applied presets this is effectively pipe order: temperature commits first and writes + `dev->chroma.wb_coeffs[]`; channelmixerrgb commits second and reads it. One caveat: + `synch_top` re-commits **only** the topmost history item, and relies on `wb_coeffs` still + being present in `dev->chroma` from a previous full commit. If temperature has never been + committed in the current session, `wb_coeffs` may be wrong on a `synch_top`. +- **`_dt_dev_load_pipeline_defaults()`** iterates in **reverse** pipe order (last module + first): + + ```c + for(const GList *modules = g_list_last(dev->iop); + modules; + modules = g_list_previous(modules)) + { + dt_iop_reload_defaults(modules->data); + } + ``` + + So channelmixerrgb's `reload_defaults()` runs **before** temperature's. + +### Why reverse? (and what it is *not*) + +The source gives no rationale for the reverse direction. Checked against the code, it is +**not** a chromatic-adaptation (CAT) deduplication mechanism, despite the intuitive theory +that "the last instance should claim the CAT first." CAT-token ownership is in fact +**order-independent**, for three reasons: + +- `_declare_cat_on_pipe()` (channelmixerrgb.c) resolves a conflict between multiple + `channelmixerrgb` instances through `dt_iop_is_first_instance()`. Its conflict branch can + take the token away from a previous claimant: + + ```c + if(chr->adaptation == NULL) + chr->adaptation = self; // first to register wins + else if(chr->adaptation == self) + ; // already ours + else if(dt_iop_is_first_instance(self->dev->iop, self)) + chr->adaptation = self; // earlier in the pipe: take over + ``` + + So the **first instance in `dev->iop` always ends up owning the token**, regardless of who + registered first. Tracing two eligible instances shows that forward iteration would dedup + the *defaults* more cleanly than reverse: reverse leaves both instances with active-CAT + defaults until a later commit re-resolves the token. +- `_declare_cat_on_pipe()` returns early when `gui_data == NULL`, so it is a no-op during + headless default loading. It therefore cannot be the reason the loop runs in reverse. +- Duplicate `channelmixerrgb` instances do not exist yet when + `_dt_dev_load_pipeline_defaults()` runs at image load. They are created later, by user + action or history replay. + +So the safe statement is: defaults load in reverse pipe order, and CAT dedup is handled by +`_declare_cat_on_pipe()` plus `dt_iop_is_first_instance()` — **not** by the loop direction. +The real hazard the reverse order creates is producer/consumer staleness for modules that read +shared state in `reload_defaults()`, described next. + +### What `dt_dev_reset_chroma()` actually resets + +`dt_dev_reset_chroma()` (develop.c) clears only three fields: + +```c +chr->adaptation = NULL; +chr->temperature = NULL; +for(int c = 0; c < 4; c++) chr->wb_coeffs[c] = 1.0f; +``` + +It does **not** reset `D65coeffs`, `as_shot`, or `late_correction`. Those are initialized only +by `dt_dev_init_chroma()`, at dev-context creation — not on each image switch. + +`channelmixerrgb.reload_defaults()` calls `_get_d65_correction_ratios()`, which reads +`dev->chroma.D65coeffs[]` and `as_shot[]`. Because temperature's `reload_defaults()` has not +run yet (reverse order) and `dt_dev_reset_chroma()` leaves those fields alone, channelmixerrgb +can read **stale values from the previously loaded image**. The visible effect: the default +illuminant xy that the Reset button restores can be computed from the wrong camera's daylight +reference right after an image switch. + +The `wb_coeffs` staleness (which affects active params through `commit_params()`) **is** fixed +by `dt_dev_reset_chroma()`. The `D65coeffs` / `as_shot` / `late_correction` staleness (which +affects **defaults only**) remains. It is a known limitation, not a mitigated bug. + +### Developer rule + +A module's `reload_defaults()` **cannot** assume that earlier-in-pipe modules have already +populated the shared `dev->chroma` state. The field that `dt_dev_reset_chroma()` clears +(`wb_coeffs`) is safe to read, because it holds a neutral 1.0. The fields it does **not** clear +(`D65coeffs`, `as_shot`, `late_correction`) may still hold stale data from the previous image +when `reload_defaults()` runs. + +--- + +## 6. Pipeline execution detail + +How pixels actually flow once the pipes are marked dirty: + +- `dt_dev_process_image_job()` (develop.c) is the entry point. It locks the pipe and captures + `dev->timestamp`. +- `dt_dev_pixelpipe_process()` calls `_dev_pixelpipe_process_rec()` (pixelpipe_hb.c), which + recurses from the last module back toward the input, pulling each module's input on demand. +- For each module, the cache key is the **cumulative hash** of all upstream `piece->hash` + values (each set in `dt_iop_commit_params()` from op, instance, params, and blend_params), + plus the color-profile information. On a key match, the cached output is reused and + `process()` is skipped. On a miss, `process()` (or `process_cl()` on the GPU) runs and the + result is stored. +- `modify_roi_in()` and `modify_roi_out()` run on every module during the ROI-propagation + phase, before processing, translating between the output-side and input-side regions. Under + the `ZOOMED` flag only the ROIs change — the module hashes do not — so cache hits are + possible for any module whose cached output region still covers the requested region. +- The cache is a fixed-size LRU, and eviction is automatic. There is no explicit invalidation + on a param change; a changed hash simply misses. +- After all modules finish, `pipe->status` becomes `VALID` and a finished signal (for example + `DT_SIGNAL_DEVELOP_PREVIEW_PIPE_FINISHED`) triggers the GTK redraw. +- The full and preview pipes run identical logic at different input resolutions and can run in + parallel on separate threads. + +--- + +## Key symbols + +Grouped by file. Look these up by name; the surrounding block is described in the relevant +section above. + +**develop.c** + +| Symbol | Role | +|---|---| +| `dt_dev_load_image` | Top-level image load | +| `dt_dev_read_history_ext` | Loads defaults/presets and builds `dev->history` from the DB | +| `_dt_dev_load_pipeline_defaults` | Calls `reload_defaults` on all modules, in reverse pipe order | +| `dt_dev_reset_chroma` | Clears `adaptation`, `temperature`, `wb_coeffs` only | +| `dt_dev_init_chroma` | Initializes the full `dev->chroma`, at dev-context creation | +| `dt_dev_add_history_item` / `_dev_add_history_item_ext` | Add/update a history entry; choose `TOP_CHANGED` vs `SYNCH` (the `focus_hash` test lives here) | +| `dt_dev_pop_history_items` / `_ext` | Reset to defaults, then replay history (used by undo/redo) | +| `dt_dev_reload_history_items` | Undo/redo and history-panel navigation | +| `dt_dev_module_remove` | Removes a module instance and prunes its history entries | +| `dt_dev_invalidate_all` | Marks all pipes dirty, bumps `dev->timestamp` | +| `_dev_auto_apply_presets` | Auto-presets; gated on `DT_IMAGE_AUTO_PRESETS_APPLIED` | +| `dt_dev_process_image_job` | Pixel-processing entry point | + +**imageop.c** + +| Symbol | Role | +|---|---| +| `dt_iop_reload_defaults` | Wrapper: calls the module's `reload_defaults()` then `dt_iop_load_default_params()` | +| `dt_iop_load_default_params` | Copies `default_params` into `params` | +| `dt_iop_commit_params` | Translates params into `piece->data`; sets `piece->hash` | +| `dt_iop_gui_changed` | Slider/widget change entry point | +| `dt_iop_is_first_instance` | True for the earliest instance of an op in `dev->iop` | +| `_gui_off_callback` | Enable/disable toggle handler | +| `_gui_reset_callback` | Reset-to-defaults handler | +| `dt_iop_gui_duplicate` | Add a module instance | + +**pixelpipe_hb.c / .h** + +| Symbol | Role | +|---|---| +| `DT_DEV_PIPE_*` flags | Pipe change flags (defined in pixelpipe_hb.h) | +| `dt_dev_pixelpipe_change` | Dispatches on the change flag | +| `dt_dev_pixelpipe_synch_top` | Re-commits the top module only | +| `dt_dev_pixelpipe_synch_all` | Re-commits all history items in order | +| `_dev_pixelpipe_process_rec` | Recursive per-module processing | + +**Other** + +| Symbol | File | Role | +|---|---|---| +| Basic-hash helper (cumulative cache hash) | pixelpipe_cache.c | Builds the cache key from image id, profiles, and all modules up to a position | +| `_declare_cat_on_pipe` | channelmixerrgb.c | Claims/resolves the CAT token | +| `temperature.commit_params` | temperature.c | Writes `dev->chroma.wb_coeffs` | +| `channelmixerrgb.commit_params` | channelmixerrgb.c | Reads `dev->chroma` | + +## See also + +- `IOP_Module_API.md` — the callback API reference (signatures, the two `reload_defaults` + jobs). +- `pixelpipe_architecture.md` — pipeline architecture, including the ordering-asymmetry note. From cd2f446ccfb4a044861e985030b2f8b6d40d39e7 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 31 May 2026 16:19:24 +0200 Subject: [PATCH 12/18] Doc updates: wb + color calibration --- dev-doc/IOP_Module_API.md | 5 +- dev-doc/Module_Groups.md | 2 +- dev-doc/Module_Lifecycle.md | 199 ++++++++++++------------------ dev-doc/README.md | 1 + dev-doc/pixelpipe_architecture.md | 8 +- 5 files changed, 88 insertions(+), 127 deletions(-) diff --git a/dev-doc/IOP_Module_API.md b/dev-doc/IOP_Module_API.md index 1594f8159822..79e34fb74e0b 100644 --- a/dev-doc/IOP_Module_API.md +++ b/dev-doc/IOP_Module_API.md @@ -4,6 +4,7 @@ This guide documents the functions that darktable Image Operation (IOP) modules See also: - [Pixelpipe Architecture](pixelpipe_architecture.md) for pipeline data flow and caching. +- [Module Lifecycle](Module_Lifecycle.md) for when these callbacks fire and what the system does around them. - [Introspection System](introspection.md) for parameter management. - [GUI Architecture](GUI.md) for GUI events, callbacks, thread safety, and widget reparenting. @@ -389,6 +390,8 @@ Some modules use `reload_defaults()` to populate shared state in `dev->chroma` ( Example: `temperature.c` always computes the camera's as-shot WB coefficients and D65 reference coefficients from the image's EXIF data and writes them into `dev->chroma.as_shot[]` and `dev->chroma.D65coeffs[]`. `channelmixerrgb.c` reads these values during its own `commit_params()` to perform chromatic adaptation. If `reload_defaults()` did not run unconditionally, `channelmixerrgb` would have no access to the camera's native WB — even when the white balance module itself is disabled or absent from the history. +For the complete producer/consumer map, the reverse-order default-loading reasoning, and which `dev->chroma` fields each module actually reads, see [iop/wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md). + #### Where `default_params` is consumed **Initial image load.** Inside `dt_dev_read_history_ext()`, `_dt_dev_load_pipeline_defaults()` calls `reload_defaults()` for every module (in reverse pipe order) and copies each `default_params` into `params`. The function then adds the workflow default modules and any auto-presets, and builds `dev->history` **directly from the database rows** — it does *not* call `dt_dev_pop_history_items`. A module with a history row runs with the saved params from that row; a module without one keeps the image-specific `default_params` just loaded. This is true on both the first open of an image and the reopen of an edited one; the only difference is whether the database has any rows for that module. @@ -400,7 +403,7 @@ Example: `temperature.c` always computes the camera's as-shot WB coefficients an So `default_params` is consumed in two ways: as the starting point for modules with no (or not-yet-replayed) history, and as the value the per-module "Reset to defaults" button restores. It also sets the visual "default" markers on GUI sliders (via `dt_bauhaus_slider_set_default()`). -For the full sequence — load order, the `dev->chroma` side effects, and the reverse-iteration hazard — see `Module_Lifecycle.md`. +For the full sequence — load order, the `dev->chroma` side effects, and the reverse-iteration hazard — see [Module_Lifecycle.md](Module_Lifecycle.md). ### Pipe Lifecycle Functions diff --git a/dev-doc/Module_Groups.md b/dev-doc/Module_Groups.md index 7fb27a9c69be..331076785bf8 100644 --- a/dev-doc/Module_Groups.md +++ b/dev-doc/Module_Groups.md @@ -9,7 +9,7 @@ Darktable uses a tabs-based interface in the right panel. Each tab corresponds t - **Technical**: Basic corrections (demosaic, lens, crop). - **Grading**: Color and tone grading. - **Effects**: Artistic effects. -- **Quick Access Panel**: (Special case, detailed in `Quick_Access_Panel.md`). +- **Quick Access Panel**: (Special case, detailed in [Quick_Access_Panel.md](Quick_Access_Panel.md)). ## Implementation (`modulegroups.c`) diff --git a/dev-doc/Module_Lifecycle.md b/dev-doc/Module_Lifecycle.md index 7337e1844db6..044e95233f89 100644 --- a/dev-doc/Module_Lifecycle.md +++ b/dev-doc/Module_Lifecycle.md @@ -1,7 +1,7 @@ # Module Lifecycle: What Happens When the User Interacts -`IOP_Module_API.md` documents the callback API: what each function signature means and what -a module must implement. This document is its companion. It describes *when* those callbacks +[IOP_Module_API.md](IOP_Module_API.md) documents the callback API: what each function signature +means and what a module must implement. This document is its companion. It describes *when* those callbacks fire and *what else* the system does around them — pipe change flags, cache invalidation, history replay, and cross-module data flow. @@ -22,7 +22,9 @@ a full reference. ### History stack (`dt_dev_history_item_t`) -An ordered list of per-module parameter snapshots on `dt_develop_t`. `dev->history_end` is +An ordered list of per-module parameter snapshots on +[`dt_develop_t`](pixelpipe_architecture.md#dt_develop_t) (the main darkroom session state). +`dev->history_end` is the active cursor: only the entries in the range `[0, history_end)` are "live". Note that a history item's **`enabled` flag is stored separately from `params`** — it is not part of the params buffer. Toggling a module on or off and editing its parameters are therefore two @@ -54,7 +56,8 @@ pixelpipe_hb.h. `synch_all` re-commits every module, but a module whose hash did not change still produces a cache hit, so its `process()` is skipped. In other words, the flag controls *committing*, not -*reprocessing*. Reprocessing is decided per module by the cache (see §6). +*reprocessing*. Reprocessing is decided per module by the cache (see +[section 6](#6-pipeline-execution-detail)). ### Pipe status flags @@ -63,7 +66,9 @@ current), `INVALID` (unusable, for example during teardown). ### `focus_hash` -A hash of the currently focused module widget, updated by `dt_iop_request_focus()`. Inside +A module is **focused** when the user opens its panel or clicks into one of its widgets; the +darkroom tracks a single focused module at a time. `dt_iop_request_focus()` records that module +and updates `dev->focus_hash`, a hash of the focused module's widget. Inside `_dev_add_history_item_ext()` (develop.c), the relevant test is roughly: ```c @@ -86,27 +91,25 @@ the image id and pipe profiles and then walks all modules up to the requested po When the key matches a cached entry, the output is reused and `process()` is skipped. Stale entries are not actively deleted on a param change; they simply stop being looked up, and the -fixed-size LRU evicts them later. +fixed-size LRU evicts them later. For the full caching model see +[pixelpipe_architecture.md](pixelpipe_architecture.md#pipeline-caching). ### `dev->chroma` (shared WB/CAT state) A struct on `dt_develop_t` used by white balance (`temperature.c`, the producer) and color calibration (`channelmixerrgb.c`, the consumer) to communicate. There is **no signal**; -synchronization is implicit, through the order in which `commit_params()` runs (see §4 and -§5). Temperature has two distinct write sites: - -- `reload_defaults()` writes `as_shot[]` and `D65coeffs[]`. This happens once per image load, - **even if temperature is disabled or absent from the history**, so channelmixerrgb always - has the camera's native WB reference available. -- `commit_params()` writes `wb_coeffs[]`, `chr->temperature`, and `chr->late_correction`, on - every parameter change. - -`channelmixerrgb.commit_params()` reads these values. +synchronization is implicit, through the order in which `commit_params()` runs (see +[section 4](#4-cross-module-interactions) and [section 5](#5-pipeline-ordering-asymmetry)). +Temperature writes the camera WB reference in `reload_defaults()` and the live coefficients in +`commit_params()`; channelmixerrgb reads them. The full field-by-field producer/consumer map and +the reverse-load reasoning live in their own doc — see +[wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md). ### `module->params` vs `module->default_params` `default_params` is **image-specific**: it is set by `reload_defaults()` (see -`IOP_Module_API.md`). `params` is the **live editing state**. +[IOP_Module_API.md](IOP_Module_API.md#reload_defaults---per-image-defaults)). `params` is the +**live editing state**. `dt_dev_pop_history_items_ext()` resets every module to its `default_params` and then replays the history entries on top. As a result, a module with no history entry runs with its image-specific defaults, not with compile-time constants. @@ -124,7 +127,9 @@ callers. 1. **`_dt_dev_load_raw()`** triggers the rawspeed decode through the mipmap cache (this blocks), then reads the decoded image struct into `dev->image_storage`. The large mipmap - buffer is released right after the decode; `image_storage` keeps the metadata. + buffer is released right after the decode; `image_storage` keeps the metadata. (The *mipmap + cache* holds pre-scaled copies of each image at several resolutions, used for fast thumbnail + and preview generation — see [mipmap](https://en.wikipedia.org/wiki/Mipmap).) 2. Mark all pipes `DIRTY` and set `loading = TRUE`. 3. **`dt_iop_load_modules()`** builds the IOP module list (`dev->iop`). 4. **`dt_dev_read_history_ext()`** does everything else — defaults, presets, and history @@ -135,18 +140,21 @@ that happens inside `dt_dev_read_history_ext()`. ### Inside `dt_dev_read_history_ext()` (develop.c) -This function applies saved history by **building `dev->history` directly from the database**. -It does *not* call `pop_history_items`. The reset-then-replay `pop` path is the undo/redo -path (§3f), not the initial-load path. +This function applies saved history by **building `dev->history` directly from the database** — +it allocates one history item per stored row and appends it to the list. (This is a different +mechanism from the reset-then-replay path that undo/redo uses later; that one is described in +[section 3f](#3f-undo-redo-and-history-panel-navigation).) **For every image** — this is the `if(!no_image)` block, and it runs whether or not the image has saved history: - Clear the scratch `memory.history` table. -- **`dt_dev_reset_chroma()`** clears part of the shared WB state (§5 lists exactly which - fields): `temperature` and `adaptation` become `NULL`, and `wb_coeffs[]` is reset to 1.0. +- **`dt_dev_reset_chroma()`** clears part of the shared WB state + ([section 5](#5-pipeline-ordering-asymmetry) lists exactly which fields): `temperature` and + `adaptation` become `NULL`, and `wb_coeffs[]` is reset to 1.0. - **`_dt_dev_load_pipeline_defaults()`** calls `dt_iop_reload_defaults()` on every module in - **reverse** pipe order (§5 explains the direction). This sets each module's image-specific + **reverse** pipe order ([section 5](#5-pipeline-ordering-asymmetry) explains the direction). + This sets each module's image-specific `default_params`. Because it goes through the wrapper, it also copies them into `params` (via `dt_iop_load_default_params()`). Temperature additionally populates `dev->chroma.as_shot[]` and `D65coeffs[]` as a side effect. @@ -189,11 +197,13 @@ This direct call is a deliberate wrapper bypass: it skips the framework's wholes `default_params → params` copy (`dt_iop_load_default_params()`). It still refreshes `default_params` and the `dev->chroma` reference data, and temperature's own body also writes a few `self->params` fields directly (`preset`, `late_correction`). So `params` is partially -updated, not fully resynced — see the `reload_defaults` caveat in `IOP_Module_API.md`. +updated, not fully resynced — see the `reload_defaults` caveat in +[IOP_Module_API.md](IOP_Module_API.md#reload_defaults---per-image-defaults). The function then re-reads `history_end` from `main.images`, and — when the GUI is attached — calls `dt_dev_pipe_synch_all()` and `dt_dev_invalidate_all()` so the pipes will commit and -reprocess. The pipes stay `DIRTY`, and the pipeline threads run on the next redraw (§6). +reprocess. The pipes stay `DIRTY`, and the pipeline threads run on the next redraw +([section 6](#6-pipeline-execution-detail)). --- @@ -276,7 +286,7 @@ So a removed instance's history entries are **actively deleted**, not merely ski headless context, where `gui_attached` is false, this pruning block does not run — but interactive removal is the case this document covers. -### 3f. Undo / redo / history-panel navigation +### 3f. Undo, redo, and history-panel navigation This is a **distinct path** from the initial image load. During an active editing session it is the main way `module->params` are rewritten from saved entries. @@ -299,19 +309,14 @@ re-commits after the replay. ### 4a. Temperature (WB) → ChannelMixerRGB (through `dev->chroma`) -There is no signal; synchronization is implicit and ordered by iop_order: - -- During `synch_all` or `synch_top`, `dt_iop_commit_params()` runs in pipe order. -- `temperature.commit_params()` writes `dev->chroma.wb_coeffs[]`, `chr->late_correction`, and - the `chr->temperature` pointer. -- `channelmixerrgb.commit_params()` runs **after** it and reads `wb_coeffs[]` to build its - chromatic-adaptation matrix. -- As a once-per-image-load side effect, `temperature.reload_defaults()` writes - `dev->chroma.as_shot[]` and `D65coeffs[]` **unconditionally** — even when temperature is - disabled or absent from the history — because channelmixerrgb needs the camera's native WB - reference to compute its default illuminant. -- For late correction, temperature sets `chr->late_correction` and channelmixerrgb reads it. - A WB-mode switch raises `SYNCH`, so both re-commit in order. +There is no signal; synchronization is implicit and ordered by iop_order. Because temperature is +upstream of channelmixerrgb, a full `synch_all` runs `commit_params()` in pipe order: +`temperature.commit_params()` writes `dev->chroma.wb_coeffs[]` first and +`channelmixerrgb.commit_params()` reads it afterward to build its chromatic-adaptation matrix. +(`synch_top` re-commits only the topmost history item, so it is not evidence that an upstream +producer has just run.) The full producer/consumer field map, the once-per-load `reload_defaults` +side effects, and why reverse-order default loading stays correct are covered in +[wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md). ### 4b. Color-profile change (colorin / colorout) @@ -326,16 +331,14 @@ and everything downstream reprocesses. ## 5. Pipeline ordering asymmetry Two operations walk the module list in **opposite** directions, which is a common source of -confusion. +confusion. (See also the +[ordering-asymmetry note](pixelpipe_architecture.md#pipeline-ordering-asymmetry) in +pixelpipe_architecture.md.) -- **`commit_params()` (through `synch_all`)** iterates the history in history-list order. For - auto-applied presets this is effectively pipe order: temperature commits first and writes - `dev->chroma.wb_coeffs[]`; channelmixerrgb commits second and reads it. One caveat: - `synch_top` re-commits **only** the topmost history item, and relies on `wb_coeffs` still - being present in `dev->chroma` from a previous full commit. If temperature has never been - committed in the current session, `wb_coeffs` may be wrong on a `synch_top`. -- **`_dt_dev_load_pipeline_defaults()`** iterates in **reverse** pipe order (last module - first): +- **`commit_params()` (through `synch_all`)** iterates the history in history-list order, which + is effectively pipe order: an upstream module commits before a downstream one, so any state it + writes into shared structures is in place when the downstream module reads it. +- **`_dt_dev_load_pipeline_defaults()`** iterates in **reverse** pipe order (last module first): ```c for(const GList *modules = g_list_last(dev->iop); @@ -346,74 +349,20 @@ confusion. } ``` - So channelmixerrgb's `reload_defaults()` runs **before** temperature's. - -### Why reverse? (and what it is *not*) - -The source gives no rationale for the reverse direction. Checked against the code, it is -**not** a chromatic-adaptation (CAT) deduplication mechanism, despite the intuitive theory -that "the last instance should claim the CAT first." CAT-token ownership is in fact -**order-independent**, for three reasons: - -- `_declare_cat_on_pipe()` (channelmixerrgb.c) resolves a conflict between multiple - `channelmixerrgb` instances through `dt_iop_is_first_instance()`. Its conflict branch can - take the token away from a previous claimant: - - ```c - if(chr->adaptation == NULL) - chr->adaptation = self; // first to register wins - else if(chr->adaptation == self) - ; // already ours - else if(dt_iop_is_first_instance(self->dev->iop, self)) - chr->adaptation = self; // earlier in the pipe: take over - ``` - - So the **first instance in `dev->iop` always ends up owning the token**, regardless of who - registered first. Tracing two eligible instances shows that forward iteration would dedup - the *defaults* more cleanly than reverse: reverse leaves both instances with active-CAT - defaults until a later commit re-resolves the token. -- `_declare_cat_on_pipe()` returns early when `gui_data == NULL`, so it is a no-op during - headless default loading. It therefore cannot be the reason the loop runs in reverse. -- Duplicate `channelmixerrgb` instances do not exist yet when - `_dt_dev_load_pipeline_defaults()` runs at image load. They are created later, by user - action or history replay. - -So the safe statement is: defaults load in reverse pipe order, and CAT dedup is handled by -`_declare_cat_on_pipe()` plus `dt_iop_is_first_instance()` — **not** by the loop direction. -The real hazard the reverse order creates is producer/consumer staleness for modules that read -shared state in `reload_defaults()`, described next. - -### What `dt_dev_reset_chroma()` actually resets - -`dt_dev_reset_chroma()` (develop.c) clears only three fields: - -```c -chr->adaptation = NULL; -chr->temperature = NULL; -for(int c = 0; c < 4; c++) chr->wb_coeffs[c] = 1.0f; -``` - -It does **not** reset `D65coeffs`, `as_shot`, or `late_correction`. Those are initialized only -by `dt_dev_init_chroma()`, at dev-context creation — not on each image switch. - -`channelmixerrgb.reload_defaults()` calls `_get_d65_correction_ratios()`, which reads -`dev->chroma.D65coeffs[]` and `as_shot[]`. Because temperature's `reload_defaults()` has not -run yet (reverse order) and `dt_dev_reset_chroma()` leaves those fields alone, channelmixerrgb -can read **stale values from the previously loaded image**. The visible effect: the default -illuminant xy that the Reset button restores can be computed from the wrong camera's daylight -reference right after an image switch. - -The `wb_coeffs` staleness (which affects active params through `commit_params()`) **is** fixed -by `dt_dev_reset_chroma()`. The `D65coeffs` / `as_shot` / `late_correction` staleness (which -affects **defaults only**) remains. It is a known limitation, not a mitigated bug. + So a downstream module's `reload_defaults()` runs **before** an upstream one's. The source + documents no rationale for the direction. ### Developer rule A module's `reload_defaults()` **cannot** assume that earlier-in-pipe modules have already -populated the shared `dev->chroma` state. The field that `dt_dev_reset_chroma()` clears -(`wb_coeffs`) is safe to read, because it holds a neutral 1.0. The fields it does **not** clear -(`D65coeffs`, `as_shot`, `late_correction`) may still hold stale data from the previous image -when `reload_defaults()` runs. +populated shared state (such as `dev->chroma`), because under reverse iteration they have not run +yet. Shared state that a `reload_defaults()` depends on must therefore be **reset to a neutral +value before the reverse default-load** — which the framework does via `dt_dev_reset_chroma()` +just before `_dt_dev_load_pipeline_defaults()`. + +The worked example — how white balance and color calibration share `dev->chroma`, and why this +reset keeps color calibration's defaults image-local despite the reverse order — is in +[wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md). --- @@ -426,7 +375,9 @@ How pixels actually flow once the pipes are marked dirty: - `dt_dev_pixelpipe_process()` calls `_dev_pixelpipe_process_rec()` (pixelpipe_hb.c), which recurses from the last module back toward the input, pulling each module's input on demand. - For each module, the cache key is the **cumulative hash** of all upstream `piece->hash` - values (each set in `dt_iop_commit_params()` from op, instance, params, and blend_params), + values (each set in + [`dt_iop_commit_params()`](IOP_Module_API.md#commit_params---transform-ui-parameters-into-processing-data) + from op, instance, params, and blend_params), plus the color-profile information. On a key match, the cached output is reused and `process()` is skipped. On a miss, `process()` (or `process_cl()` on the GPU) runs and the result is stored. @@ -455,8 +406,7 @@ section above. | `dt_dev_load_image` | Top-level image load | | `dt_dev_read_history_ext` | Loads defaults/presets and builds `dev->history` from the DB | | `_dt_dev_load_pipeline_defaults` | Calls `reload_defaults` on all modules, in reverse pipe order | -| `dt_dev_reset_chroma` | Clears `adaptation`, `temperature`, `wb_coeffs` only | -| `dt_dev_init_chroma` | Initializes the full `dev->chroma`, at dev-context creation | +| `dt_dev_reset_chroma` | Resets shared WB state to neutral before reverse default-load | | `dt_dev_add_history_item` / `_dev_add_history_item_ext` | Add/update a history entry; choose `TOP_CHANGED` vs `SYNCH` (the `focus_hash` test lives here) | | `dt_dev_pop_history_items` / `_ext` | Reset to defaults, then replay history (used by undo/redo) | | `dt_dev_reload_history_items` | Undo/redo and history-panel navigation | @@ -473,7 +423,6 @@ section above. | `dt_iop_load_default_params` | Copies `default_params` into `params` | | `dt_iop_commit_params` | Translates params into `piece->data`; sets `piece->hash` | | `dt_iop_gui_changed` | Slider/widget change entry point | -| `dt_iop_is_first_instance` | True for the earliest instance of an op in `dev->iop` | | `_gui_off_callback` | Enable/disable toggle handler | | `_gui_reset_callback` | Reset-to-defaults handler | | `dt_iop_gui_duplicate` | Add a module instance | @@ -493,12 +442,16 @@ section above. | Symbol | File | Role | |---|---|---| | Basic-hash helper (cumulative cache hash) | pixelpipe_cache.c | Builds the cache key from image id, profiles, and all modules up to a position | -| `_declare_cat_on_pipe` | channelmixerrgb.c | Claims/resolves the CAT token | -| `temperature.commit_params` | temperature.c | Writes `dev->chroma.wb_coeffs` | -| `channelmixerrgb.commit_params` | channelmixerrgb.c | Reads `dev->chroma` | + +For the white-balance ↔ color-calibration symbols (`temperature.*` / `channelmixerrgb.*` and the +`dev->chroma` helpers), see +[wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md#key-symbols). ## See also -- `IOP_Module_API.md` — the callback API reference (signatures, the two `reload_defaults` - jobs). -- `pixelpipe_architecture.md` — pipeline architecture, including the ordering-asymmetry note. +- [IOP_Module_API.md](IOP_Module_API.md) — the callback API reference (signatures, the two + [`reload_defaults`](IOP_Module_API.md#reload_defaults---per-image-defaults) jobs). +- [pixelpipe_architecture.md](pixelpipe_architecture.md) — pipeline architecture, including the + [ordering-asymmetry note](pixelpipe_architecture.md#pipeline-ordering-asymmetry). +- [wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md) — the white-balance ↔ + color-calibration interaction through `dev->chroma` (worked example of the ordering principle). diff --git a/dev-doc/README.md b/dev-doc/README.md index 7c071ce43a7c..08acb26fa396 100644 --- a/dev-doc/README.md +++ b/dev-doc/README.md @@ -9,6 +9,7 @@ This guide covers building Image Operation (IOP) modules for darktable's darkroo |------|-------------| | **[IOP_Module_API.md](IOP_Module_API.md)** | Module API reference: params_t vs data_t, processing, commit_params, lifecycle | | **[pixelpipe_architecture.md](pixelpipe_architecture.md)** | Pixelpipe data flow, caching, ROI, ordering asymmetry | +| **[Module_Lifecycle.md](Module_Lifecycle.md)** | When callbacks fire: image load, edits, history replay, pipe flags, cross-module data flow | | **[introspection.md](introspection.md)** | Introspection system for parameters and GUI | | **[Shortcuts.md](Shortcuts.md)** | The Action/Shortcut system and `dt_action_def_t` | | **[Module_Groups.md](Module_Groups.md)** | Module grouping explanation and `default_group()` | diff --git a/dev-doc/pixelpipe_architecture.md b/dev-doc/pixelpipe_architecture.md index d7f2728ebd0b..69085d7ae138 100644 --- a/dev-doc/pixelpipe_architecture.md +++ b/dev-doc/pixelpipe_architecture.md @@ -4,6 +4,10 @@ The **pixelpipe** is the core image processing engine of darktable. It is respon ## Core Structures +### `dt_develop_t` +Defined in `src/develop/develop.h`. +This is the main darkroom session state — one per open image. Besides the pixelpipes (below) it holds the module list (`dev->iop`, a `GList` of `dt_iop_module_t`), the history stack (`dev->history` plus the `dev->history_end` cursor), and shared inter-module state such as `dev->chroma` (white-balance / chromatic-adaptation data passed from `temperature.c` to `channelmixerrgb.c`). Most lifecycle operations — image load, history replay, parameter commits — act on this structure. For the event-by-event walkthrough see [Module_Lifecycle.md](Module_Lifecycle.md). + ### `dt_dev_pixelpipe_t` Defined in `src/develop/pixelpipe_hb.h`. This structure represents a single instance of a processing pipeline. A `dt_develop_t` (the main development state) holds several pipes: @@ -105,9 +109,9 @@ Two key pipeline operations iterate modules in **different** orders: - **`commit_params()`** runs in **forward** pipe order (e.g., temperature before channelmixerrgb). This is the normal processing direction. - **`_dt_dev_load_pipeline_defaults()`** runs in **reverse** pipe order (e.g., channelmixerrgb before temperature). This happens during history reset and default loading. -This asymmetry matters for modules that communicate via shared state. For example, `temperature.c` writes white balance coefficients into `dev->chroma.wb_coeffs`, and `channelmixerrgb.c` reads them during `commit_params()`. During forward processing, temperature commits first and the data is available. But during reverse-order default loading, channelmixerrgb runs first — before temperature has written its values. This caused a bug where stale values from a previous image or history state influenced the defaults. +This asymmetry matters for modules that communicate via shared state. For example, `temperature.c` writes white balance coefficients into `dev->chroma.wb_coeffs`, and `channelmixerrgb.c` reads them during `commit_params()`. During forward processing, temperature commits first and the data is available. During reverse-order default loading, channelmixerrgb runs first — before temperature has refreshed its values — so any shared state a `reload_defaults()` depends on must be reset to a neutral value beforehand. -**Consequence:** Shared state (like `dev->chroma`) must be properly reset before reverse-order iteration to prevent this class of ordering-dependent bug. +**Consequence:** Shared state (like `dev->chroma`) must be reset before reverse-order iteration. The framework does this via `dt_dev_reset_chroma()` immediately before `_dt_dev_load_pipeline_defaults()`; the neutral `wb_coeffs` it writes is what keeps the dependent defaults image-local. For the full white-balance ↔ color-calibration interaction, see [iop/wb_and_colorcalibration](iop/wb_and_colorcalibration/README.md). ## Introspection Connection From 5baae497ba6fc5ace8771508b64dc3e38a595213 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 31 May 2026 20:24:04 +0200 Subject: [PATCH 13/18] wb: removed 'as-shot to reference' (same as 'as shot' + 'prepare data for color calibration' (late_correction)); fixed behaviour when WB disabled; removed unused code --- src/common/exif.cc | 2 +- src/common/illuminants.h | 25 --------- src/develop/develop.h | 4 +- src/iop/temperature.c | 106 +++++++++++++-------------------------- 4 files changed, 37 insertions(+), 100 deletions(-) diff --git a/src/common/exif.cc b/src/common/exif.cc index ccc867df64a4..64f1cbebc3aa 100644 --- a/src/common/exif.cc +++ b/src/common/exif.cc @@ -2061,7 +2061,7 @@ static bool _exif_decode_exif_data(dt_image_t *img, Exiv2::ExifData &exifData) dt_dng_illuminant_t illu[3] = { DT_LS_Unknown, DT_LS_Unknown, DT_LS_Unknown }; // make sure for later testing fallback later via - // `find_temperature_from_raw_coeffs` if there is no valid illuminant + // `find_illuminant_xy_from_as_shot_coeffs` if there is no valid illuminant dt_mark_colormatrix_invalid(&img->d65_color_matrix[0]); #if EXIV2_TEST_VERSION(0,27,4) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index fc74b0e1710b..be9373869790 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -513,31 +513,6 @@ static inline float planckian_normal(const float x, const float t) } -DT_OMP_DECLARE_SIMD() -static inline void blackbody_xy_to_tinted_xy(const float x, const float y, const float t, const float tint, - float *x_out, float *y_out) -{ - // Move further away from planckian locus in the orthogonal direction, by an amount of "tint" - const float n = planckian_normal(x, t); - const float norm = sqrtf(1.f + n * n); - *x_out = x + tint * n / norm; - *y_out = y - tint / norm; -} - - -DT_OMP_DECLARE_SIMD() -static inline float get_tint_from_tinted_xy(const float x, const float y, const float t) -{ - // Find the distance between planckian locus and arbitrary x y chromaticity in the orthogonal direction - const float n = planckian_normal(x, t); - const float norm = sqrtf(1.f + n * n); - float x_bb, y_bb; - CCT_to_xy_blackbody(t, &x_bb, &y_bb); - const float tint = -(y - y_bb) * norm; - return tint; -} - - DT_OMP_DECLARE_SIMD() static inline void xy_to_uv(const float xy[2], float uv[2]) { diff --git a/src/develop/develop.h b/src/develop/develop.h index 731fbd0f58c0..8ef369fbf171 100644 --- a/src/develop/develop.h +++ b/src/develop/develop.h @@ -147,8 +147,8 @@ typedef struct dt_dev_viewport_t - the currently used wb_coeffs in temperature module - D65coeffs and as_shot are read from exif data f) - late_correction set by temperature if we want to process data as following - If we use the new DT_IOP_TEMP_D65_LATE mode or enable this in user modes of temperature.c - and don't have any temp parameters changes later we can calc correction ratios + If we enable this for non-D65 modes of temperature.c and don't have any + temp parameters changes later we can calc correction ratios to take white-balanced (neutralized) data to D65 */ typedef struct dt_dev_chroma_t diff --git a/src/iop/temperature.c b/src/iop/temperature.c index 35145a1146cd..65b87e5e4ab1 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -53,7 +53,7 @@ DT_MODULE_INTROSPECTION(5, dt_iop_temperature_params_t) #define DT_IOP_LOWEST_TINT 0.135 #define DT_IOP_HIGHEST_TINT 2.326 -#define DT_IOP_NUM_OF_STD_TEMP_PRESETS 5 +#define DT_IOP_NUM_OF_STD_TEMP_PRESETS 4 // If you reorder presets combo, change this consts #define DT_IOP_TEMP_UNKNOWN -1 @@ -61,7 +61,6 @@ DT_MODULE_INTROSPECTION(5, dt_iop_temperature_params_t) #define DT_IOP_TEMP_SPOT 1 #define DT_IOP_TEMP_USER 2 #define DT_IOP_TEMP_D65 3 -#define DT_IOP_TEMP_D65_LATE 4 static void _gui_sliders_update(dt_iop_module_t *self); @@ -85,9 +84,7 @@ typedef struct dt_iop_temperature_gui_data_t GtkWidget *btn_asshot; //As Shot GtkWidget *btn_user; GtkWidget *btn_d65; - GtkWidget *btn_d65_late; GtkWidget *temp_label; - GtkWidget *balance_label; GtkWidget *check_late_correction; int preset_cnt; int preset_num[54]; @@ -194,11 +191,21 @@ int legacy_params(dt_iop_module_t *self, } if(old_version == 4) { + const int legacy_d65_late = 4; const dt_iop_temperature_params_v4_t *o = (dt_iop_temperature_params_v4_t *)old_params; dt_iop_temperature_params_v5_t *n = malloc(sizeof(dt_iop_temperature_params_v5_t)); memcpy(n, o, sizeof(dt_iop_temperature_params_v4_t)); - n->late_correction = FALSE; + if(o->preset == legacy_d65_late) + { + n->preset = DT_IOP_TEMP_AS_SHOT; + n->late_correction = TRUE; + } + else + { + n->preset = o->preset > legacy_d65_late ? o->preset - 1 : o->preset; + n->late_correction = FALSE; + } *new_params = n; *new_params_size = sizeof(dt_iop_temperature_params_v5_t); *new_version = 5; @@ -742,30 +749,21 @@ void commit_params(dt_iop_module_t *self, d->preset = p->preset; - gboolean effective_late_correction = FALSE; - - if(p->preset == DT_IOP_TEMP_D65_LATE) - effective_late_correction = TRUE; - else if(p->preset == DT_IOP_TEMP_D65) - effective_late_correction = FALSE; - else - effective_late_correction = p->late_correction; + const gboolean effective_late_correction = + p->preset == DT_IOP_TEMP_D65 ? FALSE : p->late_correction; d->late_correction = effective_late_correction; - chr->late_correction = effective_late_correction; + // When the WB module is disabled it applies no coefficients (wb_coeffs are + // published as 1.0 above), so it makes no promise that the data is white + // balanced. Do not ask colorin (or any other consumer) to "finish" a + // correction that never started, otherwise disabling WB would still pull the + // image to D65 via D65coeffs/1.0. + chr->late_correction = piece->enabled ? effective_late_correction : FALSE; - chr->temperature = piece->enabled ? self : NULL; /* 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. */ - if(p->preset == DT_IOP_TEMP_D65_LATE) - chr->late_correction = TRUE; - else if(p->preset == DT_IOP_TEMP_D65) - chr->late_correction = FALSE; - else - chr->late_correction = p->late_correction; - chr->temperature = piece->enabled ? self : NULL; if(dt_pipe_is_preview(pipe) && !piece->enabled) dt_iop_set_module_trouble_message(self, NULL, NULL, NULL); @@ -1198,7 +1196,6 @@ static inline const char *_preset_to_str(const int preset) case DT_IOP_TEMP_SPOT: return "by spot"; case DT_IOP_TEMP_USER: return "user defined"; case DT_IOP_TEMP_D65: return "camera reference"; - case DT_IOP_TEMP_D65_LATE: return "as shot to reference"; default: return "other"; } } @@ -1209,10 +1206,8 @@ static void _update_preset(dt_iop_module_t *self, int mode) dt_dev_chroma_t *chr = &self->dev->chroma; dt_iop_temperature_gui_data_t *g = self->gui_data; - const gboolean is_current_reference = (p->preset == DT_IOP_TEMP_D65_LATE) || - (p->preset == DT_IOP_TEMP_D65); - const gboolean is_new_mode_manual = (mode != DT_IOP_TEMP_D65_LATE) && - (mode != DT_IOP_TEMP_D65); + const gboolean is_current_reference = p->preset == DT_IOP_TEMP_D65; + const gboolean is_new_mode_manual = mode != DT_IOP_TEMP_D65; if(is_current_reference && is_new_mode_manual) { @@ -1222,17 +1217,13 @@ static void _update_preset(dt_iop_module_t *self, int mode) p->preset = mode; - if(mode == DT_IOP_TEMP_D65_LATE) - { - chr->late_correction = TRUE; - } - else if(mode == DT_IOP_TEMP_D65) + if(mode == DT_IOP_TEMP_D65) { chr->late_correction = FALSE; } else { - // For manual modes, use the parameter + // For non-D65 modes, use the parameter chr->late_correction = p->late_correction; } @@ -1250,9 +1241,6 @@ void gui_update(dt_iop_module_t *self) { dt_iop_temperature_gui_data_t *g = self->gui_data; dt_iop_temperature_params_t *p = self->params; - dt_iop_temperature_params_t *d = self->default_params; - - d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; const gboolean true_monochrome = dt_image_monochrome_flags(&self->dev->image_storage) & DT_IMAGE_MONOCHROME; @@ -1285,13 +1273,7 @@ void gui_update(dt_iop_module_t *self) gboolean found = FALSE; const dt_dev_chroma_t *chr = &self->dev->chroma; // is this a "as shot" white balance? - if(dt_dev_equal_chroma((float *)p, chr->as_shot) && (p->preset == DT_IOP_TEMP_D65_LATE)) - { - dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_D65_LATE); - found = TRUE; - } - - else if(dt_dev_equal_chroma((float *)p, chr->as_shot)) + if(dt_dev_equal_chroma((float *)p, chr->as_shot)) { dt_bauhaus_combobox_set(g->presets, DT_IOP_TEMP_AS_SHOT); p->preset = DT_IOP_TEMP_AS_SHOT; @@ -1439,8 +1421,6 @@ void gui_update(dt_iop_module_t *self) p->preset == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), p->preset == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - p->preset == DT_IOP_TEMP_D65_LATE); _color_temptint_sliders(self); _color_rgb_sliders(self); @@ -1593,8 +1573,8 @@ void reload_defaults(dt_iop_module_t *self) dt_iop_temperature_params_t *d = self->default_params; dt_iop_temperature_params_t *p = self->params; - d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; - d->late_correction = dt_is_scene_referred(); + d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; + d->late_correction = p->late_correction = dt_is_scene_referred(); float *dcoeffs = (float *)d; for_four_channels(k) @@ -1703,7 +1683,8 @@ void reload_defaults(dt_iop_module_t *self) { for_four_channels(k) dcoeffs[k] = as_shot[k]; - d->preset = p->preset = DT_IOP_TEMP_D65_LATE; + d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; + d->late_correction = p->late_correction = TRUE; } else { @@ -1713,6 +1694,7 @@ void reload_defaults(dt_iop_module_t *self) dcoeffs[2] = coeffs[2]/coeffs[1]; dcoeffs[3] = coeffs[3]/coeffs[1]; dcoeffs[1] = 1.0f; + d->late_correction = p->late_correction = FALSE; } } } @@ -1750,7 +1732,6 @@ void reload_defaults(dt_iop_module_t *self) dt_bauhaus_combobox_add(g->presets, C_("white balance", "user modified")); // old "camera neutral", reason: better matches intent dt_bauhaus_combobox_add(g->presets, C_("white balance", "camera reference")); - dt_bauhaus_combobox_add(g->presets, C_("white balance", "as shot to reference")); g->preset_cnt = DT_IOP_NUM_OF_STD_TEMP_PRESETS; memset(g->preset_num, 0, sizeof(g->preset_num)); @@ -1760,7 +1741,6 @@ void reload_defaults(dt_iop_module_t *self) _gui_sliders_update(self); dt_bauhaus_combobox_set(g->presets, p->preset); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), p->preset == DT_IOP_TEMP_D65_LATE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_asshot), p->preset == DT_IOP_TEMP_AS_SHOT); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_user), FALSE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), FALSE); @@ -1836,7 +1816,6 @@ static gboolean _btn_toggled(GtkWidget *togglebutton, const int preset = togglebutton == g->btn_asshot ? DT_IOP_TEMP_AS_SHOT : togglebutton == g->btn_d65 ? DT_IOP_TEMP_D65 : - togglebutton == g->btn_d65_late ? DT_IOP_TEMP_D65_LATE : togglebutton == g->btn_user ? DT_IOP_TEMP_USER : 0; if(!gtk_toggle_button_get_active(GTK_TOGGLE_BUTTON(togglebutton))) @@ -1879,8 +1858,6 @@ static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) pos == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), pos == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - pos == DT_IOP_TEMP_D65_LATE); gboolean show_finetune = FALSE; dt_dev_chroma_t *chr = &self->dev->chroma; @@ -1910,9 +1887,6 @@ static void _preset_tune_callback(GtkWidget *widget, dt_iop_module_t *self) case DT_IOP_TEMP_D65: // camera reference d65 _temp_params_from_array(p, chr->D65coeffs); break; - case DT_IOP_TEMP_D65_LATE: // as shot wb just for now - _temp_params_from_array(p, chr->as_shot); - break; default: // camera WB presets { gboolean found = FALSE; @@ -2188,21 +2162,11 @@ void gui_init(dt_iop_module_t *self) (g->btn_d65, _("set white balance to camera reference point\nin most cases it should be D65")); - g->btn_d65_late = dt_iop_togglebutton_new(self, - N_("settings"), - N_("as shot to reference"), NULL, - G_CALLBACK(_btn_toggled), FALSE, 0, 0, - dtgtk_cairo_paint_bulb_mod, NULL); - gtk_widget_set_tooltip_text - (g->btn_d65_late, - _("set white balance to as shot and later correct to camera reference point,\nin most cases it should be D65")); - // put buttons at top. fill later. g->buttonbar = dt_gui_hbox(dt_gui_expand(g->btn_asshot), dt_gui_expand(g->colorpicker), dt_gui_expand(g->btn_user), - dt_gui_expand(g->btn_d65), - dt_gui_expand(g->btn_d65_late)); + dt_gui_expand(g->btn_d65)); dt_gui_add_class(g->buttonbar, "dt_iop_toggle"); g->presets = dt_bauhaus_combobox_new(self); @@ -2253,7 +2217,7 @@ void gui_init(dt_iop_module_t *self) gtk_widget_set_tooltip_text(g->check_late_correction, _("ensures the white balance coefficients are treated as a reference for the color calibration module.\n" - "keep this checked if you use the modern scene-referred workflow but want to manually adjust white balance.")); + "keep this checked for non-reference white balance in the modern scene-referred workflow.")); GtkWidget *box_enabled = dt_gui_vbox(g->buttonbar, g->check_late_correction, @@ -2318,9 +2282,9 @@ void gui_cleanup(dt_iop_module_t *self) void gui_reset(dt_iop_module_t *self) { dt_iop_temperature_gui_data_t *g = self->gui_data; - dt_iop_temperature_params_t *d = self->default_params; + dt_iop_temperature_params_t *p = self->params; - const int preset = d->preset = dt_is_scene_referred() ? DT_IOP_TEMP_D65_LATE : DT_IOP_TEMP_AS_SHOT; + const int preset = p->preset; dt_iop_color_picker_reset(self, TRUE); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_asshot), @@ -2329,8 +2293,6 @@ void gui_reset(dt_iop_module_t *self) preset == DT_IOP_TEMP_USER); gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65), preset == DT_IOP_TEMP_D65); - gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(g->btn_d65_late), - preset == DT_IOP_TEMP_D65_LATE); _color_finetuning_slider(self); _color_rgb_sliders(self); From 7f51e3acf0c9c5cd5a813d0307e1f3e515362dd7 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sat, 6 Jun 2026 22:26:04 +0200 Subject: [PATCH 14/18] 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 --- src/common/illuminants.h | 6 ++-- src/iop/channelmixerrgb.c | 71 +++++++++++++++++++++------------------ src/iop/temperature.c | 16 +++++---- 3 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index be9373869790..b38f8b4594bb 100644 --- a/src/common/illuminants.h +++ b/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); diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 1c1b1f6a5c8e..84267c391b2d 100644 --- a/src/iop/channelmixerrgb.c +++ b/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 (details)"), _("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 (details)"), _("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); } } diff --git a/src/iop/temperature.c b/src/iop/temperature.c index 65b87e5e4ab1..0556fb0bd675 100644 --- a/src/iop/temperature.c +++ b/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, From 051e6e2fd41bea7962c14fdd9ff9691453255862 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 7 Jun 2026 09:31:25 +0200 Subject: [PATCH 15/18] wb: don't modify p->x, p->y in _preview_pipe_finished_callback; switch left-over gui->reset to DT_ENTER|LEAVE_GUI_UPDATE --- src/iop/channelmixerrgb.c | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 84267c391b2d..c48e7f2fa371 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -3105,23 +3105,16 @@ static void _preview_pipe_finished_callback(gpointer instance, dt_iop_module_t * dt_iop_channelmixer_rgb_params_t *p = self->params; if(p->illuminant == DT_ILLUMINANT_FROM_WB) { - // The WB coefficients might have changed in the temperature module. - // We need to re-calculate the chromaticity (x, y) to update the GUI feedback. - float x = p->x; - float y = p->y; + // WB coefficients might have changed temperature.c; recalculate xy and update the GUI + float x, y; if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) { - p->x = x; - p->y = y; - - darktable.gui->reset++; - // Update derived GUI widgets (CCT label, color patch) + DT_ENTER_GUI_UPDATE(); _update_approx_cct(self); _update_illuminant_color(self); - // Update Hue/Chroma sliders to match the new coordinates - dt_aligned_pixel_t xyY = { p->x, p->y, 1.f }; + dt_aligned_pixel_t xyY = { x, y, 1.f }; dt_aligned_pixel_t Lch; dt_xyY_to_Lch(xyY, Lch); @@ -3129,7 +3122,7 @@ static void _preview_pipe_finished_callback(gpointer instance, dt_iop_module_t * dt_bauhaus_slider_set(g->illum_x, rad2degf(Lch[2])); dt_bauhaus_slider_set(g->illum_y, Lch[1]); - darktable.gui->reset--; + DT_LEAVE_GUI_UPDATE(); } } From c24c307e3282d28947f04ef70b9cbf9b995b9d41 Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 7 Jun 2026 09:34:40 +0200 Subject: [PATCH 16/18] wb: minor clean-up in channelmixerrgb (GUI struct field names for clarity), no need to allocate + copy xy in illuminants.h find_illuminant_xy_from_wb_coeffs; no need for 0 check after isnormal --- src/common/illuminants.h | 7 ++----- src/iop/channelmixerrgb.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/common/illuminants.h b/src/common/illuminants.h index b38f8b4594bb..42f27f19d04e 100644 --- a/src/common/illuminants.h +++ b/src/common/illuminants.h @@ -444,7 +444,7 @@ static gboolean get_CAM_to_XYZ(const dt_image_t * img, float(* CAM_to_XYZ)[3]) static inline gboolean _wb_coeffs_invalid(const dt_aligned_pixel_t wb_coeffs, const int num_coeffs) { for(int k = 0; k < num_coeffs; k++) - if(!dt_isnormal(wb_coeffs[k]) || wb_coeffs[k] == 0.0f) return TRUE; + if(!dt_isnormal(wb_coeffs[k])) return TRUE; return FALSE; } @@ -463,10 +463,7 @@ static gboolean find_illuminant_xy_from_wb_coeffs(const dt_image_t *img, const d return FALSE; } - float x, y; - WB_coeffs_to_illuminant_xy(CAM_to_XYZ, wb_coeffs, &x, &y); - *chroma_x = x; - *chroma_y = y; + WB_coeffs_to_illuminant_xy(CAM_to_XYZ, wb_coeffs, chroma_x, chroma_y); return TRUE; } diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index c48e7f2fa371..7ce18adcb722 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -148,8 +148,8 @@ typedef struct dt_iop_channelmixer_rgb_gui_data_t GtkWidget *scale_grey_R, *scale_grey_G, *scale_grey_B; GtkWidget *normalize_R, *normalize_G, *normalize_B, *normalize_sat, *normalize_light, *normalize_grey; GtkWidget *color_picker; - float xy[2]; - float XYZ[4]; + float colorchecker_xy[2]; + float ai_wb_XYZ[4]; point_t box[4]; // the current coordinates, possibly non rectangle, of the bounding box for the color checker point_t ideal_box[4]; // the desired coordinates of the perfect rectangle bounding box for the color checker @@ -1724,8 +1724,8 @@ static void _extract_color_checker(const float *const restrict in, dt_D50_XYZ_to_xyY(illuminant_XYZ, illuminant_xyY); // save the illuminant in GUI struct for commit - g->xy[0] = illuminant_xyY[0]; - g->xy[1] = illuminant_xyY[1]; + g->colorchecker_xy[0] = illuminant_xyY[0]; + g->colorchecker_xy[1] = illuminant_xyY[1]; // and recompute back the LMS to be sure we use the parameters that // will be computed later @@ -2207,7 +2207,7 @@ void process(dt_iop_module_t *self, // as scratch space since we will be overwriting it afterwards // anyway _auto_detect_WB(in, out, data->illuminant_type, roi_in->width, roi_in->height, - ch, RGB_to_XYZ, g->XYZ); + ch, RGB_to_XYZ, g->ai_wb_XYZ); dt_dev_pixelpipe_cache_invalidate_later(piece->pipe, self->iop_order, "AI RGB: "); dt_iop_gui_leave_critical_section(self); } @@ -3002,8 +3002,8 @@ static void _commit_profile_callback(GtkWidget *widget, dt_iop_gui_enter_critical_section(self); - p->x = g->xy[0]; - p->y = g->xy[1]; + p->x = g->colorchecker_xy[0]; + p->y = g->colorchecker_xy[1]; p->illuminant = DT_ILLUMINANT_CUSTOM; _check_if_close_to_daylight(p->x, p->y, &p->temperature, NULL, NULL); @@ -3063,8 +3063,8 @@ static void _develop_ui_pipe_finished_callback(gpointer instance, dt_iop_module_ return; dt_iop_gui_enter_critical_section(self); - p->x = g->XYZ[0]; - p->y = g->XYZ[1]; + p->x = g->ai_wb_XYZ[0]; + p->y = g->ai_wb_XYZ[1]; dt_iop_gui_leave_critical_section(self); _check_if_close_to_daylight(p->x, p->y, @@ -4548,7 +4548,7 @@ void gui_init(dt_iop_module_t *self) g->delta_E_in = NULL; g->delta_E_label_text = NULL; - g->XYZ[0] = NAN; + g->ai_wb_XYZ[0] = NAN; #ifdef AI_ACTIVATED DT_CONTROL_SIGNAL_HANDLE(DT_SIGNAL_DEVELOP_UI_PIPE_FINISHED, _develop_ui_pipe_finished_callback); From a10e8e03e8dbc58a093d247cdc8107e98268ceaf Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 7 Jun 2026 10:44:32 +0200 Subject: [PATCH 17/18] wb: opposed.c hash: late_correction is a param of temperature (is before opposed), so already part of hash --- src/iop/hlreconstruct/opposed.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/iop/hlreconstruct/opposed.c b/src/iop/hlreconstruct/opposed.c index fb198a830db8..89ad86555609 100644 --- a/src/iop/hlreconstruct/opposed.c +++ b/src/iop/hlreconstruct/opposed.c @@ -42,8 +42,7 @@ static dt_hash_t _opposed_hash(dt_dev_pixelpipe_iop_t *piece) { const dt_iop_highlights_data_t *d = piece->data; dt_hash_t hash = dt_dev_pixelpipe_piece_hash(piece, NULL, FALSE); - hash = dt_hash(hash, &d->clip, sizeof(d->clip)); - return dt_hash(hash, &piece->module->dev->chroma.late_correction, sizeof(int)); + return dt_hash(hash, &d->clip, sizeof(d->clip)); } static inline float _calc_linear_refavg(const float *in, const int color) From ff4c93fbb8619d83a5b50d28765f8bd4bb337e5d Mon Sep 17 00:00:00 2001 From: Kofa Date: Sun, 7 Jun 2026 15:18:26 +0200 Subject: [PATCH 18/18] wb: tiny clean-up: unnecessary parens, accidental duplication --- src/iop/channelmixerrgb.c | 10 +++++----- src/iop/temperature.c | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/iop/channelmixerrgb.c b/src/iop/channelmixerrgb.c index 7ce18adcb722..da2b48b0cf1b 100644 --- a/src/iop/channelmixerrgb.c +++ b/src/iop/channelmixerrgb.c @@ -2238,7 +2238,7 @@ void process(dt_iop_module_t *self, dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &x, &y)) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2255,7 +2255,7 @@ void process(dt_iop_module_t *self, // which don't come from the EXIF this time). float x, y; - if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) + if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2366,7 +2366,7 @@ int process_cl(dt_iop_module_t *self, dt_aligned_pixel_t correction_ratios; _get_d65_correction_ratios(self, correction_ratios); - if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &(x), &(y))) + if(find_illuminant_xy_from_as_shot_coeffs(&(self->dev->image_storage), correction_ratios, &x, &y)) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -2383,7 +2383,7 @@ int process_cl(dt_iop_module_t *self, // which don't come from the EXIF this time). float x, y; - if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &(x), &(y))) + if(find_illuminant_xy_from_wb_coeffs(&(self->dev->image_storage), self->dev->chroma.wb_coeffs, &x, &y)) { // Convert illuminant from xyY to XYZ dt_aligned_pixel_t XYZ; @@ -3201,7 +3201,7 @@ void commit_params(dt_iop_module_t *self, // if illuminant is from camera or user WB coefficients, x and y are set on-the-fly at // commit time, so we need to set adaptation too if(p->illuminant == DT_ILLUMINANT_CAMERA || p->illuminant == DT_ILLUMINANT_FROM_WB) - _check_if_close_to_daylight(x, y, NULL, NULL, &(d->adaptation)); + _check_if_close_to_daylight(x, y, NULL, NULL, &(d->adaptation)); d->illuminant_type = p->illuminant; // Convert illuminant from xyY to XYZ diff --git a/src/iop/temperature.c b/src/iop/temperature.c index 0556fb0bd675..befcbd3a70f6 100644 --- a/src/iop/temperature.c +++ b/src/iop/temperature.c @@ -1577,7 +1577,7 @@ void reload_defaults(dt_iop_module_t *self) dt_iop_temperature_params_t *d = self->default_params; dt_iop_temperature_params_t *p = self->params; - d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; + d->preset = DT_IOP_TEMP_AS_SHOT; d->late_correction = p->late_correction = dt_is_scene_referred(); float *dcoeffs = (float *)d; @@ -1663,7 +1663,7 @@ void reload_defaults(dt_iop_module_t *self) STR_YESNO(another_cat_defined), daylights[0], daylights[1], daylights[2], as_shot[0], as_shot[1], as_shot[2]); - d->preset = p->preset = DT_IOP_TEMP_AS_SHOT; + d->preset = DT_IOP_TEMP_AS_SHOT; // White balance module doesn't need to be enabled for true_monochrome raws (like // for leica monochrom cameras). prepare_matrices is a noop as well, as there