Flat curve editor

[Beep6581]

Originally reported on Google Code with ID 519

Some tools needs a Flat Curve Editor, like the HSV equalizer, for better control over
the modification than an equalizer.

The priority is High, because i'd like to make it for the upcoming release (it will
be very hard thoug...).

Reported by natureh.510 on 2011-02-02 00:56:49

[Beep6581]

Reported by natureh.510 on 2011-02-08 10:42:01

• Status changed: Started

[Beep6581]

Reported by natureh.510 on 2011-02-20 23:14:13

• Labels added: Milestone-3.0-b1
• Labels removed: Milestone-3.0-b2

[Beep6581]

Here is a first patch to test this new functionnality. It's not completly stable, especially
when closing the software after playing with the HSV tool in the Batch Tool panel...

The tangential handles are not visible yet, and thus can't be modified, but everything
is in place in the backend.

It has to be applied to the last revision of Branch3 (changset #8466eb48c1).

Emil, could you look at your HSV tool to check my modifications ?

Reported by natureh.510 on 2011-03-06 01:29:57

---

• Attachment: FlatCurveEditor-V1.patch

[Beep6581]

I forgot to mention : commit occured during the development of this function, they are
not applied in this patch but i'm doing it right now.

Reported by natureh.510 on 2011-03-06 10:14:47

[Beep6581]

I'll have a look.

Reported by ejm.60657 on 2011-03-06 13:29:58

[Beep6581]

I need some relevant files:

-- Configuring done
CMake Error at rtengine/CMakeLists.txt:20 (add_library):
  Cannot find source file "flatcurves.cc".  Tried extensions .c .C .c++ .cc
  .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx

CMake Error in rtengine/CMakeLists.txt:
  Cannot find source file "diagonalcurves.cc".  Tried extensions .c .C .c++
  .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp .hxx .in .txx

Reported by ejm.60657 on 2011-03-06 13:57:12

[Beep6581]

I presume I may need the associated header files as well...

Reported by ejm.60657 on 2011-03-06 13:58:14

[Beep6581]

There no specific header file. Here is the complete patch, including the missing files.

I've checked the different commit that occured during the development of this patch,
and it seems that this patch is up-to-date, but don't blame me if i've missed something
:-/, i've done my best to keep the synch.

Reported by natureh.510 on 2011-03-06 14:20:12

---

• Attachment: FlatCurveEditor-V1b.patch

[Beep6581]

Compiles OK; I get a 

Program received signal:  “EXC_BAD_ACCESS”.

at line 62 of curveeditor.cc when it tries to display the flat curve...

Reported by ejm.60657 on 2011-03-06 15:19:32

[Beep6581]

The png files does not seem to be part of the patch :-/ I really need to look at how
to create a patch with binary files inside...

Here are the missing icon files that have to be copied to rtdata/images. Gtk throw
an exception if they are not there. I guess that it's what happenned.

Reported by natureh.510 on 2011-03-06 15:32:45

---

• Attachment: flatCurveIcons.zip

[Beep6581]

Nope; same error.  The zip file created a flatCurveIcons folder within rtdata/images,
I also copied the contents directly to rtdata/images, still I get the same error.

Reported by ejm.60657 on 2011-03-06 15:46:59

[Beep6581]

You have to run CMake again to take them into account in the "install" process. You
still can copy them manually to their destination folder to avoid that for the moment.

Reported by natureh.510 on 2011-03-06 15:49:23

[Beep6581]

Now I have a slightly different error (after re-cmake); still EXC_BAD_ACCESS, but it
is in popupcommon.cc line 134 (being called from popuptogglebutton.cc, line 35, called
from flatcurveeditor.cc, line 62).

Reported by ejm.60657 on 2011-03-06 16:05:52

[Beep6581]

flatcurveeditor.cc doesn't exist. I guess you're talking about curveeditor.cc

I also presume that you checked that the png files are in the RT/images/ destination
folder ? And you applied the patch above Branch3

I still think that there's something wrong with those png files. Do they have the right
filenames ? Did you deleted the flatCurveIcons subfolder before running CMake (yes,
the png files have to be placed in /images/) ?

Could it be related to access rights ?

Reported by natureh.510 on 2011-03-06 16:42:32

[Beep6581]

Sorry, my bad -- I forgot that my IDE doesn't put icons and language strings in the
destination folder; I have to first do a command line make install, and then go back
to the IDE.  All is well now that I have done that.  I'll play a little and report
back...

Reported by ejm.60657 on 2011-03-06 17:09:07

[Beep6581]

It seems to be working for me.  Pretty cool :D

I actually wanted all three curves to be flat curves, so that the user can adjust H,S,V
as a function of V.  The default should be flat, ie output = input.  The usage of the
curve is to denote changes in the affected quantity as a function of H (as opposed
to the usual curve type, which plots eg output L as a function of input L).  Since
the vertical axis in general is not denoting the same quantity as the horizontal axis,
the usual curve type doesn't make much sense which is why we need the flat curve editor.

Eventually it might be nice to have the horizontal and vertical axes user-selectable,
so that there are say three copies of the tool, then the user can choose the vertical
and horizontal axis for each, eg choose the vertical axis to be H and the horizontal
axis to be V instead of the other way around and one is adjusting for instance color
casts in shadows instead of darkening/lightening a particular color.  But that's not
for 3.0.

I'll have to look into what you've done for the H vs H curve and see how it's working.
 Then I think we'll want to make the other two work the same way.

A user-friendliness question: Would it be possible to overlay a rainbow (hue gradient)
along the bottom, so that the user can see what hues are being adjusted?

Reported by ejm.60657 on 2011-03-06 17:27:00

[Beep6581]

Tha flat curve editor now open the door to new curve adjustments, and your idea of selecting
the axis is very onteresting, we could plan to do it for 3.1, without any limitation
on the number of curve.

My revised implementation of the Saturation and Value curve (i.e. as a function of
themselves) can also be keept somewhere else, but it's not in the freeze, so maybe
for 3.1. Onovio offers them along the one you are talking about, and it can be interesting
for some people.

I've also reorganized the code to be able to create new curve types more easilly.

"I actually wanted all three curves to be flat curves, so that the user can adjust
H,S,V as a function of V."

You meant as a function of H, do you ?

For the user friendlyness, i've planned to do much more than that, but i'm conentrating
at providing the full control over the curve first so we can go to RT Beta1, then i'll
add the decorative yet usefull matter.

Reported by natureh.510 on 2011-03-06 17:54:34

[Beep6581]

Here's a patch to do what I intended (to be applied on top of Hombre's):

Reported by ejm.60657 on 2011-03-06 18:01:00

---

• _Attachment: HSV_allflatcurves.patch_

[Beep6581]

I doesn't apply above mine, but instead of mine. I'll look in the diff file for your
modifications.

Reported by natureh.510 on 2011-03-06 18:11:10

[Beep6581]

I could apply FlatCurveEditor-V1b.patch  without any problem, but all the files got
rejected when I tried to apply HSV_allflatcurves.patch (the output was massive...).

First I used hg import patchfilename --no-commit, but commit was needed so I committed
the first one to my local repo.
Is there any trick to apply two patches after each other?

Reported by gyurko.david@e-arc.hu on 2011-03-06 20:22:16

[Beep6581]

As Hombre said, I was mistaken -- HSV_allflatcurves.patch includes the changes in FlatCurveEditor-V1b.patch,
rather than being applied on top of it.

Reported by ejm.60657 on 2011-03-06 21:39:15

[Beep6581]

The Value editor doesn't seem to have any effect : does this line (413 ?) of improcfun.cc
really have to use "s" ?

valparam *= (1-SQR(SQR(1-s)));

Reported by natureh.510 on 2011-03-06 23:51:26

[Beep6581]

Please replace the preceding line

float valparam = 0.01*(vCurve->getVal((double)h)-0.5);

by

float valparam = (vCurve->getVal((double)h)-0.5);

ie delete the multiplier 0.01; this should fix the problem.

Reported by ejm.60657 on 2011-03-07 00:13:06

[Beep6581]

Would it be possible to add a standard spline flat curve?  I am thinking this curve
editor will be useful in a number of situations, for instance I am thinking of adding
one for the NR tool, so that one can more accurately tune the NR as a function of luminance;
but the style of the current flat curve tool is not well adapted to that -- for this
and many other applications, one wants a curve that is not restricted to having the
control points being maxima or minima, but rather to simply have the curve pass through
the point and join smoothly to the other points.  It might also be helpful to have
the option for the curve to be periodic or not, depending on the application.

Reported by ejm.60657 on 2011-03-07 14:31:16

[Beep6581]

I know nothing about the Spline curve type and its equations, but i can do a NURBS curve
type (i.e. "control cage") if you want. Optional periodicity can also be done without
too much effort.

My priority is to finish this curve type, then what should be the priority : adding
useful decoration on this curve type, or create your requested curve type ?

By the way, do you find my new curve type based on maxima/minima really useful, or
does a regular "Cage control" curve type be a best choice ? I'll keep it anyway, since
i've already done more than 50% of the job, but i'd like to know.

Reported by natureh.510 on 2011-03-07 14:41:50

[Beep6581]

I haven't played with it enough to form an opinion.  It looks like a variant of the
cosine-based interpolation of the HSV sliders.  I suspect for playing with H,S,V as
a function of H it is useful, since one can then target a specific Hue and push/pull
its characteristics.  

BTW, the sliders were eight in the original implementation simply to give people enough
control, ie narrow enough bands being adjusted by the sliders; I always thought six
would make more sense color-wise, then they are centered on the primaries RGB and CMY
(ie in order RYGCBM). Since one now has very precise control with the flat curve, it
might make sense to just have six control points centered on these primary colors as
the initial default.  The user then has them at handy reference values, and can always
add more points for more precision.

Another usability suggestion: There is no indication when one moves a point that it
is in the neutral position; would it be possible to add something like that (eg the
control point changes color when at the neutral vertical position)?  I can imagine
a user wanting to shift control points left or right, but neutral, in order to shift
the endpoints of the adjustment window, but then not have any point outside that window
being affected.  Without such an indicator the user may inadvertently miss setting
the endpoints at neutral and then there will be slight adjustments applied outside
the intended window.

I would suggest that the priority is finishing this curve type, including making the
user friendly decorations.  The other curve type(s) can be a follow-up.  For the NR
application I have in mind, NURBS might be better in fact than spline, but either shouldn't
be too hard to implement.  The code is probably already present, spline is the first
nontrivial curve type on the diagonal curve tool.

Reported by ejm.60657 on 2011-03-07 15:00:47

[Beep6581]

The six control points already present in the default curve are already centered to
the RBG CMY colors.

In the final version of this curve type, you won't be able to move a point in both
directions at the same time : you'll have to select one control point first on which
you'll operate, then there will be a vertical and horizontal handle, so you will only
move the point in one direction at the same time.

There will also be a "snap to 0 + snap vertically to next/previous point" option, but
having a visual feedback that the point is on the neutral axis is a nice idea.

Regarding the spline curve type, i think that the algorithm is buggy right now... It
behave weirdly even when the curve has only 4 control points.

Reported by natureh.510 on 2011-03-07 15:22:09

[Beep6581]

Here is a new version more complete. You still have to apply it above Branch3, and still
have to copy the 2 png files contained in the zip file of comment 10 to the rtdata/images
folder (and run cmake again if this is your first try).

What is missing is the snapTo feature, but i don't know which key to use, there's none
left (i wanted ti use Alt, but it's already used to show the Highlight).

I also need to add some more visual graphical elements and it will be ready to be pushed
:)

I also hope that the use of this curve editor will be quite intuitive. Everything is
done with the left mouse button...

Reported by natureh.510 on 2011-03-11 01:23:58

---

• Attachment: FlatCurveEditor-V2.patch

[Beep6581]

It compiled fine, but I can't seem to move any of the control points.

Reported by ejm.60657 on 2011-03-11 14:24:05

[Beep6581]

This should help : http://www.rawtherapee.com/forum/viewtopic.php?p=19334#19334

Reported by natureh.510 on 2011-03-11 17:14:57

[Beep6581]

Yes it does help.  It seems very fiddly -- I like the idea of being able to freeze motion
in vertical or horizontal direction, but it now requires many more mouse clicks to
adjust the curve.  How about restoring the previous method of control point setting,
with left mouse controlling H+V, middle mouse V only, right mouse H only?  Or some
combination of shift and/or control keys if you want to keep everything on the left
mouse button?

Reported by ejm.60657 on 2011-03-11 17:54:06

[Beep6581]

Also, instead of the single horizontal arrow that allows the range of the control point
to be adjusted (a nice addition BTW), I would make it adjusted by the scroll wheel.
 One would have to come up with an acceptable workaround for tablet users.

Reported by ejm.60657 on 2011-03-11 18:05:34

[Beep6581]

"How about restoring the previous method of control point setting, with left mouse controlling
H+V, middle mouse V only, right mouse H only?"

Where did you saw that behaviour ?

