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

[WIP] sigmoid tone mapping module #7820

Closed
wants to merge 17 commits into from

Conversation

jandren
Copy link
Contributor

@jandren jandren commented Jan 12, 2021

Hi darktable developers!
I have wanted to contribute to the darktable source for some time now and I finally found the time to do some "hacking"/exploration this Christmas. I actually managed to make something interesting so I figured I just as well could present myself with a pull request.

Anyway here is the results from some late nights during Christmas, I hope you find it interesting!

Sigmoid?

I have recently been dealing with strictly positive data for classification at work which has given me a reason to study appropriate probability density functions for this kind of data. Two examples are the log-logistic and the weibull which also have easy closed for definitions of the cumulative distribution function, cdf. It struc me that these functions really resembled the S-curve used for tone mapping of images and their cdf properties are pretty nice with any value [0.0, inf] mapping to (0, 1) in a smooth way. The slope of this ramp is also determined by a single shape parameter.

Compared to the base curve presets

My first test was to compare directly with the base curve presets as those are hardcoded in the module and therefore easy to extract. They look like in the left plot bellow when stack ontop of each other. Kind of similar but not the same. The right plot show what would happen if the base curves where adopted to a scene referred workflow and fix middle grey as 0.18. They all look very similar all the sudden. The two sigmoid functions are plotted ontop. The scale parameter is calculated such that they also hit the point (0.18, 0.18) while the shape is picked to resemble the base curves. I'm actually surprised at both how tightly the base curves algined with each other as well as how well the sigmoids followed them. Notice how the weibull distribution converge towards 1 much quicker then the log-logistic, this is the most notable difference between the two, weibull gives punchier highlights. The log-logistic on the other hand gives a lot of room for preserving details in the highlights!

Base

Another way of plotting the same data is to plot the derivative of the above curves. Perry Sprawls has a good article about this kind of curve which is best described as a contrast curve. Its easy to see why its a bit tricky to tone your image using custom base curves! Even some of the presets wiggle like crazy back and forth! The average trend is pretty clear though and both the log-logistic and weibull pdfs follow the trend fairly well.

Contrast

Any conclusions from the above? Well I would say its promising, especially when the following is taken into consideration:

  • Its controlled by a single easy to understand parameter
  • A well defined curve with a smooth stable derivative
  • Results is always within the display safe bounds [0, 1]
  • Supports very very high dynamic range, no weird out bounds behaviors etc.

I had loved to include the corresponding curves for darktable filmic, blender filmic and ACES RRTODT but its a bit more work then just extracting the base curve presets so I haven't put in the time to do that.

Pics or it didn't happen

I don't have any sharable portraits laying around so my sisters dog will be the main subject instead. The picture has only been edited by adjusting exposure and white balance. The difference between them is the tone mapping which is either my sigmoid method, filmic v4 or base curve with the "Canon EOS like" preset and preserve colors = luminance / none.

DSC_5870_sigmoid
Sigmoid, log-logistic, contrast = 1.4

DSC_5870_filmic
Filmic, slightly tuned white and black levels.

DSC_5870_base_luma
Base curve, Canon EOS like, preserve colors = luminance

DSC_5870_base_non
Base curve, Canon EOS like, preserve colors = none

I also found this problematic image on the darktable pixls forum https://discuss.pixls.us/t/a-study-in-blue/21706
and I think it gave some interesting results worth sharing.

image
Lightroom edit by Egocentrix

RPN_Kick-In_20180830_3392_sigmoid
Sigmoid, contrast = 0.89, slight white balance change and adjusted exposure. That contrast is very very low but worked well for this particular image as it could fit both the intense lights as well as the much darker surrounding in our viewable [0, 1] range.

RPN_Kick-In_20180830_3392_filmic
Filmic with tuned black and white level, as well as the same white balance and exposure as for the sigmoid.

Compared to ACES

The ACES-dev repository has a set of reference images that can be downloaded and compared against.
See: https://github.com/ampas/aces-dev/tree/master/images
Their scene to display transforms has been used in a lot of really good looking movies so I wondered how the sigmoid would compare. The following is the result when using the image /ACES/syntheticChart.01.exr from the linked library.

syntheticChart 01_ODT Academy sRGB_100nits_dim ACES reference ODT: sRGB, 100 nits, dim light

syntheticChart 01_sigmoid
sigmoid, log-logistic, contrast = 1.4

syntheticChart 01_filmic
filmic, default values.

syntheticChart 01_base_lum
Base curve, Canon EOS like, preserve colors = luminance

syntheticChart 01_base_no
Base curve, Canon EOS like, preserve colors = none

Please try it!

I had loved to get some feedback and see examples from your pictures. I personally so far love how it does one thing very well and then cooperates with the exposure and tone equalizer modules for good results.
Here are some tips to get you going:

  • Push that exposure! I averaged around +2 EV for my pictures. Do not mind this, its just a matter of middle grey being defined differently.
  • Pay attention to the highlight reconstruction. It can really make a big difference to change to "Lch" or "color" instead of "clip".
  • Weak saturation in brighter parts of an image? Use the tone equalizer to reduce the brightness instead of trying to push saturation sliders!
  • Weak whites? Change to weibull
  • Missing details in the whites? Change to log-logistic

Missing for being properly done done

  • Better name, I feel like it should say what it does rather then what it is. Suggestions are welcome!
  • Histogram with EVs as x axis. 0 EV = 0.18
  • Plot curve on the histogram similar to the base curve
  • Whatever optimization that might be needed

@aurelienpierre
Copy link
Member

  1. Please don't duplicate the tone mapping module. Your logistic equations are very easy to put it-place of the filmic spline, it's a minor change.
  2. Filmic default values are tuned for DSLR output, with white at + 4EV. The chart you give here in HDR and has white at +16 EV, so filmic defaults don't apply, and the highlights reconstruction kicks-in. Also, filmic uses chroma preservation but has a basic independent channels mode too, so you are comparing apples to oranges. Here is the closest filmic match with chroma preservation set to none:
    syntheticChart 01-censure_01
    And here is in chroma preservation mode set to power norm with white properly set:
    syntheticChart 01-censure_02

I don't think this module brings anything new or better. First of all, duplicating the feature is unnecessary. Then the default filmic spline is already quite close to the logistic curve. I would rather fix the highlights reconstruction, that obviously fails here, and add an option to remove the desaturation at extreme luminances if that's what you want.

@phweyland
Copy link
Contributor

I've tried it.
Results are really impressive with only 2 sliders (I add the exposure one) !
I like it ! Well done !

@todd-prior
Copy link

@phweyland
I have not attempted to use extra code like this before is there a best practice way to do this?? The code is on Jardens git correct??

@parafin
Copy link
Member

parafin commented Jan 13, 2021

@aurelienpierre, some interesting artifacts are happening with middle stripe in filmic screenshots (especially on the left).

@chhil
Copy link

chhil commented Jan 13, 2021

@todd-prior

See #7201 (comment)
#7201 (comment)
and around it. Some instructions on how to go about doing it.

Personally, I look at the change and if the change is simple , I merge it manually into the master I have checked out.

I also did the following and it worked.

