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

filmicrgb: add rational function to toe/shoulder #8764

Merged
merged 1 commit into from
Apr 24, 2021

Conversation

aurelienpierre
Copy link
Member

Rational polynomial designed by MrTeatime (https://discuss.pixls.us/t/proposal-for-updated-filmic-curve-parameterisation-in-darktable/23774/101)
This is guaranteed to not over/under-shoot but the contrast is a bit muted near black/white.
It is introduced in addition to soft/hard contrast intents as a "safe" method.

Rational polynomial designed by MrTeatime (https://discuss.pixls.us/t/proposal-for-updated-filmic-curve-parameterisation-in-darktable/23774/101)
This is guaranteed to not over/under-shoot but the contrast is a bit muted near black/white.
It is introduced in addition to soft/hard contrast intents as a "safe" method.
@aurelienpierre aurelienpierre added feature: enhancement current features to improve difficulty: trivial some changes in a couple of functions scope: image processing correcting pixels labels Apr 23, 2021
@@ -173,7 +175,7 @@ typedef struct dt_iop_filmicrgb_params_t
float black_point_target; // $MIN: 0.000 $MAX: 20.000 $DEFAULT: 0.01517634 $DESCRIPTION: "target black luminance"
float white_point_target; // $MIN: 0 $MAX: 1600 $DEFAULT: 100 $DESCRIPTION: "target white luminance"
float output_power; // $MIN: 1 $MAX: 10 $DEFAULT: 4.0 $DESCRIPTION: "hardness"
float latitude; // $MIN: 0.01 $MAX: 100 $DEFAULT: 25.0
float latitude; // $MIN: 0.01 $MAX: 100 $DEFAULT: 33.0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you've changed the default latitude unconditionally, is that intentional ? is that needed for the new safe mode ? if not maybe should be part of another PR, at least I'm not sure it is good to change that default as people are now used to the module... or maybe there is a strong point doing that ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the name suggests, "safe" is safer as a default because users can blindly push contrast all the way, it will never overshoot. But safe is less contrasty so a bit more latitude is good.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that the default was also moved to safe mode.

Copy link
Member

@TurboGit TurboGit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good !

@TurboGit TurboGit merged commit 5005d87 into darktable-org:master Apr 24, 2021
@TurboGit TurboGit added this to the 3.6 milestone Apr 24, 2021
@TurboGit TurboGit added the documentation-pending a documentation work is required label Apr 24, 2021
@elstoc elstoc added documentation-complete needed documentation is merged in dtdocs and removed documentation-pending a documentation work is required labels May 20, 2021
@aurelienpierre aurelienpierre deleted the filmic-safe branch December 12, 2022 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: trivial some changes in a couple of functions documentation-complete needed documentation is merged in dtdocs feature: enhancement current features to improve scope: image processing correcting pixels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants