search

Beep6581 / RawTherapee

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

Display curve node input/output values #2593

Closed Beep6581 closed 8 years ago

Beep6581 commented 8 years ago

Originally reported on Google Code with ID 2610

I think the Curves Tool (Exposure Panel, Lab Adjustments, etc) need a textboxes with
the input-output values of each node of the curve. This is a very useful improvement
for a fine curve design when we work with step wedges o densitometric scales, where
we work with numeric values as a output and the eyedropper for pick input values from
the image.

This textboxes are a common tool in the most raw developers and image editors because
are very useful (may be imperative) for a fine tunning or design of tone response curves.

Thanks

Reported by info@jpereira.net on 2014-12-18 00:36:25

Beep6581 commented 8 years ago

Reported by entertheyoni on 2014-12-18 09:56:37

Beep6581 commented 8 years ago

Reported by natureh.510 on 2015-02-22 09:16:58

Beep6581 commented 8 years ago 
This issue seems very important to José ;), I'll add direct value edition asap (in March
i'd say).

Reported by natureh.510 on 2015-02-22 09:19:27

Beep6581 commented 8 years ago 
Here is the first patch that add numerical edition of the points.

At the moment, the range is [0;1] for all curve type, and all axis. We could quite
easily add the range management (contextual to each curve), but one should review all
curve's in RT to get the X&Y range of each one, and I'm not the right guy for that.

How to use that feature (see the screenshot):
1. Click on the "Edit" toggle button
2. Lit one point by moving your mouse over it and press Enter. This will freeze the
Editor and enable the input values (they are Read Only otherwise)
3. Edit your values
4. Click on the curve and press Enter again to disable the input buttons and enable
the Editor again

