Gamma handling in curves is broken

[Beep6581]

Originally reported on Google Code with ID 2850

I've noted that if you apply the same tonecurve on a TIF and as on a RAW the result
is vastly different, which is bad. The reason is the following:

Raw files are converted to linear float, but get sRGB gamma assigned for the curves,
so the curves works in sRGB gamma space (~2.2), and the histogram you see inside the
curve is sRGB gamma. This is as it should be, curves are much better used with a gamma
(or else the shadow range get tiiiny at the start), and sRGB gamma is a good default.

TIFF files are loaded with their native gamma, but then in the colorspace conversion
step they are linearized to linear float, so the internal representation becomes identical
to that of the RAW. However, there is no gamma assigned for them, so the curves will
operate in linear gamma (1.0). This makes the curve behave differently, and curves
on gamma 1.0 makes no sense as the shadow range gets tiny.

I think this is broken. Curves should always operate with the same gamma.

However, although I think the sRGB gamma as used for raws is good, the way they are
applied seems broken. The gamma is pre-applied with a standard RGB curve, then the
tonecurve operates on that, and then the gamma is reversed with a standard RGB curve.
How I would expect it to work is that the tonecurve step is fed with linear data and
then the tonecurve LUT is modulated with the gamma, that is all the curve action is
made with the targeted curve type.

Reported by torger@ludd.ltu.se on 2015-07-22 09:33:19

[Beep6581]

Reported by torger@ludd.ltu.se on 2015-07-22 09:34:40

• Blocking: #2817

[Beep6581]

Reported by torger@ludd.ltu.se on 2015-07-22 09:36:52

• Blocking: #2374

[Beep6581]

This is a one-line patch that makes tonecurves for TIFF/JPEG/PNG behave the same as
for raws, ie with sRGB gamma rather than linear gamma.

Reported by torger@ludd.ltu.se on 2015-07-22 09:45:32

---

• Attachment: tcgamma-patch1.txt

[Beep6581]

I'll make a more elaborate patch that cleans up some gamma stuff... seems to be much
legacy code from the integer days.

Reported by torger@ludd.ltu.se on 2015-07-22 09:56:36

• Status changed: Started
• Labels added: Priority-High
• Labels removed: Priority-Low

[Beep6581]

One aspect with this is that curve behavior of TIFF files will change, breaking compatibility.

I guess that can be a pretty severe issue for some users?

Reported by torger@ludd.ltu.se on 2015-07-22 09:59:18

[Beep6581]

We've never been good about compatibility, that's why we recommend the users to keep
older version if they want to open their source image and produce identical results
as before. Personal standard profiles will have to be updated though, but in my opinion,
it's better to target image quality and usability over compatibility.

About your patch, I can't comment since it's not my playground.

Reported by natureh.510 on 2015-07-22 11:08:33

[Beep6581]

I'm working on a bigger patch of the cleanup/simplification kind, and learning stuff
as I go. I'll make a more thorough description of what happens with that.

Reported by torger@ludd.ltu.se on 2015-07-22 12:10:44

[Beep6581]

Anders, I won't have access for the most part of today, but will join back as soon as
I have it.

Reported by michaelezra000 on 2015-07-22 12:32:53

[Beep6581]

Here's the larger patch, which makes the following

 - removes getGamma() from imagesource as it makes no sense with floating point 
   internals, internal is gamma 1.0 always.
 - removes the forward gamma / inverse gamma step on image data before/after tone 
   curve, and instead apply gamma to the tone-curve itself
     - That is the curve algorithms will work on linear data, instead RGB-gamma as

       before.
     - Gamma thus becomes only a GUI thing, ie curves are view and edited in sRGB 
       gamma space, but the processing pipeline always see linear data.
 - Tiff curves now behave as raw curves, ie always on sRGB gamma.

