Improved tonemapping

[Beep6581]

Originally reported on Google Code with ID 1237

This patch aims to improve "tonmapping"
1) it controls the gamut before "tonemapping" and corrects colors with an algorithm
similar to that of "vibrance" (Munsell correction)
2) it changes the luminance after "tonemapping" to restore the histogram in similar
values ​​before applying "tonemapping" (if Strength > 0)
3) corrects the colors (as in 1)) after "tonemapping"

It is enabled by a checkbox "avoid color shift"

Reported by jdesmis on 2012-02-03 11:50:38

• Blocking: #1130

---

• _Attachment: tone_map0.patch_

[Beep6581]

This is all about the histogram shift?

That's too easy to fix. Effectively, tone mapping replaces the blurred L with L^CompressionExponent.

So, if you want to preserve histogram, replace the output luminance with L^(1/CompressionExponent).
Like, change that loop to:

    float iCompressionExponent = 1.0f/CompressionExponent;
    for(i = 0; i != n; i++){
        float ce = expf(Source[i] + u[i]*(CompressionExponent - 1.0f)) - eps;
        float ue = expf(u[i]) - eps;
        Source[i] = expf(Source[i]) - eps;
        Compressed[i] = powf(ce, iCompressionExponent) + DetailBoost*(Source[i] - ue);
    }

I haven't tested it but believe it'll work.

I wouldn't think of doing this on my own because I use tone mapping to bring out shadows,
histogram shift is essential there.

Reported by nonbasketless on 2012-04-13 14:51:18

[Beep6581]

Oh, and I believe it leaving the 0 to 1 range is due to nonzero DetailBoost. That could
even cause negative output. Clip it.

Reported by nonbasketless on 2012-04-13 14:52:52

[Beep6581]

I'm sorry, one more post. This is more correct and more efficient:

    float iCompressionExponent = 1.0f/CompressionExponent;
    for(i = 0; i != n; i++){
        float ce = expf(iCompressionExponent*(Source[i] + u[i]*(CompressionExponent - 1.0f)))
- eps;
        float ue = expf(u[i]) - eps;
        Source[i] = expf(Source[i]) - eps;
        Compressed[i] = ce + DetailBoost*(Source[i] - ue);
    }

Reported by nonbasketless on 2012-04-13 14:58:45

[Beep6581]

I don't see shadow lifting as the issue here (perhaps Jacques does), but rather the
possibility that the white point is moved -- though I don't see that in images I use
for testing this tool.  

Jacques, could you point me to a raw file where the range is changed in the way that
you say (0-1 lifted to 0.3-1.3)?  I would have thought that the behavior of this tool
would be more like 0-1 gets lifted to 0.3-1 (ie dynamic range compression).

Reported by ejm.60657 on 2012-04-13 15:12:26

[Beep6581]

I personnally thought that it would act like a local contrast tool, but with the ability
to cleverly modify the data to fit in the target range, which is not the case if i
have understood correctly. E.g. if the different tools modify the image beyond 0>1
(let's say -0.05 > 1.52), EPD would compress the tone (without hueshift if possible)
so that new bounds are 0>1.

Sorry if this comment is nonsens.

Reported by natureh.510 on 2012-04-13 15:48:02

[Beep6581]

Emil

You can find on my web site :
http://jacques.desmis.perso.neuf.fr/RT/

2 photos of flowers :
1) _ASC4209.NEF
2) _ASC5477.NEF
These two pictures as a test for me , for "tonemapping" (images "normal" for which
one seeks to obtain beautiful effects), and also for "Munsell"

Profile=neutral  
working profile=sRGB   Ouput profile=RT_srGB
Strength=2

For 1) mini before=0.03  maxi before=0.72
       mini after =0.35  maxi after=1.56

For 2) mini before=0.05  maxi before=0.81
       mini after=0.32  maxi after=1.69

Reported by jdesmis on 2012-04-13 16:41:21

[Beep6581]

I just tried with the amendment proposed by Ben  (  iCompressionExponent) ... Now it
feels like to function properly

The "Equalizer" is from my point of view, not useful now ... Have a look at some pictures.

If everything is OK tomorrow, I modify the patch to keep only corrections "Munsell"

Good job :)

Reported by jdesmis on 2012-04-13 17:50:13

[Beep6581]

I quickly removed the unnecessary code ...

Introducing the new patch with:
1) the improvement made by Ben
2) correction Munsell

Can you test! :)

Reported by jdesmis on 2012-04-13 19:09:25

---

• _Attachment: tone3_map.patch_

[Beep6581]

