Beep6581 / RawTherapee

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

Retinex New process for gain and offset #3146

Open Desmis opened 8 years ago

Desmis commented 8 years ago

I just push a new branch : "retinexgain"

@heckflosse

Can you have a look :)

Thank you

Jacques

heckflosse commented 8 years ago

@Desmis yes :)

heckflosse commented 8 years ago

@Desmis Jacques, I had a look at the source and found no Issues. I didn't test it yet, but it will get enough tests during my review and code cleanup. I'll continue the review and cleanup as soon as you pushed the modifications :)

Ingo

Desmis commented 8 years ago

@heckflosse There is now, 2 branch for Retinex (retinexgain and retinex_fix) ! Is that keeps the two, in which case why not "clean and fix bug" with only one branch? Or do you have to work separately! No problem for me, when I create new branch, I thought you would delete "retinex_fix" (I don't verify!!)

jacques

heckflosse commented 8 years ago

@Desmis Jacques, I forgot to remove retinex_fix branch :)

Desmis commented 8 years ago

@heckflosse Ingo I'll push soon a change (almost GUI) and others improvments :)

jacques

Desmis commented 8 years ago

I just push a change with a) improvment for GUI (I hope it's an improvment), to try to be more explicit b) re-introduce "scale" (I had suppressed some mounth ago)..it's useful for haze images

Jacques

iliasg commented 8 years ago

.. indeed "scale" was useful One change I would like is at the default transmission curve. As it is now (with the left side below zero), I usually take black clipping at the shadows and I need to rise the left at zero to be fine.

Desmis commented 8 years ago

@iliasg it depends of each image... it's very easy to do what you want...

for the record, the left part of the curve is on the foreground on the right background

I'll make that change quickly But as I say, you can not have your cake and eat it too :)

Desmis commented 8 years ago

@iliasg

done :)

iliasg commented 8 years ago

Ohh thanks .. now I have to learn how to compile in the new environment .. then test ..

heckflosse commented 8 years ago

@Desmis Jacques, can I include the changes I mentioned here or should I wait?

Desmis commented 8 years ago

@heckflosse

Ingo

As says someone : "yes you can" :)

heckflosse commented 8 years ago

@Desmis Jacques, I pushed. You can continue :)

Desmis commented 8 years ago

@heckflosse Ingo I'll test..tomorrow :) jacques

heckflosse commented 8 years ago

:)

Desmis commented 8 years ago

@heckflosse

Ingo Finally I test it tonight, singing Connemara...(I don't drink Guinness the evening!)

No differences in output, even with CS layers differences Time reduction about 80 to 100ms for a D5000 NEF

Good job :)

heckflosse commented 8 years ago

@Desmis Jacques, thanks for testing and enjoy the Connemara :+1:

Desmis commented 8 years ago

I push a change

Desmis commented 8 years ago

I just merge master in retinexgain :)

@heckflosse Ingo, you planned late February to improve "Retinex gain" and merge with master in march !

what do I do ? Jacques

heckflosse commented 8 years ago

@Desmis Jacques, it got completely out of my focus, sorry :( I'll try to do that next week. Ingo

Desmis commented 8 years ago

@heckflosse

Ingo, no problem :)

heckflosse commented 8 years ago

@Jacques, I just made a break at my rgbcurvesspeedup branch and will take a look at optimizations to your branch now :)

Desmis commented 8 years ago

@heckflosse Ingo, thank you very much :) Jacques

heckflosse commented 8 years ago

@Desmis Jacques, can you have a look at this line please? Currently the (int) affects the value of 2.8 which then truncates it to 2.0. Is that intended? Ingo

Desmis commented 8 years ago

@heckflosse

Ingo

You are true... as always... We must replace int nei = (int) 2.8f * deh.neigh; by int nei = (int) (2.8f * deh.neigh);

Thank you for this work :)

heckflosse commented 8 years ago

@Desmis Jacques, I pushed some minor speedups and some cleanups to your branch. I also made some annotations (search for TODO in the diff) in code for further improvements. But these (small) improvements require your branch and my 'rgbcurvespeedup' branch to be be merged into master. So :+1: to make a pull request for your branch to get it into master branch. As soon as my 'rgbcurvespeedup' branch is merged into master, I'll make the retinex TODO changes.

Ingo

Desmis commented 8 years ago

@heckflosse

Ingo, no problem at all :)

I do not understand when and who will merge "retinexgain" with master...Is it you in some days...or me now after testing ?

I also will try your changes in "rgbcurvesspeedup"

Jacques

heckflosse commented 8 years ago

@Desmis Jacques, I can merge retinexgain into master if you like

Ingo

Desmis commented 8 years ago

@heckflosse

Ingo OK, you can :) Thank you again

heckflosse commented 8 years ago

@Desmis Jacques, I just merged it into master

Desmis commented 8 years ago

@heckflosse

Thank you :)

Beep6581 commented 8 years ago

@Desmis @heckflosse I didn't get much opportunity to use RawTherapee in the last few months, but I had a good go today, and wow! Retinex was stable, fast and lead to very good results!

scrot_2016-05-28 215842