mchhil@DESKTOP-MKCA8HN:~/darktable$ git remote add jandren https://github.com/jandren/darktable.git
mchhil@DESKTOP-MKCA8HN:~/darktable$ git fetch jandren
From https://github.com/jandren/darktable
 * [new branch]          30bit                        -> jandren/30bit
 * [new branch]          align_image_stack            -> jandren/align_image_stack
 * [new branch]          aurelienpierre-patch-1       -> jandren/aurelienpierre-patch-1
 * [new branch]          battery-indicator            -> jandren/battery-indicator
 * [new branch]          camera-import-metadata       -> jandren/camera-import-metadata
 * [new branch]          colorspace-263               -> jandren/colorspace-263
 * [new branch]          colorzones-zoom              -> jandren/colorzones-zoom
 * [new branch]          d810-base-curve              -> jandren/d810-base-curve
 * [new branch]          darktable-1.0.x              -> jandren/darktable-1.0.x
 * [new branch]          darktable-1.1.x              -> jandren/darktable-1.1.x
 * [new branch]          darktable-1.2.x              -> jandren/darktable-1.2.x
 * [new branch]          darktable-1.4.x              -> jandren/darktable-1.4.x
 * [new branch]          darktable-1.6.x              -> jandren/darktable-1.6.x
 * [new branch]          darktable-2.0.x              -> jandren/darktable-2.0.x

 * [new branch]          darktable-2.2.x              -> jandren/darktable-2.2.x
 * [new branch]          darktable-2.4.x              -> jandren/darktable-2.4.x
 * [new branch]          darktable-2.6.x              -> jandren/darktable-2.6.x
 * [new branch]          darktable-3.0.x              -> jandren/darktable-3.0.x
 * [new branch]          darktable-3.2.x              -> jandren/darktable-3.2.x
 * [new branch]          detachable                   -> jandren/detachable
 * [new branch]          dualiso                      -> jandren/dualiso
 * [new branch]          experimental_plugins         -> jandren/experimental_plugins
 * [new branch]          film-emulation               -> jandren/film-emulation
 * [new branch]          filmic-autotune              -> jandren/filmic-autotune
 * [new branch]          fix-filmic-autotune-latitude -> jandren/fix-filmic-autotune-latitude
 * [new branch]          fix-filmic-sse               -> jandren/fix-filmic-sse
 * [new branch]          fix-graduatednd              -> jandren/fix-graduatednd
 * [new branch]          fix-on-off                   -> jandren/fix-on-off
 * [new branch]          hacking                      -> jandren/hacking
 * [new branch]          lua                          -> jandren/lua
 * [new branch]          master                       -> jandren/master
 * [new branch]          next                         -> jandren/next
 * [new branch]          po-ui-refactor               -> jandren/po-ui-refactor
 * [new branch]          po/crop-angle-visibility     -> jandren/po/crop-angle-visibility
 * [new branch]          preview-timeout              -> jandren/preview-timeout
 * [new branch]          raw-video                    -> jandren/raw-video
 * [new branch]          rawdenoise                   -> jandren/rawdenoise
 * [new branch]          sigmoid_tone_mapping         -> jandren/sigmoid_tone_mapping
 * [new branch]          srw-makernote-bug            -> jandren/srw-makernote-bug
mchhil@DESKTOP-MKCA8HN:~/darktable$ git checkout master
Already on 'master'
Your branch is up to date with 'origin/master'.
mchhil@DESKTOP-MKCA8HN:~/darktable$ git merge jandren/sigmoid_tone_mapping
Merge made by the 'recursive' strategy.
 src/common/colorspaces_inline_conversions.h |   4 +
 src/common/iop_order.c                      |   2 +
 src/iop/CMakeLists.txt                      |   1 +
 src/iop/sigmoid.c                           | 329 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 336 insertions(+)
 create mode 100644 src/iop/sigmoid.c
mchhil@DESKTOP-MKCA8HN:~/darktable$

Now you can build DT as you always did and the sigmoid module will be available in the build.
image

If you want to revert these

This will remove the merged files.
git reset --hard origin/master

This will show you the remote adds that you have done
git remote -v
jandren https://github.com/jandren/darktable.git (fetch)
jandren https://github.com/jandren/darktable.git (push)
origin https://github.com/darktable-org/darktable.git (fetch)
origin https://github.com/darktable-org/darktable.git (push)

This will remove the remote add we did earlier
git remote rm jandren

And you will be back to the kosher dt master branch.

@chhil
Copy link

chhil commented Jan 13, 2021

@jandren
Running darktable-generate-cache from commandline I get

cannot get iop-order for sigmoid instance 0

@TurboGit
Copy link
Member

@jandren
Running darktable-generate-cache from commandline I get

cannot get iop-order for sigmoid instance 0

To fix that an entry in dt_ioppr_get_iop_order_list() is needed.

@phweyland
Copy link
Contributor

I have not attempted to use extra code like this before is there a best practice way to do this??

I just did:
git fetch https://github.com/darktable-org/darktable pull/7820/head:sigmoid_tone_mapping

This creates another new branch (sigmoid_tone_mapping) you can compile.

@paul-pw
Copy link
Contributor

paul-pw commented Jan 13, 2021

  1. Please don't duplicate the tone mapping module. Your logistic equations are very easy to put it-place of the filmic spline, it's a minor change.

