Rawtherapee: Multiple control points for "locallab branch"

[Desmis]

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 :

• in improccoordinator.cc just near call to "aloListener->localChanged(dataspot, sp);" (line 903) (todo |= M_LUMACURVE)
• in locallab.cc, localcomputed_ with adjusterChanged() and others changes,
  but all is bad..sometimes it seems to work...but no!

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

[heckflosse]

@Desmis

Hello Jacques,

I just compiled your branch but can not find the new controls. Can you please post a screenshot?

[Desmis]

@heckflosse

Hello Ingo

Here a screenshot.
http://jacques.desmis.perso.neuf.fr/RT/multi.jpg

You can see just above "shape", the 2 sliders "Control points and "Help to Move Control point"

Jacques

[heckflosse]

@Desmis

Hello Jacques,

I must have been completely blind or confused. Now, after building again, I see it. Thank you!

Ingo

[Desmis]

@heckflosse

Hello Ingo
I think, I have found a way to solve (perhaps partially) "events"...Can I push a change, or wait ?
Thank you

Jacques

[heckflosse]

Push :)

[Desmis]

@heckflosse

Ingo and all

I think I have partially solved the problem of managing events (I just push a change to locallab).
Nevertheless not everything works properly, including "inverse" function.

Of course there are still issues to resolve, such as the optimization, but now I think we should be able to test properly
:)

[sguyader]

@Desmis
I just tried your latest changes and it looks very promising. In my quick test I was able to work on 3 separate areas, and it is easy to jump from one to another with the slider.
I only noticed that when using "noise" in one area, modifying things in another area was changing slightly the preview of the "noise" area (I guess it makes RT recalculate the random component of the noise distribution, which in fact is not really a problem).
Great job Jacques!

[Desmis]

@sguyader
Sébastien

Thank you for this quick test :)

[Desmis]

I add controls for events :

• for inverse (color light, blurr, retinex);
• for the 2 methods : cursor and retinex

I hope no error...to test :)

if we admit that it works correctly, you can ask the following questions:
1) is there enough control points (actually 5). It is easy to fix another value, 3, 6, 10,...or make configurable via option ?
2) with the same general process, then there must be other types of control points : noise, sharpening, spot-removal, ...? (for this last point "spot removal", it needs a file with spot to elaborate algorithm)

Jacques

[Floessie]

@Desmis Bonjour Jacques,

is there enough control points (actually 5). It is easy to fix another value, 3, 6, 10,...or make configurable via option ?

I didn't have a look at the code let alone the whole feature, so please bear with me if this is a daft question: Why does the number of control points have to be fixed? I could image -- from a user's perspective -- that adding a CP when you need one would be nice. Sorry, if that idea is stupid for some obvious reason...

Best
Flössie

[Desmis]

@Floessie
Excuse my very bad english :)
In fact, in an ideal world, it would be that the user does not have to make that choice.

In this case, I was led to make choices based on a hypothetical algorithm without being sure that will go run.
Now it's run...

Probably we should be able, from the basic algorithm I imagined, find other solutions. I am open to any concrete proposal
:)

[Floessie]

@Desmis

Excuse my very bad english :)

I don't have to: You write quite understandably. :)

Probably we should be able, from the basic algorithm I imagined, find other solutions. I am open to any concrete proposal

From the code I see, that CP 0 is special, as it is the base for the other 4 CPs. Apart from that, they seem equal.

Could it be, that the main problem for not having a dynamic number of CPs is the way they are passed between functions as int**? If so, storing them in a map of maps (e.g. std::map<Item, std::map<Subitem, int>>) could help (and would also make it more readable). Best would be a class holding CPs and a class modelling those CPs. But again, I'm not really familiar with the code. So, sorry if this is rubbish.

Best
Flössie

[Desmis]

@Floessie

Hello
My english is bad, but also I am a bad computer!

For me no problem, if you think, it is better to write the code in another way : dynamic CP, mutex, etc.
You can change without problem :)

thanks in advance!

Jacques

[Floessie]

@Desmis

My english is bad, but also I am a bad computer!

:joy: Biggest understatement of the year. I mean: Who is the innovator here?

You can change without problem

I will, as time permits. Where can I find the names or meanings of the control points' attributes? (Edit: It's in the params.locallab.) I could at least help you with the storage classes and the mutex problem, but no so much with the GUI.

HTH
Flössie

[Desmis]

@Floessie

I'll try tomorrow, to explain how control points'attributes works :)

Thank you again
Jacques

[Desmis]

@Floessie
I see your edit...
I describe summarily the principle :

Principle of the algorithm

I don't use layers, or layers mask...
Each control point has is own controls : transition, scope, etc.
I think we can easily use the same principle for others controls (noise, spot removal, etc.)
I think also we can combined algorithms from several control points...but not now :)

2 files are principally used :

• improccoordinator.cc and locallab.cc

2 files are also used but read only and directly derived from improccoordinator.cc

• dcrop.cc and simpleprocess.cc

Iplocallab.cc is slightly modified, I replace the luminosity curve, by a calculation

2 parameters are not differenciated and are the same for all Control points:

• transmission gain : 1) I don't know how to save parameters in a file; 2) I think it is not important because, Transmission gain is often a caracteristic of an entire image
• avoid color shift

