Beep6581 / RawTherapee

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

Skin targeting for CBDL (hue curve?) #2134

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 2150

Would be nice to have a hue curve for CBDL, same as the one for Defringe, to target
only skin :] Or instead of a curve, some other method of targeting only skin.

Reported by entertheyoni on 2013-12-21 15:58:47

Beep6581 commented 9 years ago
Hombre, I was waiting your "colored gradient"...thank you very much :)

I changed the scale function dirpyr_equalizer

"static const int scales[8] = {1,2,4,8,16,32,64,128};" by dividing by skip...

I rounded to lower value (>=1).
We must now verify that everything is working properly !

More, I reducted a little the effect of blur.

Michael :it is not that simple, because the artifacts are mainly due to the chroma
component.  By cons is do we have no interest in a "blur" function such as Photoshop
... Now the code is ready. ?

Hombre : pour ma part, je suis prêt à travailler sur le U-point ou équivalent. Pas
de problèmes  :)

Reported by jdesmis on 2014-02-06 08:22:24


Beep6581 commented 9 years ago
Looks very nice!

However the non-100% preview is still very different to the saved image:
http://filebin.net/ivt7ow9c7e

Also I noticed some frame-like artifacts when zoomed out and using unrealistically
large CbDL values:
http://i.imgur.com/jcOJCCf.png

Reported by entertheyoni on 2014-02-06 12:49:15

Beep6581 commented 9 years ago
DrSlony

"However the non-100% preview is still very different to the saved image:"

It is "normal" you cannot cut pixel...
*If zoom=100% all values are normally calculated;
*I zoom=50% or under, values are calculated but false for low values of contrast
  ex: level 1: with zoom 100%  ==> scale="2"
      level 1: with zoom  25%  ==> scale="1"  because "2/skip"  (skip=4) leads to scale
< 1.
However it gives a pretty good overview even more it approaches 100%.

Maybe it must be mentioned in the documentation.

Or we act as before only with zoom = 100% or above.

:)

Reported by jdesmis on 2014-02-06 13:09:51

Beep6581 commented 9 years ago
Yes I understand the problem. We must decide whether it's worth keeping. On the one
hand, it gives a general idea. On the other hand, sometimes this general idea can be
very misleading. Is it worth it?

Reported by entertheyoni on 2014-02-06 20:29:28

Beep6581 commented 9 years ago
I did not try the latest patch yet but would like to comment that preview at lower zoom
levels is quite valuable.

Reported by michaelezra000 on 2014-02-06 20:38:51

Beep6581 commented 9 years ago
What if the downsampling algorithm was lanczos instead of the current ?.

It would be a nice start for OpenCL to take the full picture and do the resampling
in GPU .. 

Reported by iliasgiarimis on 2014-02-06 20:59:35

Beep6581 commented 9 years ago
Jacques, just to clarify, I suggested the masked blurring not to deal with the white-noise
artifacts (which could be due to chroma, etc), but to avoid posterization in tonal
transitions on the edges of the areas where effect gets applied to.

Reported by michaelezra000 on 2014-02-07 01:05:12

Beep6581 commented 9 years ago
If you examine the algorithm developed by Emil, you can see:
1) there are a plurality of contrast levels, which depend on a variable "scale"

2) this variable "scale" can take the values ​​1, 2, 4, 8, 16, 32, 64, 128
With 5 levels currently anticipated, it uses 1,2,4,8,16

3) these values ​​affect filter a low pass, but basically this level reflects the pixel
size of the area affected by the algorithm.

With the image at 100%, these are 5 levels 1,2,4,8,16 which correspond to the same
levels of pixels.

RT construction is made in such manner as to accelerate the preview working on a skip
factor of a reduced picture.
At 100% skip=1, at 50% skip=2, at 25% skip=4, etc.

The processed image therefore skip times less pixels in width and height.

To keep the same contrast effect, it is necessary to reduce the number of pixels affected
by the contrast.

Therefore, to skip=4, the factor "scale" should be :
1/4, 2/4, 4/4, 8/4, 16/4
but as you can not put a value of less than 1 pixel, this become:
1, 1, 1, 2, 4
It is good for levels 2, 3 and 4, but excessive for level 0 and 1.

