Highlight reconstruction broken, grey highlights!

[Beep6581]

Originally reported on Google Code with ID 752

What is the problem?
Highlights are grey instead of white in the saves image!
The preview looks fine.

What steps will reproduce the problem?
1. Load overexposed photo
2. Highlight recovery amount 50
3. Highlight recovery threshold whatever, 0
4. Highlight reconstruction whatever BUT ENABLED! e.g. luminance
The preview looks fine, the highlights are recovered and what isn't recovered is white.
A bad surprise awaits you when you save a photo! The highlights are grey instead of
white!

Gentoo 64
Branch: default
Version: Dev-3.1m5.14
Changeset: c57194569448
Compiler: GCC 4.4.5
Processor: generic x86
System: Linux
Bit depth: 64 bits
Gtkmm: V2.22.0
Build type: release
Build flags:  -mtune=generic
Link flags:  -mtune=generic
OpenMP support: ON
MMAP support: ON
Rawzor support: OFF

Haven't tested branch_3.0 for this yet.

Reported by entertheyoni on 2011-06-08 01:18:51

[Beep6581]

http://i.imgur.com/b7Umc.png

Reported by entertheyoni on 2011-06-08 01:22:09

[Beep6581]

Yet another annoying example of having four separate image pipelines.  This is getting
old, real fast.

Reported by ejm.60657 on 2011-06-08 01:35:23

[Beep6581]

@DrSlony

What is the preview at 1:1 scale ?

@Emil

Did you already identifyed the four pipelines ?

Reported by natureh.510 on 2011-06-08 14:15:12

[Beep6581]

It's grey at 1:1, looks fine at non-1:1.
Default branch only, branch_3.0 is fine.

Reported by entertheyoni on 2011-06-08 20:57:10

[Beep6581]

I bisected this to:
The first bad revision is:
changeset:   1167:498786e5dc30
branch:      defloat
parent:      1165:14e676790851
user:        Emil Martinec <ejm.60657@gmail.com>
date:        Thu Apr 07 14:40:47 2011 -0500
summary:     re-enabling some functions mistakenly turned off from float branch testing.

Although with all honesty this might be wrong, because I should have ran "sh clean.sh"
before every cmake && recompilation but I didn't, I only added "sh clean.sh" when I
noticed that bisect was being quirky. It needs to be verified.

Reported by entertheyoni on 2011-06-08 23:01:14

[Beep6581]

I re-ran it with "sh clean.sh" from the beginning and I traced it down to 1167 again
without any quirks this time. Has to be it.

Reported by entertheyoni on 2011-06-08 23:38:08

• Labels added: 3.1

[Beep6581]

http://code.google.com/p/rawtherapee/source/detail?r=498786e5dc30bad994d787d165ce8062a17d9416

Reported by entertheyoni on 2011-06-08 23:40:24

[Beep6581]

Someone please confirm... anyone? This is a severe bug.

Reported by entertheyoni on 2011-06-10 07:48:09

[Beep6581]

Confirmed. But solving that is another story... :-/

Reported by natureh.510 on 2011-06-10 07:53:26

[Beep6581]

Try this patch. It's definetly a bug, the question is, if it's this. My test image does
not show gray before, having trouble to repro.

Reported by oduis@hotmail.com on 2011-06-16 19:08:17

• Status changed: PatchSubmitted

---

• Attachment: ColorCorr.patch

[Beep6581]

Not much testing interest in this one... committed to DEFAULT now a bit early, since
it's small and critical.
Please let me know if it also fixes this problem.

Reported by oduis@hotmail.com on 2011-06-17 05:00:49

• Status changed: PatchApplied

[Beep6581]

Hi Olli, I am not sure how to test this. what should I be looking at?
(going to sleep now though. 1:25 AM)

Reported by michaelezra000 on 2011-06-17 05:25:27

[Beep6581]

What is the purpose and effect of correction_YIQ_LQ?

Reported by michaelezra000 on 2011-06-17 05:33:03

[Beep6581]

