Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ remain right for what is fine to miss: progress, confirmation, a passing state.
| Read this | For |
|---|---|
| [`doc/iop-notes.md`](doc/iop-notes.md) | per-module findings (ashift, retouch, toneequal, …) |
| [`doc/color-picker.md`](doc/color-picker.md) | what a module picker's min/max are, and how their pixels are located |
| [`doc/drawlayer.md`](doc/drawlayer.md) | the drawlayer module in full |
| [`doc/retouch-result-memo.md`](doc/retouch-result-memo.md) | retouch's per-shape memo |
| [`doc/gtk-patterns.md`](doc/gtk-patterns.md) | layout, focus, repaint and threading patterns |
Expand Down
7 changes: 7 additions & 0 deletions data/anselconfig.xml.in
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,13 @@
<shortdescription>restrict histogram to selection</shortdescription>
<longdescription/>
</dtconfig>
<dtconfig>
<name>ui_last/colorpicker_display_extremes</name>
<type>boolean</type>
<default>false</default>
<shortdescription>display the pixels holding the minimum and maximum a module picker reads</shortdescription>
<longdescription/>
</dtconfig>
<dtconfig>
<name>ui_last/colorpicker_large</name>
<type>boolean</type>
Expand Down
59 changes: 59 additions & 0 deletions doc/color-picker.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Module color pickers: where the minimum and maximum come from

*Established on top of `fda814dd0c`, 2026-11-14. Measurements on top of `6a9005ef9c`, 2026-11-13.*

## What a module receives

