Beep6581 / RawTherapee

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

Uniform Perceptual Lab option #1297

Closed Beep6581 closed 8 years ago

Beep6581 commented 8 years ago

Originally reported on Google Code with ID 1313

This adds a new feature to the Lab control; it makes it optionally work in Uniform Perceptual
Lab space. This should lead to fewer color shifts when editing.
More info here: http://www.brucelindbloom.com/index.html?UPLab.html

You have to create a directory "(RTroot)/iccprofiles/internal" and then unpack this
icc into there:
http://www.brucelindbloom.com/downloads/CIELab_to_UPLab2.icc.zip

However the usual problems with LittleCMS/ICCs also apply here:
- Unprecise (using integer arithmetic inside)
- Clips highlights etc. (however since the recovers is a stage before it should not
disturb too much)
- Slower (not optimized yet, don't know if the tool itself is useful enough)

Reported by oduis@hotmail.com on 2012-04-07 15:50:38

Beep6581 commented 8 years ago
This is the performance optimized version.

Reported by oduis@hotmail.com on 2012-04-07 21:26:47


Beep6581 commented 8 years ago
Hi Olli, this will be a great addition!

Some feedback with issues that would need resolving: 
1. When clicking "Uniform Perceptual Lab" there is a color change in the preview even
when no Lab parameter is changed from 0.
2. Engaging this option adds magenta tint to skintones
3. Histogram shows strong posterization with this option enabled
4. Changes are not reflected in the thumbnails

Reported by michaelezra000 on 2012-04-08 01:08:21

Beep6581 commented 8 years ago
When I click UPLab I get EXC_BAD_ACCESS; here is the backtrace:

(gdb) bt
#0  0x000000010217c5e7 in cmsDoTransform ()
#1  0x0000000100472288 in rtengine::LabImage::ConvertFromUPLab (this=0x1063a0020) at
/Users/ejm/math/imaging/rawconvert/C_code/rawtherapee-default/rtengine/labimage.cc:71
#2  0x000000010045ccdb in rtengine::ImProcCoordinator::updatePreviewImage (this=0x103ae3000,
todo=6, cropCall=0x0) at /Users/ejm/math/imaging/rawconvert/C_code/rawtherapee-default/rtengine/improccoordinator.cc:285
#3  0x000000010045d83e in rtengine::ImProcCoordinator::process (this=0x103ae3000) at
/Users/ejm/math/imaging/rawconvert/C_code/rawtherapee-default/rtengine/improccoordinator.cc:661
#4  0x000000010046293a in sigc::bound_mem_functor0<void, rtengine::ImProcCoordinator>::operator()
(this=0x106907e28) at mem_fun.h:1787
#5  0x0000000100462955 in sigc::adaptor_functor<sigc::bound_mem_functor0<void, rtengine::ImProcCoordinator>
>::operator() (this=0x106907e20) at adaptor_trait.h:251
#6  0x0000000100462978 in sigc::internal::slot_call0<sigc::bound_mem_functor0<void,
rtengine::ImProcCoordinator>, void>::call_it (rep=0x106907df0) at slot.h:103
#7  0x0000000100fff002 in call_thread_entry_slot ()
#8  0x0000000100f0565c in g_thread_create_proxy ()
#9  0x00007fff89f57fd6 in _pthread_start ()
#10 0x00007fff89f57e89 in thread_start ()
Current language:  auto; currently c++
(gdb) 

Reported by ejm.60657 on 2012-04-08 01:45:59

Beep6581 commented 8 years ago
Found an interesting paper on the subject: http://www.ppenteado.net/idl/penteado_visualize2010.pdf

@Emil, just to be sure, did you unpack the new icc into (RTroot)/iccprofiles/internal

Reported by michaelezra000 on 2012-04-08 01:57:53

Beep6581 commented 8 years ago
Emil-Martinecs-MacBook-Pro:internal ejm$ pwd
/Users/ejm/math/imaging/rawconvert/C_code/rawtherapee-default/iccprofiles/internal
Emil-Martinecs-MacBook-Pro:internal ejm$ ls
CIELab_to_UPLab2.icc
Emil-Martinecs-MacBook-Pro:internal ejm$ 

Reported by ejm.60657 on 2012-04-08 02:42:32

Beep6581 commented 8 years ago
Good morning,
Quick take before breakfast: Emil, you're right, I didn't update the cmake script,
so you currently have to copy the ICC manually to the execution release folder.
Michael, I also noticed the red changes, however did not mention that it's already
shifting when no parameter was changed. However I wonder the code is the same for back
and forth...  will have a look

