Beep6581 / RawTherapee

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

converting RT image pipeline to float #427

Closed Beep6581 closed 8 years ago

Beep6581 commented 8 years ago

Originally reported on Google Code with ID 437

I started a new branch to convert the RT image processing pipeline to floating point
throughout.  So far I have ported most of the Lab image processing to floating point.
 I am not sure all the image processing tools are working as desired, but basic exposure
adjustments seem to be OK.

The next step will likely be to star converting the RGB processing to floating point,
in particular raw conversion.

For the moment, I would like to ask that I be the only one allowed to check in changes
to this branch; of course, anyone is free to post patches for me to vet for check-in.

Reported by ejm.60657 on 2010-12-26 17:15:30

Beep6581 commented 8 years ago
It always puzzled me why float was not used from the beginning, probably for smaller
memory footprint and speed. I am really looking forward to this change - from both
simpler coding point of view and likely better image quality. 

This will further be useful if RT will get extended into HDR processing in the future.

Reported by michaelezra000 on 2010-12-26 18:18:35

Beep6581 commented 8 years ago
That's what I would guess too; only Gabor knows for sure though.  But I am frustrated
by the limitations of short integer formats for data storage; it makes it very convoluted
to keep some headroom for highlights, and one ends up clipping the data all the time
at intermediate steps of the calculations.  Also one can't do proper black point processing,
it's suboptimal for exposure compensation by large amounts, does unnecessary clipping
of narrow gamut working spaces, won't do HDR, etc etc.  Once I get this in place I
think one of the first steps will be to implement proper gamut mapping (I want to do
some form of perceptual intent treatment of out-of-gamut colors) and a revision of
the highlight recovery/reconstruction tools.

Reported by ejm.60657 on 2010-12-26 19:43:19

Beep6581 commented 8 years ago
AFAIK performance was considered better with int, but I'll contact Gábor.
However, since then the computers have became much faster. Performance should be a
key point for RT (think of netbooks for example), but this should be aimed by scalability
(like the cache-size settings in preferences), and quality is more important than performance
until the usability does not deteriorate.

Int -> float should solve a lot of problems IMHO, I really like this initiative. Thanks
in advance.

Reported by gyurko.david@e-arc.hu on 2010-12-26 22:01:56

Beep6581 commented 8 years ago
if everything becomes float, does this open an opportunity to use GPU and run x-times
faster?

Reported by michaelezra000 on 2010-12-26 22:18:31

Beep6581 commented 8 years ago
Using integers instead of floats for performance reasons (at the cost of precision and
convenience) can on modern CPUs often turn out to be a poor choice. There are cases
when float processing is actually faster, but more common the float code does not become
as much slower as one expected so it is simply not worth making it integer.

Back in the days when there was separate floating point units and Intel had not started
to call their CPUs Pentium integer math was indeed always much faster, and I think
this historical fact still lives although no longer true.

Using SIMD (SSE) for float processing where possible can increase perfomance greatly,
and that is probably easier to make use of than the GPU, although that too is an interesting
feature.

Reported by torger@ludd.ltu.se on 2010-12-26 23:04:41

Beep6581 commented 8 years ago
Keep attention to memory footprint also! Just now we have some problems when more photos
are opened in editor.

Reported by ffsup2@yahoo.it on 2010-12-26 23:21:24

Beep6581 commented 8 years ago
May be the thumbnail processing should remain int for smaller memory footprint

Reported by michaelezra000 on 2010-12-26 23:33:31

Beep6581 commented 8 years ago
Perhaps I should make it clear -- "only" the image processing pipeline is being converted
to float in this branch of 3.1alpha (aka default); the output image arrays remain 16-bit
for image editing, and either 8-bit or 16-bit for thumbs (depending on setting in preferences).
 So the costs for memory are that the arrays rawData, red, green and blue are converted
to float from unsigned short.

Rawtherapee is a memory hog, but I don't think the culprit is the storage of the image
arrays themselves.

Reported by ejm.60657 on 2010-12-27 00:00:39

Beep6581 commented 8 years ago
Emil

It is a great step forward in improving quality.

Reported by jdesmis on 2010-12-27 06:16:51

Beep6581 commented 8 years ago
Some first results...

default branch, changeset 744:
http://theory.uchicago.edu/~ejm/pix/20d/posts/ojo/RT-default.jpg