Which means that the image will look the same as 100% for levels 2, 3 and 4, but has
a higher contrast for levels 0 and 1.

for zoom=50%, only level 0 is too contrasted...unless the slider 0 is at zero

#Ilias, perhaps "Lanczos algorithm" is possible, but it needs to rewrite all, including
calling features in "dcrop.cc" "improccordinator.cc"...I don't know if it is used somewhere
in RT , and I don't know how to do?

#Michael: It is touching to skin that generates artifacts  (chroma), not only in transition,
but every where there are hue/chroma /luma as skin...(and there are many many pixels
others than skin concerned...accross the image, and I don't see how to do, although
it is conceivable ??

Reported by jdesmis on 2014-02-07 07:44:59

Beep6581 commented 9 years ago
I don't think that the lanczos filter is worth a rewrite of the call chain. At least
it's negligible compared to the wavelength factor.

Speaking about this, we should take care of the last 1 scale and ignore previous 1
factors, plain simple. Users can't expect to see pixel level enhancements when zoomed
out.

Reported by natureh.510 on 2014-02-07 14:20:43

Beep6581 commented 9 years ago
I agree, when zooming out, let's ignore the finer CDBL

Reported by michaelezra000 on 2014-02-07 14:39:25

Beep6581 commented 9 years ago
Here a patch (the same) that take into account new default!

Hombre : tu t'es proposé pour changer les tooltip #31

Excuse my bad English, but when you say "let's ignore the finer CDBL" ...

a) Is that the algorithm should ignore low values ​​- which will lead for skip>1 to
zero the action of corresponding sliders  (if scale/skip < 1)  (easy to do)

b) Or is the user - that the magnification is low and it is difficult to see the differences
- which should ignore the low values

:)

Reported by jdesmis on 2014-02-08 06:22:17


Beep6581 commented 9 years ago
Oh it is not the good file 

Sorry

Reported by jdesmis on 2014-02-08 07:36:40


Beep6581 commented 9 years ago
Hi Jacques, sorry I could not understand a) vs b)... What I though is that if there
is no purpose of sharpening a pixel that cannot be seen. So if a slider is acting on
spacial frequency that is *not displayed with the original contents due to zoom*, there
is no need to display effect of that slider.

Reported by michaelezra000 on 2014-02-08 19:02:46

Beep6581 commented 9 years ago
I agree with #61 a), let's hope it makes the preview quite similar to the rendered output!
Will test 17b.patch today/tomorrow.

Reported by entertheyoni on 2014-02-08 19:30:40

Beep6581 commented 9 years ago
oui, il s'agit de la solution a).

pour les tooltips, j'essaierais de m'en occuper demain après-midi, sur la base du dernier
patch en date.

Reported by natureh.510 on 2014-02-09 01:06:32

Beep6581 commented 9 years ago
Here are the patch who does what is expected in a)

For "Lab mode" and "CIECAM"

:)

Reported by jdesmis on 2014-02-09 07:28:05


Beep6581 commented 9 years ago
Sorry, didn't managed to find enough time for all that I had to do this sunday.

Is "algo" really removed, or should it be kept for later implementation? If removed,
then it'd be better to have a clean patch with everything related removed.

I'm also ambivalent on the CBDL preview when zoomed out: it' still not realistic when
compared to the output at the same zoom level. If it can't be a real preview (i.e.
quite close), should we preview anything? It might confuse users and we'll see complains.
The problem is for zoom rates that are not a multiple of 2, 4, 8, ... i guess.

Reported by natureh.510 on 2014-02-09 23:26:23

Beep6581 commented 9 years ago
I have comment "algo" and not removed, I am not sure the actual algo is the best...to
put it differently, there is no rationality in skin color, and the current algorithm
is empirical, just change a few things for a less good appearance  :)

For the second point, I change a little the manner of calculate "scale" for zoom rates
that are not a  multiple of 2, 4, 8, 16...I "rounded" the division to the nearest integer...but
there will always be some (small) differences.

Reported by jdesmis on 2014-02-10 08:08:34