Reported by natureh.510 on 2011-03-11 20:46:24

[Beep6581]

It wasn't there; I meant that previously, the left mouse button controlled H+V together,
and that in order to have the nice capability (just added with this second version)
to lock one and only adjust the other, one could use the other mouse buttons, and the
scroll wheel to adjust the distance away from the control point that is influenced
by that control point.  Or use combinations of shift and control keys.  It's just that
having to click (and it takes me sometimes several tries to get the correct point)
and then only be able to adjust one direction, then have to find the right spot to
click and adjust the other direction and then again to adjust the range, and do that
for each point, and then find out that the first point needs more adjustment, etc,
I found it very cumbersome to achieve the desired curve.  The first version was much
quicker, so I was trying to suggest a way to have that quick adjustment capability,
while retaining the finer control possibilities of the second patch.

Reported by ejm.60657 on 2011-03-11 21:26:49

[Beep6581]

I agree that it should be more efficient to avoid entering in a point editing mode,
i didn't thought about using the middle and right mouse button (the last time i did
it, for the popuptoggle button, everybody complained about the fact that they had to
use the right mouse button...)

Also, what would be more nartural :
- using B2 and Ctrl+B2 for the one axis displacement + B3 and Ctrl B3 for the tangent
adjustment ;
- using B2 and B3 for the one axis displacement + Ctrl B2 and Ctrl B3 for the tangent
adjustment ?

Reported by natureh.510 on 2011-03-11 23:00:13

[Beep6581]

Is it possible to use the shift key as well?  If so then how about

Left button allows free movement of the selected point
Ctrl+Left button allows only vertical motion
Shift+Left button allows only horizontal motion
Scroll wheel adjusts the range of the control point

Reported by ejm.60657 on 2011-03-11 23:21:29

[Beep6581]

This is what I see directly after I start RT:

[New Thread 0x7fffe8286700 (LWP 7628)]
[New Thread 0x7fffe7a85700 (LWP 7629)]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff43d5d40 in Gtk::Menu::reposition() ()
   from /usr/lib/libgtkmm-2.4.so.1

On Ubuntu 10.10 64-bit. 

Reported by paul.matthijsse4 on 2011-03-12 13:39:50

[Beep6581]

Did you installed the png files that you have to download from comment 10? Don't forget
to rerun cmake.

If you know how to integrate binaries files in a patch, i will appreciate ;)

Reported by natureh.510 on 2011-03-12 14:19:30

[Beep6581]

Ok, I oversaw the install-png part, now it works fine. Intriguing tool, but it will
take some time to get familiar with it. First results look promising!

Regards, Paul. 

Reported by paul.matthijsse4 on 2011-03-12 16:44:27

[Beep6581]

Here is a new version of the flat curve editor, with bug correction and a new color
feedback mechanism that will be usable for the diaginal curve editor too.

I didn't revised the way that are handled the elements yet, but it will be the next
step.

I'll also bring back the histogram (in the V channel only i think), but i'll need your
help Emil. You are also free to update the 'HSVEqualizer::colorForValue' function if
you think that the colored feedback is wrong.

Is this colored feedback enough or do i have to create colored range in the bottom
(X axis) and left (Y axis) side of the graph. If you want me to do them, i'd need a
preview of how it should look like for each H/S/V curve. But can we really use an absolute
Y scale for S and V ?

You still have to apply this patch over Branch3, and add the png files from comment
#10 to rtdata/images (if not already done)

See also http://www.rawtherapee.com/forum/viewtopic.php?p=19391#19391

Reported by natureh.510 on 2011-03-14 00:10:00

---

• Attachment: FlatCurveEditor-V3.patch

[Beep6581]

Thanks, Hombre; I'll have a look when I have time.  

I don't think a histogram is appropriate for this kind of tool; more important is having
hue information on the horizontal axis.  I'd have to think about what should be on
the vertical axis, if anything.  I'll try to put something together in terms of a mockup.
 My first thought is to have the horizontal axis be S=V=.5, and H varying along the
axis.  Then for H vs H, when the control point is the output color one sees what color
is being mapped to what output color; for S vs H, one sees the change in saturation,
and for V vs H one sees the change in V for the control point.

Reported by ejm.60657 on 2011-03-14 00:57:47

[Beep6581]

I still don't understand what is the objection to allowing the original behavior that
the left mouse button controls 2D movement of the control point.

Reported by ejm.60657 on 2011-03-14 01:29:02

