Better color handling of the Tone curve

[Beep6581]

Originally reported on Google Code with ID 1529

Discussion about tone curve's rendering started here: http://code.google.com/p/rawtherapee/issues/detail?id=1527

The goal is the be able to limit or avoid color shift, or find an algorithm to make
them pleasant like Adobe's tone curves.

For compatibility issue, the old behavior should be kept thanks to a "Method" combo
(like the one for the demozaicing algorithm).

Pardon me in advance if i don't participate much on the algorithm/technical discussion
of this issue ;)

Reported by natureh.510 on 2012-08-20 14:46:17

[Beep6581]

First question: does one called for by "tone curve"?
Excuse my bad english ...

1) is it a curve that acts only on the luminance? In this case, it is L=f(L) in "Lab
adjustements" but in RGB mode it is very difficult to make a curve "luminance" without
acting partly on the color ...

2) is it a curve that acts on the gamma (as "output gamma") which acts on the luminance
within an ICC profile? (it is very difficult - to my knowledge - to make profiles "input"
with LCMS)

3) Is it a curve that is within a profile "input" that can change the contrast and
saturation. For example "ProfileMaker5" produce profiles of such high quality? (portrait,
landscape, etc.). And in "reproduction mode" these profiles are probably the best (low
deltaeE94)

4) Is a set of curves which acts seperately on the luminance and chromaticity as there
are in "Lab adjustements" and has maximum flexibility? These "set" allow a total control
on "color shift" 

5)?

Reported by jdesmis on 2012-08-20 15:42:32

[Beep6581]

I think it is about RTs tone curve in the exposure section.

The dicussion started on the DCP tone curve support thread. Hombre likes the visual
results of the Adobe curve (baked into some DCPs). It's technically a simple algorithm
it seems, I ported it from the Adobe reference to RT, see ApplyToneCurve() (dcp.cc,
line 462)

Then Michael joined in, having discussed with Emil privately, that the problem would
be that the current RT Lab L curve (though it avoids color shift) seems to affect saturation/hue/chromacity.

I offered to port the Adobe reference tone curve algorithm to our normal (exposure)
tone curve as additional mode (e.g. with a checkbox). But since I am no specialist
on this topic I don't know if the algorithms is worth it. Since it looks less sophisticated
than e.g. your new Munsell stuff I want to avoid useless controls and GUI bloating.
Your opinion on this would also we interesting.

Reported by oduis@hotmail.com on 2012-08-20 19:47:49

[Beep6581]

I don't understand. The "Lab L curve", does not affect saturation/hue/chromaticity.

To be sure, just choose as "working profile" Prophoto and disable "avoid color shift"
to prevent control gamut and correction Munsell.

When you change the "Lab L curve", only the L values ​​change, which is what is expected.

Reported by jdesmis on 2012-08-20 20:26:57

[Beep6581]

I cannot follow the problems with the (new) Lab curve either. However Michael put some
link and description here: http://code.google.com/p/rawtherapee/issues/detail?id=1527#c22

Could you share your opinion about Adobes reference curve (position see above)?
(Good night)

Reported by oduis@hotmail.com on 2012-08-20 20:34:08

[Beep6581]

Sorry, i should have been more precise: i was referring to the Tone curve of the Exposure
tool. Actually, as far as i know, each channel is affected by that single curve, which
obviously will introduce colorshift (the RGB curve's tool ( which is the same tool
than the Exposure/Tone curve but for each channel separatly) is not sufficient to adress
the problem.

I don't know if we have to use Adobe's algo (well, their sample code), i'm quite confident
that Jacques can do wonders in this field ;).

About comment #1 point 3: using tone curve embedded in the input profile is what Olli
did in his last patch, but for from DNG/DCP only, and with Adobe's sample code. Jacques,
if you know other filetype and/or algo for applying the curve, we would be glad to
hear you :)

Reported by natureh.510 on 2012-08-20 20:40:56

[Beep6581]

I will post illustrations tomorrow evening EST to clearly demonstrate the issue.The
plugin in the link I provided also allows to see the difference of curves in different
applications. Would be invaluable to have a curve that could alter lightness without
any percieved changes in hue, saturation and  chromaticity across full range of brightness
and colors. This should be a new curve, and will likely be used as a primary tool in
RT. All existing curves then can provide flavoring for fine tuning to artistic taste.
I just hope it is possible:)

Reported by michaelezra000 on 2012-08-21 01:16:05

[Beep6581]

The current tone curve in the exposure tool acts multiplicatively on each RGB channel,
multiplying each by a common factor.  I find it curious that this leads to perceptual
changes in hue/saturation -- I would have thought that maintaining the ratios of the
three color stimuli would lead to the same apparent color, but apparently not.

Reported by ejm.60657 on 2012-08-21 03:53:12

[Beep6581]

Here is a quote from article I mentioned in the other thread, sheding some light on
how Adobes Curves work.
(Thomas Knoll=Lead of Photoshop etc., CR= Adobe Camera Raw)

"Thomas Knoll was asked how these Curves algorithms affect saturation and hue. His
reply indicated that in both Photoshop and CR the Curves are designed to moderately
boost saturation with contrast, resulting in saturation effects that resemble those
from film tone curves, which many years of looking at photographs from film has taught
our visual perception to like. He said the programming is easier without the saturation
boost, but included it because after extensive testing they determined the results
are generally not pleasing without it. The developers of “Raw Shooter” (since acquired
by Adobe) came to the same conclusion independently. CR Curves have a hue lock. They
map the minimum and maximum RGB values (in linear ProPhoto RGB) through the tone curve,
and the middle value is interpolated to exactly preserve hue. The PS-RGBc does not
have a hue lock. But setting the PS-RGBc to Luminosity Blend Mode preserves the colour
of the underlying layer. Using the Lab L* Curve for this purpose has only fair colour
consistency as L* is changed holding a* and b* constant. To corroborate this, one page
from colour scientist Bruce Lindbloom is referenced: (http://www.brucelindbloom.com/index.html?UPLab.html)."

Reported by oduis@hotmail.com on 2012-08-21 05:35:12

[Beep6581]

Means Adobes curve is optimized for visual pleasingness, and not color correctness.
This explains Hombres findings, and I'll take back my comments that it might be less
sophisticated than ours.

I'll try to come up with an experimental patch to port this to our tone curve, to find
out if such a "visual pleasing" tone curve mode would be a good addition to RT.

This shouldn't stop the dicussions about a color correct Lab based curve with Michael
though.

Reported by oduis@hotmail.com on 2012-08-21 05:44:45

[Beep6581]

Providing a curve that modify the image in a pleasant manner in a single shot is what
most users are waiting for (including me). So providing such a tone curve beside the
existing "color correct" ones is a must have would be very welcomed :). The tone curve
embedded in the DCP profile now optionnally usable may also benefit from it.

Regarding the special color profiles from Bruce L., it seems that it does what RT does
in a dedicated functions coded by Jacques (located in colors.cc iirc). Jacques, can
you evaluate if the ICC profile would be a better or simpler candidate for preserving
hues? Can we evaluates both methods effectiveness (if possible)? I for one wouldn't
like to see duplicated functions :-/ (self made & provided by Lcms+ICC profile).

Reported by natureh.510 on 2012-08-21 07:56:29

[Beep6581]

Bruce Lindblooms ICC has already been tried, it doesn't work correctly (see issue 1297,
which I had to close).

Reported by oduis@hotmail.com on 2012-08-21 08:07:06

[Beep6581]