Beep6581 commented 9 years ago
I tested issue2150-18.patch preview vs saved when using all sliders including those
ignored in the preview, and then using just the sliders not ignored in the preview
(the ignored ones set to "1"):
1a http://i.imgur.com/DcML681.png
1b http://i.imgur.com/0BJ6IG1.png
2a http://i.imgur.com/XEVyjAq.png
2b http://i.imgur.com/J982lwb.jpg
3a http://i.imgur.com/pa6iSf6.png
3b http://i.imgur.com/XqJHo24.png
4a http://i.imgur.com/oydM9hB.png
4b http://i.imgur.com/EkVjBhp.jpg

Observations:
- Fine CbDL levels have almost no impact on a much scaled-down saved image, so ignoring
them in the preview is acceptable.
- Those levels which do have an affect are rendered accurately-enough in the preview.
- Works good, let's keep it! :)

In fact the biggest difference between the preview and saved images is not the CbDL
sharpness, but that the preview uses a low quality downscaling algorithm while the
saved image uses a smooth Lanczos ones. If the preview were to use Lanczos, I believe
that would make using this as well as several other tools in RT easier too because
the preview would match the output better (e.g. tone mapping).

Reported by entertheyoni on 2014-02-10 08:14:45

Beep6581 commented 9 years ago
I tested the preview vs saved of _18 vs _19 at 16% and 25% zoom. _18 matches the saved
image better than _19 in all cases.

Reported by entertheyoni on 2014-02-10 08:50:17

Beep6581 commented 9 years ago
So which one to keep? I find V18 to be quite off. Hadn't had time to test the latest
one, sorry.

About the labels, I don't have the time to change that, I'm very busy, but at least,
could you remove "(radians x100) in the label, and make a 2-3 line tooltip for the
Artifact checkbox (the actual one doesn't help at all) please? Then, I guess it should
be committable!? I'll change the labels afterwards.

Please don't change the scaling method, at least do not make it conditional. I can
investigate later this week (or after the commit) to use the lanczos scaling method,
with a slight speed penalty.

Reported by natureh.510 on 2014-02-10 21:37:00

Beep6581 commented 9 years ago
Hombre, please wait with lanczos scaling. I actually try a speedup for it.

Ingo

Reported by heckflosse@i-weyrich.de on 2014-02-10 22:30:45

Beep6581 commented 9 years ago
No problem, I have another big bone to work on ;)

Reported by natureh.510 on 2014-02-10 23:25:03

Beep6581 commented 9 years ago
:-)

Reported by heckflosse@i-weyrich.de on 2014-02-10 23:56:17

Beep6581 commented 9 years ago
This patch restores the previous way of managing the zoom; ie it does not matter up
to 100%; whether in "lab mode" or "CIECAM mode".

I tried "Lanczos" by adding a "Lanczoslab" mode, but I stopped because:
a) Hombre says it will do
b) Ingo says it will improve this function.
Nevertheless, I left the code (with comment) to change it if necessary with "skip"
and let the code "Lanczoslab" (very little difference with Lanczos rgb).

Unless I did not understand pipelines RT, it is essential if you want the preview is
the same as the TIFF (JPG) output:
a) from a total image (not as a partial as actually), otherwise there will always be
differences (? maybe I'm wrong);
b) ignore differences due to color management and differences due to gamma (between
preview and output  - old issue)
C) act with the algorithm "Lanczos" in Lab and CIECAM to view in skip mode (20%...33%..50%)..

I also slightly changed the labels and tooltip.

Except opposite view and after users have checked - both "lab" and "CIECAM" - everything
works correctly; I'll commit shortly :)

Reported by jdesmis on 2014-02-11 10:13:04


Beep6581 commented 9 years ago
Hombre "So which one to keep? I find V18 to be quite off."
Did you find it to work better in any of the other patches? I find patch 18 to be most
accurate to the saved image so I'm in favor of having it work as in patch 18. Patch
19's preview was much too soft at <100%. Patch 18's differences were mostly in fine
sharpness, but I'm guessing that difference will be gone when the preview is switched
to use Lanczos.

Preview of CbDL at less than 100% zoom does not work with patch 20, and at 100% zoom
the preview looks identical to unpatched, so I don't know what to test.
"Hue skin" should be changed to "Skin hue".