It false color correction in RAW, a very cryptic name for it ;-)
It was always-on and set to 1 in full processing before the patch, a clear bug. My
suspicion is that it's also responsible for the highlight problems DrSlony described
above. But I cannot repro clearly.

Reported by oduis@hotmail.com on 2011-06-17 05:53:13

[Beep6581]

false color in raw... is that correction of color aliasing on micro level? 
If that is the case, you are right, no need to run it at < 100%.

But I am surprised if it would have effect on a global image appearance.

Reported by michaelezra000 on 2011-06-17 06:00:28

[Beep6581]

Just have a look at the RAW tab, there is a slider for false colors suppression. The
bug was that this was always switched on and set to 1, regardless of your setting there.
Difference is faint, but visible e.g. in blown highlights. It might have cause DrSlonys
problems.

Reported by oduis@hotmail.com on 2011-06-17 06:11:48

[Beep6581]

I think we may be close to putting a face on the enemy! The patch didn't fix this issue,
but it did help me find something that might help.

1- Starting with the neutral profile, load a photo with blown highlights, e.g. http://www.rawtherapee.com/shared/test_images/overexposed_bkt3.pef

2- Zoom in to 1:1 because this bug is only visible at 1:1 and in the saved image.

3- Set Highlight Recovery Amount (HRA) to 100, threshold (HRT) doesn't matter, let's
say 0.

4- Turn Highlight Reconstruction on, doesn't matter which one, let's say luminance.
Notice how all the highlights are grey, #D9D9D9 here if HRA=100 and HRT=0. http://i.imgur.com/FGoRJ.png

5- In the RAW tab, change False Color Suppression Steps (FCSS) from 1 to 0. The highlights
go much brigter, from #D9D9D9 to #ECECEC! http://i.imgur.com/69Bhr.png

