Skip to content

Commit

Permalink
temperature.c : add warning message
Browse files Browse the repository at this point in the history 
advertize the use of D65 RGB coeff on the pipe,
simplify channelmixerrgb handling
  • Loading branch information
@aurelienpierre
aurelienpierre committed Nov 12, 2020
1 parent 24067dd commit 9c3f183
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 60 deletions.
1 change: 1 addition & 0 deletions src/develop/develop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ void dt_dev_init(dt_develop_t *dev, int32_t gui_attached)

dev->proxy.exposure.module = NULL;
dev->proxy.chroma_adaptation = NULL;
dev->proxy.wb_is_D65 = FALSE;

dev->rawoverexposed.enabled = FALSE;
dev->rawoverexposed.mode = dt_conf_get_int("darkroom/ui/rawoverexposed/mode");
Expand Down
3 changes: 3 additions & 0 deletions src/develop/develop.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ typedef struct dt_develop_t
// only used to display warnings in GUI of modules that should probably not be doing white balance
dt_iop_order_entry_t *chroma_adaptation;

// is the WB module using D65 illuminant and not doing full chromatic adaptation ?
gboolean wb_is_D65;

} proxy;

// for the overexposure indicator
Expand Down
111 changes: 51 additions & 60 deletions src/iop/channelmixerrgb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,44 @@ static inline void repack_3x3_to_3xSSE(const float input[9], float output[3][4])
}


static void declare_cat_on_pipe(struct dt_iop_module_t *self)
{
// Advertise to the pipeline that we are doing chromatic adaptation here
dt_iop_channelmixer_rgb_params_t *p = (dt_iop_channelmixer_rgb_params_t *)self->params;
dt_iop_order_entry_t *this
= dt_ioppr_get_iop_order_entry(self->dev->iop_order_list, "channelmixerrgb", self->multi_priority);

if(this == NULL) return; // there is no point then

if(self->enabled && !(p->adaptation == DT_ADAPTATION_RGB || p->illuminant == DT_ILLUMINANT_PIPE))
{
// We do CAT here so we need to register this instance as CAT-handler.
if(self->dev->proxy.chroma_adaptation == NULL)
{
// We are the first to try to register, let's go !
self->dev->proxy.chroma_adaptation = this;
}
else
{
// Another instance already registered.
// If we are lower in the pipe than it, register in its place.
if(this->o.iop_order < self->dev->proxy.chroma_adaptation->o.iop_order)
self->dev->proxy.chroma_adaptation = this;
}
}
else
{
if(self->dev->proxy.chroma_adaptation != NULL)
{
// We do NOT do CAT here.
// Deregister this instance as CAT-handler if it previously registered
if(self->dev->proxy.chroma_adaptation == this)
self->dev->proxy.chroma_adaptation = NULL;
}
}
}


static void update_illuminants(struct dt_iop_module_t *self);
static void update_approx_cct(struct dt_iop_module_t *self);
static void update_illuminant_color(struct dt_iop_module_t *self);
Expand Down Expand Up @@ -976,6 +1014,7 @@ void process(struct dt_iop_module_t *self, dt_dev_pixelpipe_iop_t *piece,
break;
}
}
declare_cat_on_pipe(self);
}

static void _develop_ui_pipe_finished_callback(gpointer instance, gpointer user_data)
Expand Down Expand Up @@ -1013,44 +1052,9 @@ static void _develop_ui_pipe_finished_callback(gpointer instance, gpointer user_

--darktable.gui->reset;

dt_dev_add_history_item(darktable.develop, self, TRUE);
}

static void declare_cat_on_pipe(struct dt_iop_module_t *self)
{
// Advertise to the pipeline that we are doing chromatic adaptation here
dt_iop_channelmixer_rgb_params_t *p = (dt_iop_channelmixer_rgb_params_t *)self->params;
dt_iop_order_entry_t *this
= dt_ioppr_get_iop_order_entry(self->dev->iop_order_list, "channelmixerrgb", self->multi_priority);

if(this == NULL) return; // there is no point then
gui_changed(self, NULL, NULL);

if(self->enabled && !(p->adaptation == DT_ADAPTATION_RGB || p->illuminant == DT_ILLUMINANT_PIPE))
{
// We do CAT here so we need to register this instance as CAT-handler.
if(self->dev->proxy.chroma_adaptation == NULL)
{
// We are the first to try to register, let's go !
self->dev->proxy.chroma_adaptation = this;
}
else
{
// Another instance already registered.
// If we are lower in the pipe than it, register in its place.
if(this->o.iop_order < self->dev->proxy.chroma_adaptation->o.iop_order)
self->dev->proxy.chroma_adaptation = this;
}
}
else
{
if(self->dev->proxy.chroma_adaptation != NULL)
{
// We do NOT do CAT here.
// Deregister this instance as CAT-handler if it previously registered
if(self->dev->proxy.chroma_adaptation == this)
self->dev->proxy.chroma_adaptation = NULL;
}
}
dt_dev_add_history_item(darktable.develop, self, TRUE);
}


