search

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 versus Tone-mapping - newretinex branch #2969

Closed Desmis closed 8 years ago

Desmis commented 9 years ago

I am trying to develop an algorithm (personnel) to allow Retinex simulate a "tone-mapping effect" I will post a change probably tomorrow... :)

heckflosse commented 9 years ago

Jacques, I'm working on making retinex usable for non raw images (tiff,jpg). This requires some code movements between different source files. I hope it will not conflict too much with your changes. Should I wait with my changes until you posted your changes and then make them based on your changes?

Ingo

Desmis commented 9 years ago

Salut Ingo

I'll post my change tomorrow. I have introduced an iteration loop in "ipretinex.cc", and various adjustments to ipretinex...and of cource GUI.... No changes to Rawimagesource.cc. I think your proposition is good (wait and change after).

Thank you :)

Jacques

heckflosse commented 9 years ago

Ok :)

Desmis commented 9 years ago

Ingo

I just push the change..I call the new branch "retinexadd" to 'allow' other things that Tone-mapping

Amicalement Jacques

Beep6581 commented 9 years ago

Cool, can't wait to test! :)

heckflosse commented 9 years ago

Jacques, I'll need some more time for non-raw retinex. So please don't hesitate to push new changes. I'll push my changes this week (don't know exactly when).

Ingo

Desmis commented 9 years ago

Ingo, OK:)

I just push a change with;

Nota :

:)

Desmis commented 9 years ago

I have update "Rawpedia" (always in french) to 'explain' "Retinex versus Tone-mapping" :)

Beep6581 commented 9 years ago

Interesting results can be had with this addition, let's keep it.

Jacques what do you think about renaming:

Easier to understand?

Desmis commented 9 years ago

DrSlony We can rename as we want, although the 3 "old" names seem more in line.

Ok for "Gain" and "Contrast" Ok for "offset" and "Brightness" But for "Neighboring pixels", "Sigma" seems closer to me.

Do you want, I update...?

Beep6581 commented 9 years ago

I understand they are more inline with the research behind Retinex, but for users (photographers) I think "sigma" means... not much? If you're ok with renaming, let's get some more opinions before making a decision.

Desmis commented 9 years ago

then OK to "radius"...but the value is not radius, as indeed sigma !

So I update ?? Very easy to do :)

Desmis commented 9 years ago

But if one is in the semantics, it would be reviewing many names in RT, eg :

Desmis commented 9 years ago

I just push these changes, and also a small modification to "gaussian gradient"

Desmis commented 9 years ago

Just another change, to take into account "Highlight method" for "Gaussian gradient"

I update Rawpedia !

Desmis commented 9 years ago

Ingo

Can I push a change ...tomorrow morning, or must I wait ?

heckflosse commented 9 years ago

Jacques, no problem. I wait.

Desmis commented 9 years ago

I just push a change with a "Mask method" (I took the idea with Russell Cottrell plugin). You can used 3 functions

Ingo, now you can :)

Desmis commented 9 years ago

I think it is easy to reduce by 50% the time wavelet, but now I await the TIFF / JPG change Ingo

I'll update Rawpedia !

heckflosse commented 9 years ago

Jacques, I'll need a bit to code a good solution for TIFF/JPG retinex. But it's no problem if you change the functions 'retinex', 'retinexpreparebuffers', 'retinexpreparecurves' and 'MSR' meanwhile. So just go on :+1: It's mostly copy&paste to include modified versions of the functions into my changes.

Ingo

Desmis commented 8 years ago

Ingo Thank you :)

I just push a change with:

Desmis commented 8 years ago

I have updated Rawpedia (always in french)... For me both "Mask method" + Preview transmission" is a major advance for Retinex!

Beep6581 commented 8 years ago

I haven't tried today's changes out yet, but this sounds like something similar which was added to the Wavelet tool, so I wanted to say that it would be good if one could not just preview but also save the mask image (for use in GIMP/PS/etc), and in that case it should not be called "Preview" (the similar option in Wavelet was also initially called "Preview" though it influenced the saved image) but "Process".

Desmis commented 8 years ago

No problem, it's only a semantic problem...When you send to TIFF/JPEG "mask" or "Transmission" are saved :)

Desmis commented 8 years ago

It should be noted that export the image "Transmission" is possible, but in my opinion of no use ...

Some remarks of semantics (again):

Desmis commented 8 years ago

Ingo Can I push a change without it bothering you ? The files concerned are "retinex.cc (GUI)" and MSR function in "ipretinex.cc" Thank you :)

heckflosse commented 8 years ago

Jacques, of course.

Desmis commented 8 years ago

I just push a chnage ==> I have add; 1) an auto mode to Process "Transmission"...I don't think it's better or worst than the other, but different.. 2) always in Process a "Unshap mask"...you can generate images with high or very high radius Unsharp mask, with some settings of retinex ... :)