TIFF look is drastically changed as gamma is now sRGB instead of 1.0 in the curve.
That it was 1.0 before (rather than the file's gamma?) is probably a legacy bug introduced
when RT became floating point. I think it makes more sense to always have sRGB gamma
in the tone curves regardless of gamma in file, so curves become compatible between
files.

RAW look is virtually exactly the same for standard, film-like and weighted, as they
RGB-gamma application does not change much for those curve types.

The luminance curve can get a slight different look for some curves (a little bit more
or less saturation). The reason for this is that the curve now works on linear data
instead of RGB-gamma-distorted data, and I think that is the way it should be. For
the luminance formula to work as intended it must be linear data anyway, so it's behaving
more correct now than before.

Reported by torger@ludd.ltu.se on 2015-07-22 15:13:15

• Status changed: PatchSubmitted

---

• Attachment: tcgamma-patch2.txt

[Beep6581]

New patch:
 - Further minor cleanups
 - Fixed curve pipette buffer handling
 - RGB curves now also are defined in sRGB gamma (was gamma 1.0 before, no wonder 
   shadows were hard to target...)

(Note that Lab-related curves are use L for lightness which is already perceptually
mapped, similar in scale to sRGB gamma, so this gamma mess is only related to RGB stuff
and tonecurves)

Reported by torger@ludd.ltu.se on 2015-07-22 17:48:53

---

• Attachment: tcgamma-patch3.txt

[Beep6581]

The RGB curve part of the patch will naturally also break compatibility. Again I assume
that back in the integer days it operated on the working space gamma, and with float
it became gamma 1.0, which doesn't make sense in the GUI.

Summary so far:

With this patch I intend to make gamma handling clean:
 - Tonecurve 1&2 and RGB-related curves are displayed with sRGB gamma regardless of

   input file or working space
 - Actual curve application is done floating point linear, ie the sRGB gamma is only
   for the gui and curve definitions.

Behavior changes, breaks backwards compatibility:
 - TIFF/JPEG/PNG tonecurve 1&2 now sRGB gamma and thus same as for RAW (before 1.0)
 - RGB curves gamma now sRGB (before 1.0)
 - Minor look change for luminance and satval blend curves due to curve application

   in linear space instead of the old (incorrect) way of first applying an RGB-curve

   gamma. Standard, film-like and weighted are not visibly affected.

Reported by torger@ludd.ltu.se on 2015-07-22 17:58:26

[Beep6581]

The R, G and B curves are much more usable with this patch.

There are tiny color and saturation differences in raw files comparing un/patched,
easy to see using amsterdam.pef and Pop 2 L, see the tree reflections in the water,
or using http://rawtherapee.com/shared/test_images/pentax_k10d_09_20.pef when you move
the curve midpoint to the bottom-right corner.

Reported by entertheyoni on 2015-07-22 20:31:57

[Beep6581]

Noted that the B&W before after curves were also gamma 1.0, fixed that in attached patch.

Personally I think it's important that we anchor this design so when we add a new tool
with a curve we think over the range so the curve actually becomes useful. Noone can
think it's good to have shadow range pressed into the 1/20th first range of the curve.

This patch is quite painful though due to the compatibility breaks, but as it is now
is genuinely poor design(tm) and we need to fix that anyway at some point.

Reported by torger@ludd.ltu.se on 2015-07-22 20:57:04

---

• Attachment: tcgamma-patch4.txt

[Beep6581]

Not that I want to stress folks, but my started work on the new Perceptual curve (issue
2817) is halted by this issue, and it's raining heavily here today so I can't make
use of my vacation anyway, so I'd like to have a decision soon on how to deal with
this. A decision is more important than actually testing this patch.

The patch itself is quite trivial, the big thing is the behavior change, and establishing
a clear design direction which is:

 - We should not have gamma 1.0 curves, but rather have sRGB as standard unless it's

   logical to have something else (like L-scaled curve for Lab)
 - The classic ICC/integer-pipeline thinking of that "working profile" also affects

   gamma, ie Prophoto would cause curves to operate on gamma 1.8, AdobeRGB on 2.2 
   etc, is definitely put into history books, and we clearly state that the pipeline

   is floating point linear and if any tool internally needs a different encoding it

   should translate to/from it's desired encoding in/out from the tool (I don't 
   think we have any such tool now by the way).
 - Curves should of course have the same scaling regardless of file format, ie not

   suddenly become a different gamma (as for TIFF/JPEG/PNG today)

