Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spot Removal not working properly #7026

Open
PDeb108 opened this issue Apr 3, 2024 · 7 comments
Open

Spot Removal not working properly #7026

PDeb108 opened this issue Apr 3, 2024 · 7 comments

Comments

@PDeb108
Copy link

PDeb108 commented Apr 3, 2024

The problem I am facing in RT is using the spot removal module. I am working with it at the finishing stage of the work. What is happening is that as soon as I click the button on the right to activate the spot (marked with a red circle in the bottom frame). The exposure of the entire picture increases to the point when I am unable to see the spot. The top frame is when I have not activated the spot creation button, and you can see the spot.
This is when I am using the most recent dev version of RT (RawTherapee_7023merge_5.10-182-g918ef5e5c_win64_release)

My system is Intel I5/16GB/Intel graphics processor/Windows 11
The situation is similar even when I use the stable release 5.10

Below is the nature of the problem

a065cb07e48ccf9878335f8d1052380dbe3104b6

@Desmis
Copy link
Collaborator

Desmis commented Apr 5, 2024

@Lawrence37 @PDeb108

@Hombre57 - I believe it was you who led this change a few years ago, can you look. Thanks.

I confirm the bug found by @PDeb108
The operation of "Spot Removal" seems to be linked to the Raw file and the lenses used. In many cases everything works fine. In this case it does not work, or poorly, or can cause a system crash.

The problem was found during the challenge to test "lacam16n2" but it seems that this malfunction occurs whatever the branch, and probably dates back a long time
https://discuss.pixls.us/t/rawtherapee-processing-challenge/42647/66

Here the file Raw, and the pp3
https://drive.google.com/file/d/1w6sn1BQ8FQ7uSTd3quceJcyNU5K_nCCZ/view?usp=sharing
https://drive.google.com/file/d/16HVp5w3_Oas9qzEj_2KHYSpKClpzsTWK/view?usp=sharing

I've tried several changes that make it work less badly.
In Dcrop.cc and Improcoordinator.cc - better (I think) cleaning of variables (copying part of the ART code, which is very little different), but I don't think that's it
In refreshmap.h #define M_spot change from 16 to 22
I also change but it's only cosmetic the icon in Tollbar.cc

It seems that: crop, Lens profile, Input profile have a strong impact.
It seems also that : Haze removal and Graduated Filter have an impact.
I try to disable all "expanders" without real effect.

It works only with "neutral".

I used as a basis the current work of the "cbdlciecam" #7017 branch because as @PDeb108 uses Ciecam and certain functions which created artifacts, I preferred to start from a healthier base to eliminate possibilities (it seems that Ciecam16, Wavelets or LA are not involved... to check)

This looks like a pipeline problem, plus this type of process is not in my area of expertise

I create a new branch "spot_cbdlciecam"

Jacques

@Desmis Desmis added the type: bug Something is not doing what it's supposed to be doing label Apr 5, 2024
@Hombre57
Copy link
Collaborator

Hombre57 commented Apr 5, 2024

Hi @PDeb108 , @Desmis ,

Since spots are located at fixed positions in the initial image, entering the edit mode disable all transform tools that you could have already set, so you can preview the image in its initial state. That's what the rtengine::TweakOperator has been created for. See line 866 of spot.cc for all tools forced to neutral. The last group on line 883 is for better performance, because the spot tool is already time consuming if there are numerous big spots. If new tools are added to RT, this list might have to be updated (probably the local editing tools should be disabled here).

Tone curves are not disabled IIRC, and that's perhaps why it looks over exposed. Keep in mind that spots should be removed in the beginning of your editing process to avoid this kind of problem, even with a temporary exposure compensation. There's no magic solution here I'm afraid.

@Desmis
Copy link
Collaborator

Desmis commented Apr 5, 2024

@Hombre57

Thank you for the explanation.
I think you are true (of course, because you know the tool for having designed it). "Keep in mind that spots should be removed in the beginning of your editing process"

I think to improve the operation we should add to Spot.cc (tweaksparams)

  1. pparams.locallab= LocallabParams();
  2. pparams.toneCurve = ToneCurveParams();

And
pparams.locallab.enabled = false;
pparams.toneCurve.hrenabled = false;
pparams.toneCurve.histmatching = false;
pparams.toneCurve.expcomp = 0.;

and perhaps others parameters.
What do you think ? and who does it?

Jacques

@PDeb108
Copy link
Author

PDeb108 commented Apr 5, 2024

@Hombre57

Thank you for the explanation. As a user, I don't understand the technicalities you have mentioned, however, I gather, that spot-removal is a resource-intensive process and should be done before doing anything else, much like setting the white balance first. Doing it later might even cause crashes. I am yet to try it on any photos as yet. will give you a feedback

Might I suggest that the module be moved from the detail group to the exposure group to focus on the need to use it early in the work process

@Hombre57
Copy link
Collaborator

Hombre57 commented Apr 5, 2024

@Desmis I've not installed any IDE since I changed laptop, so could you please do that change ? I wouldn't reset ToneCurveParams(); though, nor force pparams.toneCurve.expcomp to 0, because it's the only remaining way to change the exposure to better see spots.

So the patch would look like :

pparams.locallab.enabled = false;
pparams.toneCurve.hrenabled = false;  // not sure for this one, it could be useful for ExpComp w/o performance penalty
pparams.toneCurve.histmatching = false;

No need to reset the entire LocallabParams(); if you have an enabled parameter that can do the job.

@PDeb108 Tools are sorted into tabs depending on their functionality, and tabs are organized (somewhat) by their usage count. That's why RAW is almost the last tab while being the first thing happening in the process pipeline. So I think Spot is in the right tab.

@PDeb108
Copy link
Author

PDeb108 commented Apr 6, 2024

That is perfectly logical. Thanks for the input. BTW, although I created the issue, I am not sure whether it should be closed by me or not. Someone more knowledgeable may update the status. Thank You

@PDeb108 PDeb108 closed this as completed Apr 6, 2024
@PDeb108 PDeb108 reopened this Apr 6, 2024
@Lawrence37
Copy link
Collaborator

Closing because the behavior is expected.

@Lawrence37 Lawrence37 removed the type: bug Something is not doing what it's supposed to be doing label Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants