Beep6581 / RawTherapee

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

Wavelet levels - improvements #2684

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2702

I open a new issue for wavelet ... because issue2594 may overflow and it does not have
much meaning.

I brought three changes - improvements:
1) choice between sliders and curve for chroma - I have no attachment to curve, just
see issue2594 #0. I stand by "curve" for compatibility "pp3"

2) improvement in edge sharpness

3) I have moved "Daubechies coefficient" from Preferences to "Wavelet settings" and
add 3 "levels":
* D2 Haar
* D10 
* D14

Now : D2, D4 (default), D6, D10, D14
==> enhanced edge detection, reduce artifacts...

Reported by jdesmis on 2015-03-03 11:45:30

Beep6581 commented 9 years ago
here the patch

Ingo can you have a look to lines about 550 in ipwavelet.cc (Levwava before / after)
:)

Reported by jdesmis on 2015-03-03 11:56:05


Beep6581 commented 9 years ago
Jacques, I'll have a look

Reported by heckflosse@i-weyrich.de on 2015-03-03 12:20:41

Beep6581 commented 9 years ago
I had a look. Looks good :-)

Reported by heckflosse@i-weyrich.de on 2015-03-03 12:57:22

Beep6581 commented 9 years ago
Thank you :)

Reported by jdesmis on 2015-03-03 13:07:00

Beep6581 commented 9 years ago
Jacques, Thanks for the new wave:) The original issue is so long thta I agreen, new
one is necessary. I'd like to mentions a few points:

1. I would suggest to move "(Daubechies coefficients)" to tool tip, and change label
to "Edge Quality". The effect seems to be related to edge anti-aliasing.

2. Regarding the threshold for Edge sharpness. Would it be possible to change so that
with higher values the effect to non-sharp areas would be reduced?

3. One more thing - edge halos. You can use this image for experimenting: http://i.imgur.com/Mqz5oZ1.jpg
Note how strong the edge halos get when contrast sliders are moved to the right. Is
there any way to eliminate such halos?

Reported by michaelezra000 on 2015-03-03 16:04:26

Beep6581 commented 9 years ago
Hello Michael

Excuse my bad english, but I don't understand all...I am an old french papy !! :)

1. What do you mean by "move '(Daubechies coefficients)' to tool tip"...what is tool
tip ? If it is "preference", I don't agree..because this paremeters are to use with
both features...as levels...and results are different for each image (in function of
local contrast)

2. I don't understand !!

3. No way to reduce halo  (wavelet is very very different from "RL deconvolution" and
"unsharp mask")...but you can use : 
 a) daubechies coef
 b) reduce first level,and or modify threshold (which is not a threshold !!)
 c) interact with "contrast level" (if they are not used...), eg reduce first(s) levels...

:)

Reported by jdesmis on 2015-03-03 16:20:45

Beep6581 commented 9 years ago
:)
1. I am referring to label text and the "tool tip" which is opening when user holds
mouse over that dropdown.

2. Threshold - basically to avoid sharpening smooth out of focus areas.

3.b. what is this threshold?:)

Reported by michaelezra000 on 2015-03-03 17:48:41

Beep6581 commented 9 years ago
Michael, 3b) I recently also had some problems to understand all the edge sharpness
controls. Here's a sheet where you can play with the values and see the sharpening
factor for each level in last row.

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-03 18:04:24


Beep6581 commented 9 years ago
I forgot to mention that I made the sheet before Jacques' latest changes. Don't know
whether the calculations still fit to last patch.

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-03 18:19:59

Beep6581 commented 9 years ago
Thanks, Ingo, very cool. So it appears that "Threshold" is kind of dampening for contribution
of higher levels:
At threshold 0,   all levels are engaged, progressively lower from level 0 to 8.
At threshold 100, mostly the levels 0,1 are engaged

Although when threshold is adjusted, the edge factor at levels 0 and 1 are changing
also.

Reported by michaelezra000 on 2015-03-03 18:38:27

Beep6581 commented 9 years ago
Michael, for me it also was very instructive to see the effect of 'radius' slider:

For radius values 0 to <30 the max. factor is always at level 0, but biggest when radius
is 0
For radius value 30 the max. factor is the same at level 0 and level 1
For radius values >31 to <90 the max. factor is always at level 1, but biggest when
radius is 60
For radius value 90 the max. factor is the same at level 1 and level 2
For radius values >90 the max. factor is at level 2

Maybe we find a way to make this more clear to the user (choosing a different label
and scale for the radius control...?)

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-03 22:13:26

Beep6581 commented 9 years ago
replace 'is the same' with 'is nearly the same' in last comment

Reported by heckflosse@i-weyrich.de on 2015-03-03 22:16:22

Beep6581 commented 9 years ago
Michael, I had a look at the new code now. Seems I have to rework the sheet...

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-03 22:38:53

Beep6581 commented 9 years ago
It's only valid when 'First level' is set to 'Unchanged'.

Reported by heckflosse@i-weyrich.de on 2015-03-03 22:40:54

Beep6581 commented 9 years ago
Hi Ingo, I find this progressivity quite usable. The end result feels intuitive. But
it would be useful to document this detailed design for reference.

Reported by michaelezra000 on 2015-03-03 23:43:51

Beep6581 commented 9 years ago
Michael, I also find it usable after I understood how it works :-)
No problem for my mind to transfer the 'radius' values into values of 'sharpness according
to level'. But wouldn't it be more useful to change the slider from

radius : 0....10....20....30....40....50....60....70....80....90....100

to

level  : 0................1....................................2......

if that's possible.

And, yes, it would be useful to document this

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-03 23:58:39

Beep6581 commented 9 years ago
Scale in #16 is not correct. It's just to illustrate the idea...

Reported by heckflosse@i-weyrich.de on 2015-03-04 00:00:16

Beep6581 commented 9 years ago
I wonder if there is a way to translate the radius scale into pixels?
The scale you suggested could be interpreted as "dominant wavelet level (for edge detection)".
Here is the same excel with plotted graphs.
Since wavelet levels are integers it may be confusing to express radius as dominant
wavelet level, since radius value would then have decimals, falling between the levels.
As it is now it is pretty continuous, but could be more intuitive if expressed as pixels,
although i cannot say it is photographically necessary but might be helpful.

Reported by michaelezra000 on 2015-03-04 02:35:02

Beep6581 commented 9 years ago

Reported by michaelezra000 on 2015-03-04 02:35:38


Beep6581 commented 9 years ago
Thank you Ingo and Michael, for this work to explain what "in pedagogical terms", what
the algorithm. You now understand the difficulties that I have to speak in English
of this algo and find the right vocabulary (radius which is not a radius, threshold
is not a threshold, etc.)
:)

Ok to move "Daubechies coefficients" in "tool tip", but not the label...I think "Quality"
is good, because D2...D14 affects all levels between 0 and 3 and others but with very
very small effect, with edge but also contrast, chroma...

Another point, the result will generally not be copied by partial-paste (or pp3), and
results will be different from an image to others, because original local contrast
(for level 0 or for others levels) depends of each image, for the same settings. Reduce
or reinforce "strength" is a way around the difficulty...I'll look (but wihout promises)
to take into account original local contrast !!

Reported by jdesmis on 2015-03-04 07:12:05

Beep6581 commented 9 years ago
Here a new patch

This algorithm take into account real local contrast
I use a thresholdadjuster with : value 0, 10, 35, 100

** 0 10*10 35*35 100*100 substantially correspond to the true distribution of low value,
mean, standard-deviation and max (eg 5, 50, 400, 4000  => L amplitude 0..32000)==>
different for each image / level...but enough to have an algorithm that responds to
common sense

** Bottom left ==> minimal low value for local contrast (not 0, but real=5..)
** Top left ==> average reference value (for each level)
** Top Right==> standard deviation (for each level)
** Bottom right ==> Max for positif and negatif contrast (for each level)

If we move each sliders to the left, local contrast is reduced
If we move sliders to the right local contrast is increased

I think after rapid test that top-right and bottom-right are usually enough...but top-left
and bottom can be used to increase contrast !!

I hope no error, and not too bug in algorithm!!

:)

Reported by jdesmis on 2015-03-04 15:24:31


Beep6581 commented 9 years ago
Welcome back to more hostile climates, Jacques :)

I haven't had a chance to test the patch and it may be a few days before I can, but
I wanted to comment on some of the things discussed:

#0 "choice between sliders and curve for chroma"
Due to the idiosyncrasies of that curve (i.e. only being sampled at 9 points, no indication
of where those points lie, not adapting to the number of wavelet levels used, etc)
I think sliders are better.

"I stand by "curve" for compatibility "pp3""
We have not released any stable version of RawTherapee with the new Wavelet Levels
tool, and users have been warned, so backwards compatibility should not be a factor
in our decisions.

Considering how 'strange' the curve is, I think removing it entirely and having just
sliders is the best choice.

#5.1 "move "(Daubechies coefficients)" to tool tip, and change label to "Edge Quality""
I fully support that. We may want to consider removing this entirely before 4.3 if
there is an all-round best performer.

Reported by entertheyoni on 2015-03-04 20:50:06

Beep6581 commented 9 years ago
Michael, your illustration how Wavelet Edge Sharpening works is great :-)

Jacques, I'll have a look at your latest patch tomorrow.

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-04 23:52:26

Beep6581 commented 9 years ago
Here the patch, with changes #22

I have keep code for chroma curve (GUI, Rtengine) for ulterior usage !

:)

Reported by jdesmis on 2015-03-05 08:11:48


Beep6581 commented 9 years ago
Jacques, though it's very nice from gcc to optimize the unused

float* av_LL = new float [tileheight*tilewidth];

away, I would prefer to delete it at the end or (better) not to allocate it at all.
We can't rely on the capabilities of optimizer. Maybe gcc 4.6.x used for win32 builds
does not optimize it away and we have a big memory leak then.

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 09:50:38

Beep6581 commented 9 years ago
Jacques, I want to make a patch which speeds up Aver() and Sigma(). Ok?

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 10:26:42

Beep6581 commented 9 years ago
Ingo

No problem, you can optimize "Aver()", "Sigma()" and also "av_LL" when it is no used
!

Thank you :) 

Reported by jdesmis on 2015-03-05 10:52:00

Beep6581 commented 9 years ago
Jacques, before I post a patch, I have a question:

In function Sigma() you calculate 

else if(DataList[count] < -thres) {variN += SQR(DataList[count] - averageNeg);  

So when you run into this case DataList[count] is always < 0, but averageNeg is always
positive (I verified that with a printf). That leads to wrong values of variN and in
consequence also of sigmaNeg. Fortunately sigmaNeg is not used anywhere, only SigmaPlus
is used later in ContAllL().

Now I don't know what to do with this code in my patch. There are 4 possibilities:

1.) Ignore the wrong calculation and just optimize.

2.) Correct the wrong calculation, though it's not used, then optimize.

3.) Remove the wrong calculation, then optimize.

4.) Make a correction to sigma function, so that the actually lost sigma for values
< -thres are considered too, the optimize.

Which solution do you prefer?

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 13:03:18

Beep6581 commented 9 years ago
Hello Ingo

I think 4) it's better...even actually I have not use sigmaN...but may be !

Thank you :)

Reported by jdesmis on 2015-03-05 13:09:40

Beep6581 commented 9 years ago
Ok, that's also the solution I prefer.

Reported by heckflosse@i-weyrich.de on 2015-03-05 13:13:31

Beep6581 commented 9 years ago
Jacques, here's the patch. 

1.) Speedups for Aver() and Sigma() (small speedup for preview, big speedup in full
processing). 

2.) corrected the calculation of sigmaNeg number 2.) from #28

3.) Made some changes for a thing I think it was buggy in old code (ipwavelet.cc line
1145 to 1221)
    This leads to differences before/after patch when the Local Contrast Control is
not at default settings, but I think, it's correct now.

If my changes for 3.) are correct, I can try to speedup this part as well.

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 15:21:09


Beep6581 commented 9 years ago
Hello Ingo

Thank you for all these optimizations...

Effectively there were a bug in lines 1145/1221...I could not find the reaction of
the algorithm "normal", but now everything is working properly, it is the same algorithm
but works well.
In the patch 4a, I modified the parameters for eedg_maxe to be more sensitive==> SQR.

Thank you again :)

Reported by jdesmis on 2015-03-05 16:15:41


Beep6581 commented 9 years ago
Here's the patch with the speedup for lines 1145/1221.

I tested full processing the usual D800 NEF using wavelet, but used only Edge Sharpening
with this values:

Radius 15;Strength 19;Thresold 10;First Level 'Unchanged';Bottom-left 1;Top-left 12;Top-right
38;Top-left 99

Processing time for WaveletcontAllL with patch 4a : 2231 ms
Processing time for WaveletcontAllL with patch 5  :  144 ms

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 17:51:22


Beep6581 commented 9 years ago
Radius 15;Strength 19;Thresold 10;First Level 'Unchanged';Bottom-left 1;Top-left 12;Top-right
38;Top-left 99

              ^

              this should be Bottom-right

Reported by heckflosse@i-weyrich.de on 2015-03-05 17:52:44

Beep6581 commented 9 years ago
I am impressed by the speed of the calculation! How with the same algorithm, dividing
the processing time by 15! ...with the same result  :)

Ingo you are the best !

Reported by jdesmis on 2015-03-05 18:01:51

Beep6581 commented 9 years ago
+ 1!!!:)

Reported by michaelezra000 on 2015-03-05 18:46:47

Beep6581 commented 9 years ago
Jacques :-) 
The key was to move the cbrt outside the loop.
That reduced the processing time to 281 ms.
Then moving remaining calculations outside the loop => 144 ms

Ingo

Reported by heckflosse@i-weyrich.de on 2015-03-05 18:49:45

Beep6581 commented 9 years ago
Michael :-)

Reported by heckflosse@i-weyrich.de on 2015-03-05 19:12:44

Beep6581 commented 9 years ago
Jacques, local contrast control is very interesting.
I like that threshold UI is very compact, but is it most suitable?
Threshold control enforces the trapezoid shape of the curve, so that for example the
Top Left slider can not have value higher than Top right slider, etc.. My guess is
that they could be independent sliders. If that's the case, as much as I like the concise
look of this control, it is probably better represented by individual sliders.

Reported by michaelezra000 on 2015-03-05 19:32:18

Beep6581 commented 9 years ago
I find that use of the edge sharpness tool increases image acuity. I makes the images
captured by heavily antialiased sensor look like the digital back files:)

While testing, I noticed that Gamut controls/hue-tones does not work correctly. It
does not target the selected hues, e.g. blue sky.

Reported by michaelezra000 on 2015-03-05 21:39:51

Beep6581 commented 9 years ago
Jacques, also, to the last point, could the hue targetting/protection apply to the edge
sharpness also?

Reported by michaelezra000 on 2015-03-05 21:44:10

Beep6581 commented 9 years ago
I replace the "threshold slider" by a curve (complex) that take into account :
* real local contrast
* average
* standard deviation
* max
for each level! But the same for positive and negative values of contrast...

I note in "Tool tip" the principle...

For control hue/tones...as I write before, control of Hue in levels seems to be very
very difficult...in terms of mathematics ...I'll look :)

Reported by jdesmis on 2015-03-07 11:38:23


Beep6581 commented 9 years ago
Jacques, I tried to use the new control, but must admit that it is a bit difficult to
use.. I was actually more successful in getting a usable result with the previous simpler
control. I thing simplifying UI with more straight-forward sliders may be a better
direction, it would significantly cut down the editing time to get usable result. 