Reported by oduis@hotmail.com on 2012-04-08 06:56:51

Beep6581 commented 8 years ago
Hello Olli
First of all thank you for this patch, you did a good job with "CIELab_to_UPLab2.icc"

I compared the colors with and without UPLab on the picture of my target 468 colors.

I chose as general profile "neutral"
Working profile = Prophoto; Output Profile = Prophoto
with "auto-matched camera-specific profile" (I also tried with "Camera Standard")
I chose these settings to avoid the maximum problems of gamut, Clip, etc..

I checked five colors, before and after UPLab without changing any settings (no saturation,
no contrast, no vibrance, etc.. Nothing)
Grey:
* Without UPlab: L = 70 a=0 b=1
* UPlab: L =70 a=0 b=3

Red:
* Without UPlab: L =44 a=72 b=36
* UPLab: L = 44 a=74 b=17

Green:
* Without UPLab L =54 a=-61 b = 15
* UPLab: L = 54 a =-61 b =10

Blue:
* Without UPLab L = 37 a =-29 b =-71
* UPLab: L = 37 a =-28 b =-74

Yellow:
* Without UPLab: L = 84 a =-2 b =99
* UPLab: L=84 a=-7  b=87

I do not see any anomalies in the code "Labimage.cc", may be should try a different
"intent"?

I also made a brief comparison with "vibrance"  (with blue-purple):
* activating or not "avoid color shift"
* Activating or not UPlab

reference color (without vibrance)  L=23 a=39 b=-55  

Acivating vibrance (50) :
* not activating "avoid color shift" : L=23 a=59 b=-82   
* activating "avoid color shift" : L=23 a=66 b=-76  (Good Munsell correction)
* activating UPlab (no activating "avoid color shift") : L=23 a=54 b=-92

I also found the same abnormalities as Michael 

Reported by jdesmis on 2012-04-08 07:36:58

Beep6581 commented 8 years ago
I copied into release/iccprofiles and still got a segfault.

Reported by ejm.60657 on 2012-04-08 13:51:16

Beep6581 commented 8 years ago
Jacques, I am a little confused as to how to interpret your numbers.  UPLab is a different
color space than Lab, so comparing Lab numbers doesn't necessarily make much sense
(like comparing RGB values in sRGB and prophoto).  How should we interpret your results?

Reported by ejm.60657 on 2012-04-08 13:55:31

Beep6581 commented 8 years ago
Sorry, we're over at my parents house for Easter.
@Emil: iccprofiles\internal is the folder.
@Jacques: I think the problem to be solved first is why converting back and forth does
any changes at all. If the image is nowhere near a gamut border it must be the same.

Reported by oduis@hotmail.com on 2012-04-08 15:27:32

Beep6581 commented 8 years ago
Olli

Values ​​are measured with the "preview", so they are in Lab values ​​before and after
conversion UPlab.
Normally if there is no treatment (saturation, brightness, etc..), values ​​before
/ after should be identical.

This means that either:
* There is an anomaly in the calls for UPLab (LCMS,labimage.cc, etc.)
* the conversion and vice versa with UPlab "CIELab_to_UPLab2" is bad; which may be
due to several reasons which can only be resolved by the creator of the ICC profile
(default in LUT, Bradford adaptation, etc..)

Reported by jdesmis on 2012-04-08 15:47:47

Beep6581 commented 8 years ago
OK, I got it not to segfault.  The result for me is to totally desaturate the blues...

Reported by ejm.60657 on 2012-04-08 15:53:19

Beep6581 commented 8 years ago
BTW, how does the comment at the bottom of

http://www.brucelindbloom.com/index.html?WorkingSpaceInfo.html

about swapped tags affect us?

Reported by ejm.60657 on 2012-04-08 16:07:35

Beep6581 commented 8 years ago
Emil

In my case, under Windows7 64-bit, the same image of my target 468 color (your "screen-capture.jpg"),
seems more correct, but by measuring, we see significant difference (the ones I mentioned
above)

I tried with the other profile provided by B.Lindbloom "CIELab_to_UPlab.icc", results
are less bad, but still not satisfactory.

Reported by jdesmis on 2012-04-08 16:59:50

