color balance rgb can make extreme highlights go black when using darktable UCS saturation formula

[benp456]

I have observed this issue with darktable 4.0, both CPU and OpenCL.

Steps to reproduce: Open an image and disable all non-mandatory modules Enable color balance rgb and drag the highlights slider in the perceptual brilliance section to the right Enable exposure and increase the exposure until there are severely overexposed regions in the image

The issue seems to occur only with moderate to large highlight boosts when there are already overexposed highlights.

Expected behaviour is the highlights clip at white, but at a certain point while increasing exposure they will go black or sometimes grey

If the source image contains sufficiently bright highlights then the exposure module is not needed to reproduce the bug

Please see attached image and xmp for a nonsynthetic example (excuse the poor photography but I was trying to see what kind of sunstars I could get with my new lens!)

With darktable UCS saturation formula

With JzAzBz saturation formula

Image and xmp below should show the issue but I think it's easy to reproduce on any image with bright highlights DSC_4844.zip DSC_4844.NEF.xmp.txt

[aurelienpierre]

Did you set the white fulcrum slider in mask tab to the proper white value (using the color picker) ?

[benp456]

I had not tried that, but doing it now, the slider has only a minor effect on which pixels go black. Using the color picker and selecting the sun for example does not change the qualitative result.

[aurelienpierre]

I see what you did. The brilliance boost for highlights is far too high, which means you are making white "whiter". Since we are working in a perceptual space, white cannot be brightened without voiding all the assumptions needed by the color space. I suspect you are shifting the design use case of color balance by using it for contrast, but it can only be used to make colors paler or darker. Brilliance boost up to +50% or so are fine in highlights, but more will create problems.

The relevant code is here:

      const float radius = hypotf(HCB[1], HCB[2]);
      const float sin_T = (radius > 0.f) ? HCB[1] / radius : 0.f;
      const float cos_T = (radius > 0.f) ? HCB[2] / radius : 0.f;
      const float DT_ALIGNED_PIXEL M_rot_inv[2][2] = { { cos_T,  sin_T }, { -sin_T, cos_T } };

      float P = HCB[1];
      float W = sin_T * HCB[1] + cos_T * HCB[2];

      float a = fmaxf(1.f + d->saturation_global + scalar_product(opacities, saturation), 0.f);
      const float b = fmaxf(1.f + d->brilliance_global + scalar_product(opacities, brilliance), 0.f);

      const float max_a = hypotf(P, W) / P;
      a = soft_clip(a, 0.5f * max_a, max_a);
      const float P_prime = (a - 1.f) * P;
      const float W_prime = sqrtf(sqf(P) * (1.f - sqf(a)) + sqf(W)) * b;

      HCB[1] = fmaxf(M_rot_inv[0][0] * P_prime + M_rot_inv[0][1] * W_prime, 0.f);
      HCB[2] = fmaxf(M_rot_inv[1][0] * P_prime + M_rot_inv[1][1] * W_prime, 0.f);

when b gets too high, the problem appears. HCB[2] needs to be clamped between 0 and 1, clipping to 0 only ensures black doesn't go negative, but lets white go whiter and fuck up the whole space.

[aurelienpierre]

But after more tests, the trick above will lead to non-smooth white blotches even where the algo didn't degrade to full black like the example above, so it's damaging valid uses in order to protect against insane values. That's heading to a won't fix.

[benp456]

Thanks for the explanation - certainly it's easy to avoid.

[flannelhead]

@aurelienpierre : this patch

diff --git a/src/common/colorspaces_inline_conversions.h b/src/common/colorspaces_inline_conversions.h
index 102543a1a8..907f64278d 100644
--- a/src/common/colorspaces_inline_conversions.h
+++ b/src/common/colorspaces_inline_conversions.h
@@ -1376,7 +1376,7 @@ static inline void dt_UCS_JCH_to_xyY(const dt_aligned_pixel_t JCH, const float L
   */

   // should be L_star = powf(JCH[0], 1.f / cz) * L_white but we treat only the case where cz = 1
-  const float L_star = JCH[0] * L_white;
+  const float L_star = MIN(JCH[0] * L_white, 2.09888f);
   const float M = powf(JCH[1] * L_white / (15.932993652962535f * powf(L_star, 0.6523997524738018f)), 0.8322850678616855f);

   const float U_star_prime = M * cosf(JCH[2]);

is enough to get rid of these black areas. After all there's a hard limit to lightness in dt UCS, as commented in

static inline float dt_UCS_L_star_to_Y(const float L_star)
{
  // WARNING: L_star needs to be < 2.098883786377, meaning Y needs to be < 3.875766378407574e+19
  return powf((1.12426773749357f * L_star / (2.098883786377f - L_star)), 1.5831518565279648f);
}

Probably it's best to sanitize to honor that after making adjustments in JCH or HSB.

[aurelienpierre]

This fix has unexpected side effects that need investigation.

[ptilopteri]

since you apparently already know what the "unexpected side effects" are, would it not follow that you would provide those small details? tks,

[github-actions[bot]]

This issue did not get any activity in the past 60 days and will be closed in 365 days if no update occurs. Please check if the master branch has fixed it and report again or close the issue.

[github-actions[bot]]

This issue was closed because it has been inactive for 300 days since being marked as stale. Please check if the newest release or nightly build has it fixed. Please, create a new issue if the issue is not fixed.