5.5-rc1 curve behaviour


#1

It seems the new curve algorithm can behave in unexpected ways:
Screenshot%20from%202018-12-05%2018-41-05
By setting just a single point, it is possible to create the step behaviour shown.
Somehow I like the idea of the new curves - but this version needs more control points to create a simple smooth round curve. (Even if the control point is in the middle it tends more towards two straight lines meeting in the middle.)


#2

conflicting
It does let me add points to where I think the curve is supposed to be!


(Ingo Weyrich) #3

Looks like a bug when more than one curve point is at y = 0.
Ping @agriggio


(Alberto) #4

Sounds like a bug in the GUI though, given what @HIRAM demonstrated (i.e. the curve seems to be computed correctly).

Anyway:

I hope this will make people happy.


(Alberto) #5

And here’s the same thing with the old cubic splines:
52


(Ingo Weyrich) #6

@agriggio Imho no need to fix it for 5.5 release!


(Alberto) #7

If you are talking about the GUI bug, I definitely agree (I had no intention of fixing it, in fact :slight_smile:

However, restoring the old curve behaviour is now or never IMHO. It doesn’t make sense to remove it for 5.5 and add it back for 5.6.


(Ingo Weyrich) #8

Does restoring the old behaviour mean we will not have the new curve behaviour?


(Alberto) #9

No, we’ll have both (just try the PR mentioned above):
24


(Andrew) #10

I’m happy, thanks.
All, a note about the old method… There have been several comments about adding/changing a point having a “global” effect which means other parts of the curve change. There is a simple workaround if you don’t want that. Simply have two points fairly close together and this acts as a clamp. That is, moving the curve on one side of the clamp has negligible effect on the curve that’s on the other side of the clamp. A second point can be easily added close to an existing one by placing it with CTRL + mouse click - it will jump exactly onto the curve. So the old curve can be nice and smooth if you want that. Or by placing more points, you can imitate the more “local” behaviour of the new curve.


#11

Question: Which curve is used by the Auto-Matched Tone Curve, new or old?


(Alberto) #12

new


(Franz Trischberger) #13

Which means that you have to move two points when you want to change the curve in that region. Makes it more difficult to immediately see the result. Also you can easily delete a node if you drag one node over the other.


(Andrew) #14

This is true. But once you’ve dragged one away, then you immediately see the result as you move the remaining one. It takes me a while to do a curve so having to do this removing thing once or twice isn’t much of an overhead for me.


(Glenn Butcher) #15

Sorry for the late input, just got back from the woods…

I’ve considered this in my software, and I’ve come to the conclusion that I’m wedded to Tino Kludge’s spline algorithm until the heat death of the universe. The spline algorithm that forms the foundation of a curve tool really becomes the basis of the image manipulation with respect to determining input->transform->output, and changing it renders a good many curves as defined by a set of control points based on the previous spline as plain wrong.

I explored a more ‘local’ spline algorithm, but abandoned the effort when i realized 1) the legacy of control point lists I already had, and 2) I can make my Tino Kludge spline be local, but I can’t make a local algorithm easily provide the sweeping curves that distort my image tones less than, say, a lift in one area that then produces a relatively flat slope that really distorts the tones of a particular band.

I do a good bit of what Andrew describes, using two close points to anchor the remaining slope. Works for me…


(Flössie) #16

Hmm, those zeros are from rtengine::DiagonalCurve::getVal() which is not GUI specific as far as I can tell.


(Alberto) #17

You’re right, I was a bit too quick in dismissing this. I’ll take a look when time permits :+1:


(Flössie) #18

@agriggio Alberto, I think I found the culprit: It’s the special case for 0 and 1. Though I don’t know how to fix that…


(Alberto) #19

yes, I’m working on it… thanks for the hint!


(Alberto) #20

Here’s the patch. I’ll push later today

diff --git a/rtengine/diagonalcurves.cc b/rtengine/diagonalcurves.cc
--- a/rtengine/diagonalcurves.cc
+++ b/rtengine/diagonalcurves.cc
@@ -326,6 +326,9 @@
     if (p1_y == p2_y && (p1_y == 0 || p1_y == 1)) {
         for (i = 1; i < n_points-1; ++i) {
             t = p1_x + space * i;
+            if (t >= p2_x) {
+                break;
+            }
             res_x.push_back(t);
             res_y.push_back(p1_y);
         }