With the proposed change, the tool acts more like local contrast enhancement than DR
compression, and there is a lot more haloing if one starts to play with the sliders.
 I think that more careful consideration needs to be undertaken to get the luminance
effect right without making the local contrast preservation much worse and artfact-prone.

Reported by ejm.60657 on 2012-04-13 19:37:45

[Beep6581]

With tone3_map.patch, echo what Emil said. The tool has been demoted to what looks like
a USM that works at all preview zoom levels, nothing more.

Reported by entertheyoni on 2012-04-13 21:36:20

[Beep6581]

Hombre could you please upload the patch I tested in comment 33 if you still have it?

Reported by entertheyoni on 2012-04-13 21:55:34

[Beep6581]

You are right, the old setting favors lowlights, and the new microcontrast.
I made a patch that combines the advantages of one and another and retains "Munsell"

I kept the previous sliders, but by changing their names and features.
"Microcontrast" replaces "Equalizer": for low values ​​(0.2) "tonemapping" behaves
much like the patch "tone2_map23", for high values​​, it behaves like "tone3_map".

I put an "average" setting that of course can evolve.

Reported by jdesmis on 2012-04-14 06:40:32

---

• _Attachment: tone4_map.patch_

[Beep6581]

You've all done a great job, and I've been thinking how to make this tool friendlier
to use.

 http://www.cs.huji.ac.il/~danix/epd/ 
The demonstration video on EPD at around 03:30 shows an image being adjusted smothly
in real time. The lector says this is done through blending of the pre-processed images
(quoted from memory, cant check now as im typing through a phone). Is this a viable
option for RT?

Secondly, their sliders are simply named "coars", "fine" and one other. Would controling
EPD in RT with such sliders be possible to implement?

Reported by entertheyoni on 2012-04-14 08:48:31

[Beep6581]

Those three settings are different scales. They do a multiscale decomposition, while
mine's just one.

The other settings are there, just hidden. If we did this it'd end up more complicated
and slower.

I don't see what the problem is, for most cases the default settings are right, Strength
is all that's needed. For people interested there are more options.

Still think this is just being made more complicated. It's not a miracle all in one
image enhancement thing, you know. The settings I put in there control just the tone
mapping, other operations should be other places. My opinion.

Reported by nonbasketless on 2012-04-14 19:16:24

[Beep6581]

other opinions, other assessments?

Reported by jdesmis on 2012-04-15 05:04:32

[Beep6581]

Since you asked for it, I am of the same opinion as Ben - just making it more complex
when the practical gains are very faint is generally not a good idea. For me the use
case for this tool is shadow lightup on contrasty images, think that direction should
be kept.
Also like to second DrSlony: I use it just seldom since the preview is inaccurate and
it's slow. So the improvement efforts of tone mapping should better go there from my
point of view.

Reported by oduis@hotmail.com on 2012-04-15 08:12:27

[Beep6581]

To reiterate, I don't think we need new sliders here.  Munsell correction and a fix
for the white point issue should be sufficient (ie keep the same effect as the existing
tool, just renormalize by an overall exposure compensation so that the white point
is not affected).

Reported by ejm.60657 on 2012-04-15 13:00:02

[Beep6581]

After a bit more exploration with image Jacques' red flower, I think the problem there
is that the tool acts on luminance which is only about 20-25% comprised of the red
channel and about 70% green; greens are relatively low in the image and so get boosted,
so that when the boosted luminance together with untouched ab channels is split apart
again into RGB, the R is pushed into clipping.  Since RT does not clip, one can bring
the image back into range by using the exposure slider.

Reported by ejm.60657 on 2012-04-15 15:17:20

[Beep6581]

So I don't know much about color science, and forgive me if this is a dumb question:
if Munsell color space works better, why not use it for everything?

(I did read up on it and it sounds pretty good)

If you guys want to write a function which adjusts chrominance according to a specified
change in luminance, more power to you. Mantiuk's experiment, though, suggests that
the problem is very subjective and he states that it seemed best to replace L, keep
ab, and let user play with saturation later if desired. If you want to go deeper though,
his experimental results in figure 9 might help. For tone mapping, Mantiuk's c is the
CompressionExponent.

Same paper:

http://www.cs.ubc.ca/~mantiuk/pdfs/mantiuk09cctm.pdf

Concerning white point shift, that shouldn't happen. I'd recommend setting DetailBoost
= 0 (requires rebuild) and seeing if that helps. Utility of DetailBoost is questionable,
but it seemed to usually help and I didn't want to add another slider, so in its current
state it's affected by Strength.

