colorcalibration also for manual wb

[kofa73]

Addresses #19873

What this would buy us:

• technical
  • when the dual white balance + color calibration method was introduced, white balance used the 'camera reference' (D65) multipliers. Colours, including neutral areas, were fixed in color calibration, via proper CAT. The drawback was that some algorithms are hurt by the not-quite-accurate white balance.
    Quoting 1c5df70, which introduced as-shot to reference - the excerpt below is about the use of reference (D65) multipliers:

    There is one drawback with this approach. Some modules in the pipe would like to have "perfect white balance" correction - the rgb channels all have the same value for any greytone.

    Examples:

    1. Some hightlights reconstruction algos take the other channels or surrounding data into account and modify data "towards white".
    2. raw chromatic aberration correction also iterates from channel differences. Good "white is white" coeffs help significantly here.
    3. Some demosaicers also have slightly improved output on pixelpeeping level.

  • we now take this further, allowing the user to choose RGB multipliers via any method (sliders, colour picker, temperature + tint) supported by the white balance module, to provide neutrals that are even better than the camera's estimate
  • finally, we allow color calibration to use those user-provided multipliers ('as set in white balance module', in addition to 'as shot in camera' introduced in 1c5df70).
• usability
  • these changes allow users to work with controls they are familiar with from other software, like 'temperature' and 'tint', and setting white balance in white balance, a more discoverable place for newcomers.

Changes:

• added a new checkbox, prepare data for color calibration to white balance temperature.c to explicitly control the late_correction flag in 'user' / 'manual' modes
  • the checkbox is only visible in such modes (late_correction is always off in 'camera reference' mode, always on in 'as-shot to reference')
  • if switching from 'camera reference' or 'as-shot to reference' to a 'user' mode, the checkbox is automatically ticked if color calibration is enabled
• replaced reference chr->as_shot with chr->wb_coeffs in highlights.c, colorin.c, opposed.c and segbased.c
• illuminants.h:
  • added a new illuminant, DT_ILLUMINANT_FROM_WB, plus handling in conditionals, similar to DT_ILLUMINANT_CAMERA, but relying on dev->chroma->wb_coeffs
  • find_temperature_from_raw_coeffs:
    • renamed to find_temperature_from_as_shot_coeffs
    • extracted the logic to find_temperature_from_wb_coeffs, to allow directly using coefficients from temperature.c, instead of always going through ratios relative to the D65 values
    • added param wb_coeffs to illuminant_to_xy for absolute coefficients from temperature.c; renamed param custom_wb to correction_ratios to express what it represents; passed to find_temperature_from_wb_coeffs in DT_ILLUMINANT_FROM_WB mode
• channelmixerrgb.c:
  • _dev_is_D65_chroma no longer needs as_shot multipliers in late_correction mode
  • renamed _get_white_balance_coeff to _get_d65_correction_ratios
  • wired up the calls to find_temperature_from_wb_coeffs to handle DT_ILLUMINANT_FROM_WB

Draft for now, to allow discussion (and I haven't tested much, but wanted to make the changes visible to others; plus, I need to clean up some temp files)

[ralfbrown]

Did you intend to include cmixer_orig.c.txt in the PR?
And are the two .md files in the right place?

[kofa73]

Did you intend to include cmixer_orig.c.txt in the PR? And are the two .md files in the right place?

See the end of the description:

Draft for now, to allow discussion (and I haven't tested much, but wanted to make the changes visible to others; plus, I need to clean up some temp files)

[kofa73]

@ralfbrown : extra files removed, thank for your patience

[jenshannoschwalm]

A first complete code reading and remembering your comments before; generally i think this is a minor but very good improvement and will certainly help with some issues. Also it make the intention more clear and fixes some common misunderstandings.

I spotted some places like this:

dt_aligned_pixel_t correction;
  for_four_channels(k)
  {
    if(late && chr->wb_coeffs[k] > 1e-6f)
      correction[k] = chr->D65coeffs[k] / chr->wb_coeffs[k];
    else
      correction[k] = 1.0f;
  }

Could you explain if/why this is nevessary or is it just code-style?

[kofa73]

The key is the line:

correction[k] = chr->D65coeffs[k] / chr->wb_coeffs[k];

This used to be something like (below is one concrete example):

chr->D65coeffs[0] / chr->as_shot[0]

The new denominator comes from chr->wb_coeffs[k] (value set in temperature.c), which means it may be set using one of the sliders. The sliders may have a value of 0, so we must protect the division:

typedef struct dt_iop_temperature_params_t
{
  float red;     // $MIN: 0.0 $MAX: 8.0
  float green;   // $MIN: 0.0 $MAX: 8.0
  float blue;    // $MIN: 0.0 $MAX: 8.0
  float various; // $MIN: 0.0 $MAX: 8.0
  int preset;
} dt_iop_temperature_params_t;

One option would be to restrict the values to MIN: 0.1 or so, then we can avoid adding the protection everywhere.

If I misunderstood the question, please let me know.

[kofa73]

Any updates? There have been no new commits for a while, just resolving conflicts.

[TurboGit]

@kofa73 : This gets under the radar and I had many things to do for the AI integration. The discussion on pixl.us was difficult and my motivation was quite down for some time... I'm still a simple human :) I'll get back to this for 5.6.

[TurboGit]

Ok, I see this has been reviewed and blessed by @jenshannoschwalm so I'll test as soon as the conflict is resolved. The code looks ok to me, I just need to play with this to have a better understanding of the workflow. TIA.

[kofa73]

I'm running the usual panel review on it (eating my own dog food) :-)

Update: oh boy. :-D

[TurboGit]

I'm not sure to understand the workflow here.

If you click on "prepare data for color calibration" we don't have the warning about duplicate WB setting. This is fine.

Now as soon as I change the temp or hue in the panel the user defined tab is selected (pencil). Fine but the image is not changed in any way. Actually you can drag de sliders in all direction and it changes nothing. That's really unexpected and certainly something even worst than the current situation with the two possible WB settings.

Also why in the last tab (ref camera, the light bulb) there is no check for "prepare data for color calibration"? Again this is quite confusing.

All in all, the UI/UX is not good to me and far more messy than the current one. Maybe there is room for improvement?

[kofa73]

as soon as I change the temp or hue in the panel the user defined tab is selected (pencil). Fine but the image is not changed in any way

Yes, also discovered by the bots. I'll take care of it, not a big deal.

Also why in the last tab (ref camera, the light bulb) there is no check for "prepare data for color calibration"?

prepare data for color calibration is simply the late correction exposed, and camera reference does not use it (no late correction is needed, as the data is already "correct": it uses the camera reference multipliers, there's nothing to "correct" (convert from arbitrary WB multipliers to reference multipliers)). Previously, late correction was only used by "as shot to reference", and "to reference" is exactly what we need for the user modes, as well (in colorin, undo the multipliers applied in white balance and apply camera reference multipliers, so color calibration receives reference-referred data).

All in all, the UI/UX is not good to me and far more messy than the current one. Maybe there is room for improvement?

Sure, and I thank you for the feedback. I have no better idea, unfortunately: late correction has to be set somehow. Maybe I can check if there's a color calibration instance with CAT on the pipeline, and adjust it automatically.