Solving issue 2137: "Moving HLR to a better place to preserve RT's structure"

rtengine/improccoordinator.cc

@@ -81,7 +81,7 @@ ImProcCoordinator::ImProcCoordinator ()
 
       pW(-1), pH(-1),
       plistener(NULL), imageListener(NULL), aeListener(NULL), hListener(NULL),acListener(NULL), abwListener(NULL),
-      ahlListener(NULL),resultValid(false), changeSinceLast(0), updaterRunning(false), destroying(false)
+      resultValid(false), changeSinceLast(0), updaterRunning(false), destroying(false)
     {}
 
 void ImProcCoordinator::assign (ImageSource* imgsrc) {
@@ -190,6 +190,19 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
             highDetailRawComputed = false;
     }
 
+    // Updating toneCurve.hrenabled if necessary
+    // It has to be done there, because the next 'if' statement will use the value computed here
+    if (todo & M_AUTOEXP) {
+        if (params.toneCurve.autoexp) {// this enabled HLRecovery
+            if (ToneCurveParams::HLReconstructionNecessary(histRedRaw, histGreenRaw, histBlueRaw) && !params.toneCurve.hrenabled) {
+                // switching params.toneCurve.hrenabled to true -> shouting in listener's ears!
+                params.toneCurve.hrenabled=true;
+
+                // forcing INIT to be done, to reconstruct HL again
+                todo |= M_INIT;
+            }
+        }
+    }
 
     if (todo & (M_INIT|M_LINDENOISE)) {
         MyMutex::MyLock initLock(minit);  // Also used in crop window
@@ -232,15 +245,6 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
 
         // Will (re)allocate the preview's buffers
         setScale (scale);
-		bool hlrbool=false;
-		int thresholdHLR=50;
-		 if (settings->verbose) printf("HLredzero=%i   HLgreenzero=%i  HLblurzero=%i \n", histRedRaw[0], histGreenRaw[0], histBlueRaw[0]);
-		 if (settings->verbose) printf("HLredMax=%i   HLgreenMax=%i  HLblueMax=%i \n", histRedRaw[255], histGreenRaw[255], histBlueRaw[255]);
-		if (params.toneCurve.autoexp) {// this enabled HLRecovery
-		//500 arbitrary values to enabled HLrecovery : not to enabled for too low values
-		if(histRedRaw[255]>thresholdHLR || histGreenRaw[255]>thresholdHLR || histBlueRaw[255]>thresholdHLR   || histRedRaw[0]>thresholdHLR || histGreenRaw[0]>thresholdHLR || histBlueRaw[0]>thresholdHLR) {params.toneCurve.hrenabled=true;hlrbool=true;} }
-            if (ahlListener) {if(hlrbool==true) ahlListener->HLChanged (hlrbool);//enabled Highlight recovery
-		}
 
         imgsrc->getImage (currWB, tr, orig_prev, pp, params.toneCurve, params.icm, params.raw);
         //ColorTemp::CAT02 (orig_prev, &params)	;
@@ -285,13 +289,12 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
     if (todo & M_AUTOEXP) {
         if (params.toneCurve.autoexp) {
             LUTu aehist; int aehistcompr;
-			//histRedRaw, histGreenRaw, histBlueRaw 
             imgsrc->getAutoExpHistogram (aehist, aehistcompr);
             ipf.getAutoExp (aehist, aehistcompr, imgsrc->getDefGain(), params.toneCurve.clip, params.toneCurve.expcomp,
                     params.toneCurve.brightness, params.toneCurve.contrast, params.toneCurve.black, params.toneCurve.hlcompr, params.toneCurve.hlcomprthresh);
             if (aeListener)
                 aeListener->autoExpChanged (params.toneCurve.expcomp, params.toneCurve.brightness, params.toneCurve.contrast,
-                                            params.toneCurve.black, params.toneCurve.hlcompr,params.toneCurve.hlcomprthresh);
+                                            params.toneCurve.black, params.toneCurve.hlcompr,params.toneCurve.hlcomprthresh, params.toneCurve.hrenabled);
         }
     }
 

rtengine/improccoordinator.h

@@ -141,7 +141,6 @@ class ImProcCoordinator : public StagedImageProcessor {
         AutoExpListener* aeListener;
         AutoCamListener* acListener;
         AutoBWListener* abwListener;
-        AutoHLListener* ahlListener;
 
         HistogramListener* hListener;
         std::vector<SizeListener*> sizeListeners;
@@ -214,7 +213,6 @@ class ImProcCoordinator : public StagedImageProcessor {
         void setHistogramListener(HistogramListener *h)  {hListener = h; }
         void setAutoCamListener  (AutoCamListener* acl)  {acListener = acl; }
         void setAutoBWListener  (AutoBWListener* abw)  {abwListener = abw; }
-        void setAutoHLListener  (AutoHLListener* ahl)  {ahlListener = ahl; }
 
         void saveInputICCReference (const Glib::ustring& fname);
 

rtengine/procparams.cc

@@ -47,6 +47,14 @@ const char *RAWParams::methodstring[RAWParams::numMethods]={"amaze","igv","lmmse
 const char *RAWParams::ff_BlurTypestring[RAWParams::numFlatFileBlurTypes]={/*"Parametric",*/ "Area Flatfield", "Vertical Flatfield", "Horizontal Flatfield", "V+H Flatfield"};
 std::vector<WBEntry*> WBParams::wbEntries;
 
+bool ToneCurveParams::HLReconstructionNecessary(LUTu &histRedRaw, LUTu &histGreenRaw, LUTu &histBlueRaw) {
+    if (options.rtSettings.verbose)
+        printf("histRedRaw[  0]=%07d, histGreenRaw[  0]=%07d, histBlueRaw[  0]=%07d\nhistRedRaw[255]=%07d, histGreenRaw[255]=%07d, histBlueRaw[255]=%07d\n",
+                histRedRaw[0], histGreenRaw[0], histBlueRaw[0], histRedRaw[255], histGreenRaw[255], histBlueRaw[255]);
+
+    return histRedRaw[255]>50 || histGreenRaw[255]>50 || histBlueRaw[255]>50 || histRedRaw[0]>50 || histGreenRaw[0]>50 || histBlueRaw[0]>50;
+}
+
 void WBParams::init() {
     // Creation of the different methods and its associated temperature value
     wbEntries.push_back(new WBEntry("Camera"              ,WBT_CAMERA,      M("TP_WBALANCE_CAMERA"),        0, 1.f,     1.f));

rtengine/procparams.h

@@ -24,6 +24,7 @@
 #include <cassert>
 #include <cstdio>
 #include <cmath>
+#include "LUT.h"
 
 class ParamsEdited;
 
@@ -180,8 +181,6 @@ class Threshold {
 class ToneCurveParams {
 
     public:
-        bool hrenabled;
-		Glib::ustring method;
 
         enum eTCModeId {
             TC_MODE_STD,               // Standard modes, the curve is applied on all component individually
@@ -192,6 +191,8 @@ class ToneCurveParams {
 
         bool        autoexp;
         double      clip;
+        bool        hrenabled;  // Highlight Reconstruction enabled
+        Glib::ustring method;   // Highlight Reconstruction's method
         double      expcomp;
         std::vector<double>   curve;
         std::vector<double>   curve2;
@@ -202,9 +203,10 @@ class ToneCurveParams {
         int         contrast;
         int         saturation;
         int         shcompr;
-        int         hlcompr;
+        int         hlcompr;        // Highlight Recovery's compression
-        int         hlcomprthresh;
+        int         hlcomprthresh;  // Highlight Recovery's threshold
 
+        static bool HLReconstructionNecessary(LUTu &histRedRaw, LUTu &histGreenRaw, LUTu &histBlueRaw);
 };
 
 /**

rtengine/refreshmap.cc

@@ -31,7 +31,7 @@ RGBCURVE,         // EvExpComp,
 RGBCURVE,         // EvHLCompr,
 RGBCURVE,         // EvSHCompr,
 RGBCURVE,         // EvToneCurve1,
-ALLNORAW,          // EvAutoExp,
+AUTOEXP,          // EvAutoExp,
 AUTOEXP,          // EvClip,
 LUMINANCECURVE,   // EvLBrightness,
 LUMINANCECURVE,   // EvLContrast,

rtengine/rtengine.h

@@ -242,12 +242,13 @@ namespace rtengine {
             virtual ~AutoExpListener() {}
             /** This member function is called when the auto exposure has been recomputed.
               * @param brightness is the new brightness value (in logarithmic scale)
-              * @param, bright is the new ...
+              * @param bright is the new ...
               * @param black is the new black level (measured in absolute pixel data)
               * @param contrast is the new contrast values
               * @param hlcompr is the new highlight recovery amount 
-              * #param hlcomprthresh is the new threshold for hlcompr*/
+              * @param hlcomprthresh is the new threshold for hlcompr
-            virtual void autoExpChanged (double brightness, int bright, int contrast, int black, int hlcompr, int hlcomprthresh) {}
+              * @param hlrecons set to true if HighLight Reconstruction is enabled */
+            virtual void autoExpChanged (double brightness, int bright, int contrast, int black, int hlcompr, int hlcomprthresh, bool hlrecons) {}
     };
 
     class AutoCamListener {
@@ -264,12 +265,6 @@ namespace rtengine {
 			
  	};		
 	
-	class AutoHLListener {
-		public :
-            virtual ~AutoHLListener() {}
-			virtual void HLChanged (bool hlrbool) {}
-			
- 	};		
 
     /** This class represents a detailed part of the image (looking through a kind of window).
       * It can be created and destroyed with the appropriate members of StagedImageProcessor.
@@ -352,7 +347,6 @@ namespace rtengine {
             virtual void        setPreviewImageListener (PreviewImageListener* l) =0;
             virtual void        setAutoCamListener      (AutoCamListener* l) =0;
             virtual void        setAutoBWListener     	(AutoBWListener* l) =0;
-            virtual void        setAutoHLListener     	(AutoHLListener* l) =0;
 
             virtual ~StagedImageProcessor () {}
 

rtengine/rtthumbnail.cc

@@ -304,6 +304,7 @@ Thumbnail* Thumbnail::loadFromRaw (const Glib::ustring& fname, RawMetaDataLocati
 
 	Imagefloat* tmpImg = new Imagefloat(tmpw, tmph);
 	if (ri->isBayer()) {
+		// demosaicing! (sort of)
 		for (int row = 1, y = 0; row < height - 1 && y < tmph; row += vskip, y++) {
 			rofs = row * width;
 			for (int col = firstgreen, x = 0; col < width - 1 && x < tmpw; col+= hskip, x++) {
@@ -311,10 +312,10 @@ Thumbnail* Thumbnail::loadFromRaw (const Glib::ustring& fname, RawMetaDataLocati
 				int g = image[ofs][1];
 				int r, b;
 				if (FISRED(filter,row,col+1)) {
-					r = (image[ofs + 1][0] + image[ofs - 1][0]) >> 1;
+					r = (image[ofs + 1    ][0] + image[ofs - 1    ][0]) >> 1;
 					b = (image[ofs + width][2] + image[ofs - width][2]) >> 1;
 				} else {
-					b = (image[ofs + 1][2] + image[ofs - 1][2]) >> 1;
+					b = (image[ofs + 1    ][2] + image[ofs - 1    ][2]) >> 1;
 					r = (image[ofs + width][0] + image[ofs - width][0]) >> 1;
 				}
 				tmpImg->r(y,x) = r;
@@ -351,18 +352,9 @@ Thumbnail* Thumbnail::loadFromRaw (const Glib::ustring& fname, RawMetaDataLocati
 					continue;
 				double fr = r - ur;
 				double fc = c - uc;
-				fImg->r(row,col) = (tmpImg->r(ur,uc) * (1 - fc)
+				fImg->r(row,col) = (tmpImg->r(ur,uc)*(1-fc) + tmpImg->r(ur,uc + 1)*fc) * (1-fr)   +   (tmpImg->r(ur + 1,uc)*(1-fc) + tmpImg->r(ur + 1,uc + 1)*fc) * fr;
-						+ tmpImg->r(ur,uc + 1) * fc) * (1 - fr)
+				fImg->g(row,col) = (tmpImg->g(ur,uc)*(1-fc) + tmpImg->g(ur,uc + 1)*fc) * (1-fr)   +   (tmpImg->g(ur + 1,uc)*(1-fc) + tmpImg->g(ur + 1,uc + 1)*fc) * fr;
-						+ (tmpImg->r(ur + 1,uc) * (1 - fc)
+				fImg->b(row,col) = (tmpImg->b(ur,uc)*(1-fc) + tmpImg->b(ur,uc + 1)*fc) * (1-fr)   +   (tmpImg->b(ur + 1,uc)*(1-fc) + tmpImg->b(ur + 1,uc + 1)*fc) * fr;
-								+ tmpImg->r(ur + 1,uc + 1) * fc) * fr;
-				fImg->g(row,col) = (tmpImg->g(ur,uc) * (1 - fc)
-						+ tmpImg->g(ur,uc + 1) * fc) * (1 - fr)
-						+ (tmpImg->g(ur + 1,uc) * (1 - fc)
-								+ tmpImg->g(ur + 1,uc + 1) * fc) * fr;
-				fImg->b(row,col) = (tmpImg->b(ur,uc) * (1 - fc)
-						+ tmpImg->b(ur,uc + 1) * fc) * (1 - fr)
-						+ (tmpImg->b(ur + 1,uc) * (1 - fc)
-								+ tmpImg->b(ur + 1,uc + 1) * fc) * fr;
 			}
 		delete tmpImg;
 		tmpImg = fImg;
@@ -660,7 +652,7 @@ IImage8* Thumbnail::processImage (const procparams::ProcParams& params, int rhei
         }
 
 /*
-    // apply highlight recovery, if needed		-- CURRENTLY BROKEN DUE TO INCOMPATIBLE DATA TYPES; DO WE CARE???
+    // apply highlight recovery, if needed		-- CURRENTLY BROKEN DUE TO INCOMPATIBLE DATA TYPES, BUT HL RECOVERY AREN'T COMPUTED FOR THUMBNAILS ANYWAY...
     if (isRaw && params.toneCurve.hrenabled) {
         int maxval = 65535 / defGain;
         if (params.toneCurve.method=="Luminance" || params.toneCurve.method=="Color")

rtengine/simpleprocess.cc

@@ -100,6 +100,16 @@ IImage16* processImage (ProcessingJob* pjob, int& errorCode, ProgressListener* p
 
     PreviewProps pp (0, 0, fw, fh, 1);
     imgsrc->preprocess( params.raw, params.lensProf, params.coarse);
+
+    if (params.toneCurve.autoexp) {// this enabled HLRecovery
+        LUTu histRedRaw(256), histGreenRaw(256), histBlueRaw(256);
+        imgsrc->getRAWHistogram(histRedRaw, histGreenRaw, histBlueRaw);
+        if (ToneCurveParams::HLReconstructionNecessary(histRedRaw, histGreenRaw, histBlueRaw) && !params.toneCurve.hrenabled) {
+            params.toneCurve.hrenabled=true;
+            // WARNING: Highlight Reconstruction is being forced 'on', should we force a method here too?
+        }
+    }
+
     if (pl) pl->setProgress (0.20);
     imgsrc->demosaic( params.raw);
     if (pl) pl->setProgress (0.30);

rtgui/batchtoolpanelcoord.cc

@@ -310,6 +310,11 @@ void BatchToolPanelCoordinator::panelChanged (rtengine::ProcEvent event, const G
         for (size_t i=0; i<selected.size(); i++) {
             initialPP[i].toneCurve.autoexp = pparams.toneCurve.autoexp;
             initialPP[i].toneCurve.clip = pparams.toneCurve.clip;
+
+            // at this stage, we don't know if HL Reconstruction will be enabled or not (depending on the raw histogram),
+            // so we're forcing its paramseditd value to false (i.e. mixed state)
+            pparamsEdited.toneCurve.hrenabled = false;
+
             selected[i]->applyAutoExp (initialPP[i]);
         }
 

rtgui/tonecurve.cc

@@ -266,22 +266,6 @@ void ToneCurve::autoOpenCurve  () {
     shape->openIfNonlinear();
     shape2->openIfNonlinear();
 }
-int HLChangedUI (void* data) {
-    GThreadLock lock;
-    (static_cast<ToneCurve*>(data))->HLComputed_ ();
-    return 0;
-}
-
-void ToneCurve::HLChanged  (bool hlrbool){
-    nexthlrbool= hlrbool;
-    g_idle_add (HLChangedUI, this);
-}
-bool ToneCurve::HLComputed_ () {
-    enaconn.block (true);
-    hrenabled->set_active (nexthlrbool);
-    enaconn.block (false);
-    return false;
-}
 
 void ToneCurve::write (ProcParams* pp, ParamsEdited* pedited) {
 
@@ -365,19 +349,18 @@ void ToneCurve::hrenabledChanged () {
     }
 
     if (listener) {
-        if (hrenabled->get_active ()){
+        // Switch off auto exposure if user changes enabled manually
-            listener->panelChanged (EvHREnabled, M("GENERAL_ENABLED"));
-        }
-        else {    // Switch off auto exposure if user changes enabled manually
         if (autolevels->get_active() ) {
             autoconn.block(true);
             autolevels->set_active (false);
             autoconn.block(false);
             autolevels->set_inconsistent (false);
         }
+        if (hrenabled->get_active ())
+            listener->panelChanged (EvHREnabled, M("GENERAL_ENABLED"));
+        else
             listener->panelChanged (EvHREnabled, M("GENERAL_DISABLED"));
     }  
-    }  
 }
 void ToneCurve::methodChanged () {
 
@@ -520,7 +503,11 @@ void ToneCurve::neutral_pressed () {
     brightness->setValue(0);
     black->setValue(0);
     shcompr->setValue(50);
+    enaconn.block (true);
     hrenabled->set_active (false);
+    enaconn.block (false);
+    if (!batchMode)
+        hlrbox->hide();
     if (!black->getAddMode()) shcompr->set_sensitive(!((int)black->getValue ()==0)); //at black=0 shcompr value has no effect
     contrast->setValue(0);
     //saturation->setValue(0);
@@ -612,6 +599,8 @@ void ToneCurve::waitForAutoExp () {
     saturation->setEnabled (false);
     curveEditorG->set_sensitive (false);
     toneCurveMode->set_sensitive (false);
+    hrenabled->set_sensitive(false);
+    method->set_sensitive(false);
 }
 
 int autoExpChangedUI (void* data) {
@@ -619,7 +608,7 @@ int autoExpChangedUI (void* data) {
     return 0;
 }
 
-void ToneCurve::autoExpChanged (double expcomp, int bright, int contr, int black, int hlcompr, int hlcomprthresh) {
+void ToneCurve::autoExpChanged (double expcomp, int bright, int contr, int black, int hlcompr, int hlcomprthresh, bool hlrecons) {
 
     nextBlack = black;
     nextExpcomp = expcomp;
@@ -627,6 +616,7 @@ void ToneCurve::autoExpChanged (double expcomp, int bright, int contr, int black
     nextContrast = contr;
     nextHlcompr = hlcompr;
     nextHlcomprthresh = hlcomprthresh;
+    nextHLRecons = hlrecons;
     g_idle_add (autoExpChangedUI, this);
 }
 
@@ -643,6 +633,8 @@ void ToneCurve::enableAll () {
     saturation->setEnabled (true);
     curveEditorG->set_sensitive (true);
     toneCurveMode->set_sensitive (true);
+    hrenabled->set_sensitive(true);
+    method->set_sensitive(true);
 }
 
 bool ToneCurve::autoExpComputed_ () {
@@ -656,6 +648,13 @@ bool ToneCurve::autoExpComputed_ () {
     black->setValue (nextBlack);
     hlcompr->setValue (nextHlcompr);
     hlcomprthresh->setValue (nextHlcomprthresh);
+    enaconn.block (true);
+    hrenabled->set_active (nextHLRecons);
+    enaconn.block (false);
+    if (nextHLRecons)
+        hlrbox->show();
+    else if (!batchMode)
+        hlrbox->hide();
     if (!black->getAddMode()) shcompr->set_sensitive(!((int)black->getValue ()==0)); //at black=0 shcompr value has no effect
     enableListener ();
 

rtgui/tonecurve.h

@@ -27,7 +27,7 @@
 #include "mycurve.h"
 #include "guiutils.h"
 
-class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableToolPanel, public rtengine::AutoExpListener, public rtengine::AutoHLListener,public CurveListener {
+class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableToolPanel, public rtengine::AutoExpListener,public CurveListener {
 
   protected:
     // from HLRecovery
@@ -36,7 +36,6 @@ class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableTool
     sigc::connection    methconn;
     sigc::connection    enaconn;
     bool                lasthrEnabled;
-    bool                nexthlrbool;
   
     Gtk::HBox* abox;
     Gtk::HBox* hlrbox;
@@ -69,6 +68,7 @@ class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableTool
     int nextBlack;
     int nextHlcompr;
     int nextHlcomprthresh;
+    bool nextHLRecons;
 
   public:
 
@@ -90,7 +90,7 @@ class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableTool
     void clip_changed ();
     bool clip_changed_ ();
     void waitForAutoExp ();
-    void autoExpChanged (double expcomp, int bright, int contr, int black, int hlcompr, int hlcomprthresh);
+    void autoExpChanged (double expcomp, int bright, int contr, int black, int hlcompr, int hlcomprthresh, bool hlrecons);
     bool autoExpComputed_ ();
     void enableAll ();
     void curveChanged (CurveEditor* ce);
@@ -103,8 +103,6 @@ class ToneCurve : public Gtk::VBox, public AdjusterListener, public FoldableTool
     void updateCurveBackgroundHistogram (LUTu & histToneCurve, LUTu & histLCurve, LUTu & histCCurve, LUTu & histCLurve, LUTu & histLLCurve, LUTu & histLCAM, LUTu & histCCAM, LUTu & histRed, LUTu & histGreen, LUTu & histBlue, LUTu & histLuma);
  
     void setRaw (bool raw);
-    void HLChanged (bool hlrbool);
-    bool HLComputed_  ();
 
     void hrenabledChanged ();
     void methodChanged ();

rtgui/toolpanelcoord.cc

@@ -363,7 +363,6 @@ void ToolPanelCoordinator::initImage (rtengine::StagedImageProcessor* ipc_, bool
         ipc->setAutoExpListener (toneCurve);
         ipc->setAutoCamListener (colorappearance);
         ipc->setAutoBWListener (blackwhite);
-        ipc->setAutoHLListener (toneCurve);
 
         ipc->setSizeListener (crop);
         ipc->setSizeListener (resize);