search

Beep6581 / RawTherapee

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

Multiple control points for "locallab branch" #3421

Closed Desmis closed 5 years ago

Desmis commented 7 years ago

Hello all After 2 mounth whithout RawtherapeeI, I updated "locallab" branch with a modification allowing several "Control points". I put 5 control points, but it's easy to add more or less. The proposed system is far from perfect, there are still many issues to be resolved. But despite all the bugs, I prefer to distribute this change: 1) for opinions of principle ... is that we continue to do this way or we abandon ? 2) to get help from other developers, especially to handle events, and others optimizations or problems.

Now if you are very tolerant, and if you follow a rigorous process, it is possible to control 5 Points, for "Color and light", "Blur and Noise" and "Retinex"..in 2 modes : Preview and TIF/JPG

However I was not able to properly run the events, and I installed a work around.

To select control point, I added a slider with 5 positions, but due to my bad events management, I was forced to add another control that I called "Help to move control point". When you want to add (or move) a control point, you must manually change "help to move control point" (0 to 1, or 1 to 0... a check point does not work), just after change control point.

It works, but with bugs...I have tried many solutions for events :

Sometimes the system crash...

Monitoring "control points" is provided by a file * .mip (text file). eg for a file _ASC4029.NEF, a file _ASC4029.NEF.mip is create near the NEF file. (to prevent crash (??) or bad results, I always delete manualy "mip" file...and select "neutral" (profiles), for other testings!!)

for your advice and help.

Thank you

Jacques

Desmis commented 7 years ago

I am the only one to crash??

I know this problem ... but I think Perhaps it is due to the same problem as crash (bad events, bad refresh) ?? I think it should be possible to achieve local control modules for many but not for all, or at least without a rewrite RT (pipeline, raw process ...) : eg white balance More, local algorithms are often different overall because they do not take into account the same thing: exposure, wavelet, luminance,.... In a majority of cases, I had to "rewrite" the algorithms to account for the effect size

Desmis commented 7 years ago

I think I have found a way to solve many bugs... it remains no more than one, but important :)

Desmis commented 7 years ago

I just push a change.

I have fixed some important bugs.

Now everything seems to work correctly (of course to verify) except for using slider "chrominance" (Color and light) which in some cases leads to crash. I will try to fixed it !

After many trials, I modified the code by using LUT to store the values of the spots, then calling in Dcrop.cc by parent->xxx. Currently you can used till 500 spots, but it is easy to have more???

I no longer use Mutex.

You must delete all *.mip files, and probabaly delete cache. If you want to use 6 spot, you must at the end add one more with nothing.

Recall : algorithm are optimized if Scope < 20, and Hue scope < 20 for retinex :)

Desmis commented 7 years ago

I pushed a change I found an error in detection algorithm for chroma.... I corrected somme errors in code and some bugs :)

Desmis commented 7 years ago

I pushed a new change.

I think (I hope) I fixed the bug that brought a crash in some cases with the slider "chrominance". In fact it is a gamut problem, which then brought some corruption in "Lab adjustements". I solved this problem by enabling "avoid color shift"

I solved also an other bug...

Now I think this patch works properly, of course to check.

The system works without Mutex, without layers, without masks and the number of control points is almost unlimited.

The detection algorithm seems to work as (not for all ?) U-points (CN2) or Nik software

Of course, if Hombre provides an interface to better manage the GUI, for example with "slider at each control point", it would be great.

It must also be possible to add features such as sharpening, or perhaps CBDL, and also probably optimized this code.

:)

Desmis commented 7 years ago

I made some changes to improve performance and stability.

I suppress "delay" for changing spot (nbspot, anbspot), but you can restore the delay as before by changing the variable "Locdelay = true" in the file options

More in rare cases, for images with colors close to the gamut of Prophoto, and if one acts strongly on the "chrominance" sliders, it is possible to get a crash. I have changed the value of "higherCoef" in Gamut control, from 0.96f to 0.92f

Now, I'll begin work on "sharpening" : delay ???

:)

Desmis commented 7 years ago

I just made a change with : The possibility to make the sharpening locally as many times as necessary. I used "RL deconvolution" *You can select, "normal" or "inverse", and detecting shape / color is operational. For example you can sharpen foliage without touching the sky, or skin without others colors,...

I found and corrected many (big or little) bugs. Now with my images tests, I no longer crash...but to test on others images;

I set the number of spot by default to 8, which should be sufficient in most cases. :)

Desmis commented 7 years ago

Obviously, you must clean the cache and delete all *.mip files