Beep6581 commented 8 years ago
Regarding the swapped profile: I already pointed to the right one (the LCMS compatible).
Changed some Lab profile version and intend, no help.
Then I found that in RT we don't feed LCMS with Lab but convert it to XYZ and mark
that as RGB space (very strange, a workaround for a bug?). Tried that also, but it
didn't help.
Here is latest incarnation. I'm beginning to think that it might be a bug in profile,
as it just affects one color here (and the same obviously with Michael).

PS: I'm using the latest and greatest LCMS 2.3

@Jacques: no use to check color throughput at all. It's expected to now change if you
enable UPLab and have Lab tool all neutral.

Reported by oduis@hotmail.com on 2012-04-08 21:41:13


Beep6581 commented 8 years ago
This is the case UPlab vs. Std but all sliders in Lab neutral. Should be the same, but
mention the pink patch on the circle. It's most changed, while the rest roughly stays
the same (which is expected):
http://www.visualbakery.com/RawTherapee/Download/StdVsUPLab.jpg
Looks like a problem with the profile to me, maybe an incorrect LUT point.

Reported by oduis@hotmail.com on 2012-04-08 21:50:23

Beep6581 commented 8 years ago
Olli

I just tested the new patch "UPlab3.patch".
I put the same settings as before: neutral, etc..All cursors on "neutral"

On the target "468 colors", by measuring Lab values ​​before/after "Uniform Perceptual
Lab", I get almost the same values ​​as with the patch "UPLab2.patch" ... for example
the yellow 

Without UPLab L = 84 a =-2 b =99
With UPLab  L=84 a=-7 b=86
(it is the same for other colors)

Ie they are not good

Also the histogram is full of fish bones

Reported by jdesmis on 2012-04-09 05:29:55

Beep6581 commented 8 years ago
If you want to try, I put the 468 color target on my site (4029-li.NEF) (right target)
http://jacques.desmis.perso.neuf.fr/RT/

I have taken (arbitrarily) cells following reference (after white balance on the cell
H15) - all in Prophoto, and profile "neutral"
red : K10
green : J10
yellow : I10
grey : H15

Cell for "blue-purple" when saturation changed : K17

Reported by jdesmis on 2012-04-09 06:22:18

Beep6581 commented 8 years ago
Hi Jacques, thanks, but as pointed out the bug is already visible with the plain eye,
so you don't need a measurement here.
Since we don't see a bug in the code, it must be either the profile or LittleCMS (and
I'm using the latest version). I'll try to ask Marti or Bruce.

Reported by oduis@hotmail.com on 2012-04-09 08:35:44

Beep6581 commented 8 years ago
As you, this is what I think:
* Bug in the profile: LUT not good, not good chromatic adaptation, or ??, etc..
* Maybe bug in LCMS, but unless the problems of CLIP, I do not think

Moreover, apart from that return values ​​are not correct, there is the histogram we
can not leave like this (fish bones !!)

Reported by jdesmis on 2012-04-09 08:45:23

Beep6581 commented 8 years ago
Let's see what Marti says:
https://sourceforge.net/mailarchive/forum.php?thread_name=BLU126-DS1646849F07EAE2E6731360AB370%40phx.gbl&forum_name=lcms-user

Reported by oduis@hotmail.com on 2012-04-09 15:29:07

Beep6581 commented 8 years ago
For the records:
There have been some private mailings, including with Bruce himself.
Current status is that Bruce says there might be some gamut challenges in the ICC profile.
Emil wants to experiment a bit (the problems reported above were due to an outdated
version of LCMS on his machine), but if holds true the profile does not convert with
higher fidelity, we’ll need to shut this patch down unfortunately.

Reported by oduis@hotmail.com on 2012-04-15 10:42:15

Beep6581 commented 8 years ago
Ok, no action, I'll write this off (sniff).

Reported by oduis@hotmail.com on 2012-04-25 22:23:28

Beep6581 commented 8 years ago
Hi Olli, may be this could be re-purposed for Jacques' Munsell corrections instead?

Reported by michaelezra000 on 2012-04-26 13:41:46

Beep6581 commented 8 years ago
As I mentioned before, I would like to see a stand-alone method for Munsell corrections,
that could be called to address hue shifts that result from various tools.

Reported by ejm.60657 on 2012-04-26 14:15:32

Beep6581 commented 8 years ago
I'm with Emil, Jacques takes was not modular. We'd need some modular technic (UPLab
or Munsell or whatever) to "tweak" the Lab space mapping.
No problem about this patch though, I ported the speedup optimization part of the code
to other patches already, so it was not completely useless work.