Oh, didn't saw that one. So Jacques' functions are the only candidate then!? I moved
some methods related to Munsell correction in colors.cc, but as explained by Emil (
http://code.google.com/p/rawtherapee/issues/detail?id=1313#c30 ), we should have properly
modularized MapToMunsell(), MapFromMunsell() and MunsellCorrection(Image1, Image2)
methods. They may be created in the LabImage class (labimage.cc) and use the methods
from the Color class i guess.

I'm very busy with the XMP branch right now, so can anyone else cope with this?

Reported by natureh.510 on 2012-08-21 09:34:49

[Beep6581]

> So Jacques' functions are the only candidate then!? 
Plus the one that Michael talked about.

Reported by oduis@hotmail.com on 2012-08-21 12:27:45

[Beep6581]

There might be one more option from what we explored with Emil, although not sure it
should replace another L tonecurve I suggested.

Current RGB tonecurve leads to pleasing tones when curve is lifted above the diagonal.
Colors look dirty when curve is lowered under the diagonal. 

The current RGB tonecurve is:
//brightness/contrast and user tone curve 
r = rCurve[tonecurve[r]];                 
g = gCurve[tonecurve[g]];                 
b = bCurve[tonecurve[b]];                 

As suggested by Emil, we tried this version of RGB tonecurve:
Y = (0.299*r + 0.587*g + 0.114*b);                            
tonefactor = ((Y>0.01f && Y<MAXVAL) ? tonecurve[Y]/Y : 1.0f); 
r = rCurve[r*tonefactor];                    
g = rCurve[g*tonefactor];                     
b = rCurve[b*tonefactor];

In this case the behavior is opposite - it is cleaner for for darkened tones but results
in over-saturated tones which are brightened. This effect is the same as with the Black
slider in the Exposure section.

I tried mixing the effects of two curves but had mixed results (literally:)). So the
curve option I was thinking - we could leave the current tone curve as-is and use it
as a Brightening curve, but also create an additional curve that is more suitable for
Darkening tones. So it is not a single do-it-all RGB curve but a dual RGB curve setup.
This also is convenient for editing workflow - when I work on image in Photoshop I
create multiple curve layers, some for brightening, some for darkening. This is a convenient
separation of editing steps, as they can be iteratively fine tuned independently.

Reported by michaelezra000 on 2012-08-21 14:10:31

[Beep6581]

A small correction to comment 14 - in the Darkening curve it should be rCurve, gCurve,
bCurve (copy/paste error).

Reported by michaelezra000 on 2012-08-21 14:47:14

[Beep6581]

RawTherapee is already like a swiss knife and it has many ways to achieve the same goal.
It is very subjective from wich point a GUI becomes cluttered, but I think RT is already
more technical than photographical.

I do understand the need for more tools and personally find almost every new toy interesting.
But think from the perspective of a non-experienced, non-technical photographer: would
you really want to learn all the small tricks and tools to achieve, lets say, a custom
contrast enhancement with pleasing colors?

I think the combination, the unification of tools should be kept in mind. If only separate
tools can achieve something, then of course we need separate tools.
But technical difficulties should not be solved by creating usability difficulties.

Reported by gyurko.david@e-arc.hu on 2012-08-21 15:21:30

[Beep6581]

I'm not for creating 2 curves as proposed in comment #14, sorry, we should focus on
using only one curve, and with a checkbox to use the old algorithm (and i would add
if the loaded file contains it only) for compatibility issues.

More hints on the problem:

Let's suppose we have 8bit RGB color values like:

A(25, 52, 215)
B(25, 20, 31)
C(192, 230, 245)

Let's suppose an S-type tone curve, which compress the darkest half and expand the
highlight half.

When applying the curve, A will have strongly shifted colors, and B & C will have slightly
shifted colors, because:

A(25  Compressed, 52  Compressed, 215 Expanded) -> varies in opposite direction
B(25  Expanded,   20  Expanded,   31  Expanded) -> varies in the same direction
C(192 Expanded,   230 Expanded,   245 Expanded) -> varies in the same direction

The color is mainly set by the relative value between the componant, so whe have to
apply the tone curve in a smart manner (not the same way for each component) so that
relative  values are preserved.

Reported by natureh.510 on 2012-08-21 23:02:58

[Beep6581]

Here is a patch that you can play with, more a proof of concept than a committable patch,
that doesn't address the issue anyway. It change how the whole Exposure's tool set
is applied, by locking the hue (if you have free time to lost). But there's still room
for enhancement for the saturation and value handling i guess.

Reported by natureh.510 on 2012-08-21 23:59:32

---

• Attachment: TweakedExposureAlgo.patch

[Beep6581]

Coming along with this tweak, here are some tone curves that mimics the base curve of
what DNG Profile Editor display when rendering a Pentax K20 image. Even if made by
"eye-metter", it's very very close. Dunno if it's the same base curve for all camera
models.

Also added some partial profiles that mimic the rendering from my Pentax DSLR (boosted
deep shadow), another one less agressive for the shadows (boosted natural), and one
for portraits, based on boosted deep shadows, but with skin tone protection. Maybe
they could be committed for themselves?

Reported by natureh.510 on 2012-08-22 02:07:10

---

• Attachment: Profiles.zip

[Beep6581]

Hombre, this is a neat trick, but there are still remaining color shifts.
Here is a first take on comparisons:
http://www.michaelezra.com/Projects/RT/images_shared/RT_tonecurve_analysis_01.jpg
(7.8MB) 

Hombre's curve 1 uses: Color::hsv2rgb(h, s2, v2, r, g, b);
Hombre's curve 2 uses: Color::hsv2rgb(h, s , v2, r, g, b);

Reported by michaelezra000 on 2012-08-22 03:52:19

[Beep6581]

Also a smaller screen-size file: http://www.michaelezra.com/Projects/RT/images_shared/RT_tonecurve_analysis_01_sm.jpg

Reported by michaelezra000 on 2012-08-22 03:57:51

[Beep6581]

Here is a slight mod to Hombre's patch that allows to compare the tone curves within
a single RT instance. Value of params->toneCurve.hlcomprthresh is used to switch the
curves: 
0 - original
1 - Hombre' h lock
2 - Hombre' h, s lock

Reported by michaelezra000 on 2012-08-22 04:11:37

---

• Attachment: TweakedExposureAlgo-02.patch

[Beep6581]

Here is another illustration from CIECAM02 plugin on difference in JSH and JCH modes
when darkening or birghtening the image:
http://www.michaelezra.com/Projects/RT/images_shared/RT_tonecurve_analysis_02.jpg

It should be evident from RT_tonecurve_analysis_01 and 02 illustrations that some transforms
are natively better at darkening while others at darkening. So can the best modes be
combined *cleanly* into a single curve, or if not then lets have another look at the
2 curves option?

Reported by michaelezra000 on 2012-08-22 11:49:13

[Beep6581]

Something to note - the neutral RT image that was used as a source in above illustrations
has some clipping in the highligts of the red channel. I left it on purpose to illustrate
some other color shifts that occur in RT near MAXVAL and how this is handled across
various tools.

Reported by michaelezra000 on 2012-08-22 11:54:03

[Beep6581]

About my comment 23 - I had a bit of a mixed feeling about brightening in JSH mode -
just so used to a washed out saturation whenever we brighten (hence marked JCH brightened
result as more natural), but it seems that JSH brightens better than JCH. So, it appears
that may be a single do-it-all curve could be possible in JSH?

Reported by michaelezra000 on 2012-08-22 13:45:50

[Beep6581]

Took me a while, but here is the patch that enables the Adobe/RawShooter reference curve
using a new checkbox „film-like curve“.

Though they don’t care for color correctness, Thomas Knoll was right: Images edited
with this tone curve mode look very good. While formerly I had to brighten up using
the tone curve and then get the punch back using Lab, now it’s enough to use the film-like
curve, the intensiveness is preserved.

Apart from that agree with David: it would not be a good idea to have dozend slight
variations of the color correct curve like in that plug-in. One new color-correct tone
curve should be enough in my opinion.

Reported by oduis@hotmail.com on 2012-08-22 16:14:19

• Status changed: PatchSubmitted

---

• Attachment: AdobeToneCurve02.patch

[Beep6581]

Damned, this new algo is really worth it! :)

But:

1. You save the parameter as an integer value in the PP3 file. To stick to the pseudo
rules of this file, could you replace it by either a boolean, or if we want to think
about future method additions, use plain names instead of 'id' ?

2. You've forgot to include curveMode in ParamsEdited::combine.
While you are at it, you can also fix a copy/paste error in ProcParams::save for LensProfile:
you're referring to the same boolean (procparams.cc, line 587).

3. There an instability of the thumbnail brower that wasn't there before this patch,
afaik: when click on the thumbnail that contains an edited tone curve with the checkox
activated and on thumbnails arond, the curve applies to them too or dissapear! i had
a similar issue when developping the ThresholdAdjuster, but i don't remember what was
the cause :(.

3. You should increment the version number of the PP3 file

4. And add this new parameter in the bundled profiles.

(5.  ... but i guess it was planned, remove the printf)

Reported by natureh.510 on 2012-08-23 00:21:21

[Beep6581]

Here is the expanded comparison - I added new Adobe curve from AdobeToneCurve02.patch:
http://www.michaelezra.com/Projects/RT/images_shared/RT_tonecurve_analysis_03.jpg

Reported by michaelezra000 on 2012-08-23 03:21:04

[Beep6581]

Morning,

Thanks for testing.

1. The int was by design. It was meant for the other curve you and Michael would come
up here (so e.g. mode 2=new color correct tone curve), and internally we should really
move to ints (later enums) and not strings. But I can make them strings just for PP3
storing.

3. Haven't had such an issue. Is just just e.g. in single tab mode or something?

Will add the other stuff.

Reported by oduis@hotmail.com on 2012-08-23 05:44:26

[Beep6581]

1. Yes, i was talking about the PP3 value names. procparams handling has to be completly
refactored anyway, will do that later...
3 (the first one, sorry for having fucked the list numbering :-/). I'll look at it
tonight, when i'll be back home. I'm in multiple editor mode, didn't tested in single
tab mode.

Reported by natureh.510 on 2012-08-23 08:03:01

[Beep6581]

There is some discrepancy in the new tonal curve & histogram.
Start with underexposed image and a diagonal curve, move the top right point to left
so that it clips highlights (so pass the right end of the histogram). Now select the
new checkbox - nothing is clipped - you need to move the curve significantly more to
the left, way passed right end of its input histogram to clip highlights.

Reported by michaelezra000 on 2012-08-23 10:43:20

[Beep6581]

Olli, about the issue 3a: 
Browse to previously edited image (prev RT version) with cropping.
Open image (I use single edior) - cropping is gone, other params as well.
Current patch should be used with care on test images only until stable, as you might
loose editing settings.

Reported by michaelezra000 on 2012-08-23 11:11:59

[Beep6581]

when new checkbox is selected in Single-tab Editor, there is an extra event in history
- "Processing Profile is changed in browser"

Reported by michaelezra000 on 2012-08-23 11:25:45

[Beep6581]

@Hombre: ok, if it's redesigned anyway I'll make a primitive approach without enums
etc.

@Michael: the histogram describes the input, not the output. The processing of the
film curve is pretty different from the other algorithms, so it doesn't move the values
to clip. The histogramm is technically correct.
I do not remotely touch cropping in the patch, but I'll see what might be the problem.

Reported by oduis@hotmail.com on 2012-08-23 11:27:40

[Beep6581]

I dont think there is issue with histogram, its just that curve's behavior relative
to input histogram is different than other curves (tonal & L).

Reported by michaelezra000 on 2012-08-23 11:50:35

[Beep6581]

Yes, exactly, but that's technically correct the way it is.

Reported by oduis@hotmail.com on 2012-08-23 11:55:25

[Beep6581]

Here is an updated patch of the one from Michael and me, by setting HL compression threshold
to 3.

It integrate the Luminosity blending, by using hsl (newly integrated) instead of hsv.
I'm not sure that out of range values are properly handled, so please test it.

It is actually used for the whole Exposure tools, but it could be moved to where Olli
use the Adobde curve, to apply specifically for the tone curve and not the other tools.
Looks like you'll have to convert the checkbox to a combo Oliver :), since i like the
rendering of this new "Luminosity blending" method and would also appreciate  to keep
Adobe's method too.

Reported by natureh.510 on 2012-08-23 13:24:53

[Beep6581]

... and the patch :)

