Skip to content

color picker: locate the pixels holding a module picker's min and max - #1640

Merged
Jiyone merged 2 commits into
masterfrom
color-picker-extremes
Nov 14, 2026
Merged

Jiyone merged 2 commits into
masterfrom
color-picker-extremes

Conversation

@Jiyone

@Jiyone Jiyone commented Nov 14, 2026

Copy link
Copy Markdown
Member

A module color picker hands its module the mean, minimum and maximum of each channel over the area, each channel on its own. A module that reads the minimum or the maximum therefore gets three values from three different pixels in general, and nothing showed which pixels they were.

What changes

  • Locating. A picker declares the extremes its module reads with dt_color_picker_set_extremes(). For such a picker, and only for the input sample, dt_color_picker_helper() scans the same blurred buffer the statistics come from. For each channel it finds the first pixel in raster order holding the minimum and the maximum, so that pixel holds exactly the value the module received.
  • Positions. The proxy maps each position back to raw-norm with the inverse of the picker box's own chain. It keeps them in dev->color_picker until the next input sample, a reset, a deactivation or the widget's destruction.
  • Display. The darkroom draws a cross in each channel's colour on those pixels. A module names the channel it kept with dt_iop_color_picker_set_deciding_channel(), and that channel's mark becomes a + while the others stay ×.
  • Pixel values. dt_iop_color_picker_get_extreme_pixel() gives a module all the channels of one of those pixels, for a module that needs the whole pixel rather than one channel of it.
  • Option. The marks are drawn only while "Display min/max pixel" is checked in the color picker section of the scopes. It is stored in ui_last/colorpicker_display_extremes and off by default. Locating runs either way, since a module may read the pixel.

No module declares extremes in this PR. negadoctor's pickers are the first users, on a branch that will be rebased onto this one once it is merged.

Why strict per-channel extremes

doc/color-picker.md records the measurements, made on negadoctor's pickers over 20250921_0007:

  • One pixel chosen by luminance agrees with the per-channel extremes on most areas, with median differences below 0.006 D. On the worst areas, though, 31 % to 46 % of the pixels lie beyond it, so it no longer bounds the area.
  • Trimming the extremes removes a speck of dust from D max. But it moves the paper black away from the blacks: 0.087 at a 0.01 % trim and −0.007 at 0.1 %, against 0.106.

Testing

  • Built with clang, with no new warnings.
  • Checked in the darkroom with negadoctor's pickers: the crosses sit on the pixels each value comes from, the deciding channel is drawn as a +, and the option shows and hides them.

🤖 Generated with Claude Code

A module picker hands its module the mean, minimum and maximum of each channel over the area, each channel on its own, so a module reading the minimum or the maximum gets three values from three different pixels in general, and nothing showed which. A picker can now declare which extremes its module reads with dt_color_picker_set_extremes(). For such a picker, the helper scans the same blurred input the statistics come from for the first pixel in raster order holding each channel's extreme, which holds exactly the value the module received, and records its position and all its channels.

The proxy maps each position back to raw-norm with the inverse of the picker box's own chain and keeps it in dev->color_picker until the next input sample, a reset, a deactivation or the widget's destruction. The darkroom draws a cross in each channel's colour on those pixels; a module names the channel it kept with dt_iop_color_picker_set_deciding_channel(), and that channel's mark becomes a +. dt_iop_color_picker_get_extreme_pixel() gives a module all the channels of one of those pixels, for a module that needs the pixel rather than one channel of it.

The marks are drawn while "Display min/max pixel" is checked in the color picker section of the scopes (ui_last/colorpicker_display_extremes, off by default). Locating does not depend on it, since a module may read the pixel.

doc/color-picker.md records how the extremes are taken and why the strict per-channel extreme stays: measured on negadoctor's pickers, one pixel chosen by luminance agrees closely on most areas but no longer bounds the area on others, and trimming the extremes, which removes a speck of dust from D max, moves the paper black away from the blacks.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The locating pass walks the box with one index, row by row, so that its comparisons nest less, and copies the whole pixel with memcpy(). It takes a NULL `extremes` itself, so the helper no longer tests it at each call; the helper's branch for a buffer already in the picker space and its fallback were the same code and are now one. The darkroom marks compute once what depends on whether the module named a channel. Raster order is kept: the old and the new pass gave the same positions and pixels on 201 random boxes, ties and empty boxes included.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@Jiyone
Jiyone merged commit 43ec9b0 into master Nov 14, 2026
22 checks passed
@Jiyone
Jiyone deleted the color-picker-extremes branch November 14, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

1 participant