Skip to content

Commit

Permalink
Tone equalizer fixes
Browse files Browse the repository at this point in the history
- Fix a check in get_luminance_from_buffer()
- pseudo_solve() problems are tracked while committing the parameters.
  This means we always have valid runtim reports even if parameters didn't change and
  we re-open the image in darkroom.
- report in interactive mode as there is no pipe running
- some refactoring
- some correct use of gboolean instead of int
- Use gboolean for 'checks' when calling pseudo_solve()
  • Loading branch information
jenshannoschwalm committed Nov 23, 2024
1 parent 584d614 commit 8f1df83
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 58 deletions.
2 changes: 1 addition & 1 deletion src/common/fast_guided_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ static inline void variance_analyse(const float *const restrict guide, // I
ab[2*idx+1] = b;
}

if(input != NULL) dt_free_align(input);
dt_free_align(input);
}


Expand Down
2 changes: 1 addition & 1 deletion src/iop/colorequal.c
Original file line number Diff line number Diff line change
Expand Up @@ -1715,7 +1715,7 @@ static inline void _periodic_RBF_interpolate(float nodes[NODES],
}

// Solve A * x = y for lambdas
pseudo_solve((float *)A, nodes, NODES, NODES, 0);
pseudo_solve((float *)A, nodes, NODES, NODES, FALSE);

// Interpolate data for all x : generate the LUT
// WARNING: the LUT spans from [-pi; pi[ for consistency with the output of atan2f()
Expand Down
104 changes: 48 additions & 56 deletions src/iop/toneequal.c
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,8 @@ typedef struct dt_iop_toneequalizer_gui_data_t
// interactive view are in bounds
gboolean factors_valid; // TRUE if radial-basis coeffs are ready

gboolean commit_ok; // to be checked in process for correct smoothing pseudo_solve
// set while committing
gboolean distort_signal_actif;
} dt_iop_toneequalizer_gui_data_t;

Expand Down Expand Up @@ -663,7 +665,7 @@ static float get_luminance_from_buffer(const float *const buffer,
y }; // padding for vectorization

float luminance = 0.0f;
if(x > 0 && x < width - 2)
if(x > 1 && x < width - 2)
{
// no clamping needed on x, which allows us to vectorize
// apply the convolution
Expand Down Expand Up @@ -989,6 +991,10 @@ static inline void display_luminance_mask(const float *const restrict in,
}
}

static inline void _bad_curve(void)
{
dt_control_log(_("the interpolation is unstable, decrease the curve smoothing"));
}