@aurelienpierre, so it's possible to get the sigmoid tone mapping put in place of the filmic spline?
In that case the user would get the option of selecting between the old spline and the sigmoid tone mapping or how would this work from the users perspective?

I'm verry much a fan of extending the filmic module instead of introducing a new module especially because we already have the base curve and the filmic module doing essentialy the same as the proposed sigmoid tone mapping module.

@phweyland
Copy link
Contributor

phweyland commented Jan 13, 2021

I'm verry much a fan of extending the filmic module instead of introducing a new module

Why not, IF we can keep the simplicity and behavior of the sigmoid tone mapping.

@jakubfi
Copy link
Contributor

jakubfi commented Jan 13, 2021

This + exposure + color calibration was absolutely the fastest and easiest route from raw to "this is it!" I've ever got on one of my "worst" test images. Well done. 👏

@aurelienpierre
Copy link
Member

aurelienpierre commented Jan 13, 2021

Why not, IF we can keep the simplicity and behavior of the sigmoid tone mapping.

You do realize it's simple only because it takes shortcuts, hardcode things and is overall not future-proof ?

  • It assumes black = 0, which prevents any black point compensation,
  • it assumes white = 1, which will prevent future compatibility with HDR displays,
  • it doesn't do chroma preservation, which will induce hue shift and non-uniform changes in saturation and mess up previous color grading,
  • it doesn't allow to redistribute the dynamic range between shadows and highlights, so it's not a tone mapping but a tone curve,
  • the contrast (aka slope of the curve) is the same parameter that drives the dynamic range compression, so you can't actually preserve contrast while compressing the range. The synthetic chart provided above is clipped and there is no way to unclip it with that module while keeping a decent contrast.

So I get that people are getting excited about the 2 bites process, but this is going 2 years back. This is a parametric base curve, nothing more. It doesn't solve any HDR problem. Simplicity is fish-bait here.

@aurelienpierre, some interesting artifacts are happening with middle stripe in filmic screenshots (especially on the left).

It's the highlights reconstruction that kicks in and really doesn't like edges going from 0 to 65535 between 2 neighouring pixels. I'm currently fixing that.

@aurelienpierre aurelienpierre added the controversial this raises concerns, don't move on the technical work before reaching a consensus label Jan 13, 2021
@todd-prior
Copy link

todd-prior commented Jan 13, 2021

I have not attempted to use extra code like this before is there a best practice way to do this??

I just did:
git fetch https://github.com/darktable-org/darktable pull/7820/head:sigmoid_tone_mapping

This creates another new branch (sigmoid_tone_mapping) you can compile.

Thanks I stumbled on to this and managed to do it. It just gave me a version slightly behind where I was but it has the sigmoid module. I guess I need to learn how to merge it first and then build to stay current?? Thanks for the help both of you @chhil

@jandren
Copy link
Contributor Author

jandren commented Jan 13, 2021

Thanks, @TurboGit! Missed that one, fixed!

And thanks to people helping others with checking out a branch from a different origin.

As for compatibility with Filmic and the possibility to replace the splines there with this. I would rather like to see it as an alternative Look transform. One way of reducing confusion for the user could be to have a "look transform module" where filmic, sigmoid, and maybe even a scene-referred version of the base curve could be options. The desired method could then be chosen by a simple drop-down menu.

I appreciate your comments @aurelienpierre but I just wanted to show a small observation that I found very very interesting when learning the darktable source code 💔 I might have missed something important but I will answer as well as I can.

  • It assumes black = 0, yes you can adjust the black point in the exposure module. Including black point adjustment would duplicate functionality.
  • It assumes white = 1, I thought filmic tried to do this as well? Maybe including REC. 2100 as a working profile if REC. 2020 does not have enough dynamic range. I do not have an HDR display so this is not something I could test. My understanding is that scene-referred = [0, inf) and display-referred [0, 1] in whatever display profile you want. This conversion has to happen somewhere and this was a funnily effective and easy one to use. Please correct me if I'm wrong here.
  • it doesn't do chroma preservation, no neither does ACES in the RRT or ODT and that has been my inspiration as I really like their results. I actually tried to add it but that messed with the output and I couldn't trust that values stayed within the [0, 1] range anymore, which I assumed was desired.
  • tone mapping might be the wrong wording, tone curve, characteristic curve or something similar might be a better name. Point taken!
  • preserving contrast, I do not understand this problem. It compresses the range and more compression = more contrast. How on earth would contrast then be preserved? My understanding is that any other form of contrast adjustments should be done before the look transform with tools such as the tone equalizer and in the future, a scene-referred adopted local contrast.