[Beep6581]

I thought about displaying the knob with the destination color. I don't see the point
of having a 2D motion of the point, since 90% percent of the time, you'll want to move
one axis at a time.

I'm a rather disappointed that the forum thread about the flat curve editor don't have
the audience i wished... I need user's point of views but there's almost nobody, except
you.

I was planning to create a relativiely big graphical element over the auto-activated
control points, as well as the vertical and horizontal lines) to be able to handle
everything. I think that moving the point in both direction at a time is doable, but
there's no 'Undo' local to the curve editor, so the user will have to be carefull when
pressing the left mouse button. I don't want to use modifier key since they are already
used to slow down the motion and to snap the point to different element.

Reported by natureh.510 on 2011-03-14 09:09:19

• Status changed: PatchSubmitted

[Beep6581]

Hombre, This is a great tool, thank you for this addition! I am on vacation trip with
family and only now got a chance to give it a try. 

There is a problem adjusting the range for a selected point: 
- for a point in the center of the X axis both min and max of the range are adjustable
(this is good:) ).
- for a point on the left side from the center of the X axis only minimum of the range
are adjustable.
- for a point on the right side from the center of the X axis only maximum of the range
are adjustable.

it would also be great to have a pipette next to the square of the curve tool to add
a new dot on the curve based on the color picked from the image.

Reported by michaelezra000 on 2011-03-14 15:36:00

[Beep6581]

Thanks Michael ;)

I'm affraid that i don't understand what you explained about the range of a point.
What range are you talking about ?

I agree for the pipette, but i won't have the time to do it for RT3.0.

Reported by natureh.510 on 2011-03-14 20:20:59

[Beep6581]

Hombre, I see your point about the modifier keys -- I had forgot about using them to
fine-grain the point motion.  How about using the arrow keys instead -- up/down arrows
held down restrict motion to vertical, left/right arrows held down restrict the motion
to horizontal.  I am concerned that the present mode of operation, while better, still
requires lots of mouse clicks and repositioning to maneuver the curve into the desired
position.

I like the vertical/horizontal bars of color passing through the selected control point.
 Would it be possible that instead of being a solid color, they reflect the gradient
along that direction?  So for instance the horizontal bar has the hue gradient, the
vertical bar has eg a saturation gradient or a value gradient.  One thing I forgot
to check is whether the formula used to evaluate the tonality of the bar is the same
one used in the equalizer calculation; is it?

Reported by ejm.60657 on 2011-03-14 20:40:22

[Beep6581]

Hombre, about the range 

The left tangential handle works only for points that are in the left portion of entire
curve UI control.

The right tangential handle works only for points that are in the right portion of
entire curve UI control.

Only when selected point's X coordinate is in the middle of the X axis both tangential
handles work.

Reported by michaelezra000 on 2011-03-14 21:14:11

[Beep6581]

I used the same formula that is present in ImProcFunctions::rgbProc.

The left and right arrow key look to be already used to show/hide the left and right
panels of the editor, so i can't use them.

I'll implement a special widget that will appear above the point when the cursor is
over it, so you'll be able to move it in XY, X, Y ans the tangent widgets.

I'll look what i can do for the gradient color, i tought about it too, i didn't find
out how to implement it yet but it shouldn't be a problem.

Reported by natureh.510 on 2011-03-14 21:15:31

[Beep6581]

Michael, i think you missed comment #30 ;). Here is the way that are computed the active
areas : http://www.rawtherapee.com/forum/viewtopic.php?p=19334#19334

This is not an error, but a design choice... that will be obsolete in the next couple
of days.

Reported by natureh.510 on 2011-03-14 21:24:41

[Beep6581]

Bonsoir Hombre,

1.Don't be disappointed, be happy! :-) Users might not react that much, because this
is quite an advanced tool. It took me some time to find my way around (never saw such
a curve tool before). Took again more time to apply your curves in a sensible way to
my photos. 

First thing I thought: this is quite cool! Other thoughts: a pipet is quite welcome
here, as Michael said already: I want to edit *that* kind of green in my photo. Users
including me might also want some more visual feedback on what happens to what colors
when the curves are dragged. I see the horizontal color lines are changing their colors
(in patch v3), but maybe a little colorbox somewhere that reflects color changes is
more clear? I'd like to see as well an option to reset a certain color point to 0 (back
to the x axis). 

Reported by paul.matthijsse4 on 2011-03-14 21:43:05