Reported by natureh.510 on 2012-08-23 13:25:28

[Beep6581]

Since your patch builds on mine, let's make it vica versa.
Here is the latest version, should fix everything except the checkbox single tab weiredness
(cannot find the reason).
Just replaced that not-working-anyway checkbox with a dropdown and your new values,
and add your code.

Reported by oduis@hotmail.com on 2012-08-23 13:33:49

---

• Attachment: AdobeToneCurve03.patch

[Beep6581]

Hombre, is this a correct patch?

Reported by michaelezra000 on 2012-08-23 13:34:57

[Beep6581]

Olli, I can test in the evening only, but looking at the patch I see ckbToneCurveFilm
- is this a correct patch? Hombre's patch file seems related to another (closed) Vibrance
threshold issue VibranceThresholdMoreIntuitive03.patch 

Reported by michaelezra000 on 2012-08-23 13:47:50

[Beep6581]

Hombres patch looks like another issue.
But what should be wrong about ckbToneCurveFilm?

Reported by oduis@hotmail.com on 2012-08-23 14:35:24

[Beep6581]

May be I misunderstood, I thought you said you already replaced checkbox with a drop
down.
> Just replaced that not-working-anyway checkbox with a dropdown and your new values,
and add your code.

or is it
Just replace that not-working-anyway checkbox with a dropdown and your new values,
and add your code.
... ok, then it makes sense

Reported by michaelezra000 on 2012-08-23 14:49:59

[Beep6581]

Sorry, how one "d" can make a difference ;-)

Reported by oduis@hotmail.com on 2012-08-23 15:01:40

[Beep6581]

o_O  I just realized my mistake Michael, i didn't understood what you meant... Here
is the correct one, but i'm currently merging it with Oliver's one.

Reported by natureh.510 on 2012-08-23 18:18:52

---

• Attachment: TweakedExposureAlgo-03.patch

[Beep6581]

Hombre, this is a great curve!
There is a problem when using a darkening curve (or brightness<0) in the areas when
one channel is clipped.

Reported by michaelezra000 on 2012-08-23 23:49:27

---

• _Attachment: RT_tonecurve_HSL-1.jpg_

[Beep6581]

Will try to fix it. Could you send me you test picture (i'll delete it after the bugfix,
promised)? I don't have such portrait shot.

Reported by natureh.510 on 2012-08-24 08:02:47

[Beep6581]

Hombre, I sent you email. 
About the drop down - it would be useful to create a history entry on every change.

Reported by michaelezra000 on 2012-08-24 13:29:05

[Beep6581]

Does others agrees on creating a history entry on every change? I can see the reason
of this demand, but it would infringe RT's standard behavior, so i would vote against.
The ProfilePanel works as you wish, but for technical reason only.

Reported by natureh.510 on 2012-08-24 15:14:36

[Beep6581]

I very much agree that every change should be recorded. Actually I would prefer to have
recorded every change of any curve, but that's a different story. :)

Reported by gyurko.david@e-arc.hu on 2012-08-24 16:10:32