Beep6581 / RawTherapee

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

New Threshold curve widget #1251

Closed Beep6581 closed 9 years ago

Beep6581 commented 9 years ago

Originally reported on Google Code with ID 1267

As explained in some issues, a new threshold widget is required in some tools to get
better image quality.

Here is a first preview of this new Threshold widget. It's still innefective, and a
ThresholdAdjuster class still has to be created with label and reset button, but it's
rather trivial compared to the ThresholdSelector class.

For testing purpose, i've added this object to the Sharpening tool, in sharpening.cc.
Where could it really be used?

Comments about usability of this widget is welcome, the colors will be tuned later,
but it's already nice when used with "25-Gray-*". If you want make a test with a gradient
background, uncomment line 78 of sharpening.cc.

Reported by natureh.510 on 2012-03-04 00:47:51


Beep6581 commented 9 years ago
I forgot to mention about how to use it:

- click and drag a slider to modify it's position
- at anytime, you can press the SHIFT key to move both sliders of the same side
- at anytime, you can press the CONTROL key to slow down the move
- you can make the tooltip appear to see the values. This is my main problem with this
widget: where and when should i display the values? I guess that they can be displayed
by the future ThresholdAdjuster widget...

You can set in the constructor wether you want the curve to start on top then goes
to bottom, or the contrary, and you can set to have a single ou double threshold.

Actually, the ramp is linear, making it non linear would be way more complicated to
code, so i hope it will suffice :)

Reported by natureh.510 on 2012-03-04 00:59:16

Beep6581 commented 9 years ago
A second preview, now with the ThresholdAdjuster class (almost complete).

Could you point me to a slider that would advantageously replaced by this Threshold
widget? I'll try to do it so it can be used as coding example...

Reported by natureh.510 on 2012-03-06 03:02:33


Beep6581 commented 9 years ago
It would be nice to restrict any of the sharpening tools to act only on a user selected
tonal range (eg midtones).

Reported by ejm.60657 on 2012-03-06 03:50:13

Beep6581 commented 9 years ago
I wasn't so much thinking of this widget as a replacement for a slider, but rather as
a way to restrict that slider's effect.

Reported by ejm.60657 on 2012-03-06 03:51:15

Beep6581 commented 9 years ago
... so maybe i've worked for nothing then: a FlatCurveType whould have done the job
with better control, maybe slightly modified to restrict the number of point to 2 or
4. There's some kind of misunderstanding here.

Reported by natureh.510 on 2012-03-06 11:05:49

Beep6581 commented 9 years ago
I thought what was under discussion was an analog of Photoshop's 'Blend If' sliders
in the 'Options' panel...

Reported by ejm.60657 on 2012-03-06 11:26:23

Beep6581 commented 9 years ago
And the Threshold widget is precisely that, but we didn't discussed about the suitability
of this widget for the purpose you intend to use it. Maybe i misunderstood the requirements.
Anyway, it's there and (almost) ready to be used.

Can you say if this widget suits your need and eventually "please you"?

Reported by natureh.510 on 2012-03-06 13:54:07

Beep6581 commented 9 years ago
Yes, I just installed it and I think it's very nice.  I agree that the FlatCurveType
would do the job with more refined control, but it takes up too much real estate.

Current top use I would think is sharpening.  Another might be NR, though there the
gamma slider does much the same job.  Perhaps some of the other NR sliders could benefit,
such as impulse and defringe.  Though for defringe, I'm thinking we may want something
that allows a hue restriction -- say targeting red-green fringing, or purples; but
for that I suspect one would either want something more akin to a flat curve type,
or the ability to control several windows of hue (the color and its opponent color).

Reported by ejm.60657 on 2012-03-06 14:48:37

Beep6581 commented 9 years ago
Would be great to see if this helps to prevent salt pattern after the contrast by detail
levels.

Tone mapper tends to burn highlights, this tool may be useful there, however ultimately
flat curve would be safer to use in this tool to avoid rough tonal transitions.

I am away from the dev machine for some time, Hombre could you please share a screenshot?

Reported by michaelezra000 on 2012-03-06 15:13:47