It could also be that the curve is too powerful and with minor adjustments leads to
severe halo and ringing artifacts. I wonder if there is a way to avoid ringing artifacts
completely, to allow user control just 3 sliders - low, medium and high local contrast,
but in the engine translate user input to such a curve that produces no/minimum artifacts.

Some observations: 
1. switching curve to linear causes a crash
2. interestingly, with the bell-shaped curve halos are significantly better managed
when edge quality = D6, even better than D14. See the blue horse image and lamp post
on the background of the blue sky.
3. Strength slider should be moved to the top, as it applies to all controls in the
section - edge and local contrast. On the other hand, may be it is a bug? Should local
contrast adjustment depend on edge sharpness strength?
4. I would rename "Threshold" slider to "Detail".

Reported by michaelezra000 on 2015-03-07 14:46:54

Beep6581 commented 9 years ago
I don't think sliders is a good thing, because contrast is continuous, and I think a
curve is best...How to amplitude for each slider, with what overlap? the curve solves
this, but it must be calibrated ... Depending opinions .... Probably now there is too
much force, but it's easy to fix. In all case I'll use the same algo to use average,
standard deviation and max !

1) I'll look

2) edge quality depend of each image and repartition of local contrast...that's why
I put a choice ... The literature say for "edge" D2 is the best!!

3. Ok

4. OK

Reported by jdesmis on 2015-03-07 15:30:45

Beep6581 commented 9 years ago
Here patch 07

1 3 4 : solved

I have reduce the "general strength" of the curve about 60%....especially on the low
values of the abscissa

Be carefull to low levels where local contrast is tiny 5..10..(left absciss) I think
and also with big values (right absciss) that may cause artifacts.... And I repeat
sliders don't solved the problem under no circumstances...For this use (local contrast
with edge), it is to the user to control the system...because it transgresses the normal
distribution .... 
In fact you can not have your cake and eat it too. But I think a lot of pictures on
learning should allow for interesting effects. 
Warning, it is almost impossible due to the nature of the local contrast copy settings
to bring (pp3 partial paste) to other images.

:)

Reported by jdesmis on 2015-03-07 16:00:02


Beep6581 commented 9 years ago
Another thing !

Noise plays a fundamental role ... even small values are amplified by wavelet levels,
especially by tools like "edge local contrast".

Do not hesitate if you want to use these tools to reduce noise upstream (median, Luminance,
...etc.)

Reported by jdesmis on 2015-03-07 16:11:20

Beep6581 commented 9 years ago
Another thing ...
I can offer a choice between the "curve" where the user maximum creativity and "ThresholdAdjuster"
which have limits that can change ... and no possible to cover (overlap) the upper
part .... good because then we get considerable artifacts.

So many possibilities:
1) return to thresholdAdjuster
2) mix the two : curve + Thresholdadjuster
3) curve only 
4) suppress all

Actaully this tool is bundled with edge, which seems appropriate. But we can also consider
another form to reach all levels .... But I think'm pretty sure we will reach a very
large artifacts

Reported by jdesmis on 2015-03-07 16:48:23

Beep6581 commented 9 years ago
It seems that after edge sharpening was extended to all levels it became too repetitive
with the contrast sliders. It would be much better to have this tool focused on edges
for lower levels as originally, so it would be purpose-built with very clearly defined
action. Giving too much options. especially repetitive although is impressive but is
detrimental to usability:). I liked the action of threshold adjuster but was confined
by constraints of its sliders. If that could be addressed this will be a magical tool!:)

Reported by michaelezra000 on 2015-03-07 17:45:49

Beep6581 commented 9 years ago
I Have found bugs in my calculation!!

I'll post a new patch probably this afternoon :)

Reported by jdesmis on 2015-03-08 10:02:23

Beep6581 commented 9 years ago
here the patch !

Reported by jdesmis on 2015-03-08 12:18:09