Reported by oduis@hotmail.com on 2012-04-26 14:22:14

Beep6581 commented 8 years ago
Yes, I also see benefit to globalized implementation.
Is there any shortcut to evaluate such correction techniques before coding them into
RT?

Reported by michaelezra000 on 2012-04-26 14:27:32

Beep6581 commented 8 years ago
Not really. You could save the UI, but in this case if was just a checkbox, no big deal.

Reported by oduis@hotmail.com on 2012-04-26 14:30:36

Beep6581 commented 8 years ago
I do not think "tweak the Lab space" is a good idea.

When I designed "Munsell correction," I thought of two possible alternatives:
1) "tweak the Lab space" as does UPlab
2) change only what has changed during transformations.

The first solution, although it is attractive will bring big problems, because it transforms
two times the data set with tools approximate (LUT, gamma, Bradford ...) on data that
do not usually need ... and does not solve problems such as the conversion of space.

The second solution seems more complicated, but it works great. I have greatly simplified
the code (not functionality), I still have a level of simplification.

You can test it on "vibrancy" which corrects chromaticity.

If you want you can test on "luminance" of the image that is on my site, with the patch
"tonemapping"  http://jacques.desmis.perso.neuf.fr/RT/    (file _ASC4209.NEF). You
will clearly see the correct red / yellow, if "strength" (tone mapping) > 1.5.

I modified the patch "tonemapping" which now contains code "Munsell" Simplified (tone16_map.patch)

Reported by jdesmis on 2012-04-26 15:38:30

Beep6581 commented 8 years ago
Sorry for not making myself clear, Jacques.  What I have in mind is something like the
following:

1. Internal representation is Lab.  Cache initial Lab image.
2. Perform some image transformation on the Lab data (eg saturation, contrast, brightness).
3. Evaluate the change in Munsell colors and make hue correction.

If I understand correctly, much of this exists in your code already developed; we simply
need to convert the Lab data of the two images to Munsell, make the correction, and
go back to Lab.  Or am I missing something?  In principle we would want this not just
for Lab but also RGB transformations, so we would want to break the application apart
so that there are methods MapToMunsell (one for each source color space -- Lab, RGB,
whatever) as well as MapFromMunsell; and then a method MunsellCorrection(Image1, Image2)
that evaluates the change in Munsell hues between Image1 and Image2, then corrects
the hue of Image2.

Reported by ejm.60657 on 2012-04-26 15:56:07

Beep6581 commented 8 years ago
The steps 1,2,3 are performed by "Munsell correction". But do not forget the space conversion,
which if done will last (output) be done without correction.

This works fine for everything in Lab, but should be reassessed for RGB.

I think we should do this gradually - I am a very careful - check at each stage that
everything is fine : chromaticity (most common), then luminance, then RGB, etc. Currently
the luminance and chromaticity corrections work very well, it remains to simplify (yet)
just the ordering process

The patch "tone16_map.patch" contains all the code (to simplify again) to 'Munsell
correction "(in "ipvibrance.cc"). 
I expect the changes from "Ben" to commit "Munsell correction"  :)

Reported by jdesmis on 2012-04-26 16:17:43


Beep6581 commented 8 years ago
Applies, compiles and runs fine in Gentoo x86_64. Previous processing profiles are incompatible
- quite large color differences. This produces more saturated results, I like it. Shouldn't
it go into issue 1237?

Reported by entertheyoni on 2012-04-29 19:22:20

Beep6581 commented 8 years ago
yes, should better be discussed at tone mapping. This patch had a different goal and
a different result (if it had worked)

Reported by oduis@hotmail.com on 2012-04-29 20:23:35

Beep6581 commented 8 years ago
any chance to create a generic LAB profile which can be used with imagemagick to convert
LAB file to RGB?
i.e. I need a good colors conversion for this:
 convert calimg_LAB.psd -profile "/var/www/cp/CIELab_to_UPLab.icc"  -profile "/var/www/cp/AdobeRGB1998.icc"
/var/www/32.jpg

no acceptable results neither with CIELab_to_UPLab not with v2 of it

Reported by dmitry.kotin@kreatetechnology.com on 2013-06-25 19:00:20

Beep6581 commented 8 years ago
This discusses profiles for the RawTherapee converter. It has nothing to do with imagemagick.

Reported by oduis@hotmail.com on 2013-06-25 19:08:59