Beep6581 / RawTherapee

A powerful cross-platform raw photo processing program
https://rawtherapee.com
GNU General Public License v3.0
2.84k stars 320 forks source link

Clipped highlights look posterized #4479

Open TooWaBoo opened 6 years ago

TooWaBoo commented 6 years ago

Images taken with my pocket cam Nikon P330 and clipped highlights look posterized and washed out. In the past it looked fine, but with the latest version of RT it doesn't. I can't tell when this issue appeared the first time.

Link to RAW and pp3 https://filebin.net/qxlzm3k1w6g087je

grafik

Version: 5.4-122-gf9396f2a
Branch: dev
Commit: f9396f2a
Commit date: 2018-03-29
Compiler: gcc 7.3.0
Processor: undefined
System: Windows
Bit depth: 64 bits
Gtkmm: V3.22.0
Lensfun: V0.3.2.0
Build type: release
Build flags: -mwin32 -m64 -mthreads -msse2 -std=c++11 -march=native -Werror=unused-label -fopenmp -Werror=unknown-pragmas -Wall -Wno-unused-result -Wno-deprecated-declarations -Wno-aggressive-loop-optimizations -DNDEBUG -O3 -ftree-vectorize
Link flags: -m64 -mthreads -static-libgcc -fno-use-linker-plugin -march=native -s -O3
OpenMP support: ON
MMAP support: ON
Floessie commented 6 years ago

Could be related to #4457...

Beep6581 commented 6 years ago

Cannot reproduce in 5.4-122-gf9396f2a imgur-2018_03_31-14 57 50

Beep6581 commented 6 years ago

@Floessie I thought so too, but the PP3 uses method=blend, and changing it to "color propagation" works well.

Floessie commented 6 years ago

@Beep6581 I see. :+1: Sorry for the noise.

TooWaBoo commented 6 years ago

I get this when changing to "color propagation"

grafik

Beep6581 commented 6 years ago

@TooWaBoo can you try to eliminate unrelated tools? i.e. start from "Neutral" and find which tool causes that. I would suggest you delete or rename your options file as well, to make sure we're using the same settings.

TooWaBoo commented 6 years ago

@Beep6581 I've followed your instuctions. I think I found the culprit. It's the Profiled Lens correction -> Vignetting correction I've uploaded the .lcp-file to the link above too.

How to reproduce:

  1. Apply Neutral
  2. Check Highlight reconstruction
  3. Select P330...lcp
  4. Uncheck Distortion correction for better preview
  5. Check Vignetting correction
  6. The sky becomes posterized

There's another issue. If you then check radio button "None", Vignetting correction is still active.

Beep6581 commented 6 years ago

Confirmed both issues. There is a third issue - using neutral, there are strong magenta highlights.

Beep6581 commented 6 years ago

The posterized highlight transition is unrelated to LCP. Simply apply neutral and set white-point correction to 2. screenshot_20180331_170745

TooWaBoo commented 6 years ago

The strong magenta highlights will disappear by acivating highlight reconstruction.

cuniek commented 6 years ago

Isn't that the result of unbounded processing? EDIT: Scratch that, this is wrong white point (as @Beep6581 wrote). I made the comment because I am evaluating unbounded processing which introduced pink highlights in cameras that do not suffer from wrong whitepoint.

Beep6581 commented 6 years ago

I measured the white/black levels, tried this and that, and the white level number which eliminates magenta is nowhere near what I measured, even after scaling the measured value to 14-bit... @iliasg help, could you explain how to measure this file correctly?

    { // Quality X
        "make_model": "NIKON COOLPIX P330",
        "dcraw_matrix": [ 10321,-3920,-931,-2750,11146,1824,-442,1545,5539 ], // Adobe DNG Converter 10.2
        "ranges": { "white": 12200 }
    },
agriggio commented 6 years ago

could be a side effect of unbounded processing, if you don't enable highlight reconstruction. RT used to clip highlights, now it doesn't unless told so (ie by enabling highlight recovery). I see a couple of options, but I'm open to suggestions:

  1. restore the old behaviour
  2. introduce a new highlight reconstruction mode "clip"
  3. live with this, as there are workarounds