float branch (not yet committed changes), branched from 744:
http://theory.uchicago.edu/~ejm/pix/20d/posts/ojo/RT-float.jpg

Reported by ejm.60657 on 2010-12-27 19:11:57

Beep6581 commented 8 years ago
Whites are certainly cleaner. 
I am curious, why blacks seem to be clipped and histograms look so different.

Reported by michaelezra000 on 2010-12-27 19:32:55

Beep6581 commented 8 years ago
Yes, I agree: the upper end looks very promising, the lower is "interesting". :)

Reported by gyurko.david@e-arc.hu on 2010-12-27 20:46:00

Beep6581 commented 8 years ago
When starting RT_float, the gui appears but crashes away after one second. Terminal
output is:
Processing file ./profiles/crisp.pp3...
Processing file ./profiles/neutral.pp3...
Processing file ./profiles/default.pp3...
sc_length[0/3]=-nan 
sc_length[3/3]=-nan 
NURBS: error detected!
 i=0 nbr_points=-2147483648 ppn=62 N=4 sc_length[i/3]=-nan total_length=-nan

Ubuntu 10.10 32 bit, ec1ffd558f3d 749 float

Any idea?

Reported by paul.matthijsse4 on 2010-12-28 09:55:10

Beep6581 commented 8 years ago
Looks like you have a corrupted profile or some such, perhaps you might rebuild your
cache or put some images in a test directory and point RT there for startup by resetting
the preferences file.  I have not touched the part of the code that deals with NURBS
curves, which is what seems to have caused your error.

Reported by ejm.60657 on 2010-12-28 13:12:42

Beep6581 commented 8 years ago
I corrected some bugs in the Lab conversion and I think things are OK now.

default branch, changeset 744:ec1ffd558f3d
http://theory.uchicago.edu/~ejm/pix/20d/posts/ojo/RT-default.jpg

float branch (not yet committed changes), branched from 744:
http://theory.uchicago.edu/~ejm/pix/20d/posts/ojo/RT-float.jpg

Reported by ejm.60657 on 2010-12-28 17:19:22

Beep6581 commented 8 years ago
Shadows look great now. There is a color transition area on a shirt from blue-ish to
the cleaner white. This areas seems a bit yellow.

Reported by michaelezra000 on 2010-12-28 17:35:13

Beep6581 commented 8 years ago
I compiled changeset 769 and highligts are mostly purple and flat. Did something go
wrong during check in?

Reported by michaelezra000 on 2010-12-29 22:13:16

Beep6581 commented 8 years ago
check in of what?

Reported by ejm.60657 on 2010-12-29 22:27:36

Beep6581 commented 8 years ago
the samples you posted earlier looked very different from what I can produce with compiled
code from latest changeset - so I was wondering if any code (versions) got lost during
hg checkin process.

Reported by michaelezra000 on 2010-12-29 22:54:38

Beep6581 commented 8 years ago
Perhaps the version on Google code is not the most recent.  I'll push some more stuff
later tonight, I hope.

Reported by ejm.60657 on 2010-12-29 23:22:31

Beep6581 commented 8 years ago
A slight update has been pushed to Google code.

Reported by ejm.60657 on 2010-12-29 23:45:13

Beep6581 commented 8 years ago
BTW, my code has been added/modified in the Google code repo by usual command line 'hg
commit' followed by 'hg push' in a terminal; about as basic as it gets.  My local copy
should be identical to the one on Google code.  If other people report an issue I'll
start investigating; meantime you might try pulling a fresh copy of RT from Google
code and updating it to the float branch just to see if your issue is reproducible.

Reported by ejm.60657 on 2010-12-29 23:56:39

Beep6581 commented 8 years ago
Hmm, I looked at the flatfield and something is very weird; exposure slider does wacky
things.

Reported by ejm.60657 on 2010-12-30 00:06:45

Beep6581 commented 8 years ago
Just go the latest & compiled. Highligts are purple and flat unless darkened via -EC.

Reported by michaelezra000 on 2010-12-30 00:09:30

Beep6581 commented 8 years ago
About flatfield: joker updated the patch to be applied to changeset 766, did you apply
the FF patch to that changeset?

Reported by michaelezra000 on 2010-12-30 00:14:53

Beep6581 commented 8 years ago
float: this is is likely my mistake I had changeset 780 and yours latest is 763. Compiling
now!

Reported by michaelezra000 on 2010-12-30 00:26:20