__DT_CLONE_TARGETS__
static
Expand All @@ -1012,6 +1018,9 @@ void toneeq_process(dt_iop_module_t *self,
const size_t num_elem = width * height;
const size_t ch = 4;

const gboolean is_preview = piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW;
const gboolean is_full = piece->pipe->type & DT_DEV_PIXELPIPE_FULL;

// Get the hash of the upstream pipe to track changes
const int position = self->iop_order;
const dt_hash_t hash = dt_dev_pixelpipe_cache_hash(piece->pipe->image.id,
Expand Down Expand Up @@ -1040,7 +1049,7 @@ void toneeq_process(dt_iop_module_t *self,
dt_iop_gui_leave_critical_section(self);
}

if(piece->pipe->type & DT_DEV_PIXELPIPE_FULL)
if(is_full)
{
// For DT_DEV_PIXELPIPE_FULL, we cache the luminance mask for performance
// but it's not accessed from GUI
Expand All @@ -1058,7 +1067,7 @@ void toneeq_process(dt_iop_module_t *self,
luminance = g->full_preview_buf;
cached = TRUE;
}
else if(piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
else if(is_preview)
{
// For DT_DEV_PIXELPIPE_PREVIEW, we need to cache it too to
// compute the full image stats upon user request in GUI threads
Expand Down Expand Up @@ -1104,7 +1113,7 @@ void toneeq_process(dt_iop_module_t *self,
{
// caching path : store the luminance mask for GUI access

if(piece->pipe->type & DT_DEV_PIXELPIPE_FULL)
if(is_full)
{
dt_hash_t saved_hash;
hash_set_get(&g->ui_preview_hash, &saved_hash, &self->gui_lock);
Expand All @@ -1120,7 +1129,7 @@ void toneeq_process(dt_iop_module_t *self,
hash_set_get(&hash, &g->ui_preview_hash, &self->gui_lock);
}
}
else if(piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW)
else if(is_preview)
{
dt_hash_t saved_hash;
hash_set_get(&g->thumb_preview_hash, &saved_hash, &self->gui_lock);
Expand Down Expand Up @@ -1153,19 +1162,15 @@ void toneeq_process(dt_iop_module_t *self,
}

// Display output
if(self->dev->gui_attached && (piece->pipe->type & DT_DEV_PIXELPIPE_FULL))
if(g && is_full && g->mask_display)
{
if(g->mask_display)
{
display_luminance_mask(in, luminance, out, roi_in, roi_out, ch);
piece->pipe->mask_display = DT_DEV_PIXELPIPE_DISPLAY_PASSTHRU;
}
else
apply_toneequalizer(in, luminance, out, roi_in, roi_out, d);
display_luminance_mask(in, luminance, out, roi_in, roi_out, ch);
piece->pipe->mask_display = DT_DEV_PIXELPIPE_DISPLAY_PASSTHRU;
}
else
{
apply_toneequalizer(in, luminance, out, roi_in, roi_out, d);
if(g && !g->commit_ok && is_preview) _bad_curve();
}

if(!cached) dt_free_align(luminance);
Expand Down Expand Up @@ -1361,6 +1366,7 @@ static void gui_cache_init(dt_iop_module_t *self)
g->graph_valid = FALSE; // TRUE if the UI graph view is ready
g->user_param_valid = FALSE; // TRUE if users params set in interactive view are in bounds
g->factors_valid = TRUE; // TRUE if radial-basis coeffs are ready
g->commit_ok = TRUE;

g->valid_nodes_x = FALSE; // TRUE if x coordinates of graph nodes have been inited
g->valid_nodes_y = FALSE; // TRUE if y coordinates of graph nodes have been inited
Expand Down Expand Up @@ -1507,6 +1513,8 @@ static inline void compute_lut_correction(dt_iop_toneequalizer_gui_data_t *g,
// Compute the LUT of the exposure corrections in EV,
// offset and scale it for display in GUI widget graph

if(g == NULL) return;

float *const restrict LUT = g->gui_lut;
const float *const restrict factors = g->factors;
const float sigma = g->sigma;
Expand All @@ -1521,17 +1529,13 @@ static inline void compute_lut_correction(dt_iop_toneequalizer_gui_data_t *g,
}
}



static inline gboolean update_curve_lut(dt_iop_module_t *self)
{
dt_iop_toneequalizer_params_t *p = self->params;
dt_iop_toneequalizer_gui_data_t *g = self->gui_data;

if(g == NULL) return FALSE;

gboolean valid = TRUE;

if(g == NULL) return TRUE;
gboolean solved = TRUE;
dt_iop_gui_enter_critical_section(self);

if(!g->interpolation_valid)
Expand All @@ -1554,7 +1558,7 @@ static inline gboolean update_curve_lut(dt_iop_module_t *self)
{
float factors[CHANNELS] DT_ALIGNED_ARRAY;
dt_simd_memcpy(g->temp_user_params, factors, CHANNELS);
valid = pseudo_solve(g->interpolation_matrix, factors, CHANNELS, PIXEL_CHAN, 1);
solved = pseudo_solve(g->interpolation_matrix, factors, CHANNELS, PIXEL_CHAN, TRUE);
dt_simd_memcpy(factors, g->factors, PIXEL_CHAN);
g->factors_valid = TRUE;
g->lut_valid = FALSE;
Expand All @@ -1568,7 +1572,7 @@ static inline gboolean update_curve_lut(dt_iop_module_t *self)

dt_iop_gui_leave_critical_section(self);

return valid;
return solved;
}


Expand Down Expand Up @@ -1614,9 +1618,7 @@ void commit_params(dt_iop_module_t *self,
d->contrast_boost = exp2f(p->contrast_boost);
d->exposure_boost = exp2f(p->exposure_boost);

/*
* Perform a radial-based interpolation using a series gaussian functions
*/
/* Perform a radial-based interpolation using a series gaussian functions. */
if(self->dev->gui_attached && g)
{
dt_iop_gui_enter_critical_section(self);
Expand All @@ -1625,9 +1627,8 @@ void commit_params(dt_iop_module_t *self,
g->sigma = p->smoothing;
g->user_param_valid = FALSE; // force updating channels factors
dt_iop_gui_leave_critical_section(self);

update_curve_lut(self);

const gboolean curve_ok = update_curve_lut(self);
g->commit_ok = piece->enabled && (piece->pipe->type & DT_DEV_PIXELPIPE_PREVIEW) ? curve_ok : TRUE;
dt_iop_gui_enter_critical_section(self);
dt_simd_memcpy(g->factors, d->factors, PIXEL_CHAN);
dt_iop_gui_leave_critical_section(self);
Expand All @@ -1640,7 +1641,7 @@ void commit_params(dt_iop_module_t *self,

float A[CHANNELS * PIXEL_CHAN] DT_ALIGNED_ARRAY;
build_interpolation_matrix(A, p->smoothing);
pseudo_solve(A, factors, CHANNELS, PIXEL_CHAN, 0);
pseudo_solve(A, factors, CHANNELS, PIXEL_CHAN, FALSE);

dt_simd_memcpy(factors, d->factors, PIXEL_CHAN);
}
Expand Down Expand Up @@ -1770,21 +1771,14 @@ static void smoothing_callback(GtkWidget *slider, dt_iop_module_t *self)
if(darktable.gui->reset) return;
dt_iop_toneequalizer_params_t *p = self->params;
dt_iop_toneequalizer_gui_data_t *g = self->gui_data;

g->user_param_valid = FALSE;
p->smoothing= powf(sqrtf(2.0f), 1.0f + dt_bauhaus_slider_get(slider));

float factors[CHANNELS] DT_ALIGNED_ARRAY;
get_channels_factors(factors, p);

// Solve the interpolation by least-squares to check the validity of the smoothing param
const int valid = update_curve_lut(self);

if(!valid)
dt_control_log
(_("the interpolation is unstable, decrease the curve smoothing"));

// and possibly warn
if(!update_curve_lut(self))
_bad_curve();
// Redraw graph before launching computation
update_curve_lut(self);
gtk_widget_queue_draw(GTK_WIDGET(g->area));
dt_dev_add_history_item(darktable.develop, self, TRUE);

Expand Down Expand Up @@ -2122,11 +2116,9 @@ int mouse_leave(dt_iop_module_t *self)
}


static inline int set_new_params_interactive(const float control_exposure,
const float exposure_offset,
const float blending_sigma,
dt_iop_toneequalizer_gui_data_t *g,
dt_iop_toneequalizer_params_t *p)
static inline gboolean set_new_params_interactive(const float control_exposure,
const float exposure_offset,
dt_iop_module_t *self)
{
// Apply an exposure offset optimized smoothly over all the exposure channels,
// taking user instruction to apply exposure_offset EV at control_exposure EV,
Expand All @@ -2135,6 +2127,9 @@ static inline int set_new_params_interactive(const float control_exposure,
// Raise the user params accordingly to control correction and
// distance from cursor exposure to blend smoothly the desired
// correction
dt_iop_toneequalizer_gui_data_t *g = self->gui_data;
dt_iop_toneequalizer_params_t *p = self->params;
const float blending_sigma = g->sigma * g->sigma / 2.0f;
const float std = gaussian_denom(blending_sigma);
if(g->user_param_valid)
{
Expand All @@ -2147,17 +2142,16 @@ static inline int set_new_params_interactive(const float control_exposure,
float factors[CHANNELS] DT_ALIGNED_ARRAY;
dt_simd_memcpy(g->temp_user_params, factors, CHANNELS);
if(g->user_param_valid)
g->user_param_valid = pseudo_solve(g->interpolation_matrix, factors,
CHANNELS, PIXEL_CHAN, 1);;
g->user_param_valid = pseudo_solve(g->interpolation_matrix, factors, CHANNELS, PIXEL_CHAN, TRUE);
if(!g->user_param_valid)
dt_control_log(_("the interpolation is unstable, decrease the curve smoothing"));
_bad_curve();

// Compute new user params for channels and store them locally
if(g->user_param_valid)
g->user_param_valid = compute_channels_factors(factors, g->temp_user_params, g->sigma);
if(!g->user_param_valid) dt_control_log(_("some parameters are out-of-bounds"));

const int commit = g->user_param_valid;
const gboolean commit = g->user_param_valid;

if(commit)
{
Expand Down Expand Up @@ -2236,8 +2230,7 @@ int scrolled(dt_iop_module_t *self,

// Get the desired correction on exposure channels
dt_iop_gui_enter_critical_section(self);
const int commit = set_new_params_interactive(g->cursor_exposure, offset,
g->sigma * g->sigma / 2.0f, g, p);
const gboolean commit = set_new_params_interactive(g->cursor_exposure, offset, self);
dt_iop_gui_leave_critical_section(self);

gtk_widget_queue_draw(GTK_WIDGET(g->area));
Expand Down Expand Up @@ -3092,7 +3085,7 @@ static gboolean area_button_press(GtkWidget *widget,
{
if(self->enabled)
{
g->area_dragging = 1;
g->area_dragging = TRUE;
gtk_widget_queue_draw(GTK_WIDGET(g->area));
}
else
Expand All @@ -3117,7 +3110,7 @@ static gboolean area_motion_notify(GtkWidget *widget,
if(!self->enabled) return FALSE;

dt_iop_toneequalizer_gui_data_t *g = self->gui_data;
dt_iop_toneequalizer_params_t *p = self->params;
if(!g) return FALSE;

if(g->area_dragging)
{
Expand All @@ -3128,13 +3121,12 @@ static gboolean area_motion_notify(GtkWidget *widget,
const float cursor_exposure = g->area_x / g->graph_width * 8.0f - 8.0f;

// Get the desired correction on exposure channels
g->area_dragging = set_new_params_interactive(cursor_exposure, offset,
g->sigma * g->sigma / 2.0f, g, p);
g->area_dragging = set_new_params_interactive(cursor_exposure, offset, self);
dt_iop_gui_leave_critical_section(self);
}

dt_iop_gui_enter_critical_section(self);
g->area_x = (event->x - g->inset);
g->area_x = event->x - g->inset;
g->area_y = event->y;
g->area_cursor_valid = (g->area_x > 0.0f
&& g->area_x < g->graph_width
Expand Down Expand Up @@ -3187,7 +3179,7 @@ static gboolean area_button_release(GtkWidget *widget,
dt_dev_add_history_item(darktable.develop, self, FALSE);

dt_iop_gui_enter_critical_section(self);
g->area_dragging= 0;
g->area_dragging = FALSE;
dt_iop_gui_leave_critical_section(self);

return TRUE;
Expand Down

0 comments on commit 8f1df83

Please sign in to comment.