(I'm fine with 3 personally)

TooWaBoo commented 6 years ago

I vote for 1. restore the old behaviour I've tried to get the same results with the new behaviour, but I could'nt get it identical. (I've compared with RT5.3)

agriggio commented 6 years ago

@TooWaBoo what do you mean compared with 5.3? what I described above happened after 5.4, so the reference point should be 5.4 -- otherwise the issue is different... can you confirm that with 5.4 everything works as you expected?

TooWaBoo commented 6 years ago

@agriggio I'll test with 5.4 tomorrow.

Beep6581 commented 6 years ago

RT 5.3: imgur-2018_04_01-01 15 13

RT dev 69c38b45 (before the unbounded merge): imgur-2018_04_01-01 16 19

RT dev ed21c4ee imgur-2018_04_01-01 18 17

iliasg commented 6 years ago

@Beep6581

help, could you explain how to measure this file correctly?

Where did this WL = 12200 came from !!

On the heavylly overexposed areas the clipping is at 4094 (just one down from the 12bit top 4095 !!) .. I would put it a bit lower (say 4080) to be on the safe side

@agriggio But with RT 5.4-84-gc4933e36 we have a problem with DNG where adobe converts the NRW to 16bit DNG BL=3200, WL=65000 (not sure if this is already OK after Alberto's related to DNG WL commit)

BTW I see the white level range is restricted to 0.10-16.0 .. and this is not enough to correct the WL manually in this case (0.062 needed ..) .. I think this slider should better be in log2( ) scale (see code for pixelshift Iso adaption)

TooWaBoo commented 6 years ago

@agriggio I've tested with 5.4 and everything is fine.

agriggio commented 6 years ago

so is the pink building the desired behaviour? how about a checkbox in the raw tab that lets the user decide whether to clip or not?

TooWaBoo commented 6 years ago

@agriggio As long as I get the old behaviour back, you can do what ever you want. 😁 Now serious: I don't understand the benefit of the new behavior.

TooWaBoo commented 6 years ago

There is an issue in the shadows too. dev (new) grafik

5.4 (old) grafik

agriggio commented 6 years ago

@TooWaBoo https://filebin.net/qxlzm3k1w6g087je/DSCN0760-2.jpg For the shadows part, could you please attach a RAW file and a pp3, so that I can have a look? Thanks!

TooWaBoo commented 6 years ago

Here it is: https://filebin.net/08j6p7pn1btgx5ms

Beep6581 commented 6 years ago

On the heavylly overexposed areas the clipping is at 4094 (just one down from the 12bit top 4095 !!) .. I would put it a bit lower (say 4080) to be on the safe side

Exactly, but when I tried 4094 and 3894, it was as if RT ignored it. Then I tried scaling that value to a 14-bit range, RT didn't ignore it, but it still showed magenta.

cuniek commented 6 years ago

@TooWaBoo Change input profile from "Camera standard" to any icc, dcp, or "No profile". The problem is gone.

TooWaBoo commented 6 years ago

@cuniek But this wouldn't really help. I'm using the P330...dcp from Adobe and the problem still exists with latest dev.

cuniek commented 6 years ago

@TooWaBoo Here is a set of screenshots. I have tested different profiles (Nikon d750, Sony 580, Minolta 7D, even AdobeRGB), and 90% of them give smooth shadows. There are few that give wrong shadows, so... This is either color profile related, or black point in RT is wrong, but some color profiles are correcting that.

Notice, that Raw Black Points are fixing it too.

2018-04-01_133652

2018-04-01_133124

2018-04-01_133101

agriggio commented 6 years ago

btw, I don't want to advocate the change at any cost (in fact I think a checkbox in the raw tab is the way to go), but similar posterization can be obtained using 5.4 on different shots, at least as far as I remember -- I'll dig deeper once I get back to my machine

Beep6581 commented 6 years ago

Three different issues are being discussed:

  1. Problem 1: incorrect white levels. Setting "white": 4080 works fine for the NRW but not for DNG in dev (5.4-127-ged21c4ee at the time of writing).
  2. Problem 2: LCP vignetting correction is applied even when profiled lens correction is set to "None" - I opened issue #4480
  3. Problem 3: highlight transition to clipping is not smooth.

Let's please limit the discussion here to problem 3, as that is the title of this issue, and open a new issue for problem 1.

cuniek commented 6 years ago

@Beep6581 About problem 3 - I guess I have found the cause

line 1112

https://github.com/Beep6581/RawTherapee/commit/15813abbedb6079a0f3ec627675dcf8385939de1#commitcomment-28366769

agriggio commented 6 years ago

@cuniek according to @TooWaBoo the problem occurs with "blend", not "colour propagation"...

cuniek commented 6 years ago

Last post for today, I promise:

@agriggio You are (somehow) right, I made some comparisons and ALL HR methods are giving posterized highlights after unbounded was commited. Still the issue I mentioned adds even more posterization in case of colour propagation.

Happy Easter!

agriggio commented 6 years ago

@cuniek no problem, please feel free to add as many comments you feel appropriate! Regarding the posterization: the "trick" is to use highlight compression. Do you see posterization in the file I posted above? (https://filebin.net/qxlzm3k1w6g087je/DSCN0760-2.jpg). The pp3 is here: https://filebin.net/qxlzm3k1w6g087je/DSCN0760-2.jpg.out.pp3 It looks fine from here (but again, I'm on a crappy screen).

Beep6581 commented 6 years ago

There is a test image for examining transitions from unclipped to all-clipped: http://rawtherapee.com/shared/test_images/overexposed_sky.pef

Neutral

Beep6581 commented 6 years ago

Neutral + color propagation

agriggio commented 6 years ago

I will add the checkboxes to let the user decide whether to clip (default old behaviour)

agriggio commented 6 years ago

No need for extra checkboxes. Here's the patch that solves the problem for the highlights:

diff --git a/rtengine/improcfun.cc b/rtengine/improcfun.cc
--- a/rtengine/improcfun.cc
+++ b/rtengine/improcfun.cc
@@ -3764,13 +3764,12 @@
                         float g = std::max(gtemp[ti * TS + tj], 0.f);
                         float b = std::max(btemp[ti * TS + tj], 0.f);

-                        if (r > 65535 || g > 65535 || b > 65535) {
+                        if (max(r, g, b) > MAXVALF && min(r, g, b) < MAXVALF) {
                             filmlike_clip (&r, &g, &b);
+                            rtemp[ti * TS + tj] = r;
+                            gtemp[ti * TS + tj] = g;
+                            btemp[ti * TS + tj] = b;
                         }
-
-                        setUnlessOOG(rtemp[ti * TS + tj], r);
-                        setUnlessOOG(gtemp[ti * TS + tj], g);
-                        setUnlessOOG(btemp[ti * TS + tj], b);
                     }
                 }

For the shadows, I'm still testing my current solution -- I'll commit (both) once I'm happy

heckflosse commented 6 years ago

@agriggio

if (max(r, g, b) > MAXVALF && min(r, g, b) < MAXVALF)

somehow looks familiar

agriggio commented 6 years ago

meaning?

heckflosse commented 6 years ago

@agriggio Alberto, that was just a thought, as I remembered that I used the same pattern shortly ago for a different part of rt code...

agriggio commented 6 years ago

ah, ok... I thought I made some silly mistake :-)

heckflosse commented 6 years ago

@agriggio Alberto, it was this, which is still wip

https://github.com/Beep6581/RawTherapee/pull/4444/commits/e03646fc5b0b8746eed1540ddb5137691565dba1#diff-e653e67851b47e016aa6492071be7b5fR2429

TooWaBoo commented 6 years ago

@agriggio It's different now but still not fixed.

5.4 (Nikon D90) 2

dev (5.4-133-g16679296) (Nikon D90) 1

5.4 (Nikon P330) 4

dev (5.4-133-g16679296) (Nikon P330) 3

I'm sorry but I'm not allowed to upload the RAW files.

Beep6581 commented 6 years ago

5.4-133-g16679296 looks fine here:

Neutral: screenshot_20180403_175407

Neutral with color propagation: screenshot_20180403_175420

agriggio commented 6 years ago

@TooWaBoo can you send them in a private message maybe? (on pixls)

TooWaBoo commented 6 years ago

@agriggio Message sent on pixls

agriggio commented 6 years ago

@TooWaBoo I just pushed a fix -- hopefully this time I covered all cases :-)

TooWaBoo commented 6 years ago

@agriggio The posterize effect has gone but the "Highlight reconstruction" in RT5.4 is better. I prefere a checkbox old/new behaviour. ;-) https://filebin.net/oe21qgngapz49ytk

5.4 5 4

dev dev

agriggio commented 6 years ago

Just be a bit more generous with highlight compression