Beep6581 commented 8 years ago
still purple and flat highlights...

S:\Projects\Eclipse\RT\rawtherapee_default_437float>hg sum
parent: 763:4d1efc6fd374
 Minor bugfixes and improvements.
branch: float
commit: 3 unknown (clean)
update: (current)

Reported by michaelezra000 on 2010-12-30 00:38:40

Beep6581 commented 8 years ago
Can you local copy still produce result similar to the latest screenshot, or could this
be related to changing double to float?

Reported by michaelezra000 on 2010-12-30 00:56:10

Beep6581 commented 8 years ago
My version produces the same output as the image I posted in comment 16.

Reported by ejm.60657 on 2010-12-30 01:15:42

Beep6581 commented 8 years ago
I updated to float from my basic default clone that I had not been doing any development
on for the float branch, and got the same output and not any of the artifacts you are
reporting.  Doesn't mean there is not a problem; OSX is much more fault tolerant than
other platforms.

Reported by ejm.60657 on 2010-12-30 02:17:58

Beep6581 commented 8 years ago
What image are you using?  Do I have it?

Reported by ejm.60657 on 2010-12-30 02:20:32

Beep6581 commented 8 years ago
It is the same image with the "blue" horse.
It would have been great if it could be tested on Windows by anyone else, just to confirm.

Reported by michaelezra000 on 2010-12-30 02:53:58

Beep6581 commented 8 years ago
Michael, I can give it a try (I'm on Win 7 x64. Perhaps I need some guidance, though
:-)). I have the float branch here, rev 781. Where can I find the horse image?
Regards,
Johan

Reported by johan.andersson@sodra.com on 2010-12-30 08:44:09

Beep6581 commented 8 years ago
Hi Johan, thanks for jumping in. you can get the NEF file from this link:
http://www.timelessme.com/temp/postings/2010_MONTR_033.zip

Reported by michaelezra000 on 2010-12-30 13:00:30

