From e9f0b6ac60089550b23132b3ff24ec75d63a30f6 Mon Sep 17 00:00:00 2001
From: Alberto Griggio <agriggio@users.noreply.github.com>
Date: Thu, 1 Nov 2018 13:34:10 +0100
Subject: [PATCH] L*a*b* regions: some code cleanups

---
 rtengine/iplabregions.cc |  6 ++++--
 rtgui/colortoning.cc     | 44 ++++++++++++++++++++--------------------
 2 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/rtengine/iplabregions.cc b/rtengine/iplabregions.cc
index 1d42b39f1..2a4d431e6 100644
--- a/rtengine/iplabregions.cc
+++ b/rtengine/iplabregions.cc
@@ -75,9 +75,11 @@ void ImProcFunctions::labColorCorrectionRegions(LabImage *lab)
             float b = lab->b[y][x];
             float c, h;
             Color::Lab2Lch(a, b, c, h);
-            float c1 = lin2log(c * (327.68f / 48000.f), 10.f);
+            // magic constant c_factor: normally chromaticity is in [0; 42000] (see color.h), but here we use the constant to match how the chromaticity pipette works (see improcfun.cc lines 4705-4706 and color.cc line 1930
+            constexpr float c_factor = 327.68f / 48000.f;
+            float c1 = lin2log(c * c_factor, 10.f);
             float h1 = Color::huelab_to_huehsv2(h);
-            h1 = h1 + 1.f/6.f;
+            h1 = h1 + 1.f/6.f; // offset the hue because we start from purple instead of red
             if (h1 > 1.f) {
                 h1 -= 1.f;
             }
diff --git a/rtgui/colortoning.cc b/rtgui/colortoning.cc
index 2b80c6be2..b6e001b70 100644
--- a/rtgui/colortoning.cc
+++ b/rtgui/colortoning.cc
@@ -11,7 +11,23 @@ using namespace rtengine;
 using namespace rtengine::procparams;
 
 
-static constexpr int ID_LABREGION_HUE = 5;
+namespace {
+
+constexpr int ID_LABREGION_HUE = 5;
+
+inline bool hasMask(const std::vector<double> &dflt, const std::vector<double> &mask)
+{
+    return !(mask.empty() || mask[0] == FCT_Linear || mask == dflt);
+}
+
+
+inline float round_ab(float v)
+{
+    return int(v * 1000) / 1000.f;
+}
+
+} // namespace
+
 
 ColorToning::ColorToning () : FoldableToolPanel(this, "colortoning", M("TP_COLORTONING_LABEL"), false, true)
 {
@@ -1403,6 +1419,10 @@ void ColorToning::onLabRegionSelectionChanged()
 
 void ColorToning::labRegionGet(int idx)
 {
+    if (idx < 0 || size_t(idx) >= labRegionData.size()) {
+        return;
+    }
+    
     auto &r = labRegionData[idx];
     double la, lb;
     labRegionAB->getParams(la, lb, r.a, r.b);
@@ -1482,26 +1502,6 @@ void ColorToning::labRegionShowMaskChanged()
 }
 
 
-namespace {
-
-bool hasMask(const std::vector<double> &dflt, const std::vector<double> &mask)
-{
-    if (mask.empty() || mask[0] == FCT_Linear || mask == dflt) {
-        return false;
-    } else {
-        return true;
-    }
-}
-
-
-inline float round_ab(float v)
-{
-    return int(v * 1000) / 1000.f;
-}
-
-} // namespace
-
-
 void ColorToning::labRegionPopulateList()
 {
     ConnectionBlocker b(labRegionSelectionConn);
@@ -1524,7 +1524,7 @@ void ColorToning::labRegionPopulateList()
 
 void ColorToning::labRegionShow(int idx, bool list_only)
 {
-    bool disable = listener;
+    const bool disable = listener;
     if (disable) {
         disableListener();
     }