In improccoordinator.cc and locallab.cc
Creation of a mip file, near the file to be treated
If this file does not exist, create with defaut parameters
Creation of an array dataspot with 2 dimensions : 26, number of spot + 1:
Initialize dataspot[x][0] with params.locallab values
Read the mip file and initialize dataspot[x][sp] with new values and for each spot (sp)(except 0)

For each spot sp, except the current spot:

• attribute params.locallab with dataspot[x][sp]
• call lab_local

For current spot :

• call "aloListener->localChanged(dataspot, sp)", which call some functions in locallab.cc (GUI). It is a process create some times ago by Hombre (thanks to him)
• this function update preview GUI
• I searched a long time how to operate with various tests and finaly, I add in Locallab::localComputed_ (), a call as :

listener->panelChanged (Evlocallabanbspot, anbspot->getTextValue());

etc.
*I think perhaps we can simplify ?? this call ??
Then, attribute to dataspot[x][sp], params.locallab, and dataspot[x][0]

• call local_lab

Then save the mip file.

In locallab.cc in addition to what I just described, I add a slider to change and select the current "control spot"
We can certainly change that, for a dynamic system

I think we must optimized the code, I repeat, I am a scientist and an average computer...some functions are chinese for me

Jacques

[Floessie]

@Desmis I've just started to fix the "fake" mutexes. But I have one question: What are they supposed to protect against? Especially the one here. Should that be the same as this one?

Are they necessary at all in the light of cropMutex and mProcessing?

Best
Flössie

[Desmis]

@Floessie

The current code is the result of numerous trial and error ... and I'm not sure that everything is useful because I brought in the development of many changes.
That is why I asked the help of another developer to do a code review for its optimization.
In the case of "Mutex" I think I have come to add to manage files * .mip because during the development I had malfunctions when reading and writing, but can be is it more useful?

Also do you are working on the dynamic management of the number of spots (essentially a GUI issue) or do you prefer that I examine the feasibility?
jacques

[Floessie]

@Desmis I see. So the (single) mutex must protect the file accesses. No problem.

Also do you are working on the dynamic management of the number of spots (essentially a GUI issue) or do you prefer that I examine the feasibility?

Well, I've just started, but my plan is to support you with a class managing the *.mip files and a class representing the control points (which can then be passed around in a std::vector<> with dynamic size). I'll leave the GUI up to you. Sorry. :)

[Desmis]

@Floessie

Thank you :)
I'll work on GUI after your modifications.

Jacques

[sguyader]

@Desmis Jacques, some RT users would love to have a new Local Lab binary. The link to the one I posted in june has expired.
Would you agree if I make a new binary for Windows 64 bits, and post it as an experimental build, on the RT forum?

[Desmis]

Sébastien
No problem for me.
But it is an very experimental version.
Thank you :)
Jacques

[Desmis]

Sébastien
No problem for me.
But it is an very experimental version.
Thank you :)
Jacques

[Hombre57]

Hello Jacques,

I'm following this issue and branch from the back seat, seeing the progress and reactions from the other devs and users.

I still think that we should create a mechanism to enable general purpose local editing, and that's the reason why I returned. The soft-proofing branch and color picker branch has interfered in my agenda and will of doing that mechanism, and IMHO creating it before making the Gtk3 branch the main one would be prematurate, since it will be very invasive regarding the GUI.

Regarding the Spot removal feature, a stalled branch already exist for that and I need someone for the algorithm itself. I know that I could copy/paste it from other FOSS project, but I'm quite bad at deciphering a project's code and isolating the useful part. I could revive this branch (the GUI is mostly done) if someone is keen enough to help me on this.

So since you ask us about your branch, I'd suggest to put your branch aside and unit our force on this Big feature (again, let's make Gtk3 the new master first).

Cordialement,

---

Jean-Christophe

[Hombre57]

Btw, making a very invasive patch like Local Editing would require some kind of "mutex" developing mode, i.e. it will touch all the part of the code and other opened patch or branches would suffer from that. I'd recommend to put master completely on hold before the LocalRT can be merged to master. That's a collective decision and I wouldn't start this feature if I had to keep synchronized with an ever changing master and strongly changing LocalRT branch. I think Jacques knows what I mean.

[Desmis]

Hombre

No problem to help you for "Spot removal tool algorithm" according to my skills :)

jacques

[Floessie]

I've only invested 1.5h into the dynamic CPs, so no problem, if we don't pursue this branch as it is, but unite to get the feature mainline.

[Hombre57]

I'll merge master into the spot-removal branch and we'll try to finish it first then, and continue discussion in issue #2239.

[Desmis]

@Floessie
1) I would like to merge locallab with master
2) and more I will bring an improvement in the shape detection algorithm for each control point. This change only concern iplocallab.cc (and improcfun.h)

Can I push this change or must I wait ?
Thank you
Jacques

[Floessie]

@Desmis

1) I would like to merge locallab with master

I was referring to Hombres suggestion to develop a general purpose local editing and ditch this branch entirely.

Can I push this change or must I wait ?

Absolutely! Don't wait for me with anything. I can't contribute that often, and I have to help Ingo with some cleanups first before the RT5 release. Even if there are conflicts, I'm able to merge.

Best
Flössie

[Desmis]

Done :)

[Desmis]

I remember that to achieve multiple controls points (for luma, contrast, chroma, structure,...), there are several problems.

What emerges from the GUI with several types of solutions:
a) as currently implemented, which may seem a bit "primary", not optimized computing terms, but a priori that works
b) alternative(s) as proposed Hombre, which will probably be more intuitive, and more efficient in computing terms...

Algorithms to control local editings:
1) with layers , masks, etc. (as Photoshop...)
2) without layers (at least for the user), as U-point (control points), as Nikon CNX2 or Nik Software
The solution I propose (with Ingo optimization), is clearly 2) and can be used whatever the GUI
:)
Jacques

[michaelezra]

For best usability of local adjustments, it is best to try to design them in such a way that they could be applied in numerous ways:

1. multiple (unlimited) instances of adjustments should be possible
2. as a brush
3. as a gradient - linear and ellipse
4. as U-point

2, 3, 4 don't need to be implemented all at once, but it is worth designing this so that all methods are achievable.
I use local adjustments with camera raw (extremely) heavily within Photoshop. On average, about 10-30 per image. ACR's implementation of this feature is excellent, also with the range of tools that are part of each adjustment. This is a worthwhile reference to keep in mind:)
Sorry that I've been out of the game for so long, hope this is useful.

[Hombre57]

That's not the right issue to talk about it, but here is what I have in mind for local editing :

1. Add a double arrow button like the one of the Gradient tool to selected Slider adjustment to add _on preview_ local editing. The slider wold still be effective and used as _baseline value_, then the local editing objects would modulate the value locally. That way, we should have an _heterogeneous value_ and we'd still be compatible with the Batch editor. However the local editing wouldn't be available here, except for resetting the local edits. This is the easier part of the job.
2. On a second scheme, we could enable multi-profile per pp3, which opens a lot of possibilities ! Add to this possibility alpha channel support and fusion operator, and we're not far away of layer editing. It would let us do real HDR fusion, à la HDRMerge, or handle dual sensibility Fuji sensor, or stitch several images to create a panorama, or fusion several images to do focus stacking, etc.
3. Add bitmap support (i.e. painting) alongside the vectorial objects for local editing, but then the pp3 will have to handle binary objects, and that's not a simple affair for me.

My goal is doing point 1 first (and perhaps _only_, no promise for anything), then take some rest...

@michaelezra Instead of multiple instance of a tool (if I understood your point 1 correctly), I propose to have a single instance to keep the pipeline fixed as it is now, but modulate the value with unlimited amount of local objects. Would it suit your need ?

[michaelezra]

Hi Hombre!:) I think UI should have a single instance of the tool with sliders, etc. in the Toolbox.
Multiple local edits should be possible, each editable when its anchor point is selected in Preview panel.

If I got point 1 correctly - this would lead to ALL (except not raw & transform) RT edits available for each local edit?! That would be exquisite! In such case, it may be better to Enable local edit mode first - this will make local edit anchors visible, editable and those double arrows on the sliders.

[Hombre57]

@michaelezra No, that would be the other way round : all slider could have unlimited amount of local edits, each different. E.g. in the Exposure tool, you could have a different set of local edits for the Contrast slider and for the Saturation slider.

Of course, local editing support for a slider would not be automatic, but the mechanism should be simple enough to manually implement it in most sliders.

[michaelezra]

When doing gradient and elliptical adjustments in ACR I frequently combine e.g. exposure, highlight adjustments and contrast, even clarity in a single anchor.
If I understand correctly what you are proposing, next to every (applicable) slider there will be the local adjustment brush button. When it is clicked, all anchors for that specific slider would become visible and editable.
The downside is that it is difficult to visualize all local adjustments made as they are tied to million RT sliders. I feel it would improve usability a lot if local adjustments are essentially overlayed RT profiles (modulated by mask, gradient, etc) with full arsenal of RT sliders. This relates to point 2 in your list above. This will greatly minimize number of anchors user has to place and work with, as each anchor represents a group of adjustments - a pp3. Do you think this is doable? Of course having xmp / structured format would make it much easier to structure such pp5? files:)

[Hombre57]

This relates to point 2 in your list above.

The effect created by having layer or by having heterogeneous (i.e. variable) parameters would not be the same, so I wouldn't mix point 1 and 2. What you want is adjusting several sliders for each modifier. I have to think about it, to be able to have both solutions available (yours and mine). But we might expect a lot of Sliders to be locally editable, so you'd have to _Add new slider_ on the modifier object instead of seeing ALL of them already displayed. Is that ok ?

A single button would let you see all modifiers so you can attach a new slider to them or only display the one where the current slider is attached to.

Of course having xmp / structured format would make it much easier to structure such pp5? files:)

Probably, but who will do that ? :wink:

[Beep6581]

but then the pp3 will have to handle binary objects

Compressed and base-64 encoded text blob? Maybe one of our sibling projects has a solution for this, IIRC darktable.
If editing a 10 megapixel image, would that mean RT would have to store a 10 megapixel mask?

My dream for RT would be for it to allow me to increase or decrease the exposure of some parts (basically dodge and burn) and to warm up some parts (painting a warmer white balance, color toning or whatever else which will warm up the colors).

[Hombre57]

If editing a 10 megapixel image, would that mean RT would have to store a 10 megapixel mask?

With the help of some optimization, not necessarily. Of course, one of the question will be the choice of the alpha channel bit depth. 8 bits int ? 16 bits int ? 16 bits Half-Float ? 32 bits float ? Since the whole engine use float numbers, I'd suggest using a 32 bits float channel.

We're still hijacking Jacques' issue btw.

[Hombre57]

My dream for RT would be for it to allow me to increase or decrease the exposure of some parts (basically dodge and burn)

Dodge and burn is only available when merging 2 layers, so point 2 in my comment above.

and to warm up some parts (painting a warmer white balance, color toning or whatever else which will warm up the colors).

This will be possible with point 1.

[Desmis]

I just made another change of shape detection algorithm for lightness and chrominance. Now I also take into account "hue". The answer of the algorithm is now very close to that of "Nik Software" (which can be coupled free with Photoshop.)
The algorithm is activated as soon as scope is less than 20.
Attention algorithm does not take into account the chromatic noise, which of course will disrupt the system. It is therefore recommended to treat the noise before.

With this algorithm, I think it is not, in most cases, need to finely delineate the area to be treated

:)

[Desmis]

I just fixed 2 bugs :
1) when mouse sensor was out of preview, in some cases leads to crash
2) best algorithm for lightness
I hope this 2 fixes solved these bugs :)

[heckflosse]

@Desmis

Hello Jacques,

I hope you don't mind that I currently have not much time to look at your changes. When RT5 is releases, I'll take a look again!!!

Ingo

[Desmis]

@heckflosse

Ingo
No problem. Thank you again :)

The actual "algoritm code" contains many arbitrary values (thresholds filters, ...). Of course I can make parameterizable via "options", but before I want users assess the new algo.
:)

[Desmis]

There is still a bug, I think due to hueref, chromaref, lumaref between improccoordinator.cc and dcrop.cc

[Desmis]

I just push a change "merge with master"

1) I am working to another improvment for shape algorithm detection
2) actually we can only work with 5 spots, but there is no problem to have 10 or more. I'll just configured code to allow more spot.
3) I think I found the bug, it is "only" a pipeline problem, with refreshmap, and instructions "todo & someting". But I do not know exactly how to do
:)

[Desmis]

1) above, is done. I hope no error :)

[Desmis]

I just posted a change with:
1) improving the shape detection algorithm for part Retinex. I used the same principle as in lightness/contrast/chroma

2) clean up the code

3) allow the user to choose the number of spots. I am not come to establish a dynamic system, not the dynamic mode is impossible, but the monitoring / selection that I have set up (slider) is very difficult to operate.
You can choose the number of spots in "options" by changing the variable Nspot. By defaut the value is 3, you can select between 1 (stable) and ??.
Warning! This will bring probable incompatibilities with previous installations including the *.mip

4) I'm still not arrived to fix the bug that causes crashes. I think it is a pipeline problem, with a LUT "gamma2curve" with a passage of wrong settings (nan).
Knowing that this LUT is never used by locallab.
I left the "mutex" in place, even if used incorrectly (I do not know how!), as a precaution, but if we can solve the crash, may be able to do we remove.
:)

[Beep6581]

Compiles and runs fine here. I tried to reproduce the crash someone in the forum experienced when using 5 spots, but it didn't crash.

I did find that each spot has an affect on the other spots even when they don't touch. PP3 file with 5 spots https://filebin.net/0ie7hi3np0vbf7bl - remember to set the options file to 5 spots too. Scroll through all 5 spots, and you will see how all of them change when you don't change anything other than selecting a different spot.

I would find it useful if I could locally adjust exposure using a slider or preferably a curve, and be able to locally change white balance. Also the ability to control the edge falloff would make the locally edited areas blend in more nicely - currently edges are quite sharp.

[Desmis]

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]

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

[Desmis]

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]

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

[Desmis]

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]

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]

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;

• with high gamut or very deep blacks
• you can also try if there is no crash when moving the window in zoom 100%

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

[Desmis]

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

[Desmis]

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:

• a small spot is suitable for very fine foliage, etc.
• a large spot suitable for skin or skies...
  You can only change with slider - I don't know how to do with GUI...

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

[Desmis]

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 :

• hue proximity radius : determines the "radius" around each point to evaluate the variation of hue... : increase radius, increases processing time. Here I don't know how to do with OMP !
• Hue threshold : value from which the variation of hue is excluded from the algorithm (in radians * 100), low values excludes more!

Obviously I change all parameters (*.mip)

In some cases this algorithm improve shape detection for :

• luminance, chroma, contrast ==> Color Light
• local retinex
• I implemented for sharpen, but I don't see differences

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

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

[Desmis]

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]

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]

Enjoy your holidays Jacques!

[Desmis]

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]

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]

@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]

@heckflosse
Hello Ingo

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

[heckflosse]

@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]

@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]

@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]

I am working on local CBDL :)

[Beep6581]

All locallab issues:

3546

3421

3351

3292

2538

[Beep6581]

I found these bugs using commit e8061bb8d125597cfb0bf28d28464c3d19912622:

• The .mip files are stored in the wrong place. The console says
  mip files in=/home/morgan/.config/RawTherapee/profiles/amsterdam.pef.mip
  but they should be in cache not in config! So the correct path should be
  /home/morgan/.cache/RawTherapee/mip/amsterdam.pef.mip
• Each mip file should include a hash of the photo file in the filename, so that you can have more than one identically named photo in different folders. For example
  /home/morgan/.cache/RawTherapee/mip/amsterdam.pef.02e621096401cccf3badfefa78.mip
• When the LocalLab curves are dragged they create hundreds of history entries.
• Right-clicking on the Local Lab sub-tools, such as Color & Light, does not work - it should expand the clicked tool and collapse the others, same way as with Wavelets for example.
• The sub-tools should be turned off by default.

Enhancement requests:

• Rename "Shape" to "Settings"
• Put the "Control points" slider into "Settings"
• Make "Settings" collapsible (MyExpander) so that we don't always have to see it.

[Desmis]

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

• I don't know how to change mip files to cache
• I don't know how to suppress history messages
• I don't know how to collapse by default "expanders"

Others points seems solved .

Could you help me :)
Thank you.

Jacques

[heckflosse]

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

[heckflosse]

@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]

@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]

• The curve history patch suppresses the history spam, now I only get 1 entry, good.
• The mip patch puts the mip files into the cache, good.
• But the config-to-cache patch breaks LL, now the whole tool doesn't work https://i.imgur.com/eDb76mY.jpg
  Specifically, the preview doesn't change, but the histogram does.
• Apply Neutral. Simply enabling LL produces all of these history entries: https://i.imgur.com/hSgLjko.png
• Adjusting the on-canvas widget creates a new history entry for every adjustment: https://i.imgur.com/Ly6zmj9.png
  It would be cleaner if it did that all in one line, the way the Sharpening tool's Threshold adjuster works
  https://i.imgur.com/O1hLFYG.png
• There are two "Reset" buttons. Collapse all LL sub-tools to see them. https://i.imgur.com/lLOcsPw.png

[Beep6581]

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

[Desmis]

@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]

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

[Desmis]

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

jacques

[Desmis]

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

[heckflosse]

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

[Desmis]

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

[Beep6581]

@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]

No problem, for me you can push :)

2017-02-11 21:28 GMT+01:00 Beep6581 [e-mail]:

@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 : [phone]
http://jacques.desmis.perso.neuf.fr/

[Desmis]

@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]

@Desmis yes, please :+1:

[Beep6581]

@Desmis pushed.

[Desmis]

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

[Desmis]

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]

I just made the last changes:

• VBox changes as in Wavelet
• Merge with dev
  For 3 weeks now, I no longer see requests for changes.
  On the control points in Lab mode, I think I have finalized the system.

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

• Control points in RGB mode (exposure, etc.)
• Local white balance (???)
• Etc.
  But to begin these changes and in order not to overload the code, I prefer to initiate a packet of control points different from Local Lab.
  This assumes a merge "locallab_dev" in "dev".

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

jacques

[heckflosse]

@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]

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 [e-mail]:

@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 : [phone]
http://jacques.desmis.perso.neuf.fr/

[Hombre57]

@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]

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

[heckflosse]

Shouldn't be 5.1 released before 5.2?

[heckflosse]

@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]

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

[Floessie]

I haven't had a look at it yet (and it seems I won't be able to ATM), but as many on pixls have tested it, I'm pretty confident, it does what it advertises.

[agriggio]

@Hombre57 (and others): I don't think I'm entitled to have an opinion on
whether it is ok to merge (assuming that was the question), so I won't express
one. But since you asked, I will make a more general consideration (which you
all are free to ignore, of course!)

Personally, I don't feel the need for local edits very often, and when I do I
just go to Gimp (although I'm pretty bad at that). I have never seriously
tried locallab, I only gave it a quick look (solely as a user) a few months
back, so it is very much possible that things have changed completely in the
meantime, but I had the impression that the user interface was quite different
from what I am used to for local edits (i.e. layers and masks). Therefore, I
never really explored its possibilities. But it does seem like a highly
requested feature (judging from the traffic on the pixls forum), so kudos to
@Desmis for his work! I will try it out again and see what can be done.

If however there is someone else (@Hombre57?) that wants to investigate a
different approach, I can offer some manpower to that (but almost zero
experience though :-)

[Beep6581]

I still want to release 5.1 soon. Please hold off with merging locallab until 5.1 is branched off - locallab was not on the roadmap for 5.1. Once 5.1 is out we can merge locallab into dev and give it the several months of testing it needs in the main development branch before releasing 5.2. I cannot comment on what shape the code is in, but last time I tried it there were some issues left to be resolved. The local editing needs I have are not solved by the locallab branch and probably will not be (dodging and burning by on-canvas painting, locally changing the white balance), but it does seem to be a desired branch based on user comments in the forum.

[Desmis]

All issues have been resolved (@Beep6581, @Floessie, Dngimage, @sguyader, ...)
On principle I agree with @Beep6581

[heckflosse]

@Desmis Jacques, I started merge of dev into locallab_dev.
Please do not push to locallab_dev.
I will let you know when the merge is done.

[heckflosse]

@Desmis Jacques, merge of dev into locallab_dev is completed.

[Desmis]

@heckflosse

Ingo

Thank you very much :)

I just test and all seems to work well

Jacques

[heckflosse]

de rien :)

[Hombre57]

As @agriggio guessed, I have a totally different approach for local editing that I've already discussed, similar to what you can find in DarkTable. However it will take a little bit of time. I know that I'm speaking of this since more than 2 years, but there has been patch, corrections and port to Gtk3 in between, and because it is very invasive and not easy in a concurrent team working project...

