search

Beep6581 / RawTherapee

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

Local Adjustments + Exposure = Local Contrast display issue #6132

Open chaimav opened 3 years ago

chaimav commented 3 years ago

Toggling local contrast compounds the contrast on the displayed image. To reproduce, use the raw in this post here https://www.dpreview.com/forums/thread/4546372 -Add a full image local spot to the haze below the fireworks -Add Dynamic Range & Exposure tool -Increase exposure of haze -Toggle local contrast on and off (the more it is toggled, the more severe the issue) PP3: https://drive.google.com/file/d/1wtoq4lRb1vobLwML-BwObLRRbqikHGVL/view?usp=sharing

RawTherapee 5.8-2773. Windows 10, i3 8100

local contrast bug

chaimav commented 3 years ago

Let me know when the fix is available in the nightly so I can recheck it.

chaimav commented 3 years ago

RT 5.8-2804 I have found the same issue with checking and un-checking 'Look table' under Color Management >Auto matched camera profile > DCP look table I can produce the bug using the raw available here https://discuss.pixls.us/t/hazy-snowy-bridge/23605

Thanatomanic commented 3 years ago

@chaimav Do you mean that you start from Neutral and only toggle this checkbox? I cannot seem to reproduce that with the raw supplied.

chaimav commented 3 years ago

No. You need to add a full image Local Adjustment and change the exposure first. Change the extension of this file to pp3 look table bug.txt

Lawrence37 commented 3 years ago

I can confirm the pull request fixes the issue for local contrast but not for the look table.

Thanatomanic commented 3 years ago

@chaimav Could you try out the changes that have been made in dev? Is this issue resolved?

chaimav commented 3 years ago

@chaimav Could you try out the changes that have been made in dev? Is this issue resolved?

The issue with local contrast has been resolved with Dev 5.8-2816. However, the bug with look table has not. I also noticed that the Color Management >Auto matched camera profile > DCP > [] Tone curve checkbox has the same issue as look table.

chaimav commented 3 years ago

I have located yet another place where what I believe is the same bug shows up. Same initial steps as before: -Add a full image local spot -Add Dynamic Range & Exposure tool -Increase exposure

Now go to Color > Color Toning > Lab color correction grid and start moving the black and white dots

What is unique about this instance is that you do not need to actually enable Color Toning for the bug to show up. It happens without the tool enabled as well. color toning

I keep posting where I see the issue so as to help devs get to the root cause. Is this info helpful?

Desmis commented 3 years ago

@chaimav See PR https://github.com/Beep6581/RawTherapee/pull/6173

jacques

chaimav commented 3 years ago

I see that you keep trying to fix this by changing the pipeline order, but is that really addressing the root cause? Looking at my latest example, the color toning tool does not even have to be enabled for it to show up. Additionally, it seems that the actual image remains untouched, its just how it is displayed.

Desmis commented 3 years ago

@chaimav @Thanatomanic Have you tried the PR?

In the change I made there are 2 things. 1) change the location, because this one is managed with a "event" of lower rank 2) change the "event" because it is the one that will avoid the interaction between the processes

In RT process are managed with "event" with various parameters Ex in Improccordinator.cc: to manage Local adjustments

        if (((todo & (M_AUTOEXP | M_RGBCURVE)) || (todo & M_CROP)) && params->locallab.enabled && !params->locallab.spots.empty()) {

An each "event" (slider, combobox,...) is with "event" AUTOEXP

Procees Lab is with
        if ((todo & (M_LUMINANCE + M_COLOR)) || (todo & M_AUTOEXP)) {
and "event" LUMINANCECURVE 

Priority for events are in refresmap.h
(extract)
#include "procevents.h"

// Use M_VOID if you wish to update the proc params without updating the preview at all !
#define M_VOID       (1<<17)
// Use M_MINUPDATE if you wish to update the preview without modifying the image (think about it like a "refreshPreview")
// Must NOT be used with other event (i.e. will be used for MINUPDATE only)
#define M_MINUPDATE  (1<<16)
// Force high quality
#define M_HIGHQUAL   (1<<15)

// Elementary functions that can be done to
// the preview image when an event occurs
#define M_CSHARP      (1<<18)
#define M_MONITOR     (1<<14)
#define M_RETINEX     (1<<13)
#define M_CROP        (1<<12)
#define M_PREPROC     (1<<11)
#define M_RAW         (1<<10)
#define M_INIT        (1<<9)
#define M_LINDENOISE  (1<<8)
#define M_HDR         (1<<7)
#define M_TRANSFORM   (1<<6)
#define M_BLURMAP     (1<<5)
#define M_AUTOEXP     (1<<4)
#define M_RGBCURVE    (1<<3)
#define M_LUMACURVE   (1<<2)
#define M_LUMINANCE   (1<<1)
#define M_COLOR       (1<<0)

// Bitfield of functions to do to the preview image when an event occurs
// Use those or create new ones for your new events
#define FIRST            (M_PREPROC|M_RAW|M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR|M_MONITOR)  // without HIGHQUAL
#define ALL              (M_PREPROC|M_RAW|M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)  // without HIGHQUAL
#define DARKFRAME        (M_PREPROC|M_RAW|M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define FLATFIELD        (M_PREPROC|M_RAW|M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define DEMOSAIC                   (M_RAW|M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define ALLNORAW                         (M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define CAPTURESHARPEN                   (M_INIT|M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR|M_CSHARP)
#define HDR                                     (M_LINDENOISE|M_HDR|M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define TRANSFORM                                                  (M_TRANSFORM|M_BLURMAP|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define AUTOEXP                                                                    (M_HDR|M_AUTOEXP|M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define RGBCURVE                                                                                   (M_RGBCURVE|M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define LUMINANCECURVE                                                                                        (M_LUMACURVE|M_LUMINANCE|M_COLOR)
#define SHARPENING                                                                                                        (M_LUMINANCE|M_COLOR)
#define IMPULSEDENOISE                                                                                                    (M_LUMINANCE|M_COLOR)
#define DEFRINGE                                                                                                          (M_LUMINANCE|M_COLOR)
#define DIRPYRDENOISE                                                                                                     (M_LUMINANCE|M_COLOR)
#define DIRPYREQUALIZER                                                                                                   (M_LUMINANCE|M_COLOR)
#define GAMMA             M_VOID //M_MONITOR

But perhaps it is not that... :) This "solution" works fine for "local contrast" and "SH"

jacques

chaimav commented 3 years ago

@Desmis Is there an automated build for the PR? I am afraid I don't know how to compile it.

Don't get me wrong, I am not saying the changes won't solve the issue, but the changes seem to address one area at a time rather than the issue as a whole. Of course, I am just a user and not a dev, so you know way more of the inner working than I do.

Desmis commented 3 years ago

I don't know if there is a build Jacques

Thanatomanic commented 3 years ago

@chaimav Building is relative easy, unless you're really not comfortable working in a command line interface. PR's don't get automated builds that you can download somewhere, but maybe interesting to look for a solution for this in the future. I haven't got the experience to cook up a packaged build myself, sorry.

chaimav commented 3 years ago

I can use a command line interface, but I would need a step-by-step tutorial to follow. Preferably one where I can can copy-paste commands from :-P

Thanatomanic commented 3 years ago

@chaimav I'll message you on Pixls