Tone equalizer 2025-04-06 preview version

[marc-fouquet]

This is a preview of my proposed changes to the tone equalizer module. There is a lot of debug code still in it and there are known problems, see below.

This message only contains code aspects. I will write a detailed post from a user perspective for pixls.us.

Changes

• Introduced post_scale, post_shift and post_auto_align settings, which allow adjusting mask exposure and contrast AFTER the guided filter calculation.
  • The histogram calculation is now split into two steps. First, a very detailed histogram is calculated over a large dynamic range. The GUI histogram and internal parameters used for automatically computing post_scale and post_shift are derived from this histogram.
  • The new parameters are not actually applied to the luminance mask, but to the look-up table that is used to modify the image.
  • With post_auto_align=custom, post_scale=0, post_shift=0, the results are the same as with the old tone equalizer (I was not able to get a byte-identical export, but in GIMP the difference between my version and 5.0 was fully black in my tests).
• Changed upstream pipe change detection from dt_dev_pixelpipe_piece_hash to dt_dev_hash_plus after I noticed that the module constantly re-computed the guided filter, even though this was not necessary.
• Added experimental coloring to the curve in the GUI, it now turns orange or red when the user does something that is probably not a good idea:
  • Raising shadows/lowering highlights with the guided filter turned off.
  • Lowering shadows/raising highlights with the guided filter turned on. The user probably expects a gain in contrast here, but the guided filter will work against this.
  • Setting the downward slope of the curve to be too steep.
• UI changes:
  • Sliders (previously on the "simple" page) are now located in a collapsible section beneath the histogram.
  • Made the histogram/curve graph resizable (see issues!).
• In my efforts to understand the code, I renamed things that were named confusingly in my opinion (i.e. compute_lut_correction to compute_gui_curve) and sorted the functions. The consequence is that I have touched almost all lines of code, so diffs will not be helpful in tracking my changes.

Known issues

Tbh., I had more problems with the anatomy of a darktable module (threads, params, data, gui-data, all the GTK stuff) than with the actual task at hand.

Known problems are:

• Pulling the histogram/curve graph small causes DT to crash. I am clearly still missing something here. There is also no horizontal bar on mouseover to indicate that the graph can be resized.
• When post_auto_align is used to set the mask exposure, the values for post_scale and post_shift are calculated in PREVIEW and used in FULL. However, other pixel pipes (especially export) calculate the mask exposure on their own and may get a different result that leads to a different output.

Things I noticed about the module (a.k.a issues already present in 5.0)

• Resetting the module multiple times makes the histogram disappear until the user moves a slider.

Related Discussion

Issue #17287

[marc-fouquet]

The detailed explanation is here now: https://discuss.pixls.us/t/tone-equalizer-proposal/49314

[MStraeten]

macos build fails