I don't know if FCSS=0 fixes this, but it certainly makes the photo much more pleasing
to look at even at such an extreme HR value (100). For reference, the same settings
in branch_3.0 (HRA=100, HRT=0, HR-luminance, FCSS=doesn't matter) produce a highlight
color=#EFEFEF, almost the same as with FCSS=0 in the default branch.

As the patch doesn't fix this issue (but is certainly welcome), I'm changing the status
to "Started".

Reported by entertheyoni on 2011-06-17 20:18:32

• Status changed: Started

[Beep6581]

Then it look to me like there is no bug left:
FCSS is only executed on 100% crop or full processing, not in the normal preview (after
this fix of course). And that is the same behaviour now as 3.0 by the way, so there
is no difference between the two branches in that respect any more.

You are not forced to activate FCSS (anything >0). Obviously the algorithm interfers
with "the other" highlight recovery in special situations as you described. But thats
not really a bug from my point of view, many algorithms will interfer with each other
in certain situations, especially the numerous highlight recovery features we have.

Reported by oduis@hotmail.com on 2011-06-17 20:35:19

[Beep6581]

Sounds like there is a bug in false color suppression...

Reported by ejm.60657 on 2011-06-17 21:12:51

[Beep6581]

No bug left? We can't release a program that turns a photo to garbage where where the
photographer has no idea what's going to happen until he either saves the photo or
zooms to 100%.

What's special about the situation I am describing? What other highlight recovery?
False color suppression is supposed to prevent tinges of wrong color in places such
as along that window frame's edge in the screenshot. It should have no effect on large
flat overexposed areas - that's a bug.

Look at this comparison between the latest default and branch_3.0 builds:
http://i.imgur.com/36P0C.png
Clearly 3.1 makes the photo look much worse than 3.0 when FCSS=1.

Secondly, I don't think there should be a situation possible where highlight recovery
is unable to recover a highlight and so it just makes that area grey. The moment highlight
recovery can no longer really recover anything is the moment where it should stop trying
to.
Example: open overexposed_bkt3.pef in the 3.0B1.46. The moment that HL stops recovering
anything and starts making the whites turn grey is when the HLA slider is at 61. HLA
should not register values above 61 for this image. For 3.1m6.20 that HLA value is
53. Anything over these values makes the photo look shit.

I found an easy way to find out when to stop sliding the HLA slider: look at the histogram.
The moment that you see the colored boxes appear in the top-right corner is the moment
you should stop sliding. You want to have the brightest unrecoverable pixels white,
not grey!

Reported by entertheyoni on 2011-06-18 17:05:43

[Beep6581]

Here is the next fix (hopefully), the blown highlights are white now for me on FCSS
on.
It does not solve the "problem" though that you only see the effects of FCSS (whatever
it does good or bad) is only visible in result image or 100%. But I see this not as
a bug. And it does not guarantee that the many tools don't interact badly with each
other (like Raw White point and the color casts etc.)

Reported by oduis@hotmail.com on 2011-06-18 21:43:47

• Status changed: PatchSubmitted

---

• Attachment: FCSSBug.patch

[Beep6581]

Committed to DEFAULT since I just saw Emil needs this for #780. Please let me know if
it fixes your problem. At least I could reproduce it now with your test image.

Reported by oduis@hotmail.com on 2011-06-19 07:24:29

• Status changed: PatchApplied

[Beep6581]

Yes! FCS now correctly does what it's supposed to do at all levels without making highlights
grey. Default looks the same as branch_3.0.

Can you explain what the patch does? It no longer converts the values to int - it keeps
them as float?

As far as this issue is concerned, I think I can close it. There is another problem,
in that, as I wrote before, I don't think RT should allow highlight recovery to work
beyond what it's capable of, and turn non-recoverable whites into grey - who would
want that? HR should stop working as soon as that happens. But I will nag about that
in a new issue.

Thank you!

Reported by entertheyoni on 2011-06-19 22:09:36

• Status changed: Verified

[Beep6581]

Patch removes another relic CLIP() command that shouldn't have been in the code.

As far as allowing or not allowing reconstruction, are you saying the current algorithms
are buggy, or somehow still different from their behavior in 3.0?

Reported by ejm.60657 on 2011-06-19 22:24:29

[Beep6581]

They don't differ from 3.0. If you take "bug" to mean "unwanted behavior", then yes.
If I want a photo to have no white point (as a rule of thumb, all photos should make
full use of the histogram), I would use curves, levels, brightness or the exposure
slider to achieve that. The highlight recovery slider should not turn whites into greys,
as I wrote in previous comments.
Why is this so important to me? Simple, two reasons:
1- I would like to be able to set a default profile to any highlight recovery level,
whether it be 50 or 100, and be sure that RT will recover what it can while not making
unrecoverable areas grey - keep them white.
2- I would like to be able to quickly slider the highlight recovery slider to the right
to have RT recover what it can without making unrecoverable areas grey, and not have
to waste time slowly clicking through each value in that slider until I find the magic
spot where whites (#FFFFFF) turn grey (non-#FFFFFF).

I think it's self evident why I want unrecoverable whites to remain white, but here's
an example in case it's not:
Imagine you're working on a photo and it looks good in RT, you save it and upload it
to your site, or even worse send it for printing or sell it, only to later open a web
page with your photo, a page with a white background, and you find that what was supposed
to be white in your photo is darker than the page background, e.g. it turns out that
the stadium lamps in the corner of your photo are not white but grey, because your
eyes couldn't tell #DEDEDE apart from #FFFFFF because you use a dark theme in RT...

Reported by entertheyoni on 2011-06-19 22:57:20

[Beep6581]

Closed. Great it works for you now.
As Emil already explained the second bug was a CLIP that should not be there, plus
an integer conversion loosing precision. I lately made a large cleanup in DEFAULT to
upgrade floating point handling where missing. However I did not touch some like this
which I did not fully understand.

Reported by oduis@hotmail.com on 2011-06-20 06:00:38

• Status changed: Fixed

[Beep6581]

oduis "verified" supersedes "fixed" ;] Thank you for the fix!

Reported by entertheyoni on 2011-06-21 21:32:31

• Status changed: Verified

[Beep6581]

Sorry, you're right.

Reported by oduis@hotmail.com on 2011-06-21 21:35:09

[Beep6581]

Reported by rinni@gmx.net on 2013-01-28 14:25:42

• Status changed: Fixed