Local Adjustments Exposure compensation acts differently on clipped highlights

[Lawrence37]

Short description
Local Adjustments > Dynamic Range & Exposure > Exposure compensation changes the lightness of clipped highlights at a different rate compared to the rest of the photo.

Steps to reproduce

1. Open an image with some clipped highlights. Start with the Neutral profile.
2. Enable Local Adjustments and add a full image spot with the Dynamic Range & Exposure tool.
3. Set Scope to 100 to make the tool affect the whole image.
4. In Exposure Tools, set everything to 0 so that nothing is enabled.
5. Drag the Exposure compensation down and observe how the image changes.

Expected behavior
Clipped highlights darken at more or less the same rate as the rest of the image.

Screenshots
Without adjustment:

Clipped highlights don't darken as quickly as the rest of the image:

At this point, the clipped highlights "catch up":

Now it darkens quicker than the rest of the image:

[Desmis]

@Lawrence37

Yes, I know...and I think there is no solution...with "exposure"...

By the way, as I mention in the Rawpedia documentation, I ranked this algorithm last...and there is a reason....
https://rawpedia.rawtherapee.com/Local_Adjustments#Five_ways_to_change_the_exposure_and_lift_the_shadows
In this case - as in a majority of cases - algorithm "Tone equalizer' is much better

jacques

[Lawrence37]

Good to know.
Is it an inherent limitation of the algorithm? What is the cause/explaination of the behavior?

[Desmis]

It's inherent at this algorithm...
The main explanation is in "LA" exposure works only on L (Lab), in main it works in rgb.

On the other hand, I had thought of removing this algorithm from "LA", because if we compare it to the others (Tone equalizer, TRC, Log encoding...), it is always lower
To a lesser degree, Shadows/Highlights is also in a majority of cases inferior to others.
If you read the Darktable forum you will see that they came to similar conclusions.

gradually with LA users will have to change their behavior, by unlearning and learning new practices

Jacques

[Lawrence37]

I would not have guessed that Lab would work so poorly in this case. With the steps I mentioned, all values are clipped prior to local adjustments, so Lab should be able to handle it without issues. Am I missing something?

[Desmis]

Yes but in this case, look at the Lab values in the cloud...L=100 a=0 b=0, the algorithm is unable to deal with this

But I recall for me, the worst algorithm to handle exposure in LA is "exposure".

jacques

[Lawrence37]

I agree the other tools are more useful than Exposure compensation. Nevertheless, I attempted to find the cause.

RawTherapee/rtengine/iplocallab.cc

Lines 3090 to 3104 in d86888e

	float L = bufexporig->L[ir][jr];
	const float Llin = LIM01(L / 32768.f);
	const float addcomp = linear * (-kl * Llin + kl);//maximum about 1 . IL
	const float exp_scale = pow_F(2.f, lp.expcomp + addcomp);
	const float shoulder = (maxran / rtengine::max(1.0f, exp_scale)) * hlcompthr + 0.1f;
	const float comp = (rtengine::max(0.f, (lp.expcomp + addcomp)) + 1.f) * hlcomp;
	const float hlrange = maxran - shoulder;
	//highlight
	const float hlfactor = (2 * L < MAXVALF ? hltonecurve[2 * L] : CurveFactory::hlcurve(exp_scale, comp, hlrange, 2 * L));
	L *= hlfactor * pow_F(2.f, addcomp);//approximation but pretty good with Laplacian and L < mean, hl aren't call
	//shadow tone curve
	L *= shtonecurve[2 * L];
	//tonecurve
	lab->L[ir][jr] = 0.5f * tonecurve[2 * L];

hlfactor is wrong for clipped values. It turns out this is because exp_scale is wrong too. Why is pow_F returning bad values? Keep in mind the second argument: lp.expcomp + addcomp
pow_F is defined in two places:

RawTherapee/rtengine/opthelper.h

Line 23 in d86888e

#define pow_F(a,b) (xexpf(b*xlogf(a)))

RawTherapee/rtengine/sleef.h

Line 29 in d86888e

#define pow_F(a,b) (xexpf(b*xlogf(a)))

The one in sleef.h is redundant. After removing that one and surrounding b in parentheses, I get better results. There is still a problem when Exposure compensation is > -0.6. I noticed lp.expcomp doesn't match the values in the UI and found this:

RawTherapee/rtengine/iplocallab.cc

Lines 1650 to 1656 in d86888e

	//increase sensitivity for low values
	float proexp = lp.expcomp;
	if (std::fabs(proexp) < 0.6f) {
	float interm = std::fabs(proexp) / 0.6f;
	interm = pow(interm, 3.f);
	lp.expcomp = proexp * interm;
	}

Removing this solves the final issue, but I have no idea what the purpose of this code is and if it is a bad idea to remove it.

[Desmis]

@Lawrence37

Could you send me the code for "surrounding" (After removing that one and surrounding b in parentheses)

The last part for expcomp < 0.6 was a precedent issue....because some users found that the system varied too quickly for low values. I think now we can disabled at least for negative values...

Thank you

Jacques

[Lawrence37]

I changed (xexpf(b*xlogf(a))) to (xexpf((b)*xlogf(a))).

[Desmis]

OK for this case, but this change will affect all pow_F calls in RT...?

But, excuse my bad english...and my bad knowledge in C++.I now know what an added parenthesis () does

:)

[Lawrence37]

I briefly checked all the usages and none of them should be affected by the extra parentheses. The extra parentheses is the correct version, so if the change breaks something, it's because it wasn't using pow_F correctly in the first place. If you don't want to change pow_F, you can just change const float exp_scale = pow_F(2.f, lp.expcomp + addcomp); to const float exp_scale = pow_F(2.f, (lp.expcomp + addcomp));.

The parentheses are necessary because #define macros are just text substitutions. So here, const float exp_scale = pow_F(2.f, lp.expcomp + addcomp); will become const float exp_scale = (xexpf(lp.expcomp + addcomp*xlogf(2.f)));. Multiplication comes before addition, so the result is wrong.

[Thanatomanic]

Please wait before committing this change directly to dev, this feels like a major change that needs some testing.

[Thanatomanic]

@Lawrence37 Please see https://godbolt.org/z/jYfrc65nn
I implemented both variants of pow_F and they result in exactly the same values. I'm not sure why you see differences... Changing compiler or optimization level in Godbolt also doesn't matter.

Edit: nevermind, my example was flawed. When you enter an arithmetic expression into the pow_F it indeed expands wrongly.

[Lawrence37]

There is no difference in that example because you used simple arguments. When dealing with macros, the tricky parts come from order of operations and using code with side-effects.
Edit: I see your edit now. :)

[Thanatomanic]

@Desmis @Lawrence37 I went through all cases of pow_F and was only able to find the single case Lawrence found for this bug report where there is an unguarded arithmetic expression in the second argument. All other instances of pow_F are good and bug-free.

@Lawrence37 Feel free to update pow_F function, that should fix this bug and not affect anything else as far as I can see. FWIW you can remove the definition in sleef.h, like I also did here #6262

[Desmis]

I just create a new branch "laexposure" with 3 changes

1. remove pow_F from sleep.h
2. added a new pow_Fr in opthelper.h
   #define pow_F(a,b) (xexpf(b*xlogf(a)))
   #define pow_Fr(a,b) (xexpf((b)*xlogf(a)))
3. in iplocallab.cc I change in "exlabLocal" pow_F by pow_Fr and I remove the lines to increase sensitivity for low values

Now the behavior is better...Thanks to @Lawrence37

Jacques

[Desmis]

I just create a PR

• I remove pow_Fr
• change pow_F
• change iplocallab.cc
• clean code