Beep6581 commented 8 years ago
I tried your (Michael's) NEF in RT float branch, used the default profile, set the highlight
reconstruction to 100 and turned on highlight recovery CIElab blending as in the screen
shots above (Emil's link). Everything else was as in the default profile. Here's the
result:
http://johanandersson.smugmug.com/Other/vs/14268006_48WLT#1141431552_cWF7w-A-LB

Then I switched to the default branch, rev 786 and using the same pp3 I got this:
http://johanandersson.smugmug.com/Other/vs/14268006_48WLT#1141431669_fPguK-A-LB

To my eye this is very similar to what Emil showed above with the white shirt turning
slightly to the yellow side by enabling the highlight reconstruction CIElab blending
(why, I don't know, just an observation).

Michael, should I test something special here? I'm really not sure if my testing above
helped in any way... Just tell me!

Regards,
Johan

Reported by johan.andersson@sodra.com on 2010-12-30 13:53:16

Beep6581 commented 8 years ago
Here is what I see:
http://www.timelessme.com/temp/postings/RT_issue437_01.jpg

Compiled after this:

S:\Projects\Eclipse\RT\rawtherapee_default_437float>hg sum
parent: 763:4d1efc6fd374
 Minor bugfixes and improvements.
branch: float
commit: 3 unknown (clean)
update: (current)

S:\Projects\Eclipse\RT\rawtherapee_default_437float>hg pull
pulling from https://rawtherapee.googlecode.com/hg
searching for changes
adding changesets
adding manifests
adding file changes
added 3 changesets with 4 changes to 3 files
(run 'hg update' to get a working copy)

S:\Projects\Eclipse\RT\rawtherapee_default_437float>hg update
resolving manifests
0 files updated, 0 files merged, 0 files removed, 0 files unresolved

Reported by michaelezra000 on 2010-12-30 14:23:35

Beep6581 commented 8 years ago
Michael, I noticed that your histogram and mine doesn't match. Do you
have any other parameter set other from the default? Perhaps you could
post your pp3-file? I attach mine here.

Reported by johan.andersson@sodra.com on 2010-12-30 14:33:19

Beep6581 commented 8 years ago
Ok, that comment (#38) didn't work out as planned. Here's the file:

Reported by johan.andersson@sodra.com on 2010-12-30 14:34:31


Beep6581 commented 8 years ago
Yes Johan this is very helpful in keeping me from chasing red herrings (excuse the idiomatic
English).  Also, it would be helpful if people can do their normal image processing
to help discover any bugs in what I've done so far, and comparing output with the default
branch parent changeset 763:4d1efc6fd374 to look for differences. So far there should
be few except in highlights and near blacks, since the only major effect of the float
conversion so far on images should be that clipping of image data only occurs at the
very end upon conversion to output, rather than at many steps along the way to keep
the data within short integer bounds 0-65535.  I believe this is the source of the
difference in the white shirt in the horse image; as to why it is slightly yellow,
that may be some combination of white balance and/or the inner workings of the highlight
reconstruction algorithm.

Reported by ejm.60657 on 2010-12-30 15:02:22

Beep6581 commented 8 years ago
Johan, I tried your pp3 file and nothing changed in my display.
However, there is a strange thing happening:
I just got a fresh clone from float branch using hg from command line.
Here s the summary:
S:\Projects\Eclipse\RT\rt_float>hg sum
parent: 762:4d1efc6fd374
 Minor bugfixes and improvements.
branch: float
commit: 1 unknown (clean)
update: (current)

there is a difference in changeset numbering - 762 here vs 763 that I have in another
clone:
parent: 762:4d1efc6fd374

S:\Projects\Eclipse\RT\rawtherapee_default_437float>hg sum
parent: 763:4d1efc6fd374

Compiling now...

Reported by michaelezra000 on 2010-12-30 15:15:17

Beep6581 commented 8 years ago
Result is unfortunately the same with the new compile.
One specific thing I noticed that may be helpful in debugging is that EC, brightness
and Lab brightness sliders cannot increase L value of any pixel to a value higher than
75.

Reported by michaelezra000 on 2010-12-30 15:27:50

Beep6581 commented 8 years ago
Emil: first of all: thanks for your efforts. Second - you taught me not only some new
aspects regarding RT, but also some of the English language...!

Michael: hope the re-compilation solves your issue... If not, tell me if there's anyting
I can do/test here.

Regards,
Johan

Reported by johan.andersson@sodra.com on 2010-12-30 15:30:31

Beep6581 commented 8 years ago
Johan, could you please run "hg sum" in the directory with the float branch code and
paste the outcome here?

Reported by michaelezra000 on 2010-12-30 15:46:07

Beep6581 commented 8 years ago
Here's what I get: 
parent: 781:4d1efc6fd374
 Minor bugfixes and improvements.
branch: float
commit: 181 unknown (clean)
update: (current)

Regards,
Johan

Reported by johan.andersson@sodra.com on 2010-12-30 16:17:35

Beep6581 commented 8 years ago
Emil, could you please share your hg summary for this branch?

Reported by michaelezra000 on 2010-12-30 16:28:09

Beep6581 commented 8 years ago
Michael, that will have to wait for tonight when I'm at my dev machine.  But the version
on Google code is the same as the one on my dev machine; as I mentioned in comment
31, a fresh pull of the source from Google produced the same image as I posted.  Clearly
there is something in your setup that is different and by this point it is not likely
that we are compiling different code.

Reported by ejm.60657 on 2010-12-30 16:34:36

Beep6581 commented 8 years ago
emil-martinecs-computer:float ejm$ hg summary
parent: 747:4d1efc6fd374 tip
 Minor bugfixes and improvements.
branch: float
commit: 5 modified, 257 unknown
update: (current)

Please note that the changeset # is out of sync with Google, but the unmemorable char
string is the same as what you are working from.  I attach the patch to update to the
latest, which reverts the buggy version of pyramid NR from  issue 405 .  Apologies
for those who grabbed it, the first patch I uploaded was buggy.  This is better.

Reported by ejm.60657 on 2010-12-31 00:29:45


Beep6581 commented 8 years ago
Emil, thank you for the patch. I am traveling with family for the New Year celebration
and will be able to re-test on 1-st or 2nd of January.

Reported by michaelezra000 on 2010-12-31 14:24:23

Beep6581 commented 8 years ago
Unfortunately, my attempts to get fresh code and recompile did not alleviate the problem
on my machine yet.

Reported by michaelezra000 on 2011-01-04 14:50:14

Beep6581 commented 8 years ago
Perhaps there is something flaky going on with Windows.  Jacques reported that he can't
even get the code to launch after compiling in some versions, yet I and Jan (and apparently,
Johan) have no such issues.

Reported by ejm.60657 on 2011-01-04 15:06:49