~/src/darktable/src/iop/toneequal.c:1343:94: fatal error: format specifies type 'long' but the argument has type 'dt_hash_t' (aka 'unsigned long long') [-Wformat]
 1343 |     printf("toneeq_process PIXELPIPE_PREVIEW: hash=%ld saved_hash=%ld luminance_valid=%d\n", current_upstream_hash, saved_upstream_hash,
      |                                                    ~~~                                       ^~~~~~~~~~~~~~~~~~~~~
      |                                                    %llu
1 error generated.

[wpferguson]

Does this preserve existing edits?

Just imported some existing tone equalizer presets. Applying them has no effect, therefore I would believe this version isn't backward compatible and will break existing edits.

[ralfbrown]

Does this preserve existing edits?

Looks like a missing version bump - the code is present to convert old edits, but darktable doesn't call it since it thinks they're still the current version.

[marc-fouquet]

Thanks for your feedback. I will provide a new version in a few days which includes the changes that were suggested here.

Some advise on my two major roadblocks would be helpful:

1. The auto-alignment (post_scale/post_shift) is calculated in PIXELPIPE_PREVIEW and I would like to use the results in all pipes. It is easy to get this data to the main window (PIXELPIPE_FULL) by storing it in g. However I also need a way to apply the same values to exports (which don't have g). Since the guided filter is not scale-invariant, the results would differ somewhat if I re-calculated the alignment with pipe-local data during export.

• Does it make sense to store this data in p (from process running PIXELPIPE_PREVIEW) using fields that are not associated with GUI elements? These fields could be floats that are initialized to NaN and replaced with real values when they are available, commit_params would copy them to d.
• Are there cases in which an image has never seen a GUI at all, but still needs to process in a pipe correctly?

2. The problem with resizing the graph. I have only done two things:

• In gui_init

  g->area = GTK_DRAWING_AREA(dt_ui_resize_wrap(NULL,
    0,
    "plugins/darkroom/toneequal/graphheight"));

• and in _init_drawing

  g->allocation.height -= DT_RESIZE_HANDLE_SIZE;

• After that, resizing the graph works, but no handle is displayed and it is possible to drag the graph too small, which results in a crash.

[paperdigits]

@TurboGit please note there has been a lot of discussion in the thread on pixls. Please take that into consideration.

[AxelG-DE]

I am late to the party and I will also keep silent again for private reasons IRL.

From my perspective, I do not have many issues with ToneEQ.

There is only one thing bothered me from day-one of this module:

• mask exposure/contrast compensation sliders are on masking tab
• histogram is on the advanced tab
• For precise attenuation one needs to forward/backward jump between those two tabs.

I pretty much dislike and had long argues with the author Aurtélien Pierre by its time. He kept trying to convince me that it is not doable differently for this and that reason (as he usually behaves). That bar-indicator on masking tab did not guide me as precise as he always pretended it would be.

Nowadays I have mapped those two sliders to two rotaries on my midi-board (X-touch mini) and I can attenuate while looking at the advanced tab.

The luminance estimator / preserve details is another thing slightly above my head but from time to time I use it.

All the rest, I barely touch

After setting the mask, I hover the mouse on my image and scroll the wheel (for this I sometimes need to off/on the module as the rotaries seem to mess with the module-focus).

For me the “simple” tab can just be hidden totally. Besides, please do not clutter the advanced tab.

I hope my workflow (above) will not be destroyed and nor the old edits.

In other words: Never change a running system Thank you!

[TurboGit]

@paperdigits : Yes I've seen and followed a bit the discussion, but it became so heated that it has almost no value to me at this point so I don't follow it on pixls anymore. We'll see if part of it is moved here in a PR.

[wpferguson]

Existing user defined presets no longer work.

As for the included presets, whenever I apply one the histogram contracts to take half of the horizontal space and moves to the left edge. If I move the histogram back where I want it and apply a preset again the same problem occurs.

[wpferguson]

Showing the mask with preserve details shows no difference between no, EIGF, and averaged EIGF.

Masks are vastly different between tone equalizer on current master and this PR.

New masks...

versus current master

[jenshannoschwalm]

• Does it make sense to store this data in p (from process running PIXELPIPE_PREVIEW) using fields that are not associated with GUI elements? These fields could be floats that are initialized to NaN and replaced with real values when they are available, commit_params would copy them to d.

• Are there cases in which an image has never seen a GUI at all, but still needs to process in a pipe correctly?

Ad 1) Nope, you must not use the parameters for keeping data while processing the module. We have the global module data, you might use that to keep data shared by all instances of a module. But you'd have to implement some locking and "per instance" on your own. Unfortunately we currentlx don't have "global_data_per_instance" available.

Ad 2) Yes, exports and the cli interface.

[marc-fouquet]

@wpferguson Are you sure that the settings were the same?

The version in this PR had the bug with the version number introspection, so it did not convert existing settings correctly. It is probably better to wait for a new version to test this.

[marc-fouquet]

• Are there cases in which an image has never seen a GUI at all, but still needs to process in a pipe correctly?

Ad 2) Yes, exports and the cli interface.

So it probably does not make sense at all to depend on values that come from the GUI during export.

The core problem is the scale-dependence of the guided filter. An alternative approach for exporting would be to create a downscaled copy of the image (sized like the GUI preview), apply the GF, get the values I need and then apply them to the the full image - essentially simulating the preview calculation. Not the most efficient approach, but it would only be needed during export when auto_align is used.

[rgr59]

Not sure I understood the last post correctly, but if I did, in my opinion there would be a problem.

Firstly, for the CLI case, where there is no GUI, how can the GF computation be done on a downscaled image sized like the GUI preview? But also if there is a GUI, I think the export result must not depend on the arbitrary size of the darktable window (and thus the preview size) at the time the export is done. (Later exports of the image with unchanged history stack and same export settings must always yield identical results.)

[jenshannoschwalm]

So it probably does not make sense at all to depend on values that come from the GUI during export.

Definitely not, it won't generally work.

The core problem is the scale-dependence of the guided filter.

I didn't check /review your code in it's current state but are you sure you did setup correctly? There are some issues but we use feathering guide all over using masks and results are pretty stable to me.

About keeping data/results in per-module-instance, this has daunting me too on some other project. Will propose a solution pretty soon that might help you ...

[marc-fouquet]

Firstly, for the CLI case, where there is no GUI, how can the GF computation be done on a downscaled image sized like the GUI preview? But also if there is a GUI, I think the export result must not depend on the arbitrary size of the darktable window (and thus the preview size) at the time the export is done. (Later exports of the image with unchanged history stack and same export settings must always yield identical results.)

As far as I understand it, the preview thread calculates the navigation image shown in the top left of the GUI. However the actual image that the thread sees is much bigger (something like 1000px wide), so the navigation image must be a downscaled version. I hope (but have not yet checked) that the size of this internal image is constant.

[marc-fouquet]

I didn't check /review your code in it's current state

The code in the PR is outdated and has known problems, not much use looking at it now. I will update it as soon as I have a somewhat consistent state.

but are you sure you did setup correctly? There are some issues but we use feathering guide all over using masks and results are pretty stable to me.

Of course it is possible that I might have broken something, but as far as I am aware, I did not change anything about the guided filter calculation but only modified what happens with the results.

About keeping data/results in per-module-instance, this has daunting me too on some other project. Will propose a solution pretty soon that might help you ...

This sounds nice, but my next attempt will be trying to avoid this.

[ralfbrown]

Note that your code can figure out how much the image it has been given has been downscaled in order to determine scaled radii and the like to simulate appropriate results. A bunch of modules with scale-dependent algorithms do this. It isn't perfect, but does yield pretty stable and predictable results.

Look for piece->iscale and roi_in->scale in e.g. sharpen.c and diffuse.c. Most modules access these in process(), but it looks like tone equalizer actually accesses and caches this info in modify_roi_in().

[marc-fouquet]

I have been playing around with the tone equalizer some more and ran into a bit of a roadblock.

To recap, what I do is:

Image => Grayscale + Guided filter => Mask => Mask Histogram => Percentile values => Auto Scale/Shift values

The buffers in the different pipelines (PREVIEW with GUI and the export) have different sizes and the guided filter is scale-dependent, so when I make statistics over the mask, it is expected that there is a systematic error. The final calculated values deviate so much that there is a visible difference between the image that is shown in DT and the export.

My idea to overcome this was as to downscale the image during export to the same size as the preview and use the downscaled version to calculate the statistics (essentially simulating the preview pipe during export). However the results were still different even though the calculation was done with images of the same size and with the exact same parameters.

Then I added debugging code to write the internal buffers into files and I discovered the reason:

• The image on the left is a crop from the input of the PREVIEW pipeline.
• The image on the right is from the export pipeline. The input buffer was downscaled to the same size as PREVIEW using dt_interpolation_resample. I tested the different interpolation types, this example uses DT_INTERPOLATION_BILINEAR, which should in my understanding be the least sharp option.

However the left image (PREVIEW) is still a lot blurrier than my downscaled version. I would have expected them to be mostly the same.

Using a blurry version of the image is not ideal for my purposes. However the bigger problem is that I need to know, what happened to the image in the preview pipe, if I want to replicate the same steps in the export pipe.

I tried to find relevant parts in the DT code, but had no success so far. I am also interested in the calculation of the size of the PREVIEW pipe image, it seems like the height fluctuates the least and is between 880 and 900 pixels.

[ralfbrown]

The demosaic module downscales if the requested output dimensions to the following module are small enough. FULL is almost certainly getting more data out of demosaic than PREVIEW, which reflects in the sharper image after downscaling. Run with -d perf to see the image sizes being passed along the pixelpipe.

[marc-fouquet]

I finally have a version that I consider good enough to show it publicly. It would be nice if someone would take the time to look at my code, i.e. there are a few "TODO MF" markers with open questions.

The module should be (if there are no bugs) compatible with old edits. I have checked that with parameters "align: custom, scale: 0, shift: 0" the results are the same as in 5.0.1.

Most of my changes were not that much effort. Scaling the curve after the guided filter, coloring the curve, changing the UI (even though it still has a few issues) was not that difficult. The one thing that was hard and got me stuck for weeks is the auto alignment feature:

• If requested by the user, the module should determine the mask exposure and contrast automatically.
• These values should automatically adapt to upstream image changes.
• The result should be the same during GUI operations and during export.

The data that is available to the pixelpipes during GUI operations is different from the export. The FULL pipe may not see the whole image, so it is not suitable to calculate mask exposure/contrast.

The PREVIEW pipe sees the image completely, but it is scaled-down and pretty blurry. The guided filter is sensitive to both of these effects, so statistics on the PREVIEW luminance mask deviate significantly from statistics of the whole image.

The (unfortunately not so nice) solution this problem is to request the full image in PIXELPIPE_FULL when necessary. Of course this has an impact on performance. However in practice I found it acceptable and it only occurs when the user explicitly requests auto-alignment of the mask - so users who use the module like before should not experience performance degradation (unless I accidentally broke something, i.e. OpenMP).

Known issues:

• UI graph resizing is still broken. It is possible to drag the graph too small and crash the program.
• In the auto-align case, the UI needs both PIXELPIPE_PREVIEW and PIXELPIPE_FULL to be completed to draw the histogram. If one is missing (which can easily happen) no histogram is drawn. (Even in 5.0.1 there are similar situations, i.e. no histogram ist drawn after resetting the module twice.)
• My version has been forked from master a while ago. I have looked through the changes on master and applied most of them. The folders for the default presets are still missing, but these are not a problem. The upstream change detection (hash) has been modified, in my version I have also noticed that the original code did not work correctly, but my fix is different.

Other notes:

I have seen https://github.com/darktable-org/darktable/pull/18722 and to be honest I am not sure what it does exactly, but I wondered if it would be possible to give modules like tone equalizer access to an "early stable" version of the image - a version from after demosaic, but before any modules that users typically use to change the image. If this was possible, I would switch the calculating the luminance mask to this input, if the user requests auto-alignment, so re-calculating the mask would not be necessary that often.

Providing modules with access to an early version of the image would also have other use-cases, like stable parametric masks that don't depend on the input of their respective module.

[TurboGit]

However in practice I found it acceptable and it only occurs when the user explicitly requests auto-alignment of the mask

And only once when the auto-align is changed, right?

• UI graph resizing is still broken. It is possible to drag the graph too small and crash the program.

Indeed, you can even resize to negative size of the graph :)

• The folders for the default presets are still missing, but these are not a problem.

Indeed, we need back the hierarchical presets naming. Should be easy.

I have seen #18722 and to be honest I am not sure what it does exactly, but I wondered if it would be possible to give modules like tone equalizer access to an "early stable" version of the image - a version from after demosaic, but before any modules that users typically use to change the image. If this was possible, I would switch the calculating the luminance mask to this input, if the user requests auto-alignment, so re-calculating the mask would not be necessary that often.

A question for @jenshannoschwalm I suppose.

During my testing I found the auto-align a very big improvement indeed. I would probably have used the fully-fit as default or maybe the "auto-align at mid-tone". BTW, since the combo label is "auto align mask exposure" I would use simplified naming for the entries:

• custom
• at shadows
• at mid-tones
• at hightlights
• fully fit

[jenshannoschwalm]

I have seen #18722 and to be honest I am not sure what it does exactly, but I wondered if it would be possible to give modules like tone equalizer access to an "early stable" version of the image - a version from after demosaic, but before any modules that users typically use to change the image. If this was possible, I would switch the calculating the luminance mask to this input, if the user requests auto-alignment, so re-calculating the mask would not be necessary that often.

This is indeed all very tricky pipeline related stuff. Some comments

We always have to remember the restricted fullpipe roi available data.

1. We can only avoid that in modules very early in the pipe (as we do for highlights and demosaic for example) by enlarging roi_in and do some scale&crop to roi_out. Pretty scary stuff and it took a while to understand how we can use that safely.
2. The details threshold for masks uses a trick as it requires unmodified Y0 data, adding such (like a luminance mask) for just one module would lead to even more complicated code and would be pretty difficult to maintain.
3. Only the preview pipe - in gui mode - has access to all (though downscaled and unprecise) data. So you might calculate some stuff there and keep it either in gui data (we do that at some places) or you could use the recently introduced module->data (see #18753 for how-to)
4. You could do a temporary HQ mode for fullpipe like i did in Haze removal improvements 7e1c1065d7e3e7ccbf68ce5d2c64170c6f3b548c . That worked very good but on slow machines the performance drop was too bad so we reverted that "temporary HQ mode" approach. You could check for fullpipe in HQ mode and keep some hash protected data in that case for later re-runs but that a) would likely lead to endless forum discussions and b) exports would still differ if in HQ mode or not.