Reported by nonbasketless on 2012-04-16 00:56:28

[Beep6581]

I didn't mean that I seldom use this tool, in fact I do use it quite often, but learning
to make use of it has a steep learning curve. This very powerful tool can help in a
wide variety of photos. I use it for two purposes:
1- to compress the dynamic range so the shadows get lifted
2- to bring out detail or enhance textures of various frequencies

-

We all agree that adding sliders is a bad idea, yet I would like to ask you to consider
holding off from removing sliders until we get time to test this more. I have tested
it quite a bit, yet I still can't say which sliders are necessary and which are superfluous
- mostly due to that steep learning curve, where I'm still figuring out how having
TM-EPD enabled interacts with other tools and which combinations give the same end
result.

-

There is a problem with how the "Scale" slider affects the preview regarding zoom levels.
The actual effect is far more pronounced in the zoom@20% preview in RT than in the
rendered and resized accordingly image.
RT-4.0.8.13 unpatched:
http://i.imgur.com/YRG8c.jpg
http://i.imgur.com/0QC30.jpg

http://i.imgur.com/HQ2lC.jpg
http://i.imgur.com/iGtmc.jpg

http://i.imgur.com/MCO35.jpg
http://i.imgur.com/0gNbw.jpg

http://i.imgur.com/3dIoR.jpg
http://i.imgur.com/hAu3h.jpg

http://i.imgur.com/oDSFj.jpg
http://i.imgur.com/XOinj.jpg

If this difference is due to some processing shortcut to make the zoomed out preview
faster, then I disagree with it: it is of no use faster if it is so inaccurate.

The edge stopping slider shows the preview and rendered images correctly, it's just
the scale slider that differs so much.

At 100% the preview and saved image are sufficiently similar; there is a slight hint
of a halo in the rendered image which is not present in the 100% preview.
http://i.imgur.com/OjPpl.jpg
http://i.imgur.com/Sbmhr.jpg
It's not easily noticeable in this example. I've had another photo where the halos
were very clear, but I didn't take a screenshot with those settings.

...but adjusting tone mapping using a 100% preview makes it very difficult to use.

I will test the latest patch on Wednesday.

Reported by entertheyoni on 2012-04-16 22:37:23

[Beep6581]

I upgraded RT (4.0.8.8) couple days ago, and this morning tried to process an image
with tone mapping. Raising the exposure tone curve with tone mapping on results in
a nearly black image, something which I did a lot and never happened before.

I'd call this behavior "broken".

Reported by nonbasketless on 2012-04-16 22:51:19

[Beep6581]

Hehe, I was just writing about that in my previous post when I decided to delete it
as I was unsure it was a reproducible effect. Not sure whether this is new to 4.0.8.8,
I think I saw it before.

Seems it only happens when I use a high tone mapping strength. Maybe, like me, you
didn't often push the tone mapping strength up, and now that you did you experience
it?

Reported by entertheyoni on 2012-04-16 23:00:27

[Beep6581]

These are different problems.

What I'm describing is a drastic, incorrect change in illumination. But, on further
investigation, the first release with tone mapping has this problem too, so nevermind,
it's probably a mistake of mine, perhaps related to clipping the black point. Sorry
:-<

What you're describing is dependence of output on resolution. This is very unfortunate,
and I spent a lot of time trying to make Scale scale invariant. It's more complicated
than it sounds, not a shortcut, and if there was a way to fix it there would be other
positive implications (mainly less memory usage).

Reported by nonbasketless on 2012-04-16 23:15:35

[Beep6581]

I am pleased that Ben recognizes that there is a bug in tonemapping. That's what I say
for 3 months, but I am told that there is no bug in luminance, it is enough to play
with the existing sliders.
The three sliders that I added  have nothing to do with a "tonecurve", they try to
level the bug. But as I said many times it is only a stopgap.

All images (with or without red) see their luminance increased too much.
Try this picture that comes from Zanzibar _ASC4829.NEF
http://jacques.desmis.perso.neuf.fr/RT/

I considered the two proposals for Ben, and have incorporated and improved slightly.
(1) iCompressionExponent
(2) DetailBoost = 0

This will be my last patch, hoping that Ben can provide a solution without the addition
of three sliders
:)

Reported by jdesmis on 2012-04-17 09:14:11

---

• _Attachment: tone8_map.patch_

[Beep6581]

Pleases... heh, I feel like a douche. It sounded like you guys were seeking improvement
(title), not bug fix.

I don't think either of those are the correct solution. I'll have to see what exactly
is causing this and... fix it.

I wiped this computer and would have to setup a dev environment, but I have a suspicion
on what it might be, so if it's not too much trouble, could you test something? In
ImProcFunctions::EPDToneMap, replace whole function with attached.

Also removed desaturation, not sure why I left it there, sorry. It contradicts me recalling
over & over that Mantiuk suggests "keep chrominance".

Reported by nonbasketless on 2012-04-17 15:53:39

---

• Attachment: ipfepd.cpp

[Beep6581]

Thanks Ben! If you are at bugfixing, could you have a quick look at the tone mapping
crash reports in issue 1317 please?
Error in MultiDiagonalSymmetricMatrix::CreateIncompleteCholeskyFactorization: division
by zero. Matrix not decomposable.

Reported by oduis@hotmail.com on 2012-04-17 16:03:20

[Beep6581]

Ben, are you still working on the patch, or should we commit the given version for you?

Reported by oduis@hotmail.com on 2012-04-19 11:46:22

[Beep6581]

@Comment 75
I replaced that function, but haven't removed desaturation as I have no idea which
part.
Compilation fails with:
Building CXX object rtengine/CMakeFiles/rtengine.dir/improcfun.cc.o
/home/drslony/rawtherapee/rtengine/improcfun.cc:624:6: error: prototype for ‘void rtengine::ImProcFunctions::EPDToneMap(rtengine::LabImage*,
unsigned int)’ does not match any in class ‘rtengine::ImProcFunctions’
/home/drslony/rawtherapee/rtengine/improcfun.h:135:8: error: candidate is: void rtengine::ImProcFunctions::EPDToneMap(rtengine::LabImage*,
unsigned int, int)

and I'm too monkey to fix that myself :]

Reported by entertheyoni on 2012-04-19 13:13:32

[Beep6581]

tone8_map.patch applies and compiles fine but this is the default tone mapping result
on the left, and on the right is the best I could get with tweaking curves on top of
the tone mapping:
http://i.imgur.com/I0wn8.jpg

Reported by entertheyoni on 2012-04-19 14:26:42

[Beep6581]

Tried to correct that and make a patch from it (Ben, we usually supply patch files.
You can generate these with “hg diff > filenameandpatch.patch”). Also re-instanciated
the skip parameter, as I guess it is needed.

Reported by oduis@hotmail.com on 2012-04-19 14:46:58

---

• Attachment: Tonemap9.patch

[Beep6581]

Should I apply tone8_map.patch (139kB) before applying tone8_map.patch (1.4kB), or apply
tone8_map.patch over a clean ca7c7f52ff04 ?

Reported by entertheyoni on 2012-04-19 14:58:17

[Beep6581]

You mean tonemap9? It's Bens code only, which I assumed to work without Jacques patches.

Reported by oduis@hotmail.com on 2012-04-19 15:43:54

[Beep6581]

oops, typo, yes the second patch I mentioned was supposed to be Tonemap9.patch (1.4kB).

I spent the last hour testing tone2_map23.patch vs Tonemap9.patch. Although I can get
quite close, I still prefer the degree of freedom with the sliders tone2_map23.patch
offers. It was much more difficult to fine-tune highlights in photos using the curves
http://i.imgur.com/FvkCi.png , whereas with tone2_map23.patch I could use the highlights
and equalizer sliders without worrying about blowing up the rest of the image. Just
my two cents when you make your decisions.

Reported by entertheyoni on 2012-04-19 16:00:14

[Beep6581]

I'm in a bit of a mess with work & stuff, guys. I don't think I can avoid setting up
a new dev environment... so wait a bit please :-<

Reported by nonbasketless on 2012-04-19 21:24:35

[Beep6581]

BEWARE OF THE FALSE COLOR SUPPRESION STEP!

I've spend 2h to understand why i had a completly different output when using EPD with
one of my image, and the culprit is the false color suppression step that seems to
create severe out of gammut colors in antialiasing pixel, i.e. in transition from light
to shadows when you play with curves (e.g. place a point at 0,0.2 and 0.2,0.35 for
the Blue curve, Cange control type).

The EPD tool revealed the problem by taking into account those pixels in the 100% preview,
which lead to a very different preview than with lower zooming rates, where those pixels
are not part of the image.

To test EPD, it may be wise to put the false color suppression step to 0, as well as
in the bundled profiles.

I can post a sample picture if you want, but i won't be able to do it untill monday.

Reported by natureh.510 on 2012-04-21 01:13:30

[Beep6581]

Please post some screenshots Hombre, I didn't manage to reproduce.
How many photos did you manage to reproduce this on? So we can decide on the severity.