The way RT works today in these aspects I don't think is a deliberate design decision
but rather how things just came to be after years of cumulative changes including a
difficult transition from integer to float. So what we see now is some leftovers from
an integer pipeline which doesn't make sense with linear floating point image data.

Reported by torger@ludd.ltu.se on 2015-07-23 08:47:20

[Beep6581]

Well, I think RT is in your hands about this. You can break compatibility if it fix
misconceptions and enhance image quality, which is the case. Just let me apply your
patch in few hours, but it's a "ok to comit" for me.

Reported by natureh.510 on 2015-07-23 11:29:22

[Beep6581]

I had a quick opportunity to test the patch. It feels and looks good. Consistency in
processing  raws and tiffs needs to be corrected, so I am glad you can do it:). I did
not try to compare the usability of the curve action ranges. But having a larger horizontal
span to work with shadows and midtones is a useful feature to have.

Reported by michaelezra000 on 2015-07-23 12:25:18

[Beep6581]

This is a long requested feature, thanks Anders

By sRGB gamma do you mean the complex linear with 12.32 slope and then exp 2.4 or it's
equivalent g2.2 ?. Here I would vote for pure gamma.

Now some logical options on the gamma value to be used in curves' representation. I
think sRGB or adobeRGB gammas (both at around 2.2) come from the ancient CRT era and
should not be our starting point. We should instead start from a usability point :)

The best fit to photografic logic would be a gamma which will make the curve show the
scale in stops i.e. gamma = 2.0
The best fit to percieved brightness would be same as Lab i.e. gamma = 3 

I vote to have both the above as options, and I think Pinhuer has something ready on
this ..

Reported by iliasgiarimis on 2015-07-23 13:54:26

[Beep6581]

#17: yes sRGB is the one with the linear segment close to shadows. This is what RT has
used for tone curve 1&2 and also contrast/lightness sliders on raws all along, I'm
just expanding it to cover all other RGB-related curves and also TIFF/JPEG/PNG processing.

I don't think the actual gamma 2-3 matters that much from a usability standpoint, the
important thing is that it's some gamma and that it's the same regardless file type
and the same for all curves of the same class. Why I'm going for sRGB is because that's
what's been used so far.

I'm going to make it sRGB for the first patch, and then we can debate if we want some
change. The advantage of keeping at sRGB is that for most raw pp3s the compatibility
is kept.

To make the histogram linearly show photographic stops we would need to make a log2
scale, gamma 2 would not do it. I think sRGB is a fine choice as sRGB is still today
the most common output space. As far as I know Adobe Lightroom uses sRGB gamma for
their curve and histogram, so we are in good company on that.

If I would change it I would probably suggest a pure 2.2 gamma for purity in code,
but it wouldn't change usability and really not much in the code either.

Reported by torger@ludd.ltu.se on 2015-07-23 15:19:24

[Beep6581]

I support making curves work consistently. I'm with #6. The patch seems to work fine
so far. +1 for commit, we'll iron bugs out on the way.

If the curve shows a background histogram (why doesn't TC2?) then that histogram should
be useful and clearly correspond to the curve.

The problem of curve histograms and histogram grids not corresponding to stops affects
the main histogram too.

Reported by entertheyoni on 2015-07-23 18:33:50

[Beep6581]

Ok I think I've got enough feedback to dare to commit this now, and we can always make
adjustments later on.

I haven't had a look into the background histograms, so I don't know why TC2 doesn't
have one.

Reported by torger@ludd.ltu.se on 2015-07-23 19:30:29

[Beep6581]

Committed. Now I'm back to what I was doing before bumping into this :-)

Reported by torger@ludd.ltu.se on 2015-07-23 19:40:10

• Status changed: FixedPendingConfirmation

[Beep6581]

Reported by torger@ludd.ltu.se on 2015-07-23 22:42:45

• No longer blocking: #2817

[Beep6581]

Small fix just committed: discovered that there was the wrong gamma on the curve segment,
now fixed (committed) so it's truly sRGB gamma. The change should be barely visible.

Reported by torger@ludd.ltu.se on 2015-07-24 19:32:02