Reported by entertheyoni on 2014-02-11 12:08:27

Beep6581 commented 9 years ago
Hello DrSlony

I have the same question (with my bad english ! ?)
I have understand to keep the first one: ie without zoom (only if skip=100%)

For me, at 100% and above, all works correctly (and under 100% no action) :)

No problem "hue skin" and "skin hue" :)

Reported by jdesmis on 2014-02-11 12:16:33

Beep6581 commented 9 years ago
Hello Jacques :)
From my tests it shows that the CbDL preview of patch 18 is reliable. I tested here
many zoom levels - from 25% to 10% - and patch 18 worked best. It's not 100% perfect,
but as my screenshots in comment #69 show it is very close, so I am in favor of keeping
the preview working at any zoom!

Reported by entertheyoni on 2014-02-11 13:17:22

Beep6581 commented 9 years ago
For me no problem...

another opinion?

After that I try "de ne pas me mélanger les pinceaux" (in french) to provide the correct
version...
:)

Reported by jdesmis on 2014-02-11 13:20:31

Beep6581 commented 9 years ago
The same algorithm as in _18, but with all corrections.

I hope there is no error :)

Reported by jdesmis on 2014-02-12 08:00:04


Beep6581 commented 9 years ago
Jacques thank you! Works well.

Reported by entertheyoni on 2014-02-12 10:56:14

Beep6581 commented 9 years ago
If no other remarks:
1) I'll clean the code
2) I'll commit tomorrow.

:)

Reported by jdesmis on 2014-02-12 11:37:50

Beep6581 commented 9 years ago
Hi Jacques,
I just got to try the latest patch.

Ideally, it could improve useability if finer CDBL were a bit more visible.
I remember that in some incarnation of the patch this cause artifacts.
Do you think that simply reducing adjustment amount for the preview would solve that?
Or, may be there is some fundamental preview design limitation that would not allow
this?

About the posterization on the transition border, I can still reproduce it using that
sample NEF file I sent you. Posterization can be reduced or may be avoided when not
using extreme slider values (including the slider for "skin tones targeting/protection",
but the fact that it can be caused quite easily would limit use of this cool feature
in fine editing.

Reported by michaelezra000 on 2014-02-13 02:53:55

Beep6581 commented 9 years ago
Michael

Unfortunately (??) I do not do miracles ...

Of course you can modulate the action of the slider according to "skip", but on what
basis? For the "skip" values ​​which should cut the pixel, what value for the slider
?

Patch _19 which seemed more logical gives poorer results !

So in summary 3 posibilities:
1) only activate CBDL for zoom=100% (and still there will always be differences, especially
visible at low luminance values​​, due to the management of color and differences gamma)
2) leave as now (it is imperfect - you cannot cut the pixels ...)
3) use the Lanczos algorithm, but from the output image (or equivalent), otherwise
there will always be differences

For posterization, you give yourself the answer, but do not forget that the key factor
is a good white balance :)

Reported by jdesmis on 2014-02-13 06:59:26

Beep6581 commented 9 years ago
Here a patch with :
a) clean code
b) I have had an option variable (for testing)in "options" : CBDLlevel

default CBDLlevel=0
you can put CBDLlevel between 0 and 100

* 0 : disabled totaly effect of sliders if zoom too low - "we cannot cut pixels" (default)
* 100 : sliders keep all there effects even for low zoom  (probably the worst)
* 20 : 20% of sliders is take into account
* etc.

Reported by jdesmis on 2014-02-13 10:27:54


Beep6581 commented 9 years ago
Jacques, and you were saying no miracles:)
I tried at CBDLlevel=20 and it is perfect!
May be we should change that to be the default value?
Thanks:)

Reported by michaelezra000 on 2014-02-14 13:18:56

Beep6581 commented 9 years ago
I did some broader testing across images of various resolutions, zoom levels and CDBL
settings. My impression is that CBDLlevel=40 is a more suitable universal setting.

Reported by michaelezra000 on 2014-02-14 14:46:23

Beep6581 commented 9 years ago
Ok, Michael thanks :)

Another opinion ?

Reported by jdesmis on 2014-02-14 15:06:48

