From 8accebe4161c581c76dda4edee027b95ec109ea9 Mon Sep 17 00:00:00 2001 From: Lawrence Lee <45837045+Lawrence37@users.noreply.github.com> Date: Sun, 28 May 2023 18:15:27 -0700 Subject: [PATCH] Fix incorrect sampled L*a*b* values Use LCMS to convert values back into L*a*b*. The pipette buffer has the output or working profile applied with LCMS. Performing the inverse operation fixes the incorrect values shown in the navigator, histogram indicator bars, and lockable color pickers. --- rtengine/improcfun.cc | 20 ++++++++++++++ rtengine/improcfun.h | 2 ++ rtgui/cropwindow.cc | 10 +++---- rtgui/histogrampanel.cc | 52 +++++++++++++++++++++++++---------- rtgui/histogrampanel.h | 8 +++--- rtgui/lockablecolorpicker.cc | 23 ++++++++++++++-- rtgui/lockablecolorpicker.h | 17 ++++++++++-- rtgui/navigator.cc | 25 ++++++++++++++--- rtgui/navigator.h | 4 +-- rtgui/pointermotionlistener.h | 19 ++++++++++++- 10 files changed, 144 insertions(+), 36 deletions(-) diff --git a/rtengine/improcfun.cc b/rtengine/improcfun.cc index c3e11c076..5d527c818 100644 --- a/rtengine/improcfun.cc +++ b/rtengine/improcfun.cc @@ -5667,6 +5667,11 @@ void ImProcFunctions::rgb2lab(const Imagefloat &src, LabImage &dst, const Glib:: } void ImProcFunctions::rgb2lab(const Image8 &src, int x, int y, int w, int h, float L[], float a[], float b[], const procparams::ColorManagementParams &icm, bool consider_histogram_settings) const +{ + rgb2lab(src, x, y, w, h, L, a, b, icm, consider_histogram_settings, multiThread); +} + +void ImProcFunctions::rgb2lab(const Image8 &src, int x, int y, int w, int h, float L[], float a[], float b[], const procparams::ColorManagementParams &icm, bool consider_histogram_settings, bool multiThread) { // Adapted from ImProcFunctions::lab2rgb const int src_width = src.getWidth(); @@ -5779,6 +5784,21 @@ void ImProcFunctions::rgb2lab(const Image8 &src, int x, int y, int w, int h, flo } } +void ImProcFunctions::rgb2lab(std::uint8_t red, std::uint8_t green, std::uint8_t blue, float &L, float &a, float &b, const procparams::ColorManagementParams &icm, bool consider_histogram_settings) +{ + float l_channel[1]; + float a_channel[1]; + float b_channel[1]; + rtengine::Image8 buf(1, 1); + buf.r(0, 0) = red; + buf.g(0, 0) = green; + buf.b(0, 0) = blue; + ImProcFunctions::rgb2lab(buf, 0, 0, 1, 1, l_channel, a_channel, b_channel, icm, consider_histogram_settings, false); + L = l_channel[0]; + a = a_channel[0]; + b = b_channel[0]; +} + void ImProcFunctions::lab2rgb(const LabImage &src, Imagefloat &dst, const Glib::ustring &workingSpace) { TMatrix wiprof = ICCStore::getInstance()->workingSpaceInverseMatrix(workingSpace); diff --git a/rtengine/improcfun.h b/rtengine/improcfun.h index 36c571291..3fddbd753 100644 --- a/rtengine/improcfun.h +++ b/rtengine/improcfun.h @@ -116,6 +116,7 @@ class ImProcFunctions bool multiThread; void calcVignettingParams(int oW, int oH, const procparams::VignettingParams& vignetting, double &w2, double &h2, double& maxRadius, double &v, double &b, double &mul); + static void rgb2lab(const Image8 &src, int x, int y, int w, int h, float L[], float a[], float b[], const procparams::ColorManagementParams &icm, bool consider_histogram_settings, bool multithread); void transformLuminanceOnly(Imagefloat* original, Imagefloat* transformed, int cx, int cy, int oW, int oH, int fW, int fH); void transformGeneral(bool highQuality, Imagefloat *original, Imagefloat *transformed, int cx, int cy, int sx, int sy, int oW, int oH, int fW, int fH, const LensCorrection *pLCPMap, bool useOriginalBuffer); @@ -497,6 +498,7 @@ enum class BlurType { Image8* lab2rgb(LabImage* lab, int cx, int cy, int cw, int ch, const procparams::ColorManagementParams &icm, bool consider_histogram_settings = true); void rgb2lab(const Image8 &src, int x, int y, int w, int h, float L[], float a[], float b[], const procparams::ColorManagementParams &icm, bool consider_histogram_settings = true) const; + static void rgb2lab(std::uint8_t red, std::uint8_t green, std::uint8_t blue, float &L, float &a, float &b, const procparams::ColorManagementParams &icm, bool consider_histogram_settings = true); Imagefloat* lab2rgbOut(LabImage* lab, int cx, int cy, int cw, int ch, const procparams::ColorManagementParams &icm); // CieImage *ciec; void workingtrc(const Imagefloat* src, Imagefloat* dst, int cw, int ch, int mul, Glib::ustring &profile, double gampos, double slpos, int &illum, int prim, cmsHTRANSFORM &transform, bool normalizeIn = true, bool normalizeOut = true, bool keepTransForm = false) const; diff --git a/rtgui/cropwindow.cc b/rtgui/cropwindow.cc index c7d7348b3..0cd6f3f6c 100644 --- a/rtgui/cropwindow.cc +++ b/rtgui/cropwindow.cc @@ -403,7 +403,7 @@ void CropWindow::buttonPress (int button, int type, int bstate, int x, int y) // Add a new Color Picker rtengine::Coord imgPos; screenCoordToImage(x, y, imgPos.x, imgPos.y); - LockableColorPicker *newPicker = new LockableColorPicker(this, &cropHandler.colorParams->outputProfile, &cropHandler.colorParams->workingProfile); + LockableColorPicker *newPicker = new LockableColorPicker(this, cropHandler.colorParams.get()); colorPickers.push_back(newPicker); hoveredPicker = newPicker; updateHoveredPicker(&imgPos); @@ -1095,10 +1095,10 @@ void CropWindow::pointerMoved (int bstate, int x, int y) printf("Using \"%s\" output\n", outputProfile.c_str()); if(outputProfile==options.rtSettings.srgb) printf("OK SRGB2"); */ - pmlistener->pointerMoved (false, cropHandler.colorParams->outputProfile, cropHandler.colorParams->workingProfile, mx, my, -1, -1, -1); + pmlistener->pointerMoved (false, *cropHandler.colorParams, mx, my, -1, -1, -1); if (pmhlistener) { - pmhlistener->pointerMoved (false, cropHandler.colorParams->outputProfile, cropHandler.colorParams->workingProfile, mx, my, -1, -1, -1); + pmhlistener->pointerMoved (false, *cropHandler.colorParams, mx, my, -1, -1, -1); } } else { @@ -1145,11 +1145,11 @@ void CropWindow::pointerMoved (int bstate, int x, int y) // Updates the Navigator // TODO: possible double color conversion if rval, gval, bval come from cropHandler.cropPixbuftrue ? see issue #4583 - pmlistener->pointerMoved (true, cropHandler.colorParams->outputProfile, cropHandler.colorParams->workingProfile, mx, my, rval, gval, bval, isRaw); + pmlistener->pointerMoved (true, *cropHandler.colorParams, mx, my, rval, gval, bval, isRaw); if (pmhlistener) { // Updates the HistogramRGBArea - pmhlistener->pointerMoved (true, cropHandler.colorParams->outputProfile, cropHandler.colorParams->workingProfile, mx, my, rval, gval, bval); + pmhlistener->pointerMoved (true, *cropHandler.colorParams, mx, my, rval, gval, bval); } } } diff --git a/rtgui/histogrampanel.cc b/rtgui/histogrampanel.cc index 6bda0812e..821f6f0be 100644 --- a/rtgui/histogrampanel.cc +++ b/rtgui/histogrampanel.cc @@ -22,10 +22,11 @@ #include "options.h" #include #include -#include "../rtengine/array2D.h" -#include "../rtengine/LUT.h" #include "rtimage.h" +#include "../rtengine/array2D.h" #include "../rtengine/color.h" +#include "../rtengine/improcfun.h" +#include "../rtengine/LUT.h" using namespace rtengine; @@ -34,12 +35,20 @@ constexpr float HistogramArea::MIN_BRIGHT; using ScopeType = Options::ScopeType; + +namespace +{ + +const rtengine::procparams::ColorManagementParams DEFAULT_CMP; + +} + // // // HistogramPanel HistogramPanel::HistogramPanel () : pointer_moved_delayed_call( - [this](bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int r, int g, int b) + [this](bool validPos, const rtengine::procparams::ColorManagementParams *cmp, int r, int g, int b) { bool update_hist_area; @@ -52,9 +61,9 @@ HistogramPanel::HistogramPanel () : } else { // do something to show vertical bars if (histogramRGBArea) { - histogramRGBArea->updateBackBuffer(r, g, b, profile, profileW); + histogramRGBArea->updateBackBuffer(r, g, b, cmp); } - update_hist_area = histogramArea->updatePointer(r, g, b, profile, profileW); + update_hist_area = histogramArea->updatePointer(r, g, b, cmp); } if (histogramRGBArea) { histogramRGBArea->queue_draw(); @@ -623,9 +632,9 @@ void HistogramPanel::setHistRGBInvalid () histogramRGBArea->queue_draw (); } -void HistogramPanel::pointerMoved (bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int x, int y, int r, int g, int b, bool isRaw) +void HistogramPanel::pointerMoved(bool validPos, const rtengine::procparams::ColorManagementParams &cmp, int x, int y, int r, int g, int b, bool isRaw) { - pointer_moved_delayed_call(validPos, profile, profileW, r, g, b); + pointer_moved_delayed_call(validPos, &cmp, r, g, b); } /* @@ -797,7 +806,7 @@ void HistogramRGBArea::setShow(bool show) showMode = show; } -void HistogramRGBArea::updateBackBuffer (int r, int g, int b, const Glib::ustring &profile, const Glib::ustring &profileW) +void HistogramRGBArea::updateBackBuffer(int r, int g, int b, const rtengine::procparams::ColorManagementParams *cmp) { if (!get_realized () || !showMode || !( options.histogramScopeType == ScopeType::HISTOGRAM @@ -856,19 +865,25 @@ void HistogramRGBArea::updateBackBuffer (int r, int g, int b, const Glib::ustrin || options.histogramScopeType == ScopeType::WAVEFORM) ) { float Lab_L, Lab_a, Lab_b; - rtengine::Color::rgb2lab01(profile, profileW, r / 255.f, g / 255.f, b / 255.f, Lab_L, Lab_a, Lab_b, options.rtSettings.HistogramWorking); + ImProcFunctions::rgb2lab( + static_cast(r), + static_cast(g), + static_cast(b), + Lab_L, Lab_a, Lab_b, + cmp != nullptr ? *cmp : DEFAULT_CMP, + true); if (needLuma) { // Luma cc->set_source_rgb(1.0, 1.0, 1.0); - drawBar(cc, Lab_L, 100.0, winw, winh, s); + drawBar(cc, Lab_L, 32768., winw, winh, s); } if (needChroma && options.histogramScopeType == ScopeType::HISTOGRAM) { // Chroma - double chromaval = sqrt(Lab_a * Lab_a + Lab_b * Lab_b) / 1.8; + double chromaval = sqrt(Lab_a * Lab_a + Lab_b * Lab_b) / (255. * 188); cc->set_source_rgb(0.9, 0.9, 0.0); - drawBar(cc, chromaval, 100.0, winw, winh, s); + drawBar(cc, chromaval, 1.0, winw, winh, s); } } } @@ -1443,7 +1458,7 @@ void HistogramArea::updateBackBuffer () setDirty(false); } -bool HistogramArea::updatePointer(int r, int g, int b, const Glib::ustring &profile, const Glib::ustring &profileW) +bool HistogramArea::updatePointer(int r, int g, int b, const rtengine::procparams::ColorManagementParams *cmp) { if (!needPointer || !(scopeType == ScopeType::VECTORSCOPE_HC || scopeType == ScopeType::VECTORSCOPE_HS)) { return false; @@ -1456,7 +1471,16 @@ bool HistogramArea::updatePointer(int r, int g, int b, const Glib::ustring &prof pointer_red = r; pointer_green = g; pointer_blue = b; - Color::rgb2lab01(profile, profileW, r / 255.f, g / 255.f, b / 255.f, L, pointer_a, pointer_b, options.rtSettings.HistogramWorking); + ImProcFunctions::rgb2lab( + static_cast(r), + static_cast(g), + static_cast(b), + L, pointer_a, pointer_b, + cmp != nullptr ? *cmp : DEFAULT_CMP, + true); + L /= 327.68f; + pointer_a /= 327.68f; + pointer_b /= 327.68f; updateBackBuffer(); return true; } diff --git a/rtgui/histogrampanel.h b/rtgui/histogrampanel.h index 26b744546..7f165bb15 100644 --- a/rtgui/histogrampanel.h +++ b/rtgui/histogrampanel.h @@ -99,7 +99,7 @@ public: HistogramRGBArea(); ~HistogramRGBArea() override; - void updateBackBuffer (int r, int g, int b, const Glib::ustring &profile = "", const Glib::ustring &profileW = ""); + void updateBackBuffer(int r, int g, int b, const rtengine::procparams::ColorManagementParams *cmp = nullptr); bool getShow (); void setShow(bool show); void setParent (Gtk::Grid* p) @@ -206,7 +206,7 @@ public: void updateBackBuffer (); /// Update pointer values. Returns true if widget needs redrawing. - bool updatePointer(int r, int g, int b, const Glib::ustring &profile = "", const Glib::ustring &profileW = ""); + bool updatePointer(int r, int g, int b, const rtengine::procparams::ColorManagementParams *cmp = nullptr); void update( const LUTu& histRed, const LUTu& histGreen, @@ -260,7 +260,7 @@ public: class HistogramPanel final : public Gtk::Grid, public PointerMotionListener, public DrawModeListener, public rtengine::NonCopyable { private: - DelayedCall pointer_moved_delayed_call; + DelayedCall pointer_moved_delayed_call; protected: @@ -343,7 +343,7 @@ public: histogramArea->update(histRed, histGreen, histBlue, histLuma, histChroma, histRedRaw, histGreenRaw, histBlueRaw, vectorscopeScale, vectorscopeHC, vectorscopeHS, waveformScale, waveformRed, waveformGreen, waveformBlue, waveformLuma); } // pointermotionlistener interface - void pointerMoved (bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int x, int y, int r, int g, int b, bool isRaw = false) override; + void pointerMoved (bool validPos, const rtengine::procparams::ColorManagementParams &cmp, int x, int y, int r, int g, int b, bool isRaw = false) override; // TODO should be protected void setHistRGBInvalid (); diff --git a/rtgui/lockablecolorpicker.cc b/rtgui/lockablecolorpicker.cc index 0a08bb945..c87239b9c 100644 --- a/rtgui/lockablecolorpicker.cc +++ b/rtgui/lockablecolorpicker.cc @@ -20,15 +20,23 @@ #include "lockablecolorpicker.h" #include "options.h" #include "../rtengine/color.h" +#include "../rtengine/improcfun.h" #include "../rtengine/rt_math.h" #include "../rtengine/utils.h" #include "imagearea.h" #include "multilangmgr.h" #include "navigator.h" -LockableColorPicker::LockableColorPicker (CropWindow* cropWindow, Glib::ustring *oProfile, Glib::ustring *wProfile) +namespace +{ + +const rtengine::procparams::ColorManagementParams DEFAULT_CMP; + +} + +LockableColorPicker::LockableColorPicker (CropWindow* cropWindow, rtengine::procparams::ColorManagementParams *color_management_params) : cropWindow(cropWindow), displayedValues(ColorPickerType::RGB), position(0, 0), size(Size::S15), - outputProfile(oProfile), workingProfile(wProfile), validity(Validity::OUTSIDE), + color_management_params(color_management_params), validity(Validity::OUTSIDE), r(0.f), g(0.f), b(0.f), rpreview(0.f), gpreview(0.f), bpreview(0.f), hue(0.f), sat(0.f), val(0.f), L(0.f), a(0.f), bb(0.f) {} @@ -277,7 +285,16 @@ void LockableColorPicker::setRGB (const float R, const float G, const float B, c bpreview = previewB; rtengine::Color::rgb2hsv01(r, g, b, hue, sat, val); - rtengine::Color::rgb2lab01(*outputProfile, *workingProfile, r, g, b, L, a, bb, options.rtSettings.HistogramWorking); // TODO: Really sure this function works? + rtengine::ImProcFunctions::rgb2lab( + static_cast(255 * r), + static_cast(255 * g), + static_cast(255 * b), + L, a, bb, + color_management_params != nullptr ? *color_management_params : DEFAULT_CMP, + true); + L /= 327.68f; + a /= 327.68f; + bb /= 327.68f; if (validity != Validity::OUTSIDE) { setDirty(true); diff --git a/rtgui/lockablecolorpicker.h b/rtgui/lockablecolorpicker.h index baeea41ef..b18a56028 100644 --- a/rtgui/lockablecolorpicker.h +++ b/rtgui/lockablecolorpicker.h @@ -24,6 +24,18 @@ class CropWindow; +namespace rtengine +{ + +namespace procparams +{ + +class ColorManagementParams; + +} + +} + class LockablePickerToolListener { public: @@ -60,8 +72,7 @@ private: rtengine::Coord position; // Coordinate in image space rtengine::Coord anchorOffset; Size size; - Glib::ustring *outputProfile; - Glib::ustring *workingProfile; + rtengine::procparams::ColorManagementParams *color_management_params; Validity validity; float r, g, b; // red green blue in [0;1] range float rpreview, gpreview, bpreview; @@ -72,7 +83,7 @@ private: public: - LockableColorPicker (CropWindow* cropWindow, Glib::ustring *oProfile, Glib::ustring *wProfile); + LockableColorPicker (CropWindow* cropWindow, rtengine::procparams::ColorManagementParams *color_management_params); void draw (const Cairo::RefPtr &cr); diff --git a/rtgui/navigator.cc b/rtgui/navigator.cc index 42f605fa2..042f57a00 100644 --- a/rtgui/navigator.cc +++ b/rtgui/navigator.cc @@ -21,18 +21,26 @@ #include "previewwindow.h" #include "toolpanel.h" #include "../rtengine/color.h" +#include "../rtengine/improcfun.h" #include "../rtengine/rt_math.h" #include "options.h" using namespace rtengine; +namespace +{ + +const rtengine::procparams::ColorManagementParams DEFAULT_CMP; + +} + Navigator::Navigator() : pointer_moved_delayed_call(50, 100), currentRGBUnit(options.navRGBUnit), currentHSVUnit(options.navHSVUnit) { pointer_moved_delayed_call.setFunction( - [this](bool validPos, Glib::ustring profile, Glib::ustring profileW, int x, int y, int r, int g, int b, bool isRaw) + [this](bool validPos, const rtengine::procparams::ColorManagementParams *cmp, int x, int y, int r, int g, int b, bool isRaw) { if (!validPos) { setInvalid (x, y); @@ -61,7 +69,16 @@ Navigator::Navigator() : S->set_text (s2); V->set_text (s3); - Color::rgb2lab01(profile, profileW, r / 255.f, g / 255.f, b / 255.f, LAB_l, LAB_a, LAB_b, options.rtSettings.HistogramWorking); // TODO: Really sure this function works? + ImProcFunctions::rgb2lab( + static_cast(r), + static_cast(g), + static_cast(b), + LAB_l, LAB_a, LAB_b, + cmp != nullptr ? *cmp : DEFAULT_CMP, + true); + LAB_l /= 327.68f; + LAB_a /= 327.68f; + LAB_b /= 327.68f; getLABText (LAB_l, LAB_a, LAB_b, s1, s2, s3); LAB_L->set_text (s1); LAB_A->set_text (s2); @@ -328,9 +345,9 @@ void Navigator::getLABText (float l, float a, float b, Glib::ustring &sL, Glib:: } // if !validPos then x/y contain the full image size -void Navigator::pointerMoved (bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int x, int y, int r, int g, int b, bool isRaw) +void Navigator::pointerMoved (bool validPos, const rtengine::procparams::ColorManagementParams &cmp, int x, int y, int r, int g, int b, bool isRaw) { - pointer_moved_delayed_call(validPos, profile, profileW, x, y, r, g, b, isRaw); + pointer_moved_delayed_call(validPos, &cmp, x, y, r, g, b, isRaw); } void Navigator::cycleUnitsRGB (GdkEventButton *event) { diff --git a/rtgui/navigator.h b/rtgui/navigator.h index 4c2a3fd32..c16bf7ac7 100644 --- a/rtgui/navigator.h +++ b/rtgui/navigator.h @@ -34,7 +34,7 @@ class Navigator final : typedef const double (*TMatrix)[3]; private: - DelayedCall pointer_moved_delayed_call; + DelayedCall pointer_moved_delayed_call; Options::NavigatorUnit currentRGBUnit; Options::NavigatorUnit currentHSVUnit; @@ -61,7 +61,7 @@ public: // pointermotionlistener interface // void pointerMoved (bool validPos, int x, int y, int r, int g, int b); - void pointerMoved (bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int x, int y, int r, int g, int b, bool raw = false) override; + void pointerMoved(bool validPos, const rtengine::procparams::ColorManagementParams &cmp, int x, int y, int r, int g, int b, bool raw = false) override; void setInvalid (int fullWidth = -1, int fullHeight = -1); void getRGBText (int r, int g, int b, Glib::ustring &sR, Glib::ustring &sG, Glib::ustring &sB, bool isRaw = false) override; diff --git a/rtgui/pointermotionlistener.h b/rtgui/pointermotionlistener.h index 26ca994b0..f9fda419c 100644 --- a/rtgui/pointermotionlistener.h +++ b/rtgui/pointermotionlistener.h @@ -18,6 +18,23 @@ */ #pragma once +#include +#include + + +namespace rtengine +{ + +namespace procparams +{ + +class ColorManagementParams; + +} + +} + + class PointerMotionListener { protected: @@ -26,7 +43,7 @@ protected: public: virtual ~PointerMotionListener() = default; - virtual void pointerMoved (bool validPos, const Glib::ustring &profile, const Glib::ustring &profileW, int x, int y, int r, int g, int b, bool isRaw = false) = 0; + virtual void pointerMoved(bool validPos, const rtengine::procparams::ColorManagementParams &cmp, int x, int y, int r, int g, int b, bool isRaw = false) = 0; virtual void getRGBText (int r, int g, int b, Glib::ustring &sR, Glib::ustring &sG, Glib::ustring &sB, bool isRaw = false) { sR = "--"; sG = "--"; sB = "--"; } virtual void getHSVText (float h, float s, float v, Glib::ustring &sH, Glib::ustring &sS, Glib::ustring &sV) { sH = "--"; sS = "--"; sV = "--"; } virtual void getLABText (float l, float a, float b, Glib::ustring &sL, Glib::ustring &sA, Glib::ustring &sB) { sL = "--"; sA = "--"; sB = "--"; }