Expand Down Expand Up @@ -1963,26 +1967,6 @@ void gui_changed(dt_iop_module_t *self, GtkWidget *w, void *previous)
update_B_colors(self);
}

--darktable.gui->reset;

// Check that the white balance module has camera reference D65 set
// otherwise, this will cause trouble in CAT
double bwb[4] = { 0. };
gint is_reference_wb = TRUE;

if(!calculate_bogus_daylight_wb(self, bwb))
{
for(int i = 0; i < 3; i++)
if(self->dev->pipe->dsc.temperature.coeffs[i] != (float)bwb[i])
is_reference_wb = FALSE;
}
else
{
is_reference_wb = FALSE;
}

declare_cat_on_pipe(self);

if(self->enabled && !(p->illuminant == DT_ILLUMINANT_PIPE || p->adaptation == DT_ADAPTATION_RGB))
{
// this module instance is doing chromatic adaptation
Expand All @@ -1999,30 +1983,37 @@ void gui_changed(dt_iop_module_t *self, GtkWidget *w, void *previous)
"all performing chromatic adaptation.\n"
"this can lead to inconsistencies, unless you\n"
"use them with masks or know what you are doing."));
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), TRUE);
}
else if(!is_reference_wb)
else if(!self->dev->proxy.wb_is_D65)
{
// our first and biggest problem : white balance is being clever with WB coeffs
// our first and biggest problem : white balance module is being clever with WB coeffs
dt_iop_set_module_in_trouble(self, TRUE);
gtk_label_set_text(GTK_LABEL(g->warning_label), _("⚠ white balance module error"));
gtk_widget_set_tooltip_text(GTK_WIDGET(g->warning_label), _("the white balance module is not using the camera\n"
"reference illuminant, which will cause issues here\n"
"with chromatic adaptation. Either set it to reference\n"
"or disable chromatic adaptation here."));
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), TRUE);
}
else
{
dt_iop_set_module_in_trouble(self, FALSE);
gtk_label_set_text(GTK_LABEL(g->warning_label), "");
gtk_widget_set_tooltip_text(GTK_WIDGET(g->warning_label), "");
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), FALSE);
}
}
else
{
dt_iop_set_module_in_trouble(self, FALSE);
gtk_label_set_text(GTK_LABEL(g->warning_label), "");
gtk_widget_set_tooltip_text(GTK_WIDGET(g->warning_label), "");
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), FALSE);
}

--darktable.gui->reset;

}


Expand Down Expand Up @@ -2105,7 +2096,7 @@ void gui_init(struct dt_iop_module_t *self)

g->warning_label = dt_ui_label_new("");
gtk_label_set_line_wrap(GTK_LABEL(g->warning_label), TRUE);
gtk_box_pack_start(GTK_BOX(self->widget), g->warning_label, FALSE, FALSE, 0);
gtk_box_pack_start(GTK_BOX(self->widget), g->warning_label, FALSE, FALSE, 4);

g->adaptation = dt_bauhaus_combobox_from_params(self, N_("adaptation"));
gtk_widget_set_tooltip_text(GTK_WIDGET(g->adaptation),
Expand Down
72 changes: 72 additions & 0 deletions src/iop/temperature.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ typedef struct dt_iop_temperature_gui_data_t
GtkWidget *coeffs_toggle;
GtkWidget *temp_label;
GtkWidget *balance_label;
GtkWidget *warning_label;
int preset_cnt;
int preset_num[54];
double daylight_wb[4];
Expand Down Expand Up @@ -735,6 +736,7 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix
{
dt_iop_temperature_params_t *p = (dt_iop_temperature_params_t *)p1;
dt_iop_temperature_data_t *d = (dt_iop_temperature_data_t *)piece->data;
dt_iop_temperature_gui_data_t *g = (dt_iop_temperature_gui_data_t *)self->gui_data;

if(self->hide_enable_button)
{
Expand All @@ -749,6 +751,16 @@ void commit_params(struct dt_iop_module_t *self, dt_iop_params_t *p1, dt_dev_pix

// 4Bayer images not implemented in OpenCL yet
if(self->dev->image_storage.flags & DT_IMAGE_4BAYER) piece->process_cl_ready = 0;

if(g)
{
// advertise on the pipe if coeffs are D65 for validity check
gboolean is_D65 = TRUE;
for(int c = 0; c < 3; c++)
if(d->coeffs[c] != (float)g->daylight_wb[c]) is_D65 = FALSE;

self->dev->proxy.wb_is_D65 = is_D65;
}
}

void init_pipe(struct dt_iop_module_t *self, dt_dev_pixelpipe_t *pipe, dt_dev_pixelpipe_iop_t *piece)
Expand Down Expand Up @@ -1120,6 +1132,43 @@ void color_temptint_sliders(struct dt_iop_module_t *self)
}
}

static void display_wb_error(struct dt_iop_module_t *self)
{
// this module instance is doing chromatic adaptation
dt_iop_temperature_gui_data_t *g = (dt_iop_temperature_gui_data_t *)self->gui_data;
if(g == NULL) return;

++darktable.gui->reset;

if(self->dev->proxy.chroma_adaptation != NULL && !self->dev->proxy.wb_is_D65)
{
// our second biggest problem : another channelmixerrgb instance is doing CAT earlier in the pipe
dt_iop_set_module_in_trouble(self, TRUE);
gtk_label_set_text(GTK_LABEL(g->warning_label), _("⚠ white balance applied twice"));
gtk_widget_set_tooltip_text(GTK_WIDGET(g->warning_label), _("the color calibration module is enabled,\n"
"and performing chromatic adaptation.\n"
"set the white balance here to camera reference (D65)\n"
"or disable chromatic adaptation in color calibration."));
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), TRUE);
}
else
{
dt_iop_set_module_in_trouble(self, FALSE);
gtk_label_set_text(GTK_LABEL(g->warning_label), "");
gtk_widget_set_tooltip_text(GTK_WIDGET(g->warning_label), "");
gtk_widget_set_visible(GTK_WIDGET(g->warning_label), FALSE);
}

--darktable.gui->reset;
}


void gui_focus(struct dt_iop_module_t *self, gboolean in)
{
display_wb_error(self);
}


void gui_update(struct dt_iop_module_t *self)
{
dt_iop_temperature_gui_data_t *g = (dt_iop_temperature_gui_data_t *)self->gui_data;
Expand Down Expand Up @@ -1286,6 +1335,9 @@ void gui_update(struct dt_iop_module_t *self)
color_temptint_sliders(self);
color_rgb_sliders(self);
color_finetuning_slider(self);

display_wb_error(self);

gtk_widget_queue_draw(self->widget);
}

Expand Down Expand Up @@ -1576,6 +1628,8 @@ 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);

display_wb_error(self);
}

static gboolean btn_toggled(GtkWidget *togglebutton, GdkEventButton *event, dt_iop_module_t *self)
Expand Down Expand Up @@ -1872,10 +1926,20 @@ static void _preference_changed(gpointer instance, gpointer user_data)
color_finetuning_slider(self);
}

static void _develop_ui_pipe_finished_callback(gpointer instance, gpointer user_data)
{
dt_iop_module_t *self = (dt_iop_module_t *)user_data;
display_wb_error(self);
}


void gui_init(struct dt_iop_module_t *self)
{
dt_iop_temperature_gui_data_t *g = IOP_GUI_ALLOC(temperature);

DT_DEBUG_CONTROL_SIGNAL_CONNECT(darktable.signals, DT_SIGNAL_DEVELOP_UI_PIPE_FINISHED,
G_CALLBACK(_develop_ui_pipe_finished_callback), self);

gchar *config = dt_conf_get_string("plugins/darkroom/temperature/colored_sliders");
g->colored_sliders = g_strcmp0(config, "no color"); // true if config != "no color"
g->blackbody_is_confusing = g->colored_sliders && g_strcmp0(config, "illuminant color"); // true if config != "illuminant color"
Expand All @@ -1887,6 +1951,10 @@ void gui_init(struct dt_iop_module_t *self)

GtkBox *box_enabled = GTK_BOX(gtk_box_new(GTK_ORIENTATION_VERTICAL, DT_BAUHAUS_SPACE));

g->warning_label = dt_ui_label_new("");
gtk_label_set_line_wrap(GTK_LABEL(g->warning_label), TRUE);
gtk_box_pack_start(GTK_BOX(box_enabled), g->warning_label, FALSE, FALSE, 4);

g->mod_temp = NAN;
for(int k = 0; k < 4; k++)
{
Expand Down Expand Up @@ -2023,7 +2091,10 @@ void gui_init(struct dt_iop_module_t *self)

void gui_cleanup(struct dt_iop_module_t *self)
{
self->request_color_pick = DT_REQUEST_COLORPICK_OFF;
DT_DEBUG_CONTROL_SIGNAL_DISCONNECT(darktable.signals, G_CALLBACK(_preference_changed), self);
DT_DEBUG_CONTROL_SIGNAL_DISCONNECT(darktable.signals,
G_CALLBACK(_develop_ui_pipe_finished_callback), self);

IOP_GUI_FREE;
}
Expand All @@ -2047,6 +2118,7 @@ void gui_reset(struct dt_iop_module_t *self)
color_finetuning_slider(self);
color_rgb_sliders(self);
color_temptint_sliders(self);
display_wb_error(self);
}

// modelines: These editor modelines have been set for all relevant files by tools/update_modelines.sh
Expand Down

0 comments on commit 9c3f183

Please sign in to comment.