darktable-org / dtdocs

darktable user manual
GNU General Public License v3.0
74 stars 74 forks source link

improve handling of luminance estimators #244

Closed matt-maguire closed 1 year ago

matt-maguire commented 3 years ago

During the discussion on pixls: https://discuss.pixls.us/t/a-plea-for-more-guidance-in-the-darktable-manual/24118

it became clear that the documerntation concerning luminance estimators could perhaps be better organised in the documentation.

Luminance estimators are used in a variety of modules where the "luminance" of a pixel needs to be estimated for purposes of tone mapping. examples are:

the option may be called various names, such as "luminance estimator", "luiminance", "preserve colours", etc..

It makes sense to factor out the description of possible values into the Darkroom->prosessing_modules>curves section, and reference this section from the various modules using luminance estimators. At the moment, the most comprehensive description of possible values is in the filmicrgb module reference page (which is mainly reflective of the order in which the manual pages were written).

I suggest to pull that information out into the general "curves" page, and in the module reference pages refer back to that description in the curves page.

elstoc commented 3 years ago

Should tone equalizer be included here? All of the others are about chrominance preservation but tone equalizer is about guided filters. If they are the same thing but with different usage, how should we treat this in the documentation?

Should we align the names in the UI to make it clear?

There are some options that are available in the curves only, and not in filmic.

matt-maguire commented 3 years ago

Chrominance preservation means rather than do tone mapping on color channels separately/individually, you characterise the "luminance" of a pixel by a single value, and then scale all three RGB channels by the same percentage, based on where the luminance estimation falls on the tone mapping curve.

In tone equalizer, you characterise the "luminance" of a pixel by a single value that is used to build the mask, then you scale all 3 RGB channels by the same percentage based on your equalizer controls.

So, in both cases, you are doing the same thing, which is why tone equalizer is in the list. The only difference is that while for tone curves you can treat the RGB channels separately (preserve chrominance=no), in tone equaliser you must always use a luminance estimator to build up the mask. So, for tone equaliser, "preserve chrominace=no" is not an option.

You will get some variation about which estimators are allowed for different modules (eg. legacy modules may let you use the "L" in Lab, whereas the new scene-referred modules do not include that option, since it doesn't behave well for HDR cases). We could list out the possible estimator types for each module, and describe the superset in the generic "curves" section. Or, we can assume the use sits in front of the software and can see the list for himself by selecting the dropdown menu. I think a reference manual should have the complete info, but it makes it harder to maintain.

I think it would make sense to come up with a consistent naming scheme in the GUI, but not sure how keen our devs are to mess around with changing legacy module parameter names.

elstoc commented 3 years ago

Probably needs @aurelienpierre to weigh in here.

github-actions[bot] commented 3 years ago

This issue has not had any activity in the past 60 days and will be closed in 365 days if not updated.

aurelienpierre commented 2 years ago

Chrominance preservation means rather than do tone mapping on color channels separately/individually, you characterise the "luminance" of a pixel by a single value, and then scale all three RGB channels by the same percentage, based on where the luminance estimation falls on the tone mapping curve.

Yup. Which actually falls-back to an exposure compensation by a factor defined with a curve, instead of the flat factor defined in exposure module.

elstoc commented 2 years ago

@matt-maguire is that enough information for you to work on a PR? I'm certainly happy for the documentation to be consolidated if it will help.

github-actions[bot] commented 2 years ago

This issue has not had any activity in the past 60 days and will be closed in 365 days if not updated.

github-actions[bot] commented 1 year ago

This issue has not had any activity in the past year and has therefore been closed.