I'm a bit sad that you go so hard out on this @aurelienpierre. I have read and watched a lot of your stuff and I really love how you are pushing a scene-referred workflow within darktable! 😢

@phweyland
Copy link
Contributor

You do realize it's simple only because it takes shortcuts, hardcode things and is overall not future-proof ?

This is a draft, and it works well, at least with moderate DR, probably thanks to your work made on the pipeline and other color calibration module.
To test it, I've reedit some images developed with the modern pipeline and scene referred workflow. I've just turn off filmic, turn on sigmoid, and adjust exposure and sigmoid contrast to get (quickly) more or less the same rendering. Most of the time (not always) I've got a more pleasant image.

  • It assumes black = 0, which prevents any black point compensation,

  • it assumes white = 1, which will prevent future compatibility with HDR displays,

These are convention and you can certainly help to treat that properly.

* it doesn't do chroma preservation, which will induce hue shift and non-uniform changes in saturation and mess up previous color grading,

It would not hurt to add this, yes.

* it doesn't allow to redistribute the dynamic range between shadows and highlights, 
* the contrast (aka slope of the curve) is the same parameter that drives the dynamic range compression, so you can't actually preserve contrast while compressing the range.

With one slider this sounds right. Other modules can help, exposure, color balance or tone equalizer. In fact I prefer this way.


The current pipeline gives really very good results out of the box (again, thanks to your work in particular).
But, as other people, when I want to fix an image which doesn't look as I want, it's sometime very difficult to get it right with filmic. Very often I prefer to let filmic as is, and use other modules (tone equalizer, color balance, ...) to fix it.

On the other hand this sigmoid unique slider does exactly what I expect. I cannot say that for every slider in filmic, starting with the first one (white exposure which changes the shadows, you can justify this by all mean that remains a bit irritating 😃 ).

@aurelienpierre
Copy link
Member

aurelienpierre commented Jan 13, 2021

As for compatibility with Filmic and the possibility to replace the splines there with this. I would rather like to see it as an alternative Look transform. One way of reducing confusion for the user could be to have a "look transform module" where filmic, sigmoid, and maybe even a scene-referred version of the base curve could be options. The desired method could then be chosen by a simple drop-down menu.

Filmic is already a sigmoid, but in a normalized log space. I have to check, but since it's a more general setup than the true sigmoids here, I think you can even design a set of parameters that turn the filmic spline into a close-enough approximation of the sigmoids here.

It assumes black = 0, yes you can adjust the black point in the exposure module. Including black point adjustment would duplicate functionality.

No you can't. Black offset in exposure is non-linear operation and will make the color matching of the input profile fail. Non-linear operations can't be moved like that in the pipeline, the right thing needs to happen at the right place and time.

It assumes white = 1, I thought filmic tried to do this as well? Maybe including REC. 2100 as a working profile if REC. 2020 does not have enough dynamic range. I do not have an HDR display so this is not something I could test. My understanding is that scene-referred = [0, inf) and display-referred [0, 1] in whatever display profile you want. This conversion has to happen

The display white is parametric in filmic. For now the GUI doesn't allow to set it higher than 100% because the Linux graphic stack (actually, the Linux GPU drivers) don't fully support 10 bits HDR, so darktable doesn't export to 10 bits HDR. But the day it does, all you will have to do is move the filmic white display to 400%, and it will be supported just by changing the slider bound.

it doesn't do chroma preservation, no neither does ACES in the RRT or ODT and that has been my inspiration as I really like their results. I actually tried to add it but that messed with the output and I couldn't trust that values stayed within the [0, 1] range anymore, which I assumed was desired.

ACES don't have the same constraints as we have. They don't do paper print, so they don't care about black point. They are intended for video, that is 60 images/s, so they deliver static LUTs for speed and have to compromise on that. And they are currently working on a way to preserve chroma along the view transform because having a view transform that don't respect the artist's color intent is not acceptable. Whether you like the result or not is out of the equation : color corrections are done previously in the pipe, the view transform has to honor them.