I chose the Enter key to switch to the Edit mode because of tablet users (Wacom ones),
but a better solution would be to use the right mouse button (Michael, any objection?
 I know that you're still annoyed with the curves when using your pen tablet, but I
don't have one to test...)

Secondly, when editing a flat curve, the 4 spin buttons take a lot of space: should
we put them on 2 lines (it would take more space, but we'd be able to narrow the Editor's
width.

Reported by natureh.510 on 2015-04-03 00:26:58

Beep6581 commented 8 years ago 
Regarding the collision: I think that I will first make new scale choice and solve related
gamma problems and only atfer that I will code the discussed modifier.

>I'll try to add the modifier key in the patch from issue 2593, but it will be available
for the Custom type only, it doesn't make sens for the other types.

Could you please rephrase it? What is "custom type" and why does discussed modifier
make sense for it exclusively?

Reported by pinhuer on 2015-04-03 21:48:59

Beep6581 commented 8 years ago 
Hello: Looks good, but the input/output values shall be in accordance with the kind
of curve. For example, for a Lab curve, the I/O values will be L*, a* or b*, or if
the curve is in HSV space, the I/O values will be H, S or V, etc....
The idea is get the input value from image, with the pipette tool, and the output value
is set with the user criteria.
For me, this values, are very important in "Tone Curve" at Exposure tab, or "Lab Adjustments"
because is the right way for design a precise tone curves with densitometric step wedges.

I made a small webcast for show the workflow with ACR and Lightroom:
https://dl.dropboxusercontent.com/u/10481088/tonal-curves.mp4

Thanks for your time!

jose pereira

Reported by info@jpereira.net on 2015-04-03 22:28:44

Beep6581 commented 8 years ago 
Jose, in the example from your video, the range of each axis depend on the bit depth
used by the software (i.e. usually 0-255). We're using float values instead of integers
in RT, and I'm not sure that you want to see a range from 0 to 65535... So I think
that 0 to 100% will be fine and for most cases. The labels will be updated, but I don't
think that it's really necessary, if you understand the curve you're using.

Reported by natureh.510 on 2015-04-05 00:44:10

Beep6581 commented 8 years ago 
Hello:

Finally I recompiled RT with your patch, and I can test this improvement:

As you said, a 0-100% range it is a good idea, because matches with Lab o HSB range.
But now it is on a range 0-1 and it is not very representative of colorspace where
we are working. Work with RGB values as Adobe Camera Raw it is not useful because the
calculus it is less clear. 

Perhaps it is not necessary other new tool at the curves panel for activate and work
with this I/O textboxes, with the pipette tool is enough. This textboxes should be
a aid for set the output values when we work with the pipette tool.

The workflow should be: with the pipette tool pick up a sample over a image, create
a node on the curve and load the input values on the textbox (x). Then the user could
move the node or set the output value (y) with the keyboard, etc.

The input (x) values on the textboxes must be synchronized with the pipette tool.

The input value (x), must be equal at Lab color samples from Navigator panel. I think
it is necessary to know the gamma of workspace (1.8 or 2.2) and must be apply to input
(x) textbox. See attached image.

Perhaps it is more suitable to name to the textboxes with "input" (x value) and "output"
(y value).

I have tried to set an output value on the small arrows from the "y" textbox but the
RT crash, using RT over Ubuntu 14.04.

Other question, perhaps in relation to the curves panel usability it is the possibility
to move the nodes over the curve with the keyboard arrows, because it is more accurate
than drag with the mouse.

Thanks!

Reported by info@jpereira.net on 2015-04-05 14:47:10

Beep6581 commented 8 years ago 
Will I be able to use this to match Lab values of color chart patches in RT to measured
Lab values some external reference image?

Reported by entertheyoni on 2015-04-09 18:30:01

Beep6581 commented 8 years ago 
Yes, of course, this is the main idea, but work with color patches is a very hard work,
for a accurate chromaticity is better to use some kind of color profile. This method
is usefull for a accurate tonal adjustment or achromatic adjustment thanks to use of
step wedge o densitometric scales in our scene and the L* values, but it is completely
possible to work with color patches.

Reported by info@jpereira.net on 2015-04-09 20:41:24

Beep6581 commented 8 years ago 
José, the next path will let you add a point on the curve, like it already does, and
let you see the in/out values in the spin buttons while you're adjusting the new point.

Once created, you'll be able to use the new mechanism, i.e. edit that new point, click
in the out spin button, and either click on the arrows to move the point up or down,
or use the keyboard arrows or page up/down key to move the point up/down.

When moving the pointer over the image in Pipette mode, the spin buttons now reflects
the in/out value, live!

About your image in comment #8, I guess that you used the wrong curve for that example,
the L curve of the Lab adjustment tool would have been better, but I understand your
point. For the Exposure's curve, the "in" value is the mean of R, G and B value, not
the L channel.

Reported by natureh.510 on 2015-04-09 22:09:51

Beep6581 commented 8 years ago 
Jose yes, I am trying to match RawTherapee's output to a TIFF in the Adobe RGB space
saved using ACR, so apart for the DCP I will also probably need to use L*a*b* curves
to match ACR's baseline curve + other hidden adjustments. Matching color chart patch
values should be a good way of going about this.

Hombre good work!
Will these in/out values respect the "Use working profile for main histogram/Navigator"
Preferences setting? I guess for my needs I would need the values to represent those
in the output space (right?).

Reported by entertheyoni on 2015-04-09 22:44:28

Beep6581 commented 8 years ago 
The curve is applied on the image during the processing, and the "in" values are from
the image right before applying the curve, so I guess that it is in the Working profile
space, and possible in a different color space.

Reported by natureh.510 on 2015-04-09 22:52:42

Beep6581 commented 8 years ago 
I'd still appreciate a comment from Michael about the use of the right mouse button
(preferably) or the Enter key (actual solution), regarding the Pen tablet device.

Reported by natureh.510 on 2015-04-09 22:54:15

Beep6581 commented 8 years ago 
Hi natureh.510: I am agree with your workflow.
The "Exposure curve" depends the color space, may be a L* (Lab), B (HSB) or RGB. If
it is RGB, the curve is not equal to mean of R, G and B values, it is the weighted
mean of RGB values according to this equation:
Y’ = 0.2126*R’+0.7152*G’+0.0722*B’
Where Y' is the "luma" according to standard Rec.709
Yes! the best option it is working with L* than working with RGB. In the comment #8
I use the L* values not the RGB. I have compared the measured L* values at image and
compare with the values from textboxes, and I note that perhaps you need apply a gamma
correction to your textboxes values, but the gamma depends, I think, from workspace
ICC Color Profile (1.8 if the choice is Prophoto or 2.2 if others)

Reported by info@jpereira.net on 2015-04-09 22:56:59

Beep6581 commented 8 years ago 
Hi Hombre, sorry for the delayed response! I will try it out and report.

Reported by michaelezra000 on 2015-04-09 23:22:44

Beep6581 commented 8 years ago 
The Navigator shows you the value at the end of the pipeline, after conversion for the
display 5IIRC), it's not the same space and profile than the internal pipeline, so
spin buttons will reflect that internal value only, gamma or anything else won't be
handled.

Reported by natureh.510 on 2015-04-09 23:23:54

Beep6581 commented 8 years ago 
Hombre, here is my feedback:
I cannot report anything on tablet usage as my Intuos Wacom tablet does not work with
RT curves at all, its the old issue though. 

About the new functionality. It would be more seamless and intuitive user experience
if it would work a bit different:

A. When 1 point on a curve is selected 
1. point stays red until 
   a)user clicks away from the curve or 
   b) user selects another point or 
   c) user adds a new point
2. the numeric input boxes are enabled for entry.
3. when numeric values are changed, the point moves (as is)

B. When 0 points are selected 
1. if mouse is over the curve area the input boxes should be greyed out, not allowing
data entry, 
2. if mouse is outside the curve area the input boxes should get hidden.

The advantage of this approach is that users don't need explanation that Enter key
needs to be pressed (which is not very intuitive). However, this may be a significant
time overhead to implement and this is a niche functionality, thus instructing on pressing
the Enter key is probably acceptable.

A small bug report: enable the input boxes. click inside the input box and use scrolling
wheel to change the value. RT freeses and then crashes.

Reported by michaelezra000 on 2015-04-10 02:19:58

Beep6581 commented 8 years ago 
and one more thing: thanks for coding this feature:)

Reported by michaelezra000 on 2015-04-10 02:21:48

Beep6581 commented 8 years ago 
Thanks for the review Michael. However point #A.1 needs a dramatic code change than
using the Enter key or the right mouse button. To make things more intuitive and simpler,
we could go that way:
- Right click over a lit point to edit the values numericaly.
- Right click over another point to edit that new point.
- Right click over the free space to end the numerical edition process.

A tooltip over the Edit button would say just that, making things more intuitive.

Just for my information: if you were using your pen tablet, would you still be able
to use a right mouse button?

This patch will be finished soon, it took longer than expected :(

Reported by natureh.510 on 2015-04-10 07:56:21

Beep6581 commented 8 years ago 
The right click would make it more seamless and consistent with right-clicking in preview
area to "click away" from the selected tool. Wacom pen does have a button and it can
be assigned a right-click, depending on user preferences for the pen, so it should
be ok.

Reported by michaelezra000 on 2015-04-10 11:48:37

Beep6581 commented 8 years ago 
Here is the second version. Now it works a bit differently:
1. Enable the point edition by clicking on the dedicated button in the curve's toolbar
2. right click on a point to edit that point. The spin buttons become sensitive, the
first one get the focus and the edited spot has a special circle+disc icon
3. Edit the points position by editing the values and pressing 'Enter' or 'Tab'. For
periodic flat curves, you're not allowed to cross the left or right border. That's
something we could add in the future
4. You can edit another point by right clicking on that other point.
4. When done, right click in the graph but away from any point. The spin buttons become
insensitive again, even if their display looks almost the same (this has been set in
the theme files and could be changed easily).

Special case:
- switching to another curve end the point edition. The spin buttons remains displayed.
- when editing a point, the pipette won't update the spin buttons and you won't be
able to add or move a point from the preview. You have to stop the point edition first.
- when hiding the spin buttons by clicking on the Edit point button again, the point
editions stops.

Tips:
- like any spin buttons, you can use the mouse wheel over the up/down arrows to increase
or decrease the value. You can also use the up/down and page up/down key to change
the values with different increment.

I hope to have addressed any possible case, but you may prove me wrong :)

Reported by natureh.510 on 2015-04-29 20:55:45

Beep6581 commented 8 years ago 
I'll commit tomorrow afternoon (in 24h) if no objections.

Reported by natureh.510 on 2015-05-02 16:50:22

Beep6581 commented 8 years ago 
I like it!
Quick remarks:

3- "Edit the points position by editing the values and pressing 'Enter' or 'Tab'."
Enter does nothing here.

5- Right-clicking on a point currently does nothing. Maybe it could trigger showing
the in/out boxes, just as clicking on the icon does?

6- The language strings are missing http://i.imgur.com/y9SYnn9.png
Please keep them as short as possible otherwise I have to make the panel that ridiculous
width. If possible, CURVEEDITOR_AXIS_X=In and CURVEEDITOR_AXIS_X=Out, or if that's
too long then just X and Y, though In and Out are more clear to the user.

7- A tooltip over the button would be helpful, e.g. "Enable edition of node in/out
values.\n\nRight-click on a node to select it.\nRight-click on empty space to de-select
the node." No tooltips on the input boxes, they are intuitive.

Reported by entertheyoni on 2015-05-03 01:16:48

Beep6581 commented 8 years ago 
ok, looks great!
Just remember the importance of that the input values must match with the L* values
of our image, measure with the CIELab color samplers (from Navigation pane) for example.
The L* value for the Tone Curve at Exposure Pane. But for example at L*a*b* Adjustment
pane if you are edit the curve for a*, the input value must match with a* value from
image, etc...
Great job, I am excited to test it!

Reported by info@jpereira.net on 2015-05-03 09:40:14

Beep6581 commented 8 years ago 
A good complement to this feature would be the ability to select a point in the preview
for which the Navigator should show the values regardless of where the mouse is, so
I could use the pipette to sample a spot, then keep the Navigator reporting that spot's
values while I tweak the curve either using the mouse or the input boxes.

Reported by entertheyoni on 2015-05-03 10:31:24

Beep6581 commented 8 years ago 
Re #25: see comment #17 please. Furthermore, in reply to #15, the tone curve is applied
to the R, G and B channels independently, which introduce color shift for all but pure
red, pure green, pure blue hues, and grey color. That why I chosen to use the mean
value of the R, G and B channels for the pipette. There's no luma channel here.

What you definately want is to use the L* curve from the Lab tool.

And I'm affraid that it won't be possible to show the navigator's L value in any spin
button, for the reason explained in comment #17. You'll have to make the gamma conversion
yourself, or wait that someone will implement the suggestion from #26, which would
have sufficed you.

Reported by natureh.510 on 2015-05-03 23:36:27

Beep6581 commented 8 years ago 
Re #24:
3- Enter works fine in Win7, so it may be platform specific? Which is strange given
that it's handled by Gtk.

5- Yeah, I've forgot to says that the spin buttons have to be visible. Adding your
suggestion makes big changes, since the spin buttons are created when hitting the Edit
button for the first time. So I'd like to commit as is.

Here is the latest version of the patch, with the requested changes.

Reported by natureh.510 on 2015-05-05 20:21:16

Beep6581 commented 8 years ago 
All fine here, <enter> works now too. Thanks!

#24.5 - could right-clicking a point not be wired-up to send the same signal as clicking
on the edit button which shows the spinboxes does? Forget it if its much work, it's
just a tiny usability improvement.

Reported by entertheyoni on 2015-05-06 06:26:00

Beep6581 commented 8 years ago 
Works fine here, Thanks Hombre!

There's a printf left over in mydiagonalcurve.cc line 925

+ 1 for #29 #24.5

Ingo

Reported by heckflosse@i-weyrich.de on 2015-05-06 12:11:09

Beep6581 commented 8 years ago 
Sorry, but I don't understand: #29 and #24.5 seems contradictory

Reported by natureh.510 on 2015-05-06 17:13:40

Beep6581 commented 8 years ago 
ah do you mean the "not be"? It's English grammar... I should have just written without
the "not", to be clearer.

In both of the above comments I am suggesting that right-clicking a node point send
this signal
editPointCustom->signal_toggled().connect( sigc::bind(sigc::mem_fun(*this, &DiagonalCurveEditorSubGroup::editPointToggled),
editPointCustom) );
and activate that node for edition.

Reported by entertheyoni on 2015-05-06 17:59:56

Beep6581 commented 8 years ago 
Hombre, + 1 for #32

Reported by heckflosse@i-weyrich.de on 2015-05-06 18:30:01

Beep6581 commented 8 years ago 
Okay, I can do that if you don't mind having the spin buttons in the object tree even
if not displayed. I just wished to avoid bloated GUI for those who don't need point
edition, but that shouldn't make a big difference...

Reported by natureh.510 on 2015-05-06 18:51:22

Beep6581 commented 8 years ago 
No I think I can commit...

Reported by natureh.510 on 2015-05-08 23:15:44

Beep6581 commented 8 years ago 
Hello: I just recompiled my RT, and it is my first impressions:
1.- The relationship between the pipette tool and the input textbox run fine.
2.- At the Lab Adjustment panel when we work at L* curve, the input values match with
L* values of our image. It is great, and it is just we need for work with step wedge
charts.
3.- The right-click for fine point adjustment with the spin buttons, also work fine
and it is useful
But:
4.- At the same way that we pick up the L* values above our image, I think also is
necessary get the a* and b* values. For my workflows the a* and b* are not useful but
for coherence, the behavior of this atributes must be the same behavior than L* atribute.
5.- The same concept for curves at Exposure panel. His input values is not referred
to L* atribute, and perhaps this is not consistent with the behavior of Lab Adjustement
panel. I think that the input values must be referred to some color atribute, on the
other hand these are not useful. And if the behavior or input values is different 
through differents panels this is very confused.
6.- Work with Ubuntu 14.04 I find some errors work with the pipete tool and the right-click
for edit a point. Perhaps the most clear option it is disable the spin buttons when
pick up a sample with the pipete tool.

Good Job!

Reported by info@jpereira.net on 2015-05-09 23:20:00

Beep6581 commented 8 years ago 
Hombre, this is an excellent patch, thank you!

Reported by michaelezra000 on 2015-05-14 13:41:54

Beep6581 commented 8 years ago 
Hombre, this is an excellent patch, thank you!

Reported by michaelezra000 on 2015-05-14 13:41:59

Beep6581 commented 8 years ago 
José, as already told, the tone curves doesn't work on the Luminance channel. But maybe
it's something that RT is missing, so I've added a new Luminance mode for the tone
curve, base on Rec.709. I hope that it's fine, it can produce weird result if misused.
Note that the histogram is still the initial one.

Reported by natureh.510 on 2015-05-17 19:56:11

Beep6581 commented 8 years ago 
Hello natureh.510, perhaps I not explain right with the concepts. I don't say that it
is necessary work with luminance, lightness or brightness, I only say that it is mandatory
that the input values are referred to some color attribute of our loaded image and
they will be coherent with the panels. If we can not correlate input values with image
values, work with numerical values at curves panels is not usefull. This concept is
very common in several image editors.

I just test the last patch, with the luminance option at Exposure panel, and run fine.
With the improvements at Lab panel, and with the luminance adjustments at Exposure
panel, this work is for me a great improvement for RT.

One more time, natureh.510 thanks for your time!

Reported by info@jpereira.net on 2015-05-19 23:12:11

Beep6581 commented 8 years ago 
Hi José, I'm glad that it fulfils your requirements. Could you provide an example of
the result you can get with your method? And if possible give a link to a page that
explains it.

Reported by natureh.510 on 2015-05-20 19:57:56

Beep6581 commented 8 years ago 
Patch committed. There may still be bugs, but I'll correct them later if you find one,
I just want to benefit from the new Luminance tone curve mode. I think it's pretty
stable too.

Reported by natureh.510 on 2015-05-20 22:09:47

Beep6581 commented 8 years ago 
Hombre do you know of any new tricks that can be done with the Luminance curve mode?

Reported by entertheyoni on 2015-05-30 15:43:36

Beep6581 commented 8 years ago 
No, this new mode works best with an S-shape curve, otherwise produce unrealistic results.

Could you add this new curve mode and the new edit button to rawpedia for me please?

And flag as FIXED if you think it's fixed. (it is, for me)

Reported by natureh.510 on 2015-05-30 21:09:40

Beep6581 commented 8 years ago 
Yes, but please tell me more about it - summarize how it works, whether it operates
in the RGB color space, perhaps how it differs from the lightness curve L* in L*a*b*
space, and anything else that comes to mind.
http://rawpedia.rawtherapee.com/Exposure#Curve_Mode

Reported by entertheyoni on 2015-05-31 09:27:13

Beep6581 commented 8 years ago 
I will try to make as soon as possible as small screencast with the procedures for to
work with these improvements and steep wedge charts. My english it not right for write
a good documentation, but perhaps I can help with the concepts.

Reported by info@jpereira.net on 2015-05-31 13:59:48

Beep6581 commented 8 years ago 
The only thing is that despite you still see the R, G and B histogram (merged) on the
background, the cure operate on luminance, where
Relative Luminance Y = R*0.2126729 + G*0.7151521 + B*0.0721750 ( https://en.wikipedia.org/wiki/Relative_luminance
).

We first get the Relative Luminance value of the point, apply the curve to that value,
calculate the multiplication factor between before and after luminance, and then apply
this factor to each R, G and B component, compared to applying the curve to each component
separately for the other methods.

See rtengine/curves.h, line 682 for exact formula. I don't pretend that this formula
correspond to something conventional though.

Reported by natureh.510 on 2015-05-31 14:07:57

Beep6581 commented 8 years ago 
Each component of the pixel is boosted by the same factor, so it should theoretically
lead to less color shift, but also to less chroma boost as well and give a more neutral
result.

Reported by natureh.510 on 2015-05-31 14:11:43

Beep6581 commented 8 years ago 
Ok, just give me a few days to play around with it first :]

Reported by entertheyoni on 2015-05-31 21:02:05

Beep6581 commented 8 years ago 
I added this:
http://rawpedia.rawtherapee.com/General_Comments_About_Some_Toolbox_Widgets#Curve_Node_In.2FOut_Value

I'm still not happy with it because the explanation doesn't explain anything in practical
terms. If you can, please show how this is useful using a real-life example.

Reported by entertheyoni on 2015-06-08 16:17:22