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

Re-open Generalized Hyperbolic Stretch #7251

Open
wants to merge 180 commits into
base: dev
Choose a base branch
from
Open

Re-open Generalized Hyperbolic Stretch #7251

wants to merge 180 commits into from

Conversation

Desmis
Copy link
Collaborator

@Desmis Desmis commented Nov 14, 2024

I accidentally merged the PR GHS #7210
I reverted with the PR 7250

I open this new PR... same code

@Lawrence37
Copy link
Collaborator

Just to be clear, I am not arguing for or against changing the direction of the black point adjustment. My question was just to understand why you chose that direction. We can debate which direction is more intuitive. It's a matter of opinion. What is not up for debate is the fact that having inconsistent behavior is confusing and unintuitive. There are, of course, two possible solutions. We can change the GHS black point direction (as you have done), or we can keep the GHS black point behavior and change all the other black adjustments. This issue is no longer relevant because you made the change, but I do want everyone to know that consistency is important.

I'm curious, what happens to the image and histogram if you allow the black and white points to work with a stretch factor of 0?

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 12, 2024

@Lawrence37 @waynesutton50

It's all about one's perspective on consistency and what one expects.
Now, with the new setting, more in line with what's elsewhere in RT, the negative (positive before) Black Point values, which serve - not to actually change the black point, but to slightly increase the luminance in very low light (to improve (SP)), on a negative scale, but why not ?

My initial configuration, which has disoriented you a little, was precisely to be able to adjust the black and white points with (D) disabled, especially to not disturb the histogram.
Now the system can be initialized with D=0.001, which has almost no impact (0 is the same) on the histogram. But it will be necessary to write it in the documentation otherwise if (D) has a high value, it will play its role: raise the shadows, move the peaks, etc., and histogram will not be really useful anymore.

I am improving the text I attached, with images and new comments. But putting it in Rawpedia will be complicated.
I also reviewed the tooltips
Jacques

@waynesutton50
Copy link

For the raw black points, moving the slider to the right increases the number of clipped pixels. Moving it to the left decreases the number of clipped pixels.

I think the problem is compounded by the fact that the labels - Black(s) - are not explicit enough. For example the Black slider in Exposure works as above but the Blacks slider in the Tone Equalizer works in the opposite direction. I see that in ART, the Black slider in the Exposure tool has been renamed to "Black point compensation", perhaps for this reason.

@Lawrence37
Copy link
Collaborator

@Desmis Let me know when this pull request is ready. I see you recently added a GF commit that also affects the GHS.

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 15, 2024

@Lawrence37

For me - with all the limits of my skills, and the remarks made in the PR on "Abstract profiles", we must be able to admit that it is ready. Of course the GF still works imperfectly only in zoom++, but for me it is acceptable, in any case clearly better than the current solution. The GF issue should be kept open, but the GF-related code in this PR is configurable in every way.

I moved as it is written in the commit that this PR influenced the BP and WP which is obvious if we look at how it works. I moved it... no difficulties, and now everything is OK from this point of view.

Jacques

@Lawrence37
Copy link
Collaborator

Are some settings and features missing? Transition Gradient and ΔE Shape detection only have a single adjustment each. The Graduated Filter is not available even with Advanced complexity.
image

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 22, 2024

@Lawrence37

I am (very) glad that you found this difference in "settings" behavior. I changed it on purpose to see if anyone would notice the difference.

Why did I do this? A number of users point out the complexity of "settings", especially in relation to masks (ART, Darktable). These settings are used only rarely, so you should avoid using them in Basic mode.
The difficulty is that changing "complexity modes" can be done in the tools (Color & Light, Denoise, etc.), but not (at least I don't know how to do it) in Settings.

So I've been beating around the bush. There is a choice of default complexity mode in Preferences:
"Default complexity for Selective Editing" which I replaced with "Default complexity for Selective Editing and Settings". In 'basic' mode, the number of settings in Settings is reduced and becomes complete in 'standard' or 'advanced' (I used 'options').
This is just for 'testing', I can delete it, leave it as is, but I don't know how to do it any other way (at least, me).

For "Graduated Filter is not available even with Advanced complexity", it works in many cases, and in some cases no. The mixture of the 2 "fullimage" and "global" systems is complex (see my previous change), there must remain problem situations.

Thank you

Jacques

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 22, 2024

@Lawrence37
I tried to reconstruct the absence of GF in the GUI, but maybe there are cases.
Nevertheless - and this is new because of the important changes I made to the code for GFs. The GF is only available in Full image or Global mode. Which answers the general requests to have several GFs
Getting it to work in "Normal" mode seems very difficult to me; including finding the position of the center of the GF, positioning the "reduced" GF in the preview, etc.

I change the tooltip... But maybe there are cases with GUI in Full image or Global, where the GF does not appear

But, in reality nothing prevents leaving them, except that the bug with the current solutions seems to make it difficult to use GFs in "normal" mode - especially if the spot comes out of the preview . Depending on the solution found in the long term to definitively process GFs without problems, we can restore GFs in "normal" mode...

Jacques

@Lawrence37
Copy link
Collaborator

I did not follow all of the discussion about the masking so my understanding is limited. What I did read is that people were saying the RawTherapee approach is hard to use (not necessarily complex) because it is not as flexible as masking in darktable and ART. Do you remember where you saw the comments about the "complexity" of the mask settings? I want to read them and understand why people feel that way. It is surprising to me for two reasons. First, I have heard many more people say that the masking is not "powerful" enough than those who say the options are hard to understand. Second, the options are already hidden by default. Users must click "Show additional settings" to show the transition gradient and scope settings. Don't you think it's silly for someone to ask for additional settings then complain that there are too many settings?

The graduated filter problem is a preview problem, right? The graduated filter is applied correctly in the saved image. If so, then it is a GUI problem. Removing the graduated filter from the normal mode breaks backward compatibility. Either the graduated filter must be reintroduced in normal mode, or this pull request must be moved to the 6.0 milestone.

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 23, 2024

In fact, it is not the opinion on the (idiotic) masks that is in question, but the comparison between the masks of ART/DT and Selective Editing, when you want to make adjustments to parts of an image.

The comments are diffuse and take place either on the ART forum (or that of ART-therapee in France), that of DT and sometimes - often very unpleasantly - that of RT. The approach to treat the parts of images is very different between the "negative - masks" and "positive - Spot" approaches, the victory is on the side of history - the origin of the masks by Adobe 25 years ago (which "copied" the habits of photographers and their black and white labs, as me...). Having another approach than the traditional one, beyond the technique, is a considerable cognitive challenge (I am a sociologist...)
I don't remember exactly where this comment was, there are so many negative ones, we talk about what is wrong, what is not liked - people who like it talk about it little or rarely.
But I can delete, it's easier not to talk about it, as I deleted "under pressure" a bunch of things that for me were a considerable plus (4 to 5 different auto white balances, wavelet tools, etc.). Some users regretted it and told me about it a few years ago.

GF
For the GF it is not strictly speaking a GUI problem but the problem concerns

  • the pipeline, the difference in position of GF main and GF SE.
  • the consideration of variables created by G. Horvatz in 2006 to manage dcrop / improccoordinator, simpleprocess, etc.
  • the variables used which even if they seem identical, are each time "translated" in SE to be able to manage one or more spots.
  • the code is similar, even common, but the behavior is very different.

To say that for compatibility we will keep a (very) buggy version disturbs me, nevertheless what I propose is better than nothing.

The version I'm going to commit is better than the old one, but obviously it won't do the same thing. I'm not going to bother to recreate bad behavior for compatibility reasons.
It's no problem to restore the "normal" mode, it should "more or less" behave like the old one... probably not quite and it's impossible to do a pp3... who won't know what to change....
The code I changed in this PR allows me to configure everything, I think - about ten parameters that come from "dcrop" have been introduced in the iplocallab.cc code. I've already done a lot of tests, some have improved the system (a little), others haven't.

I think the current solution (which I'm going to commit) is more than acceptable and sufficiently compatible....For me, reproducing a system that doesn't work, for "compatibility", doesn't stop the road for a second. We leave the issue open, and then continue the reflection on the GFs....

I hope no error in my last change to restore something close to the old behavior (but better)
Jacques

@Lawrence37
Copy link
Collaborator

GUI is not necessarily the code inside rtgui. There are many components inside rtengine that are a part of the GUI too. I consider dcrop to be part of the GUI. It exists solely to efficiently render a subsection of the image at a reduced resolution so that the editor can show a preview with low latency. Whenever a user saves an image, dcrop is not used at all.

My question to you is, does the graduated filter bug affect the saved image? I have not seen an example of the bug manifesting in the saved image. If it doesn't affect the saved image, then it is a GUI bug that only affects the preview. In that case, users will expect us to preserve the output. It that is not the case, then it is acceptable to change the behavior.

@Desmis
Copy link
Collaborator Author

Desmis commented Dec 27, 2024

@Lawrence37
As I say in a last PM, I have no computer for several weeks... I am in th train between Paris and Frejus where I leave
Of course we can say. It is only semantic that preview behavior with dcop is also GUI.. no matter.
No incidence for output TIF,... this part of code is separated.
Jacques

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

Successfully merging this pull request may close these issues.

3 participants