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

Unbreak input profile : add log profile #1730

Closed
wants to merge 60 commits into
base: master
from

Conversation

Projects
None yet
7 participants
@aurelienpierre
Contributor

aurelienpierre commented Sep 19, 2018

Warning : This is my first code contribution in dt and the first time I use C and work with GUIs.

This adds a new mode of tone-mapping before color profile correction, using the same approach as video encoding Log profiles (S-log, N-log, etc.). Details and example can be seen here : https://discuss.pixls.us/t/solving-dynamic-range-problems-in-a-linear-way/9006

It retains the previous gamma method and takes care of the legacy params.

It adds an auto solver to automatically compute the pest settings, namely the dynamic range, according to the target grey and the measurements from the color sampler.

It adds masking/blending options and the ability to add in styles.

The OpenCL code is given but does not work at this point. I need help to debug it. OpenCL is enabled for the gamma method and disabled for now on the log method. See https://www.mail-archive.com/darktable-dev@lists.darktable.org/msg03355.html

aurelienpierre added some commits Sep 21, 2018

correct important maths mistakes\nI shouldn't be allowed near a keybo…
…ard without solving equations on paper first -_-\nfrom the maths, remove redondant GUI sliders of dependant variables
(hopefully) fix once for all the slider glitches while using the opti…
…mizer. change the defaults settings so avoid tone-mapping by default
make the black level user-input and remove it from the optmizitaion
add a color picker for the black input
remove the noise/black correction to avoid inaccuracies at low dynamic range - the black level has to be absolutely set in exposure module. Now the log functions will only be thresholded.
@aurelienpierre

This comment has been minimized.

Show comment
Hide comment
@aurelienpierre

aurelienpierre Sep 21, 2018

Contributor

Please find a full step-by-step tutorial of this module with examples here : https://discuss.pixls.us/t/solving-dynamic-range-problems-in-a-linear-way/9006/63?u=aurelienpierre

Contributor

aurelienpierre commented Sep 21, 2018

Please find a full step-by-step tutorial of this module with examples here : https://discuss.pixls.us/t/solving-dynamic-range-problems-in-a-linear-way/9006/63?u=aurelienpierre

@aurelienpierre

This comment has been minimized.

Show comment
Hide comment
@aurelienpierre

aurelienpierre Sep 23, 2018

Contributor

OpenCL is now working

Contributor

aurelienpierre commented Sep 23, 2018

OpenCL is now working

@TurboGit

This comment has been minimized.

Show comment
Hide comment
@TurboGit

TurboGit Oct 16, 2018

Member

Aurélien, when you have some time can you fix the merge conflicts? I'll have a look, all those nice comments push me to see more about it. Thanks.

Member

TurboGit commented Oct 16, 2018

Aurélien, when you have some time can you fix the merge conflicts? I'll have a look, all those nice comments push me to see more about it. Thanks.

aurelienpierre added some commits Oct 17, 2018

Backport GUI reviews of Pascal Obry from the colorbalance module
Speed-up the CPU by 1.8× improving vectorization and using an approximate fast log2 version
Note : OpenCl version stays with exact vectorized log2 as it makes no speed difference
@aurelienpierre

This comment has been minimized.

Show comment
Hide comment
@aurelienpierre
Contributor

aurelienpierre commented Oct 17, 2018

@TurboGit done !

@TurboGit

This comment has been minimized.

Show comment
Hide comment
@TurboGit

TurboGit Oct 17, 2018

Member

Still some merge conflicts? Have't you forgotten to push some modifications?

Member

TurboGit commented Oct 17, 2018

Still some merge conflicts? Have't you forgotten to push some modifications?

aurelienpierre added some commits Sep 12, 2018

Introduce a new gamma correction for color profiles based on video so…
…ftware

Works better than legacy gamma for high dynamic range (> 10 EV)
BREAKS THE PREVIOUS VERSION AND I DON'T KNOW WHY

Get rid of the previous code, make things more simple and try to enable OpenCL

Change to log profile, add color sampling and best  parameters guessing

improve the auto tuner, refine the noise handling

uncomment the optimization loop

improve the solver, add a black correction slider

improve the GUI, change the default settings, remove the linearity factor from optimization

comment the loop now useless

clamp output and fix GUI glitches.\n Add a error log message when pixels are < 0. and exit the optimization

fix the GUI glitches at module activation

change the black/noise level auto estimation

prevent the update of the parameters if there is already an update running

reinclude the old gamma version and add legacx params support

add the log profile function, but DOES NOT WORK YET !

clang-format

improve the auto-optimizer, do the computations in RGB, remove the solver

correct important maths mistakes\nI shouldn't be allowed near a keyboard without solving equations on paper first -_-\nfrom the maths, remove redondant GUI sliders of dependant variables

(hopefully) fix once for all the slider glitches while using the optimizer. change the defaults settings so avoid tone-mapping by default
make the black level user-input and remove it from the optmizitaion
add a color picker for the black input
remove the noise/black correction to avoid inaccuracies at low dynamic range - the black level has to be absolutely set in exposure module. Now the log functions will only be thresholded.
Add a full auto-tune button
Clamp input values after dividing by grey, which may reduce the noise amplification in log2
Correct the black color picker to take the min (not the max)
Still need to scream when negative pixels are present
Refactor & clean code
Make the optimizers more robust and consistent
Correct the UI labels
Be mone agressive on noise clamping
Use average color in black color-picker to reduce the noise detection
Add an SSE2 version which is not significantly faster
Add thread_mutex_lock around GTK events

Revert "Add an SSE2 version which is not significantly faster"

This reverts commit 539ae88.
Backport GUI reviews of Pascal Obry from the colorbalance module
Speed-up the CPU by 1.8× improving vectorization and using an approximate fast log2 version
Note : OpenCl version stays with exact vectorized log2 as it makes no speed difference
@aurelienpierre

This comment has been minimized.

Show comment
Hide comment
@aurelienpierre

aurelienpierre Oct 17, 2018

Contributor

It was src/common/colorspaces_inline_conversions.h that was in conflict, right ? It should be fixed, I have rebased.

Contributor

aurelienpierre commented Oct 17, 2018

It was src/common/colorspaces_inline_conversions.h that was in conflict, right ? It should be fixed, I have rebased.

@TurboGit

This comment has been minimized.

Show comment
Hide comment
@TurboGit

TurboGit Oct 17, 2018

Member

Yes, but you need to push in aurelienpierre:color-grading. The conflict is still there.

Member

TurboGit commented Oct 17, 2018

Yes, but you need to push in aurelienpierre:color-grading. The conflict is still there.

@TurboGit

This comment has been minimized.

Show comment
Hide comment
@TurboGit

TurboGit Oct 17, 2018

Member

Aurélien, I will close this PR. There is 60 commits, many merges with master, it is possible to merge locally but impossible to rebase due to conflicts.

I have squashed all those commits into a single one and will propose a new PR with this. A cleaner start for the integration.

Member

TurboGit commented Oct 17, 2018

Aurélien, I will close this PR. There is 60 commits, many merges with master, it is possible to merge locally but impossible to rebase due to conflicts.

I have squashed all those commits into a single one and will propose a new PR with this. A cleaner start for the integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment