Initial implementation of Color Harmonizer module.#20506
Initial implementation of Color Harmonizer module.#20506masterpiga wants to merge 4 commits intodarktable-org:masterfrom
Conversation
85d40e0 to
13ddd6d
Compare
|
@masterpiga you may have set a dangerous precedent here with the inclusion of all the documentation. While the documentation guys now love you, the devs are having an "oh s!*t" moment 🤣 . |
|
@wpferguson 🤣 Well, the documentation is mostly for myself, since I am swimming in unfamiliar waters. But then I thought that it make the job easier for reviewers, so I included it in the PR. Anyways, Claude wrote the docs, I just told it what needed documenting, so I can't take credit for that :) |
|
Sorry, after some vectorscope changes it needs a rebase for conflicts resolution. |
f9e2b08 to
f9d37fa
Compare
|
Thanks @TurboGit, merged! |
|
@masterpiga : Also please rebase onto master to fix the macOS issue reported by the CI. |
f9d37fa to
565978b
Compare
|
Very impressive module. I have tested a bit this latest version tonight and I must say I'm bluffed. I'll continue on this tomorrow, time to sleep now. |
1ee97c4 to
407ee93
Compare
TurboGit
left a comment
There was a problem hiding this comment.
As for all new modules, we also need to add it into the modulegroup.c (at least in group all and scene-referred).
It also needs an entry into po/POTFILES.in.
TurboGit
left a comment
There was a problem hiding this comment.
It seems also that an harmony is always displayed even for new images.
- Open darkroom
- Select vectorscope
- An harmony is displayed
- Move to next image
- An harmony is displayed
- ...
Good catch, thanks, I had missed that. Now images restore whatever state they had previously. |
783a75a to
a81e28f
Compare
|
@TurboGit thanks! comments addressed and rebased |
a81e28f to
63e85ed
Compare
|
@masterpiga : Can you rebase on current master, it should fix the macOS CI issue. TIA. |
63e85ed to
c91eab8
Compare
|
Thanks @TurboGit, done |
c91eab8 to
d0e4d48
Compare
TurboGit
left a comment
There was a problem hiding this comment.
Some comments after a review.
I must say that I was expecting the code a bit more complex. I don't say that I understand everything there, but clearly the code is clean.
04478f6 to
58e7337
Compare
|
Thanks @TurboGit, all done! I also added a check to enable the "auto-detect" button only after the histogram is ready. |
Overview
colorharmonizer.cl)Vectorscope integration
_on_vectorscope_harmony_changed) so dragging the guide in the vectorscope syncs back to the module's params and historyhistogram.c,lib.h)Pixelpipe position
iop_order.c)Shared utilities extracted
dt_gaussian_mean_blur()/dt_gaussian_mean_blur_cl()moved fromcolorequal.cintogaussian.has shared inline helpers;colorequal.cupdated to use themcolor_ryb.hcolor_harmony.h/color_harmony.ccolorspaces_inline_conversions.hiop_profile.hMain technical choices
See business logic implementation details for a more detailed analysis.
Perceptual color space (UCS JCH)
All hue manipulation happens in darktable's Uniform Color Space rather than HSL/HSV. Equal angular steps in UCS correspond to equal perceived hue differences, which makes the Gaussian weighting behave predictably. The cost is a heavier per-pixel pipeline (matrix multiply + chromatic adaptation + nonlinear transforms in both directions), accepted as necessary for perceptual accuracy.
RYB geometry, UCS processing
Predefined harmony rules are defined on the RYB (painter's) wheel because it matches users' intuition and the vectorscope overlay. Processing happens in UCS. The anchor hue does a round-trip UCS→RYB (to apply the geometry table offsets) then RYB→UCS (to recover processing node positions). This ensures the attraction zones are pixel-accurate with the guide lines shown in the vectorscope.
Precomputed LUTs for hue remapping
The UCS→RYB conversion involves sRGB gamma, HSV extraction, and a Gossett piecewise-linear mapping — too expensive per pixel. Two 720-entry tables are built once at init_global: the forward table fills directly from the conversion function; the inverse is found by nearest-match scan over the forward table (O(N²), N=720, runs once at startup, negligible cost). All subsequent calls are O(1) with linear interpolation.
Winner-takes-all node selection
Each pixel is attracted to its nearest harmony node only, not a weighted blend of all nodes. A blend would pull every pixel toward the geometric center of the node set — losing the palette identity and potentially averaging complementary hues into gray. Winner-takes-all preserves each zone's distinct character.
Neutral protection via hyperbolic ramp
The correction is gated by chroma / (chroma + cutoff + ε) — a smooth saturating curve that approaches zero for near-neutral colors and one for vivid ones. The cutoff = np_t³ × 0.03 cubic mapping distributes the slider's perceptual effect evenly; a linear mapping would concentrate most visible change in the bottom 20% of slider travel.
Two processing paths (CPU)
When smoothing is off, a single fused loop runs forward conversion, correction, and inverse conversion per pixel with no intermediate buffers. When smoothing is on, the forward conversion and per-pixel corrections are cached in pass 1; the correction field (not the image) is Gaussian-blurred; pass 2 applies blurred corrections from cache, skipping the expensive forward conversion. Blurring the correction field rather than hue values directly avoids circular-average artifacts near the 0/1 hue wrap point.
Always two-kernel on GPU, always two-pass on CPU when smoothing > 0
The OpenCL path unconditionally uses a map kernel + apply kernel with shared GPU buffers, even at smoothing=0. Adding a third fused kernel for that case would double the shader variants for a marginal benefit — device-local buffer reads are cheap on modern GPUs. The CPU does fuse at smoothing=0 since avoiding the intermediate allocation is free there.
Context
Earlier versions of this module have been shared with the Pixls community, where it has received some preliminary testing and a rather positive welcome.