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

curves: restore old DCT_Spline, add new DCT_CatmullRom type #5068

Closed
wants to merge 1 commit into from

Conversation

agriggio
Copy link
Contributor

@agriggio agriggio commented Dec 5, 2018

Hopefully this will make everybody happy. The new curve type (which I simply called "Custom new" in the GUI -- better names are welcome) is used only for histogram matching by default. Existing pp3's should pick up the old cubic splines without issues.

I coded this in about half an hour, so please test before merging (if you decide to merge)

@agriggio agriggio added this to the v5.5 milestone Dec 5, 2018
@Beep6581
Copy link
Owner

Beep6581 commented Dec 5, 2018

We could call the cubic spline "Rigid" and the Catmull-Rom "Flexible", that would be pretty user-friendly.
Or... "Stiffy" and "Floppy" 😄

@agriggio
Copy link
Contributor Author

agriggio commented Dec 5, 2018

As discussed on IRC, my vote for names goes to "Standard" for the old cubic splines and "Flexible" for the new ones. Regarding "Standard", I think cubic splines are sort of the de-facto standard when you talk about spline interpolation. In the photography/retouching world, it could be argued that "standard" is what the more popular tools do by default. I have never used Photoshop or Lightroom, but I just found this:

https://feedback.photoshop.com/photoshop_family/topics/point_editing_mode_for_tone_curve_control_hard_to_use_because_of_non_local_effects

Which makes me believe that the "point curves" are indeed cubic splines

@agriggio
Copy link
Contributor Author

agriggio commented Dec 6, 2018

after a night of sleep, I think I can live with "smooth"/"sharp" if std/Flexi has no fans :-)

@agriggio
Copy link
Contributor Author

agriggio commented Dec 6, 2018

OTOH, another argument in support of "Standard". Quoting from https://discuss.pixls.us/t/curves-behaviour-changed-in-development-version/10141:

In V5.4 stable release, it behaves like any other curves adjustment tool I’ve used, most notably Photoshop

@Thanatomanic
Copy link
Contributor

Just my 2 cents here, I would call the old version 'Standard' and the new one 'Precise' or 'Detailed'.

@TooWaBoo
Copy link
Contributor

TooWaBoo commented Dec 8, 2018

I suggest to use a different curve icon too.

@Beep6581
Copy link
Owner

Beep6581 commented Dec 9, 2018

  • Photoshop CS2
    curve photoshopcs2
  • RT cubic spline
    curve rt custom
  • RT Catmull-Rom spline
    curve rt catmullrom
  • GIMP-2.8.10
    curve gimp

As the Photoshop curve resembles RT's cubic spline, I'm fine with "Standard" for cubic spline and "Flexible" for the centripetal Catmull-Rom spline.

I committed a new icon, curve-catmullrom.png and curve-catmullrom-small.png

@agriggio
Copy link
Contributor Author

Can this be merged? If so, should I merge on releases first and then port to dev?

@Beep6581
Copy link
Owner

@agriggio please merge to dev, then I'll cherry-pick to release-5.5.

@agriggio
Copy link
Contributor Author

pushed

@agriggio agriggio closed this Dec 10, 2018
@agriggio agriggio deleted the curves-restore-old-add-new branch December 10, 2018 16:27
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.

None yet

4 participants