Reported by entertheyoni on 2012-04-25 21:32:38

[Beep6581]

Bump

Reported by entertheyoni on 2012-04-29 18:41:26

[Beep6581]

In this patch, no change "tonemapping" (I expect Ben's changes).

Against by this patch makes changes to "Munsell correcion". 
Functions are now callable. Everything is documented in "ipvibrance.cc"

Reported by jdesmis on 2012-04-30 08:32:30

---

• _Attachment: tone20_map.patch_

[Beep6581]

Here is a link to the DNG file: http://www.rawtherapee.com/shared/test_images/tracteur.dng

pp3 file is attached.

1. open tracteur.dng
2. zoom in to 1:1 scale
3. move the image around (the rendering should be dependant of the actual content)
4. set False Color Suppression to 0

Reported by natureh.510 on 2012-04-30 22:27:09

---

• Attachment: tracteur.pp3

[Beep6581]

Confirmed using your pp3 on other photos as well, major difference in overall brightness
relative to FCS setting.

Reported by entertheyoni on 2012-04-30 22:45:02

[Beep6581]

same comment as above ... The problem is somewhere in the code of "Edgepreservingdecomposition.cc"

Reported by jdesmis on 2012-05-01 05:59:39

[Beep6581]

Guys, just to be clear, I will fix this. I won't "forget" or defer; but right now I'm
really busy.

One of these days/weeks... I've bookmarked this and 1317 to make sure it gets done.

Reported by nonbasketless on 2012-05-02 17:52:44

[Beep6581]

Hi ben,

4.1 stable is coming (see here: http://code.google.com/p/rawtherapee/issues/detail?id=1419#c11
), i don't want to hassle you with this kind of comment, but did you managed to find
a little bit of time to look at this? It would be great to have a well defined EPD
tool for 4.1, otherwise we'll publish it as is, and we'll certainly have to change
the behaviour in future release.

Reported by natureh.510 on 2012-07-24 22:47:24

[Beep6581]

Reported by entertheyoni on 2012-07-25 20:32:24

• Blocking: #1130

[Beep6581]

What's the status here? Is anything going to be happen soon?

Reported by rinni@gmx.net on 2013-03-30 12:58:50

[Beep6581]

I think we can close this issue, because the contribution CIECAM02, solves the problem

:)

Reported by jdesmis on 2013-04-02 14:45:34

[Beep6581]

For users like me with a weak computer setup (win vista32, C2D 8400@3Ghz, 4GB from which
I can use 3GB for programs) using Tonemapping+CIECAM02 while solves the problem makes
the time consumed double (when saving we talk about mins not secs) and crashes when
I have >= 16 MB files.

With Tonemapping + Lab I can develop and save up to 24 MB and more .. but the colors
are destroyed ..

It would be very useful for me to have TonemappingLab with Munsel corrections (Luminance
correction is less useful as I can do almost the same with other controls). 
As it is now I have to increase Lab chromaticity at about the same value as the Tonemapping
strength applied but the correction is not that good this way.

Reported by iliasgiarimis on 2013-04-02 18:54:11

[Beep6581]

As I understand it .. the correction code is almost ready .. why abort it while it is
useful ..

Reported by iliasgiarimis on 2013-04-02 19:00:07

[Beep6581]

Hello Ilias

No the code is not ready ...

I started this because I noticed a malfunction in "Tone-mapping" ...
He was asked Ben to improve Tone-Mapping, but ...

When I pulled CIECAM, it turned out that using variables CIECAM almost solved the problem.
But it is not resolved to the Lab mode ... and I think unless Ben intervention, will
not be ??

Reported by jdesmis on 2013-04-03 06:41:04

[Beep6581]

Then we have to wait for Ben .. but meanwhile we have to find ways to overcome this
malfunction using available controls. And give the relative documentation to the users
..

After my above posts I checked using tonemap + CIECAM02 using the latest win32 build
by gaaned92 (4.0.10.43) and I had no problem saving a 18Mpix file .. thinks look more
stable than before ... I will continue testing with bigger files to find the new limit
..
But the problem with CIECAM02 is not only time/memory consumption. Many times using
CIECAM02 I have color shifts destroying WB. One solution I found useful then is to
manually decrease CAT02 adaptation .. Jacques, we will need your help here ... 

I am thinking of Lab Lightness (-) and Chromaticity (+) when CIECAM02 is no option.
Would some measures on CC24 target be useful to find the best values/analogies for
L\C to counteract the Tonemapper's malfunction ??.

Reported by iliasgiarimis on 2013-04-03 11:34:32