Beep6581 commented 9 years ago
Perhaps if you gave an example of the 'salt' pattern, the tool itself could be improved...

Reported by ejm.60657 on 2012-03-06 15:25:42

Beep6581 commented 9 years ago
I'll try to get the file which illustrates the problem. But it should be easy to see
in exaggerated form on mid ISO images when 1-st slider is set between 2 and 3, IIRC.
This can be mostly canceled by impulse noise reduction, but latter can eat some other
image detail. the problem is that the threshold slider provides the minimum threshold
value. What is also needed here is an additional high threshold - to tame sharpening
in very high contrast areas, which turn out to be noise sometimes. This could be done
either with an additional threshold slider or the blend-if curve.

Reported by michaelezra000 on 2012-03-06 15:42:00

Beep6581 commented 9 years ago
Here is an illustration of the salt noise on one of my test images "concert" with file
name IMG_8015.CR2, I think you should have it, if not, please let me know and I will
forward.

Reported by michaelezra000 on 2012-03-08 03:30:03


Beep6581 commented 9 years ago
Hombre, I just tried the new threshold selector - looks great!
Based on the curve, these two controls should belong to the right side of the tonal
scale.

Reported by michaelezra000 on 2012-03-08 03:56:49

Beep6581 commented 9 years ago
This tool should be called Blending as it is complimentary to Threshold, since,  generally
speaking, they can be functions of different variables. For example, Blending - based
on luminance, threshold - contrast, etc.

Reported by michaelezra000 on 2012-03-08 04:03:50

Beep6581 commented 9 years ago
Yes I have that one.

Reported by ejm.60657 on 2012-03-08 04:12:50

Beep6581 commented 9 years ago
Here is an updated patch with ThresholdAdjuster used as replacement in the Sharpen tool.
I think it can produce less salt and pepper noise (in fact more pressive) with it,
but i'm not sure to have properly modified the algorithm.

It's also used in vibrance, but the algorithm has not been modified yet (i'm not able
to do it), so it only use the bottom value.

Could you check:
 - the adjuster match your needs
 - the sharpen tool algo has been properly modified
 - Jacques: can you take a look to your vibrance tool and use this threshold ajuster
like in sharpen?

I need to polish few things and remove debug outputs, but it's mostly done. It's maybe
better usable with dark themes at the moment.

Reported by natureh.510 on 2012-06-03 15:51:55

Beep6581 commented 9 years ago
Sorry, quite some patch has been committed till yesterday. Here is the updated patch
to TIP (4.0.9)

Reported by natureh.510 on 2012-06-03 17:05:58


Beep6581 commented 9 years ago
I introduced "double threshold" with "vibrance". The cursor "top" has the same functionality
as before, the cursor "bottom" differentiates the action by changing the way is calculated
the transition zone. 

I also improved transitions in cases where the differences between "pastel" and "saturated"
are important. 

In the final result provides better transitions in extreme cases   :)

Reported by jdesmis on 2012-06-04 07:45:59


Beep6581 commented 9 years ago
Hombre, thanks for this great tool. It is nice that you made it so configurable. 
I think the threshold selector for the sharpening tools should be 
0-0-ramp up-1-1-ramp down-0-0 type
This will allow to 
1. focus sharpening on areas of medium contrast 
2. remove halos on high contrast edges

Reported by michaelezra000 on 2012-06-04 23:13:35

Beep6581 commented 9 years ago
giving it a bit more thought - would it be possible to have 
0-0-ramp up-1-1-stay straight@1-1-ramp down-0-0 type:

    * * * * *
  *           *
*               *

?

Reported by michaelezra000 on 2012-06-05 03:16:17

Beep6581 commented 9 years ago
Comment #19: Merci Jacques :) I'll test it tonight. But could you clarify: does the
upper and lower cursor have different purpose or set a ramp for a single purpose?

Comment #20: Yes, this patch already handle that, there's a flag in the object's constructor
but it's not used in Vibrance nor Sharpen.

Reported by natureh.510 on 2012-06-05 09:32:53

Beep6581 commented 9 years ago
Correction: there's no flag, but a an overloaded constructor with more parameters.