Now that @Desmis's branch is about to be committed, I will rethink about the effort to do it at all, since it's there. Not what I had expected in a user experience point of view and logic (why duplicating things again and again ?), but it's there, and there has been a lot of work done. Will RT users be satisfied and will it attract new users, I don't know (I have my idea though). Time will tell.

[heckflosse]

Here are my 2 cents:

There has a lot lot of work been done by Jacques in locallab branch. Even new methods like adding noise for example...
But I also think that the gui has to be improved because as I tried it today while merging dev into locallab the gui (especially adding control points) confused me a bit (maybe because I'm not used to it).

Also it needs some tests and cleanups (and some optimizations as well).

Maybe TOGETHER we can make an even better local editing which includes the algorithms written by Jacques and improves ease of use?

Ingo

[agriggio]

:+1:

[Desmis]

As I mentioned, the system used is close to that developed by Nik Software (with no layers, no mask,...), but with differences:

• Each spot, can be considered as an object that has several fields - to date about 70, consisting of cursors, curves, checkbox, ... -;
• Each field, grouped in packets may or may not be activated, and may have values that vary according to its nature;
• Packets are coherent sets for the user: color & light, contrast, blurr, retinex, sharpening, tone-mapping, contrast by detail level, denoise;
• The number of "spots objects" can vary from 2 to 500;
• The data of the "spots objects" are stored in a "mip" text file;
• "Spots objects" are managed - creation, modification, monitoring - in a "for" loop;
• There is no code duplication.

But of course the whole is perfectible, as Ingo justly says, especially for the addition of a spot and spot tracking.
The addition of a spot is a computer problem, "how to make a slider variable?",
And as a reminder I am a poor computer scientist.
Nevertheless the whole works and the improvements above do not question, compatibility (the code can be optimzed, clean, etc.).

Jacques

[Hombre57]

A lot of things will change from the starting point of the merge of this branch. Since we're mostly in a rolling release scheme, I'd suggest to avoid or warn users that for some time, RT 6 will be unstable regarding it's pp3/pp6 files.

On a side note, I think that adding local editing deserve a major version update, as well as PixelShift. There's no reason to stay so long (few years) with the same major version number like RT4. DynamicProfile + PixelShift = RT6 ; Local Editing = RT7. What's your opinion ?

[Floessie]

DynamicProfile + PixelShift = RT6 ; Local Editing = RT7. What's your opinion ?

While Local Editing seems like a big feature with usability and big UI impact that would justify a major number increment, to me DynamicProfile + PixelShift add some features without dramatic impact.

We shouldn't take part in that race for the fastest moving major number (like Firefox, e.g.), but stick to the semver scheme.

Just my 2¢,
Flössie

[agriggio]

I agree with @Floessie

[heckflosse]

PixelShift is not worth a major number increment. It's a feature supported by only 4 cameras and also is not final at the moment.

[agriggio]

out of curiosity (and forgive me if this is not the proper place/time for this), why was the alternative option of using layers and masks discarded? are there some obvious reasons or just personal preference for this workflow?

[Hombre57]

@agriggio

It was not discarded, however being the only dev to code the GUI, and for the reasons explained above, i.e.

there has been patch, corrections and port to Gtk3 in between, and because it is very invasive and [then] not easy in a concurrent team working project...

that's why I never started this big patch. It also require a lot of changes in the engine and the file format, which has not been designed to accomplish this... and the fact that I'm not a pro dev neither (like @Desmis), so everything takes more time. We just need more dev like you and @Floessie (some other pro dev left the project already), with solid knowledge in C++ and GUI.

On his side, @Desmis started his own patch for 2 years now (I think) with the actual and suboptimal design of the actual engine, up to the point of the [ _planned merge_ ]. My point was about giving a future-proof design that will let us add so much more possibilities to RT. Now, the urgency is to provide local editing, and nonetheless kudos to @Desmis for having done something more than usable to that point, but as I already said in other issues, we'll have to make a switch to the expected design along the actual behavior (which in essence looks like any other tool from an engine point of view - I still have to look out the engine evolution made by @Desmis). Users may be disoriented by so much back and forth design and future incompatibility, but we have valid explanations to provide (the urgency and low human resource).

[heckflosse]

@Hombre57

up to the point of the merge of today.

Which merge do you mean? I merged dev into locallab_dev, not vice versa.

[Hombre57]

I mean the planed merge of locallab_dev into dev, that we're discussing.

[heckflosse]

@Hombre57 That planed merge can not be the one from 'today' you mentioned ;-)

[Hombre57]

Comment updated.

[heckflosse]

:)

[agriggio]

@Hombre57 thanks for the explanation! I did not mean to dismiss @Desmis's work (no pun intended :wink: ), I just wanted to understand, since I was not around at the time... I have some ideas about local editing myself, but before describing them I need to think about them more and make sure that they are not completely insane :smile: When (or better, if) I get a clearer picture, I will follow up.

[Desmis]

@agriggio
There are several reasons for this choice:
1) I was a user of "Nikon Capture NX2" which had the 2 local editing modes: a) lasso and mask; b) U point; And in 95% of the cases I used the U-points.
Because I found them, as well as many users more efficient overall.
The Nikon approach (in fact Nik Software) was revolutionary and very efficient, without masks, without layers.
Since Nikon has abandoned the U-points (license problem), the version of NXD no longer excites the enthusiasm of the users.

