Internal preview window optimization; see issue #589

rtengine/dcrop.cc

@@ -51,16 +51,15 @@ Crop::~Crop () {
 }
 
 void Crop::setListener (DetailedCropListener* il) { 
-
+	// We can make reads in the IF, because the mProcessing lock is only needed for change
+	if (cropImageListener!=il) {
     parent->mProcessing.lock(); 
     cropImageListener = il; 
     parent->mProcessing.unlock(); 
 }       
+}       
 
-void Crop::update (int todo, bool internal) {
+void Crop::update (int todo) {
-
-    if (!internal)
-        parent->mProcessing.lock ();
 
     ProcParams& params = parent->params;
     cropMutex.lock ();
@@ -88,9 +87,10 @@ void Crop::update (int todo, bool internal) {
     if (needsinitupdate)
         todo = ALL;
 
+ /* Seems to be taken care of by calling improccoordinator::updatePreviewImage
     if( regenHighDetail )
     	parent->updatePreviewImage (ALL,this); // We have just set skip to 1
-
+		*/
     baseCrop = origCrop;
 
     bool needstransform  = parent->ipf.needsTransform();
@@ -113,14 +113,17 @@ void Crop::update (int todo, bool internal) {
     }
 
     // transform
-    if ((!needstransform && transCrop) || (transCrop && (transCrop->width!=cropw || transCrop->height!=croph))) {
+    // Delete transfrom if it's not necessary or outdated
+	if (transCrop && (!needstransform || transCrop->width!=cropw || transCrop->height!=croph)) {
         delete transCrop;
         transCrop = NULL;
     }
-    if (needstransform && !transCrop)
+
-        transCrop = new Image16 (cropw, croph);
+    if ((todo & M_TRANSFORM) && needstransform) {
-    if ((todo & M_TRANSFORM) && needstransform)
+		if (!transCrop) transCrop = new Image16 (cropw, croph);
     	parent->ipf.transform (baseCrop, transCrop, cropx/skip, cropy/skip, trafx/skip, trafy/skip, SKIPS(parent->fw,skip), SKIPS(parent->fh,skip));
+	}
+
     if (transCrop)
         baseCrop = transCrop;
 
@@ -177,9 +180,6 @@ void Crop::update (int todo, bool internal) {
     }
 
     cropMutex.unlock ();
-
-    if (!internal)
-        parent->mProcessing.unlock ();
 }
 
 void Crop::freeAll () {
@@ -329,7 +329,7 @@ void Crop::fullUpdate () {
     needsNext = true;
     while (needsNext) {
         needsNext = false;
-        update (ALL, true); 
+        update (ALL); 
     }
     updating = false;
 

rtengine/dcrop.h

@@ -64,7 +64,7 @@ class Crop : public DetailedCrop {
             ~Crop        ();
     
         bool hasListener () { return cropImageListener; }
-        void update      (int todo, bool internal=false);
+        void update      (int todo);
         void setWindow   (int cx, int cy, int cw, int ch, int skip) { setCropSizes (cx, cy, cw, ch, skip, false); }
         void fullUpdate  ();
         void setListener (DetailedCropListener* il);

rtengine/improccoordinator.cc

@@ -105,12 +105,15 @@ DetailedCrop* ImProcCoordinator::createCrop  () {
     return new Crop (this); 
 }
 
+// todo: bitmask containing desired actions, taken from changesSinceLast
+// cropCall: calling crop, used to prevent self-updates
 void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
 
     mProcessing.lock ();
 
     ipf.setScale (scale);
 
+	// Check if any detail crops need high detail. If not, take a fast path short cut
     bool highDetailNeeded=false;
 	for (int i=0; i<crops.size(); i++)
 		if (crops[i]->get_skip() == 1 ){
@@ -128,10 +131,7 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
     if ( todo & M_PREPROC)
     	imgsrc->preprocess( rp );
     if( todo & M_RAW){
-    	if( !highDetailNeeded ){
+    	fineDetailsProcessed = highDetailNeeded;
-    		fineDetailsProcessed = false;
-    	}else
-    		fineDetailsProcessed = true;
     	imgsrc->demosaic( rp );
     }
     if (todo & M_INIT) {
@@ -159,7 +159,7 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
 
         imgsrc->getFullSize (fw, fh, tr);
         PreviewProps pp (0, 0, fw, fh, scale);
-        setScale (scale, true);
+        setScale (scale);
         progress ("Sample ...",45);
         imgsrc->getImage (currWB, tr, orig_prev, pp, params.hlrecovery, params.icm, params.raw);
         ipf.firstAnalysis (orig_prev, &params, vhist16, imgsrc->getGamma());
@@ -168,16 +168,16 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
 
     progress ("Rotate / Distortion...",50);
     bool needstransform = ipf.needsTransform();
+    // Remove transformation if unneeded
     if (!needstransform && orig_prev!=oprevi) {
         delete oprevi;
         oprevi = orig_prev;
     }
-    if (needstransform && orig_prev==oprevi)
+        
-        oprevi = new Image16 (pW, pH);
+    if ((todo & M_TRANSFORM) && needstransform) {
-    if ((todo & M_TRANSFORM) && needstransform)
+		if (oprevi==orig_prev) oprevi = new Image16 (pW, pH);
     	ipf.transform (orig_prev, oprevi, 0, 0, 0, 0, pW, pH);
-
+	}
-
 
     progress ("Shadow/highlight...",55);
     if ((todo & M_BLURMAP) && params.sh.enabled) {
@@ -265,7 +265,7 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
     // process crop, if needed
     for (int i=0; i<crops.size(); i++)
         if (crops[i]->hasListener () && cropCall != crops[i] )
-            crops[i]->update (todo, true);
+            crops[i]->update (todo);  // may call outselves
 
     progress ("Conversion to RGB...",95);
     if (todo!=CROP) {
@@ -277,6 +277,7 @@ void ImProcCoordinator::updatePreviewImage (int todo, Crop* cropCall) {
         catch(char * str)
         {
            progress ("Error converting file...",0);
+			previmg->getMutex().unlock();
             mProcessing.unlock ();
             return;
         }
@@ -331,13 +332,10 @@ void ImProcCoordinator::freeAll () {
     allocated = false;
 }
 
-void ImProcCoordinator::setScale (int prevscale, bool internal) {
+void ImProcCoordinator::setScale (int prevscale) {
 
 if (settings->verbose) printf ("setscale before lock\n");
 
-    if (!internal)
-        mProcessing.lock ();
-
     tr = TR_NONE;
     if (params.coarse.rotate==90)  tr |= TR_R90;
     if (params.coarse.rotate==180) tr |= TR_R180;
@@ -383,8 +381,6 @@ if (settings->verbose) printf ("setscale before lock\n");
             sizeListeners[i]->sizeChanged (fullw, fullh, fw, fh);
     if (settings->verbose) printf ("setscale ends2\n");
 
-    if (!internal)
-        mProcessing.unlock ();
 }
 
 
@@ -528,29 +524,6 @@ void ImProcCoordinator::fullUpdatePreviewImage () {
     updaterThreadStart.unlock ();
 }
 
-void ImProcCoordinator::fullUpdateDetailedCrops () { 
-
-    if (destroying)
-        return;
-
-    updaterThreadStart.lock ();
-    if (updaterRunning && thread) {
-        changeSinceLast = 0;
-        thread->join ();
-    }
-
-    if (plistener)
-        plistener->setProgressState (1);
-
-    for (int i=0; i<crops.size(); i++)
-        crops[i]->update (ALL, true); 
-
-    if (plistener)
-        plistener->setProgressState (0);
-
-    updaterThreadStart.unlock ();
-}
-
 
 void ImProcCoordinator::saveInputICCReference (const Glib::ustring& fname) {
 

rtengine/improccoordinator.h

@@ -33,6 +33,8 @@ using namespace procparams;
 
 class Crop;
 
+// Manages the image processing, espc. of the preview windows
+// There is one ImProcCoordinator per edit panel
 class ImProcCoordinator : public StagedImageProcessor {
 
     friend class Crop;
@@ -95,7 +97,7 @@ class ImProcCoordinator : public StagedImageProcessor {
         void progress (Glib::ustring str, int pr);
         void reallocAll ();
         void updateHistograms (int x1, int y1, int x2, int y2);
-        void setScale (int prevscale, bool internal=false);
+        void setScale (int prevscale);
         void updatePreviewImage (int todo, Crop* cropCall= NULL);
 
         Glib::Mutex mProcessing;
@@ -130,7 +132,6 @@ class ImProcCoordinator : public StagedImageProcessor {
         int  getPreviewScale    () { return scale; }
 
         void fullUpdatePreviewImage  ();
-        void fullUpdateDetailedCrops ();
 
         int getFullWidth ()     { return fullw; }
         int getFullHeight ()    { return fullh; }

rtengine/refreshmap.cc

@@ -19,6 +19,8 @@
 #include <refreshmap.h>
 #include <procevents.h>
 
+// Mapping of which event causes what change in the image
+// given to Improccordinator::updatePreviewImage(todo)
 int refreshmap[rtengine::NUMOFEVENTS] = {
 ALL,              // EvPhotoLoaded,
 ALL,              // EvProfileLoaded,

rtengine/rtengine.h

@@ -259,8 +259,6 @@ namespace rtengine {
             virtual int         getPreviewScale () =0;
             /** Performs a full update on the preview image. The resulting image is passed to the listener. */
             virtual void        fullUpdatePreviewImage () =0;
-            /** Performs a full update on the detailed crops corresponding to the image. The resulting images are passed to the listeners of the crops. */
-            virtual void        fullUpdateDetailedCrops () =0;
             /** Returns the full width of the resulting image (in 1:1 scale). 
               * @return the width of the final image */
             virtual int         getFullWidth () =0;