preserving contrast, I do not understand this problem. It compresses the range and more compression = more contrast. How on earth would contrast then be preserved? My understanding is that any other form of contrast adjustments should be done before the look transform with tools such as the tone equalizer and in the future, a scene-referred adopted local contrast.

Contrast is the slope of the latitude, aka the zone centered on middle-grey ± something.That is independent from the dynamic range compression (aka the bounds). Having both strongly connected is going to be a problem as dynamic range increases. That's why filmic manages the latitude explicitely and independantly, then use a polynomial fit for shoulder and toe designed for gradient continuity.

I'm a bit sad that you go so hard out on this @aurelienpierre. I have read and watched a lot of your stuff and I really love how you are pushing a scene-referred workflow within darktable!

I'm sorry but this PR ignores half the technical constraints we have to deal with when doing reasonable tone mapping and destroys the workflow by duplicating features.

These are convention and you can certainly help to treat that properly.

Well I don't see how. Sigmoids go from 0 to 1. You could rescale that after the curve, but you are going to change colors too.

But, as other people, when I want to fix an image which doesn't look as I want, it's sometime very difficult to get it right with filmic. Very often I prefer to let filmic as is, and use other modules (tone equalizer, color balance, ...) to fix it.

That's because you mistake a view transform for an artistic contrast adjustment. Tone mapping is not about making the image beautiful, it's about handling the luminance conversion between color spaces that have different dynamic ranges. It's the companion of the output color profile. Just like you have a couple of intents, in output color profiles, to handle the gamut compression with different strategies, you have a couple of contrast preservation intent in filmic to handle the range compression.

And as long as people keep treating filmic as an artistic transform, they will not understand what it does nor how it behaves, and a quicker way to shoot themselves in the foot will not help them.

On the other hand this sigmoid unique slider does exactly what I expect

Try it on real HDR pictures then (16 EV and more).


I feel like we are having the "basic adjustments" module discussion all over again.

@phweyland
Copy link
Contributor

But, as other people, when I want to fix an image which doesn't look as I want, it's sometime very difficult to get it right with filmic. Very often I prefer to let filmic as is, and use other modules (tone equalizer, color balance, ...) to fix it.

That's because you mistake a view transform for an artistic contrast adjustment.

This may not be true. Sometimes, filmic transforms the image in a not desired way and I've difficulty to fix it.

Tone mapping is not about making the image beautiful,

Agreed. But it should still be natural. visually balanced.
The DR settings could be said as purely technical as possible.
But then you offer tools to manage the look, contrast, contrast balancing which are subjective anyway and these tools have their own advantages and limits (which could be different with another design).

You can say that if I'm not happy with my settings (that may happen :)) that's because I don't understand what I'm doing. I accept that. But as long as I don't succeed I feel myself unhappy and filmic guilty :).

In fact it is a bit too easy to use every time the "artistic" argument to disqualify any attempt or request to have a different approach, maybe easier to master, maybe with different artifacts, which, in some circumstances (which are not always HDR 16EV), can help to solve a difficulty or get faster results.

@phweyland
Copy link
Contributor

Black offset in exposure is non-linear operation and will make the color matching of the input profile fail. Non-linear operations can't be moved like that in the pipeline, the right thing needs to happen at the right place and time.

Does that mean we should not use the exposure black offset ? It is not only true for sigmoid ...

This said, I haven't seen any color change / artifact adjusting the black offset.
Don't we push the theory constraints a bit too far here ?

@phweyland
Copy link
Contributor

Try it on real HDR pictures then (16 EV and more).

I would love to give a try. That's for sure interesting (and maybe confusing for a newbie). But I haven't found so far any such raw file. Do you know any free resources for that ?

@todd-prior
Copy link

todd-prior commented Jan 15, 2021 via email

@chhil
Copy link

chhil commented Jan 15, 2021

  • Got sigmoid code, built dt used it a bit on a few images.
  • Now the db has sigmoid references in it.
  • I reset my master to remove sigmoid code from it.
  • Built dt. Installed dt.
  • Does not run as there are references for sigmoid and it can't find the underlying code.

Any pointers on how to get sigmoid references out from the db?
Basically, what should I look for to remove.

At the moment I have added sigmoid back, so that I can continue using DT.

(I should have known better and backed up my db :( )

@todd-prior
Copy link

todd-prior commented Jan 15, 2021 via email

@todd-prior
Copy link

todd-prior commented Jan 15, 2021 via email

@todd-prior
Copy link

todd-prior commented Jan 15, 2021 via email

@chhil
Copy link

chhil commented Jan 15, 2021

Thanks @todd-prior
This is what I ended up doing....
I searched history table where operation was sigmoid , got the image id (having a search by image id in the collection would be nice, had previously required to find an image filename by id when generate cache was used and the error at that time only gave the image id).
Used the image id in the image table and got the file name.
Went into those file in dt and disabled sigmoid and compressed history to remove its reference.

Building master without sigmoid now. I think I should be fine.

@parafin
Copy link
Member

parafin commented Jan 15, 2021

Doesn't dt do a DB backup automatically on schema updates?

@chhil
Copy link

chhil commented Jan 15, 2021

Doesn't dt do a DB backup automatically on schema updates?

I don't recall any db upgrade happening with this module.

@chhil
Copy link

chhil commented Jan 15, 2021

For the record, I did manage to build and run without sigmoid. Did have to clean the build and install directory.

@phweyland
Copy link
Contributor

Could we try to have this in 3.8?

@jakubfi
Copy link
Contributor

jakubfi commented Sep 1, 2021

Could we try to have this in 3.8?

(grabs popcorn)

@johnny-bit
Copy link
Member

Figure out a pathway to merge that into filmic rgb so that filmic becomes "tonemapper" iop with various curves and we can see it in 3.8. there's still (looks at the clock) roughly 3 months to have it done.

I still haven't tried this since filmic is "easy" enough for me... but since Jakob asked i just need to find a time slot for it.

@phweyland
Copy link
Contributor

Figure out a pathway to merge that into filmic rgb

I would like to avoid that!
There are already too many things in filmic. Then it is not so much the tone curve in itself I would like to change, but more the general UI, which over time went away from what I liked.
So I welcome an alternative which works on the same background (scene referred), with a different UI approach and which so far seems to give reasonably good results.

@aurelienpierre
Copy link
Member

There are already too many things in filmic. Then it is not so much the tone curve in itself I would like to change, but more the general UI, which over time went away from what I liked.

Filmic UI is split in tabs where you can easily ignore "display", "options" and "reconstruction" if you don't want them.

It doesn't make sense to have another module for the same feature, users are already confused enough. It's possible to make filmic have 2 modes : simple or advanced, and hide the unneeded GUI control depending on which mode is set. From what I gather (not tested the latest version), filmic would also benefit from the chroma/hue mixed preservation strategies.

@TurboGit
Copy link
Member

TurboGit commented Sep 2, 2021

I agree with @aurelienpierre and @johnny-bit that the way to go is a proper integration into Filmic. Hopefully the 3 months left for integration would be sufficient for 3.8 which will be a very nice addition to dt.

@jandren : How does this plan sound to you?

@phweyland
Copy link
Contributor

@jandren I cannot build any more... Could you rebase please ?

@TurboGit
Copy link
Member

TurboGit commented Oct 1, 2021

@jandren : Can you clarify the situation abut this PR ? Are you ok for the integration in FilmicRGB or should it be closed ? TIA.

@TurboGit
Copy link
Member

No comment since more than a month. I'll close, please reply here and I'll reopen if needed. Thanks.

@TurboGit TurboGit closed this Oct 16, 2021
@jandren
Copy link
Contributor Author

jandren commented Dec 19, 2021

@TurboGit Sorry didn't keep a close eye on this branch during the fall as I tried to hunt down a solution to merge elsewhere and then took a break for other things. I had preferred to not integrate it in filmic as filmic is making several assumptions about white and black points which aren't needed in this module as it is defined for infinity. So it's essentially not compatible with filmic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
controversial this raises concerns, don't move on the technical work before reaching a consensus feature: new new features to add scope: image processing correcting pixels
Projects
None yet
Development

Successfully merging this pull request may close these issues.