Since the end of 2012, the team of RT was in search of the Graal, how to make a local edition - without specifying what was meant by there but roughly:
a) correcting the faults on the sensor (spot removal)
b) allow local adjustments of brightness, contrast, etc.
About three years ago, I saw that Hombre had used an interesting approach to the GUI in "Graduated filter".
My first job was to adapt the work of Hombre to have an editing area, I chose a surface with 4 delimiters connected by ellipses.

Then, I started to realize the first module which included only the functionalities: lightness, contrast, chroma ... Moreover this module had only one control point.
This was followed by about a year and a half of lively exchanges on the durability of this module (relevance, portability, usability, habits, etc.),
as well as a progressive improvement of the "single point" module.
Meanwhile, Ingo has consistently brought its stone to the building, improving code and system performance - thanks to him.
From the end of the summer of 2016, I have glimpsed a solution to multiply the number of spots, to arrive at the current solution.

To understand process I propose to read Rawpedia in french "Contrôles locaux Lab".

2) The second reason is that I do not know how to do with layers and masks.
But of course nothing prohibits to propose to the users the 2 concepts (as in NX2) which are of a often different use.
It should even be possible to marry the two, for example by replacing the ellipses with Bezier curves, provided that there is no intersection in the homothety (transition).

During the development I have several times called to get skills for the GUI (addition and tracking of spots, adjustment of the size of the spot detection area, etc.).
I'd like to thank Ingo for his improvements in code and performance, as well as forum developers and testers who have tried, found malfunctions or bugs.

Of course the product needs to be improved : fixed bug, shape detection,... and especially its interface GUI, a joint work would significantly improve the product,
And if nothing opposes it once "locallab_dev" merge with dev, I would add:

• a module "rgb" with in particular "exposure"
• a "RGB-RAW" module with white balance and why not, demoisaicing
  In order not to complexify the code, I would open a new module at the GUI "local rgb"

I think we should also be able to with a derived algorithm remove the defects of the sensors.
On the other hand, paint, brush, etc. are impossible.

[heckflosse]

@Desmis
Jacques, today I had a first look at the current code of locallab_dev after a while and I think there are some things which can be optimized for speed. You know I'm not the right one to ask for improvements in gui, but for speedups you can still count on me. That will take a while because I have to analyze and test all the features you implemented step by step, but I think we can get this a bit faster.

I also apologize that I didn't optimize locallab last months, but pixelshift really took some resources (though finally it's not so many lines of code).

[Desmis]

@heckflosse
Ingo, you are "super". No problem you can optimize :)

Thank you
Jacques

[heckflosse]

@Desmis
Jacques, I would like to push a first speedup (for BlurNoise_Local) to locallab_dev, ok?

Ingo

[Desmis]

No problem :)

[heckflosse]

@Desmis
@
Jacques, I pushed a speedup for blur.
I tested with a D800 file using a blurred region of about 500x500 pixels using Radius 20.
I measured in function BlurNoise_Local in full processing mode (simpleprocess)

Before speedup: 146 ms
After speedup: 66 ms

I let the BENCHFUN active at the moment

[Desmis]

Hello Ingo
I just tested before / after speedup with D800.NEF
region about 900 * 1250 - radius = 25

Before = 1047 ms
After = 160 ms
No differences in output.

Very good job :)

Thank you

Jacques

[heckflosse]

@Desmis

Jacques, I will try to make one speedup per day now for the locallab tools (ok, at least 5 per week).
That will take a while because of the massive amount of locallab tools but should be worth the effort.

Ingo

[Desmis]

Ingo

No problem :)

Thank you.

[heckflosse]

@Desmis Jacques, I will push an about 3x speedup for locallab contrast tool soon :)

[Desmis]

Ok , thank you :)

[heckflosse]

@Desmis

Jacques, I pushed the speedup for locallab contrast tool and also merged dev into locallab_dev again.
Please test.

Ingo

[Desmis]

Hello Ingo

I tested your speedup.
1) first time threatment is about divided by 6 or 8...it's huge.

2) I think you have found - not a bug - but a malfunction in the original
algorithm when lp.sens < 40, with this code
if (lp.sens < 40.f ) {
kch = pow (kch, pa * lp.sens + pb); //increase
under 40
}
Now it is in "if else" and not outside. It's better now :)

Thank you very much.

[heckflosse]

@Desmis

Jacques, I pushed a speedup for Sharp_Local. Also cleaned the code around it a bit.
Please test.

Ingo

[heckflosse]

@Desmis

Jacques, I wanted to optimize the locallab denoise part. But then I detected a thing you probably can fix faster then I can.

To reproduce:
Pull latest locallab_dev from github (I added a StopWatch)

Open a file.
Apply neutral profile.
Add one locallab control region and apply denoise for this region.
Look at the console output

Expected output:

One line showing the processing time of locallab denoise

locallab Denoise called took 209 ms

Real output:

8 lines showing the processing time of 8 times (max number of control points is 8) locallab denoise

locallab Denoise called took 209 ms
locallab Denoise called took 199 ms
locallab Denoise called took 196 ms
locallab Denoise called took 197 ms
locallab Denoise called took 198 ms
locallab Denoise called took 197 ms
locallab Denoise called took 198 ms
locallab Denoise called took 197 ms