Desmis commented 7 years ago

I push a new change

1) I did a complete review of the algorithms by testing various parameters and thresholds - Luminance algorithm was very bugy (since a long time..more than one year) Now the algo is a little less discriminating, but without artefacts. There remains some progress to be made in areas where the color variations are very local, for example gravel soils - I don't know if it is faisable ??

2) I add the possibility to vary the size of the spot - this spot contains the references for hue, chroma, and luma:

Obviously you must clean cache and delete all *.mip files. :)

Desmis commented 7 years ago

I push a new change You must clean cache and delete all *.mip files

I have improved the shape detection algorithm, which in some cases (hue detection for very local variations) brought artifacts - I had previously disabled this function.

You can select "Standard" or "Enhanced"

In standard mode, it is as before. In Enhanced mode, the selected area, is scanned to find for each point, the hue variation (between 0 and PI). You have 2 sliders :

Obviously I change all parameters (*.mip)

In some cases this algorithm improve shape detection for :

But only if the distribution (gradient variation) is heterogeneous.

I will be away from Friday 18th, until December 6th :)

Desmis commented 7 years ago

I am working on "Local Denoise" and it work :) But by doing this I could see the same crash as before with the noisy images. I have located where the error comes from, and finally find a solution.

I will propose a change in the days that will follow my return from holidays on December 6 or 8. I'm leaving today; jacques

kazah7 commented 7 years ago

Desmis! :) It's going forward, so cool. I know it's a long way until we get it 'fully' functional, but your work is crucial here :) Thanks!

Beep6581 commented 7 years ago

Enjoy your holidays Jacques!

Desmis commented 7 years ago

Thank you kazah7 and Beep6581 :)

I have just returned from a winter break in "la Réunion". As expected I post a modification concerned "Local denoise". As for "Local sharp" I realized an optimization which considerably reduces the processing times when exiting in TIF / JPEG.

I also found other bugs and thought to have treated them ??

I think we can also optimized others local threatment as Color & Light and Retinex (next days ?) I think now the branch locallab works correctly.

jacques

Desmis commented 7 years ago

I have just done 2 optimizations that reduce processing times by at least 50%.

Of course it should be possible to further improve the speed, but it is more complicated for me (SSE, OMP, optimization of the code ...).

Depending on the size of the zones, their number, and the processes involved (the longest is denoise), processing times range from a few tenths of a second to a few seconds. I think I can make other improvements but it will be for later. It is time to take a break to evaluate this branch.... :)

heckflosse commented 7 years ago

@Desmis Jacques, welcome back :)

Of course it should be possible to further improve the speed, but it is more complicated for me (SSE, OMP, optimization of the code ...).

I will take a look later. Currently I'm working on pixelshift together with Ilias. Jacques, did you get the invitation from Ilias on pixls.us?

Ingo

Desmis commented 7 years ago

@heckflosse Hello Ingo

Yes I have seen...but I was busy with locallab. Now I'll have a look to pixelshift :) jacques

heckflosse commented 7 years ago

@Desmis

Now I'll have a look to pixelshift :)

That will be a long ride reading the complete thread and downloading all the large pixelshift raw files ;-)

heckflosse commented 7 years ago

@Desmis Jacques, I wanted to try locallab branch. But as soon as I enable Locallab, it crashes. To reproduce, open amsterdam.pef, enable locallab => crash

Edit. I forgot to delete the .mip files. Now it works. Sorry for confusion

Desmis commented 7 years ago

@heckflosse Currently, it is essential with each new version of: 1) delete .mip 2) clean the cache I think, but that goes beyond my capabilities (related to deep knowledge of pp3 files) that it would be nice to incorporate .mip files to pp3 ... How? This would in theory make updates less austere.

During the tests I had a lot, a lot, a lot,... of crash ... due to the presence of noise (even very light) especially when activating a zoom at 100%. This has disappeared, but to be confirmed. There were also crashes when many "color pickers". Since Hombre modification, no problem :)

Desmis commented 7 years ago

I am working on local CBDL :)

Beep6581 commented 7 years ago

All locallab issues:

3546

3421

3351

3292

2538

Beep6581 commented 7 years ago

I found these bugs using commit e8061bb8d125597cfb0bf28d28464c3d19912622:

Enhancement requests:

Desmis commented 7 years ago

@Beep6581 @heckflosse @Floessie @Hombre57 As I have already said

Others points seems solved .

Could you help me :) Thank you.

Jacques

heckflosse commented 7 years ago

@Desmis I will take a look at the history messages now.

heckflosse commented 7 years ago