`_sample_picker_from_cache()` (`gui/color_picker_proxy.c`) samples the module's input and output
cachelines in the preview pipe and hands the module `picked_color`, `picked_color_min` and
`picked_color_max` for its input (and the `picked_output_*` triple for its output).
`dt_color_picker_helper()` blurs the area first (`blur_2D_Bspline`) and then takes the mean,
minimum and maximum **of each channel independently**. A module that reads the minimum or the
maximum therefore gets, in general, three values from three different pixels, and most reduce them
over channels themselves (negadoctor's `v_maxf()` / `v_minf()`). When the module's quantity is
monotone in each channel, its value equals the one it would compute on a single pixel per channel:
the one holding that channel's extreme.

## Locating the extremes

A picker declares which extremes its module reads with `dt_color_picker_set_extremes()`. For such a
picker, and only for the input sample, the helper also scans the same blurred buffer for the first
pixel in raster order holding each channel's minimum and maximum (strict `<` / `>`), so the pixel
found holds exactly the value the module received. It records the pixel's buffer coordinates and
all four of its channels (`dt_color_picker_extremes_t`). The proxy converts the coordinates back to
raw-norm with the inverse of the picker box's own chain: the pixel centre over `roi->scale`, then
`dt_dev_distort_transform_plus()` from the module's `iop_order` (`FORW_INCL` for the input), then
image-abs → image-norm → raw-norm. It stores them in `dev->color_picker` with the picker they
belong to; a new input sample, a reset, a deactivation or the picker widget's destruction clears
them.

- `dt_iop_color_picker_set_deciding_channel()` lets the module name the channel whose extreme it
kept; the darkroom draws that channel's mark as a +, the others as smaller, dimmed crosses.
- `dt_iop_color_picker_get_extreme_pixel()` returns all the channels of one extreme's pixel, for a
module that needs the pixel itself rather than one channel of it.
- The marks are drawn only when `ui_last/colorpicker_display_extremes` is set. Locating does not
depend on it, since a module may read the pixel.

Locating costs one sequential pass over the area per sample; the statistics themselves already
read every pixel of it.

## Why per-channel extremes and not one pixel by luminance

Measured by dumping the blurred module input of negadoctor on 20250921_0007 (preview pipe) and
replaying its D max, bias, paper black and print exposure pickers in Python, against one pixel
chosen by its luminance (the darkest or the brightest one):

- On 300 random boxes the two agree closely: median differences below 0.006 D and 0.003 EV. But in
the worst boxes 31 % to 46 % of the pixels lie beyond the luminance pixel's value in the channel
the module keeps, so that pixel no longer bounds the area.
- On a paper black box covering most of the frame, the three channels' maxima sat within 2 px of
each other: per channel 0.100 / 0.080 / 0.106 (blue decides), 0.098 from the luminance pixel.
- A speck of dust in the sky made D max 1.605 per channel; the luminance pixel gave 0.187 D less.
Once the dust was retouched (998 px changed), the two differed by 0.014 D. The dust, not the
method, was the outlier: dust stops light, so it only reaches the pickers reading the densest
pixel.
- Trimming instead of taking the strict extreme neutralizes the dust for D max (1.425 with it at
the 99.99th percentile, 1.420 without) but moves the paper black away from the blacks: 0.087 at
the 0.01 % trim and −0.007 at 0.1 %, against 0.106.

So the strict per-channel extreme stays, and the marks show which pixel each value comes from.
64 changes: 59 additions & 5 deletions src/common/color_picker.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
Copyright (C) 2022, 2025-2026 Aurélien PIERRE.
Copyright (C) 2022 Martin Bařinka.
Copyright (C) 2022 Philipp Lutz.
Copyright (C) 2026 Guillaume Stutin.

darktable is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Expand Down Expand Up @@ -760,13 +761,62 @@ static void color_picker_helper_xtrans(const dt_iop_buffer_dsc_t *dsc, const flo
return color_picker_helper_xtrans_seq(dsc, pixel, roi, box, picked_color, picked_color_min, picked_color_max);
}

// A second pass, sequential: the statistics passes reduce per thread and do not carry positions.
// Strict comparisons keep the first pixel in raster order, the same value fminf()/fmaxf() return.
// `extremes` NULL skips it.
static void _color_picker_locate_extremes(const float *const pixel, const dt_iop_roi_t *const roi,
const int *const box, dt_color_picker_extremes_t *const extremes)
{
if(IS_NULL_PTR(extremes)) return;

dt_aligned_pixel_t min = { INFINITY, INFINITY, INFINITY, INFINITY };
dt_aligned_pixel_t max = { -INFINITY, -INFINITY, -INFINITY, -INFINITY };
for(int c = 0; c < 4; c++)
{
extremes->min[c][0] = box[0];
extremes->min[c][1] = box[1];
extremes->max[c][0] = box[0];
extremes->max[c][1] = box[1];
}

// One index over the box, row by row, so that raster order is kept.
const size_t box_width = (size_t)(box[2] - box[0]);
const size_t box_size = _box_size(box);
for(size_t k = 0; k < box_size; k++)
{
const int i = box[0] + (int)(k % box_width);
const int j = box[1] + (int)(k / box_width);
const float *const px = pixel + 4 * ((size_t)j * roi->width + i);
for(int c = 0; c < 4; c++)
{
if(px[c] < min[c])
{
min[c] = px[c];
extremes->min[c][0] = i;
extremes->min[c][1] = j;
memcpy(extremes->min_pixel[c], px, sizeof(dt_aligned_pixel_t));
}
if(px[c] > max[c])
{
max[c] = px[c];
extremes->max[c][0] = i;
extremes->max[c][1] = j;
memcpy(extremes->max_pixel[c], px, sizeof(dt_aligned_pixel_t));
}
}
}
extremes->valid = TRUE;
}

// picked_color, picked_color_min and picked_color_max should be aligned
void dt_color_picker_helper(const dt_iop_buffer_dsc_t *dsc, const float *const pixel, const dt_iop_roi_t *roi,
const int *const box, dt_aligned_pixel_t picked_color, dt_aligned_pixel_t picked_color_min,
dt_aligned_pixel_t picked_color_max, const dt_iop_colorspace_type_t image_cst,
const dt_iop_colorspace_type_t picker_cst,
const dt_iop_order_iccprofile_info_t *const profile)
const dt_iop_order_iccprofile_info_t *const profile,
dt_color_picker_extremes_t *extremes)
{
if(extremes) extremes->valid = FALSE;
dt_times_t start_time = { 0 }, end_time = { 0 };
if(dt_get_debug_flags() & DT_DEBUG_PERF) dt_get_times(&start_time);

Expand All @@ -792,9 +842,7 @@ void dt_color_picker_helper(const dt_iop_buffer_dsc_t *dsc, const float *const p
const gboolean convertible
= image_convertible && picker_convertible && (!crosses_family || !IS_NULL_PTR(profile));

if(((image_cst == picker_cst) || (picker_cst == IOP_CS_NONE)))
color_picker_helper_4ch(dsc, denoised, roi, box, picked_color, picked_color_min, picked_color_max, picker_cst, profile);
else if(convertible)
if((image_cst != picker_cst) && (picker_cst != IOP_CS_NONE) && convertible)
{
/* The picker samples module input/output buffers after the previous piece has written them.
When the requested picker colorspace differs from that buffer colorspace, we need a real
Expand All @@ -806,9 +854,15 @@ void dt_color_picker_helper(const dt_iop_buffer_dsc_t *dsc, const float *const p

_color_picker_convert_buffer(denoised, converted, (size_t)roi->width * roi->height, image_cst, picker_cst, profile);
color_picker_helper_4ch_converted(converted, roi, box, picked_color, picked_color_min, picked_color_max, picker_cst);
_color_picker_locate_extremes(converted, roi, box, extremes);
}
else // This is a fallback, better than crashing as happens with monochromes
else
{
// The buffer is already in the picker space, or no space was asked. Otherwise this is a
// fallback, better than crashing as happens with monochromes.
color_picker_helper_4ch(dsc, denoised, roi, box, picked_color, picked_color_min, picked_color_max, picker_cst, profile);
_color_picker_locate_extremes(denoised, roi, box, extremes);
}

error:;
dt_pixelpipe_cache_free_align(converted);
Expand Down
27 changes: 26 additions & 1 deletion src/common/color_picker.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
Copyright (C) 2020 Pascal Obry.
Copyright (C) 2021 Ralf Brown.
Copyright (C) 2022 Martin Bařinka.
Copyright (C) 2026 Guillaume Stutin.

darktable is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
Expand All @@ -35,12 +36,36 @@ struct dt_iop_buffer_dsc_t;
struct dt_iop_roi_t;
enum dt_iop_colorspace_type_t;

/**
* @brief Where the picker found each channel's extremes.
*
* Buffer coordinates of the first pixel, in raster order, holding `picked_color_min[c]` (`min[c]`)
* and `picked_color_max[c]` (`max[c]`) in the buffer the statistics were taken from: the input after
* the picker's denoising, converted to the picker colorspace when it differs. `min_pixel[c]` and
* `max_pixel[c]` hold all the channels of those pixels. `valid` is FALSE when the input was mosaiced,
* which the helper does not locate.
*/
typedef struct dt_color_picker_extremes_t
{
int min[4][2];
int max[4][2];
dt_aligned_pixel_t min_pixel[4];
dt_aligned_pixel_t max_pixel[4];
int valid;
} dt_color_picker_extremes_t;

/**
* @brief Compute the mean, minimum and maximum of each channel over `box`.
*
* @param extremes Optional, NULL to skip: receives where each channel's minimum and maximum lie.
*/
void dt_color_picker_helper(const struct dt_iop_buffer_dsc_t *dsc, const float *const pixel,
const struct dt_iop_roi_t *roi, const int *const box, dt_aligned_pixel_t picked_color,
dt_aligned_pixel_t picked_color_min, dt_aligned_pixel_t picked_color_max,
const enum dt_iop_colorspace_type_t image_cst,
const enum dt_iop_colorspace_type_t picker_cst,
const dt_iop_order_iccprofile_info_t *const profile);
const dt_iop_order_iccprofile_info_t *const profile,
dt_color_picker_extremes_t *extremes);

#endif // DT_COMMON_COLOR_PICKER_H

Expand Down
13 changes: 13 additions & 0 deletions src/develop/develop.h
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,8 @@ typedef struct dt_develop_t
gboolean display_samples;
gboolean live_samples_enabled;
gboolean restrict_histogram;
/* draw the crosses on the pixels holding the extremes the active module picker reads */
gboolean display_extremes;
int statistic;
struct dt_lib_module_t *histogram_module;
gboolean (*refresh_global_picker)(struct dt_lib_module_t *self);
Expand All @@ -389,6 +391,17 @@ typedef struct dt_develop_t

struct dt_iop_module_t *pending_module;
struct dt_dev_pixelpipe_t *pending_pipe;

/* Raw-norm positions of the module-input pixels holding each RGB channel's minimum
* (`extreme_points[0]`) and maximum (`extreme_points[1]`) over the last input sample of
* `extremes_picker`. `extremes_valid` holds the dt_iop_color_picker_extremes_t flags located;
* `extremes_channel` is the channel whose extreme decided the module's value, -1 when unknown.
* `extreme_pixels` holds all the channels of those pixels, as the module received them. */
float extreme_points[2][3][2];
float extreme_pixels[2][3][4];
int extremes_valid;
int extremes_channel;
const struct dt_iop_color_picker_t *extremes_picker;
} color_picker;

/* proxy for communication between plugins and develop/darkroom */
Expand Down
Loading
Loading