[Desmis]

Hello Ingo

All works fine for "Sharp_local", seedup about divided by 6.
Very good job :)

I'll look to "call denoise" :)

jacques

[Desmis]

Ingo

I look at this message. It is due to call to "denoise" with "Global quality
= enhanced + chroma denoise". In this case for each control point, there is
a call, even the user do not open any function.

To prevent when it is not usefull, I change the fist line:

  //  if (lp.noiself > 0.f || lp.noiselc > 0.f || lp.noisecf > 0.f ||

lp.noisecc > 0.f && call < 3 || noiscfactiv && lp.denoiena) {

by:

    bool execdenoi = false ;
    execdenoi = noiscfactiv && (lp.colorena || lp.tonemapena ||

lp.cbdlena || lp.sharpena || lp.retiena);
if (((lp.noiself > 0.f || lp.noiselc > 0.f || lp.noisecf > 0.f ||
lp.noisecc > 0.f) && lp.denoiena) || execdenoi) {

Are you ok, and if so, could you replace this part of code ?

Jacques

[heckflosse]

@Desmis

Jacques

I tried your approach but still get 2 calls of denoise (of course that's better than 8 calls).

I would appreciate if you could take this part because I don't want to destroy something.

When that's done, I can push further speedups for locallab denoise I already have on hold.

Next push is yours ;-)

Ingo

[heckflosse]

@Desmis

Jacques, I think I can speedup the locallab denoise by about factor 2 on top of the factor 8 we will get with your cleanup (not calling denoise for control points where it's not enabled)

[Desmis]

Ingo

I push a change:
With that change, denoise is only call :
1) if user want to use denoise
2) if qualmet=2 is selectionned (best quality to prevent artifacts due to
varaitions of hue due to chroa noise) and if one modules (color, retinex,
tone mapping, cbdl, sharp) is enabled and for each ot these modules if user
want to use them.

If think there is no error, but to confirm by you

You can now sppedup denoise.

Thank you :)
Jacques

[heckflosse]

@Desmis
Jacques, I pushed the speedup for locallab denoise. Though it's still called twice. Don't know why...

[Desmis]

Hello Ingo

I just tested your improvment.

Time threatment divide by about 2 (a little more)

​
​No diffrences in output, or ver very tiny.

For me, with simpleprocess (save TIF), I have only 1 call.

Very good job

Thank you very much

Jacques​

[heckflosse]

@Desmis
Jacques, here's the pp3 with which I still get 2 calls in simpleprocess.
D800FAR2I0200.NEF.zip

[Desmis]

Hello Ingo
I just tested this morning with your pp3 and D800

I have only one call (1), perhaps you have an other spot open (look at the
mip file - in the cache by default).

But, in some case (mostly), with TIF : JPG output, the system don't exactly
crash, but exit RT without threatment, when process is with locallab.

Jacques

[Desmis]

Hello Ingo

I build a debug version, then gdb.

With most files (eg Blue horse, or D800FAR21300.NEF), with only locallab,
and denoise (4 sliders > 0 about 40) and output TIF/JPG

In preview all works fine :)

I try : clean mip, clean pp3, without effect
In some mostly case , I received this message :

"Thread 372 received signal SIGSEV, segmentation fault
[Switching to thread 5480.0xb44]
0x00000000095631e in ?? ()"

and I join the log.txt, for another try.

log.txt

[heckflosse]

@Desmis

After deleting the mip file I get only one call for locallab denoise :+1:
I can not reproduce the crashes here.

[Desmis]

Hello Ingo

For the crash with "Denoise", it's very versatile...If I reboot my
computer, the same file does not crash, but in some cases, when I call
"external editor', it does not work, without crash

If somebody else can test ?

Jacques

[heckflosse]

@Desmis Jacques, today I pushed a lot of fixes to locallab_dev. Did you already test with this version?

[Desmis]

Hello Ingo
Yesterday I was away :)

I just tested all new changes
All seems to work fine :)

No more crash !

No differences in output, or as I said previously, very, very tiny

Time threatment divided globaly by 4 on various images

Very good job :)
I'll continue to test, specially "denoise"

Thank you very much

jacques

[heckflosse]

@Desmis
Jacques, I will make further speedups for locallab_dev next days

[Desmis]

No problem, thank you :)

[Desmis]

@heckflosse

Ingo

Are you working on "locallabdev" because otherwise I could merge with dev ?
Jacques

[heckflosse]

Jacques, you can merge dev into locallabdev. I will continue with speedups later.

[Desmis]

Ok, I'll do that next days :)

[Desmis]

Done :)
I hope no error !
jacques

[Beep6581]

I merged dev into newlocallab.

[Beep6581]

I am busy merging dev into newlocallab.

[Beep6581]

@Desmis I need to leave for an hour, I will continue merging tonight - please do not commit anything to newlocallab in the meanwhile.

[Desmis]

@Beep6581
I have commited a change 9d3429c 30 mn ago

No commit for me!
Thanks for merging

Jacques

[Beep6581]

@Desmis no problem. I merged dev into newlocallab and fixed the conflicts. You can pull now.

[Desmis]

@Beep6581
Thank you very much :)

[Beep6581]

@Desmis seems that issue #4094 is now used for newlocallab discussions, can we close this one?

[Desmis]

@Beep6581

Yes:)