Reported by natureh.510 on 2012-06-05 09:34:48

Beep6581 commented 9 years ago
 comment #21 :
The top slider has the same effect as before. The bottom slider is used to weight the
action of the upper slider to make the transition "variable"

"chromamean = (chromaSatur*limitpastelsatur + chromaPastel*limitpastelsaturbottom)/(limitpastelsaturbottom+limitpastelsatur);"

with :
 limitpastelsatur => top slider ==> "border" between "pastel" and "saturated"
 limitpastelsaturbottom => bottom slider
 chromaSatur = position of saturation slider
 chromaPastel = position of pastel slider

In conclusion, the border becomes permissive  :)

Reported by jdesmis on 2012-06-05 09:47:51

Beep6581 commented 9 years ago
Updated patch with double Threshold for the sharpening tool, including the vibrance
modification. Do you have an image or could you post screenshots on where the effect
of the double threshold is visible, compared to the single threshold version?

Reported by natureh.510 on 2012-06-05 13:48:23


Beep6581 commented 9 years ago
Thanks, will do in the evening EST

Reported by michaelezra000 on 2012-06-05 14:25:56

Beep6581 commented 9 years ago
I chose an extremely difficult case, with settings that have nothing to photography,
but that are possible. We work in shades of red :)

Reported by jdesmis on 2012-06-05 16:16:52


Beep6581 commented 9 years ago
Jacques, nice, much smoother.

Hombre, I found that the threshold range was too extreme and hard to use in the UI,
so I experimented with this in sharpening.cc line 76:
   threshold = Gtk::manage (new ThresholdAdjuster (M("TP_SHARPENING_THRESHOLD"), 0.,
2000., 20., 80., 2000., 1200., 0, false));

Here is illustration:
http://www.michaelezra.com/Projects/RT/images_shared/Threshold_11.jpg

It appears that right-side threshold does not provide smooth falloff - there is no
visible effect when bottom right slider is moved. Easy test - boost sharpening amount
to max, set top right threshold to 0, move bottom right slider - no effect.

There is a small bug in history string for the threshold in sharpening.
it shows bl, tl, blank,tr

Reported by michaelezra000 on 2012-06-06 03:02:55

Beep6581 commented 9 years ago
There was a mistake in the way that the Threshold object (double threshold) were used,
here is a correct version, but you should try with the former range aswell, it may
be working now.

Let's decide about the range of the Sharpening threshold selector, then i'll be able
to commit. I'll also have to update the bundled profiles, let me know which values
to use, or do you want to do that yourself in an upcoming patch?

Reported by natureh.510 on 2012-06-11 19:29:46


Beep6581 commented 9 years ago
Thanks, I will try in the evening.
About the range - do you know what is the meaning of the values used in the thresholds?
Once we understand the meaning we could come up with the suitable values.

Reported by michaelezra000 on 2012-06-11 19:41:29

Beep6581 commented 9 years ago
Here is illustration:
http://www.michaelezra.com/Projects/RT/images_shared/Threshold_12.jpg

Works great! The right threshold allows to control the highlights overs-harpening,
so I do think we need to thresholds here.

Reported by michaelezra000 on 2012-06-12 01:19:24

Beep6581 commented 9 years ago
and how cool it would be for RL deconvolution and contrast by detail levels!;)

Reported by michaelezra000 on 2012-06-12 03:24:14

Beep6581 commented 9 years ago
It was simple to enhance the threshold parameter of the USM sharpening tool, but RL
deconvolution and contrast by detail level don't have threshold cursor actually.

I'll try to polish the colors of the tool, then i'll b able to commit tonight.

Reported by natureh.510 on 2012-06-12 15:42:26

Beep6581 commented 9 years ago
Works great!

A tooltip briefly explaining what the x and y axes are would be good.

Reported by entertheyoni on 2012-06-12 23:55:05

Beep6581 commented 9 years ago
The colors look fine here, tried all themes + system one.
http://i.imgur.com/DygZc.png

Reported by entertheyoni on 2012-06-13 00:30:31

Beep6581 commented 9 years ago
DrSlony, I am curious, what is the theme in the top image in the screenshot (comment
34)? I like the slider.

Reported by michaelezra000 on 2012-06-13 00:41:34

Beep6581 commented 9 years ago
System default, it's called SLAVE.
http://browse.deviantart.com/?q=gallery%3Ahalf-left%2F16719304#/d48mtvn

Reported by entertheyoni on 2012-06-13 00:56:24

Beep6581 commented 9 years ago
It's the same one I used in the screenshot on our front page:
http://rawtherapee.com/images/screenshots/rt409_lago_di_braies.jpg

Reported by entertheyoni on 2012-06-13 00:57:23

Beep6581 commented 9 years ago
Could we distribute it with RT?
It is not a system default on Windows or Mac:)

Reported by michaelezra000 on 2012-06-13 01:24:47

Beep6581 commented 9 years ago
I am getting a color mismatch around the widget:
http://i.imgur.com/IVnQ7.png
I'm using my system gtk2 theme (oxygen-gtk). Works fine with the system "Raleigh" theme.

Also, I couldn't get the patch to work without first adding "#include <cmath>" otherwise
I kept getting:
error: 'fabs' was not declared in this scope

Applied to 4.0.9.14, 4d0ab8859c59

Reported by sankeytms on 2012-06-13 03:25:05

Beep6581 commented 9 years ago
sankeytms: could you tell which file you've modified? I didn't had any issue here, but
there's no harm in adding this line.

About the rendering, i'm experimentating with Gtk theming, and will try to use standard
painting methods instead of self made, but i may loose the inner color shade. I'll
also need some linux users to test with system themes tonight.

Anyway, it's fully functional and can be committed as is if you want. About the tooltip,
there's a lot of control that are only explained in the documentation.

Reported by natureh.510 on 2012-06-13 12:18:41

Beep6581 commented 9 years ago
Hombre, one thing that could be mentioned in the tool-tip as a text markup - 
"Hint: Hold the Shift key to move points independently"

P.S. I like the gradient shading:)

Reported by michaelezra000 on 2012-06-13 13:38:36

Beep6581 commented 9 years ago
Gradient shading should be a keeper. :)

Reported by gyurko.david@e-arc.hu on 2012-06-13 17:15:17

Beep6581 commented 9 years ago
@ comment 40: added include to rtengine/procparams.h

Reported by sankeytms on 2012-06-13 17:38:58

Beep6581 commented 9 years ago
I'll be available to test, catch me on IRC in 2 hours.

I agree with Michael, except to be consistent I'd put the actual key in bold, and I
wouldn't call it a hint, so:
[coordinates]
Hold the Shift key to move individual control points.

Reported by entertheyoni on 2012-06-13 17:46:58

Beep6581 commented 9 years ago
sounds good:)

Reported by michaelezra000 on 2012-06-13 18:44:37

Beep6581 commented 9 years ago
Here is my final version o fthe patch. I hope that it will finely handle the system
Linux themes.

Reported by natureh.510 on 2012-06-17 15:12:04


Beep6581 commented 9 years ago
Michael, could you propose some default values for the bundled profiles for this 2 ThresholdAdjuster
(in Sharpen and Vibrance)? Post them here for testing or send them to my email so i'll
add them to the comitted patch.

If nobody complain, i'll commit it tonight.

Reported by natureh.510 on 2012-06-17 15:15:05

Beep6581 commented 9 years ago
Hombe, I am testing patch 14, quick feedback - the value in History is still missing
the third item (see the end of comment 27). Will give more feedback in a few hours..

Reported by michaelezra000 on 2012-06-17 17:57:07

Beep6581 commented 9 years ago
I have no idea why, but it still doesn't work without #include <cmath> in rtengine/procparams.h.

Also, it still doesn't work with my theme!
http://i.imgur.com/6Ctw9.png
I'm using kde-gtk-config (as recommended by DrSlony) to get some gtk themes in kde.
I'll try other gtk themes later today.

Reported by sankeytms on 2012-06-17 18:40:54

Beep6581 commented 9 years ago
one more issue: 
Change thresholds, history changes
then press reset - history does not show a change in threshold values

Reported by michaelezra000 on 2012-06-17 18:57:31