Desmis commented 8 years ago

Hello all I am working on "wavelet" to establish an "unsharp mask" from the process "Above the level" and Background = Residual. My initial tests show that it works very well and should be able to imagine many combinations...and perhaps a "Wavelet retinex"!! Before continuing development includes a section on the GUI, I would like to know if: 1) I included these new features in "retinexadd" ? 2) or I open a new branch ?

:)

Beep6581 commented 8 years ago

@Desmis @heckflosse how happy are you with the stability and performance of this retinexadd branch? If it's good, let's merge first, then do the above in a new branch. For me the priority is releasing RT5, and that means fixing bugs and improving what's already there instead of adding new things which need months of testing.

Beep6581 commented 8 years ago

@Desmis I get a lot of this in the console:

.................................................................................
.................................................................................
.................................................................................

Could you clean up the printfs in ipretinex.cc if you don't need them?

471:                    printf("..");
568:            printf(".\n");
Beep6581 commented 8 years ago

Thank you for these additions. The Retinex tool is now very configurable, but also very hard to understand. Some of this difficulty can be removed by making the interface more clear:

  1. There is now a "Process (Preview)" combobox at the end of the Retinex tool, and it affects the whole Retinex tool, hiding most of the Retinex tool's widgets when preview is anything but "standard". The position of this combobox is semantically misleading. I think it should be moved into the very top of the Settings expander
  2. There is also a new "Mask" section, it pops out five sliders and a curve when mask method is anything but "None". Again it would be more clear to the user if this whole Mask section was at least inside a frame if not a MyExpander. Otherwise we need to append "Mask " to every mask adjuster label, e.g. from "Highlights" to "Mask highlights", from "Highlights tonal width" to "Mask highlights tonal width", etc. I think a frame or MyExpander is cleaner.
  3. The tooltip for "Mask method" says "Use the mask generated by the Gaussian function above". Which function above is the gaussian function specifically?
  4. Many of the tooltips lacked dots at the end of sentences and before \n.
  5. Please review the following tooltips, as they refer to adjusters which have been renamed and I don't know which ones these are: TP_RETINEX_GRAD_TOOLTIP TP_RETINEX_GRADS_TOOLTIP TP_RETINEX_SCALES_TOOLTIP

Please remember to run git pull before making changes, as I committed some fixes in https://github.com/Beep6581/RawTherapee/commit/d146809651e2a676e816ef0aeb3a52f4532727b7

Desmis commented 8 years ago

I have create a new branch : "newretinex" with the changes above Now you can try Retinex :)

I try to delete "retinexadd" git branch -dr origin/retinexad git says me : $ git branch -dr origin/retinexadd Branche de suivi origin/retinexadd supprimée (précédemment edd88be).

But if I look to GitHub...retinexadd is always here :)

Beep6581 commented 8 years ago

Great, I will test in the evening.

To delete the branch: git branch -D retinexadd To clean up your list of branches: git fetch origin --prune or you can go to https://github.com/Beep6581/RawTherapee/pull/3055 and click on the [Delete branch] button below the last comment.

By the way, there is a little "bell" icon for notifications in the top-right corner of every GitHub page notifications icon. If you click it you will see all places on GitHub where someone wrote your name with a @ before it, e.g. @Desmis, and all issues which you are watching where there has been activity. Checking it every time you visit GitHub is the easiest way of keeping track of discussions which are relevant to you.

Desmis commented 8 years ago

Hello DrSlony I have ben away some days... Thank you very much for your help, now I master a little GIT :) In the doc I saw for delete a branch : git branch -Dr retinexadd ! is the "r" necessary ?

Thank you again :)

Beep6581 commented 8 years ago

I'm not entirely sure, but I think that if you git branch -D retinexadd and then clean up using git fetch origin --prune, then the -r is unnecessary. Maybe you don't have to do --prune if you used -r while deleting? I don't know yet :)

Beep6581 commented 8 years ago

@Desmis here is my attempt at finding bugs. Keep in mind that I have absolutely no idea how to use this tool.

  1. Partial-paste in this branch is broken. Open a photo, apply some profile, enable retinex and set for example strength to 100. Copy the profile with the "processing profile fill mode" button pressed profile-filled Now apply the Neutral profile, which resets everything, set the "processing profile fill mode" off profile-partial and partial-paste just the Retinex tool. Result: everything gets pasted, not just Retinex.
  2. "Process (preview)" should be renamed to just "Process" as it affects both the preview and the saved image.
  3. Selecting in "Process" anything but "Standard" results in the "Contrast" slider above it disappearing.
  4. When using "Process: Mask and Unsharp Mask" mode with "Mask method: Curve Only or Gaussian Mask", only the bottom-left 1/10th of the Mask Equalizer curve makes a difference, the rest of the curve as in this screenshot makes no difference http://i.imgur.com/oGlGdmO.png
  5. In the same modes as point 3, none of the Mask frame's sliders work (Highlights, Highlights Tonal Width, etc). They have 0 effect. --update: I tried on other images with other settings, and then sometimes the sliders had an effect, so the question is how to reliably use this as a tool?
  6. When "Process" is at "Standard" then none of the "Mask" frame settings make a difference. Here there is an issue with consistency. If you decide to hide UI elements for disabled parts of the tool (as you do), then the "Mask" frame should be hidden when Process=Standard. If you decide to not hide UI elements, then the other Retinex widgets shouldn't cause elements to disappear. One or the other.
  7. At the top of the Retinex tool there is a Contrast slider. In the Settings expander, above the Brightness slider, there is another Contrast slider. Do they both do the same thing? If yes, then why have two? If no, then one needs to be renamed.