@Desmis Jacques, can you try this patch for the curves history messages please?

diff --git a/rtgui/locallab.cc b/rtgui/locallab.cc
index f0f394e..ea92313 100644
--- a/rtgui/locallab.cc
+++ b/rtgui/locallab.cc
@@ -2001,52 +2001,16 @@ void Locallab::curveChanged (CurveEditor* ce)
     if (listener && getEnabled()) {
         if (ce == cTgainshape) {
             listener->panelChanged (EvlocallabCTgainCurve, M ("HISTORY_CUSTOMCURVE"));
-            int strval = retrab->getValue();
-            //update MIP
-            retrab->setValue (strval + 1);
-            adjusterChanged (retrab, strval + 1);
-            usleep (10000); //to test
-            retrab->setValue (strval);
-
-            adjusterChanged (retrab, strval);
         }

         else if (ce == cTgainshaperab) {
             listener->panelChanged (EvlocallabCTgainCurverab, M ("HISTORY_CUSTOMCURVE"));
         } else if (ce == LHshape) {
             listener->panelChanged (EvlocallabLHshape, M ("HISTORY_CUSTOMCURVE"));
-            int strval = retrab->getValue();
-            //update MIP
-            retrab->setValue (strval + 1);
-            adjusterChanged (retrab, strval + 1);
-            usleep (10000); //to test
-            retrab->setValue (strval);
-
-            adjusterChanged (retrab, strval);
-
-
         } else if (ce == llshape) {
             listener->panelChanged (Evlocallabllshape, M ("HISTORY_CUSTOMCURVE"));
-            int strval = retrab->getValue();
-            //update MIP
-            retrab->setValue (strval + 1);
-            adjusterChanged (retrab, strval + 1);
-            usleep (10000); //to test
-            retrab->setValue (strval);
-
-            adjusterChanged (retrab, strval);
-
         } else if (ce == ccshape) {
             listener->panelChanged (Evlocallabccshape, M ("HISTORY_CUSTOMCURVE"));
-            int strval = retrab->getValue();
-            //update MIP
-            retrab->setValue (strval + 1);
-            adjusterChanged (retrab, strval + 1);
-            usleep (10000); //to test
-            retrab->setValue (strval);
-
-            adjusterChanged (retrab, strval);
-
         }

     }

Ingo

heckflosse commented 7 years ago

@Desmis Jacques This patch is for mip files

diff --git a/rtengine/improccoordinator.cc b/rtengine/improccoordinator.cc
index 4834208..bd3df74 100644
--- a/rtengine/improccoordinator.cc
+++ b/rtengine/improccoordinator.cc
@@ -739,7 +739,7 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall)
             }

             //  printf("mip file=%s \n", datalab.c_str());
-            Glib::ustring pop = options.getUserProfilePath() + "/";
+            Glib::ustring pop = options.cacheBaseDir + "/profiles/";
             Glib::ustring datal;
Beep6581 commented 7 years ago
Beep6581 commented 7 years ago

The attached patch fixes point 3 - LL doesn't work. ll_config2cache.patch.txt

Desmis commented 7 years ago

@heckflosse @Beep6581

For the first patch, it certainly reduces the number of message history, but this will disrupt the system refresh ... I added 3 functions of this type (which a priori do not serve anything else to trigger additional events), After many attempts to dynamically update the MIP files I was thinking instead of a solution where we modify in the GUI, the panelChanged function to avoid displaying a message

The second patch works well. But you also have to make the same change in dcrop.cc and simpleprocess.cc. Nevertheless Beep6581 wishes in addition to have "hash" ... where to find it, how to do? In addition it will be necessary to update cachemanager

heckflosse commented 7 years ago

@Desmis Why do you need to trigger additional events which in the end all trigger the same 'LUMINANCECURVE' in refreshmap.cc?

Desmis commented 7 years ago

If you don't trigger additionnal events, the system does not work :)

jacques

Desmis commented 7 years ago

I explain that in Rawpedia http://rawpedia.rawtherapee.com/Local_Lab_controls/fr#Quelques_fantaisies_incontournables

heckflosse commented 7 years ago

@Desmis I will read the code at the weekend to understand and maybe suggest a simpler solution afterwards.

Desmis commented 7 years ago

Ingo No problem, but the refesh dynamic is indispensable, If you can develop a less "fanciful" system, so much the better :)

Beep6581 commented 7 years ago

@Desmis I just merged locally dev into locallabgtk3, compiles and runs fine. Most importantly, I eliminated formatting differences in dcrop.cc between the two branches, so future merges regarding dcrop.cc will be easier. Would you like me to push?

Desmis commented 7 years ago

No problem, for me you can push :)

2017-02-11 21:28 GMT+01:00 Beep6581 notifications@github.com:

@Desmis https://github.com/Desmis I just merged locally dev into locallabgtk3, compiles and runs fine. Most importantly, I eliminated formatting differences in dcrop.cc between the two branches, so future merges regarding dcrop.cc will be easier. Would you like me to push?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3421#issuecomment-279173423, or mute the thread https://github.com/notifications/unsubscribe-auth/ANIIWVDc51sEqvwbVZZiN-QLvuKn56qhks5rbhnzgaJpZM4J4sx5 .

-- Jacques Desmis 407 rue Gustave Bret - Le Lagon Bleu 83600 Fréjus tel : 0483125966 http://jacques.desmis.perso.neuf.fr/

Desmis commented 7 years ago

@heckflosse I have found a very simple way to suppress all "bad" message in history. Can I push this change, after @Beep6581 push ?

Jacques

heckflosse commented 7 years ago

@Desmis yes, please :+1:

Beep6581 commented 7 years ago

@Desmis pushed.

Desmis commented 7 years ago

@heckflosse @Beep6581 I push a change that suppress unnecessary messages in history :)

Desmis commented 7 years ago

By trying locallabgt3, I found a bug that either resulted in a crash or a program exit. I searched for why and put together the commits. And I found an error in "merge wih dev 6e6761f": a variable required by localabl, is not initialized. To solve this problem and leave on a healthy basis, I recreated a new branch "locallab_dev". I updated the location of mip files - in the cache, in a mip folder. I also deleted unnecessary messages in history

Desmis commented 7 years ago

I just made the last changes:

Of course, I have other projects in progress, some may be impossible to achieve:

If nobody opposes it, I will proceed to this "merge" on Saturday, March 25th.

jacques

heckflosse commented 7 years ago

@Desmis

Hello Jacques,

I want to merge psgtk3 branch (pixelshift on gtk3) into dev this weekend. Because pixelshift as well as locallab add a lot of new gui controls, the one who merges later has more work to correct the merge conflicts. As I want to merge this weekend and you want to merge one week later, I offer to do this work for you (solving the merge conflicts when merging locallab into dev after I merged psgtk3 into dev). Would that be ok?

I will test locallab_dev tomorrow :)

Ingo

Desmis commented 7 years ago

Hello Ingo

No problem, you can merge your very good pixelshift, and I accept your offer to merge locallab for me after :)

Thank you

Jacques

2017-03-18 2:03 GMT+01:00 Ingo Weyrich notifications@github.com:

@Desmis https://github.com/Desmis

Hello Jacques,

I want to merge psgtk3 branch (pixelshift on gtk3) into dev this weekend. Because pixelshift as well as locallab add a lot of new gui controls, the one who merges later has more work to correct the merge conflicts. As I want to merge this weekend and you want to merge one week later, I offer to do this work for you (merging locallab into dev after I merged psgtk3 into dev). Would that be ok?

I will test locallab_dev tomorrow :)

Ingo

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Beep6581/RawTherapee/issues/3421#issuecomment-287505094, or mute the thread https://github.com/notifications/unsubscribe-auth/ANIIWTluECt-udyvaFT0Lrm1rzcVAcaAks5rmy1igaJpZM4J4sx5 .

-- Jacques Desmis 407 rue Gustave Bret - Le Lagon Bleu 83600 Fréjus tel : 0483125966 http://jacques.desmis.perso.neuf.fr/

Hombre57 commented 7 years ago

@heckflosse @Beep6581 @Floessie @agriggio Still 5 days before the merge. I've already expressed myself on this so I won't do it again, I'll let you do.

Hombre57 commented 7 years ago

@Beep6581 Do you still want to release RT5.2 ? Do you want this new tool be part of it ?

heckflosse commented 7 years ago

Shouldn't be 5.1 released before 5.2?

heckflosse commented 7 years ago

@Desmis

Jacques,

I will merge current dev into your branch and this way solve the merge conflicts caused by merging pixelshift, ok?

@Hombre57

I don't want (and can not elaborate) about current state of locallab branch because I didn't test it really (I will do that next days). But I assured Jacques that I will solve the merge conflicts and I will do that. That does not mean that I will merge locallab into dev (it's not up to me to decide that), but I will merge dev into locallab and solve the merge conflicts!

Desmis commented 7 years ago

No problem, you can solve all the merge conflicts :)