BTW - nice to see you "keep going" on this :-)

[marc-fouquet]

Thanks for the nice feedback, I will have a look at everything. Since I have limited time for coding, this may again take a little time.

[MStraeten]

the resizing of the graph doesn't work properly on my macos build. Hovering the mouse over the graph causes arbitrary resizing until it's height is 1 (according to darktablerc) sometimes resulting in a crash ... https://github.com/user-attachments/assets/dea0195f-f5d9-4901-b209-17f27ba35974

[MStraeten]

toggle for image historgram doesn't match to the graph display:

[marc-fouquet]

I have pushed an update, even tough not all known problems are resolved. Things take time on my end.

@MStraeten: Graph resizing is a know issue. I have improvements in the new version, but it is not completely fixed. I have never worked with GTK and the Darktable UI is still harder than pure GTK, so I have a hard time figuring out the reasons for strange behaviours.

Regarding the image histogram: This is a debug option, which I plan to remove before release. The differences you point at are basically the reason why I made it in the first place - to see how the image histogram differs from the mask histogram.

Commit message

Changed the points suggested by @TurboGit including:

• Removed unused code
• Changed descriptions for align left/right/center
• Made align "fully fit" the default
• Added underscores to the names of static functions
• Changed newlines after functions
• Removed some debug code and wrapped the rest in ifdefs
• Added hierarchical presets
• Added extra presets that use "fully fit" and a more linear curve (Compress Shadows/Highlights => v3 EIGF)

Graph:

• Graph resizing is now limited and no longer crashes Darktable :-)
• The handle displays correctly
• TODO: Dragging the graph still shows strange "scrolling up" behavior.
• TODO: There must still be some other scrolling-related bug. I once had the points in the graph move in an unexpected way when dragging inside the graph, but was not able to reproduce this.

Suggestion (not changed in code):

• Adding parentheses to expression in gtk.c line 4017 would make the code easier to read. The line breaks suggest that the ternary operator only refers to _resize_wrap_dragging.

Special cases to be tested:

• When exporting small/scaled-down versions of the image with auto shift/scale, is the result still sufficiently similar to the GUI image? In my test, the shift/scale values in the export pipeline were the same to a scale factor of 50% and deviated for smaller images.

[marc-fouquet]

I have spent quite some time in the last few weeks to fix what I presumed was a drawing area resizing bug that I had introduced. When dragging the handle under the resizable graph, while the module is not at the top of the visible part of the module list, the module list will scroll up until the module is at the top.

It took me until today to notice that other modules with resizable graphs show the same weird behaviour (Contrast Equalizer, RGB Levels, Filmic). So this is not a bug in my PR, it has to be fixed elsewhere. (Can someone try it and confirm? Should I open an issue for this?)

(The gif is from 5.0.1 Flatpak)

The other TODO bug above was probably a false alarm as well. I now plan to change debug output, so it is easier to test the most relevant parameters. I hope that the PR is ready for more testers after that.

[rgr59]

I tried with a self compiled master from today under Fedora 42.

With my installation, the behaviour depends on the setting "darkroom -> [modules] -> scroll processing modules to the top when expanded".

If this is checked (which is the default), while resizing the graph, the module flickers a little and changes it's position, but not always exactly to the top as in your example.

If unchecked, the behaviour is as it should be (according to my opinion).

[TurboGit]

Nice, tested again and see at least one issue:

• On the mask tab, if I scroll over any slider in the guided-filter collapsible section most of the time the graph is loosing the histogram. You probably miss a redraw at some point.

Also, for the mask tab I'm wondering if UI wise it is good to have only two collapsible sections? Shouldn't we have the guided-filer as a standard section (not collapsible)? It looks awkward to have only collapsible section on a tab to me.

But again, all this is details the work done is really good to me and makes the tone-equalizer lot easier to use.