I'm curious, what can the Retinex tool do to improve this image? http://filebin.net/vjdg287lng

Desmis commented 8 years ago

@Beep6581 Hello DrSlony I did a full review after your comment above 1) solved 2) solved 3) solved 4)5)6) for me everything works as it should...but Retinex tool is very versatile and depends on many parameters. So according to images action on masks will be more or less pronounced 7) solved :) I'll look to your image !

I have update Rawpedia Retinex in french, essentially change the texts to meet different locations changes

@heckflosse Ingo, 2 months ago you planned changes, what is it? I sent you a message on your home address in the last few days :)

Beep6581 commented 8 years ago

4, 5, 6) Yes, should first study the documentation you wrote on RawPedia. The rest - great :)

Desmis commented 8 years ago

I simplify GUI interface of Retinex : Process (logical), Mask method (suppress 'curve only'...it is the same functionnality with Sliders "Gaussian mask" to zero

Desmis commented 8 years ago

If you have proposition to improve ? :)

gaaned92 commented 8 years ago

In order to try newretinex, I am building a win32 build. I suggest you rebase if possible the branch at least to avoid a lot of warnings. Thanks.

Desmis commented 8 years ago

Qu'est-ce que tu entends par "rebase if possible the branch"...? :)

gaaned92 commented 8 years ago

Newretinex est très en retard sur la branche master. Ce serait bien de la remettre à niveau. Quelqu'un peut-il aider à remettre la branche newretinex au niveau de la branche master?

Can somebody help to put the retinex branch at master branch level?

gaaned92 commented 8 years ago

WIN 32 relwithdebinfo build available here :https://drive.google.com/open?id=0B2q9OrgyDEfPOEZmVlBzLWlnSWc

Desmis commented 8 years ago

Merci @gaaned92 pour ce lien :)

Je tiens à préciser (in french..ce qui correspond pour les anglophones à la même difficulté que l'anglais pour moi :) 1) les opérations "merge" sont pour moi stressante...Pour mémoire ce qui a valu que la précédente branche Retinex ait été fusionnée...tient plus de l'obstination à arriver à quelque chose et au hasard, qu'à une volonté de "fusionner malgré tout"...de plus je n'ai aucune formation "informatique" (sinon un peu de FORTRAN IV, avec cartes perforées en 1970!) 2) je ne suis pas à l'aise avec ces manipulations, qui sont - c'est un euphémisme de le dire - mal exprimées dans tous les tutoriels...j'en veux pour témoignage de l'effacement d'une branche. On me dit pour effacer la branche "reinexadd" sur le serveur: "git branch -D retinexadd."..cela ne fonctionne pas, après recherches sur le net :"git push origin --delete retinexadd", qui fonctionne 3) lorsque je propose d'importantes nouveautés en termes d'algorithmes. Je propose toujours une première version très rustique où j'ai "arrangé a minima" le GUI pour que cela fonctionne (il est évident que l'interface GUI suivra lorsque l'algo sera au point et approuvé). On me demande tout, tout se suite, c'est particulièrement contraignant, car fixer le GUI...fixe d'une certaine manière l'algorithme. Bref je suis en train de le faire pour "newwavelet", mais là encore, les choix de regroupement (qu'est-ce qui est dans quoi ?) sont purement arbitraires :)

Merci de m'avoir lu! Jacques

Beep6581 commented 8 years ago

I merged master into newretinex and fixed some merge conflicts. Builds fine. https://github.com/Beep6581/RawTherapee/commit/d282bba42956317cee273390aa9ecb7351872321 https://github.com/Beep6581/RawTherapee/network

Beep6581 commented 8 years ago

I tested it for half an hour - no crashes, no issues. It is a useful addition, +1 for merge.

Here are several versions of the same photo just to demonstrate: http://filebin.net/htxc3yofny

Desmis commented 8 years ago

Thank you DrSlony If no opposition, could you merge for me :)

Beep6581 commented 8 years ago

Sure, I'll just first wait for an "ok" from Ingo or Adam.