Beep6581 commented 9 years ago
Jacques, one observation - in some cases, specific to the test images CDBL preview for
specific slider at particular zoom may become less visible, than for some other images.
I suppose this is due to the preview image interpolation. Just something we should
keep in eye on.

Reported by michaelezra000 on 2014-02-14 15:22:08

Beep6581 commented 9 years ago
I tested CBDLlevel 0, 20, 40 and 100 on a few images at various zoom levels. The differences
are most visible at small zooms with extreme CbDL values. In my case, 1920x1080 screen
with Pentax K10D and Nikon D80 shots, zoom 16% - 25% is a real-life usable range. I
don't have any D800 portrait raws to test, those would probably be zoomed close to
10% on my screen.

Which CBDLlevel looks best depends on which CbDL slider is being tested and which zoom
level used.

In all cases, the first "0" CbDL slider looks best with CBDLlevel 0.
CbDL slider 1 looks best with CBDLlevel at 20.
Slider 2 didn't exactly match the preview even with CBDLlevel at 100.
CBDLlevel didn't seem to matter for slider 3, so I didn't test slider 4 either.

Jacques I don't think there is one CBDLlevel good for all sliders, photo resolutions
and zoom levels, but I am sure that CBDLlevel for slider 0 should be 0.
For the other sliders, 20 or 40 is a good compromise.

Luckily to see the differences I had to use quite high CbDL slider values, probably
higher than I'd use in real life.

Reported by entertheyoni on 2014-02-14 20:04:34

Beep6581 commented 9 years ago
DrSlony - 0 would disable the preview;)

Reported by michaelezra000 on 2014-02-14 21:20:51

Beep6581 commented 9 years ago
Yes that's the point with the 0 slider.

Reported by entertheyoni on 2014-02-14 21:58:38

Beep6581 commented 9 years ago
Here a new patch, which I hope will please you :)

I put in "option" 2 variables:
CBDLlevel0=0
CBDLlevel123=30
 =>the first is for the slider 0
 =>the second for the others sliders
By default, I put 0 for the first one, and 30 for the second

I limited the possible values ​​entered by the user, to prevent bad use :
* no negatives values
* <= 40 for sliders 0
* <=50 for others

If no others remarks I'll commit tomorrow...

Then I would (still) missing for 15 days (without internet connection)

:)

Reported by jdesmis on 2014-02-15 07:43:53


Beep6581 commented 9 years ago
commit to default :)

Reported by jdesmis on 2014-02-16 07:25:09

Beep6581 commented 9 years ago
Thanks!
Sliders 0, 1 and 4 match the saved image perfectly.

Sliders 2 and 3 in the preview need work, as they do not match the saved image. They
need to be much stronger in the RT preview.
http://filebin.net/8vf1nbshd9

I tested CBDLlevel123=0 vs CBDLlevel123=50. There are small differences between 0 and
50 with slider 2 at 4.00 (all other sliders at 1.00), and no differences at all with
slider 3 at 4.00 (all other sliders at 1.00). Is the patch working correctly?

Reported by entertheyoni on 2014-02-16 17:28:12

Beep6581 commented 9 years ago
yes it works correctly :)
But as I say "I do not do miracles ..." ! 

This algorithm is a workaround around the problem: "you can not cut the pixels"...So
this is an approximation that works in some cases and not in others.

Sorry !

Reported by jdesmis on 2014-02-16 18:42:22

Beep6581 commented 9 years ago
I understand Jacques, but since differences are visible at level 2, they should be even
more visible at level 3, no?

Reported by entertheyoni on 2014-02-16 18:47:48

Beep6581 commented 9 years ago
DrSlony

it is not as simple !

There is interaction between sliders from the moment he would cut pixels...

And depending on the strength of each slider, it would change the percentage, not being
sure of the result. The only solution that works is from the full-size image.

I leave soon for ten days

:)

Reported by jdesmis on 2014-02-17 10:27:13

Beep6581 commented 9 years ago
Closing issue as the patch has been committed
https://code.google.com/p/rawtherapee/source/detail?r=d3a85712685bbc603132833d2e0fddc4463efa07#

Thank you Jacques!

Reported by entertheyoni on 2014-03-11 14:02:35