Protect all Crop accesses with cropMutex (fixes #3306)

rtengine/dcrop.cc

@@ -22,8 +22,18 @@
 #include "mytime.h"
 #include "refreshmap.h"
 #include "rt_math.h"
-// "ceil" rounding
+
-#define SKIPS(a,b) ((a) / (b) + ((a) % (b) > 0))
+namespace
+{
+
+    // "ceil" rounding
+    template<typename T>
+    constexpr T skips(T a, T b)
+    {
+        return a / b + static_cast<bool>(a % b);
+    }
+
+}
 
 namespace rtengine
 {
@@ -33,7 +43,7 @@ extern const Settings* settings;
 Crop::Crop (ImProcCoordinator* parent, EditDataProvider *editDataProvider, bool isDetailWindow)
     : PipetteBuffer(editDataProvider), origCrop(nullptr), laboCrop(nullptr), labnCrop(nullptr),
       cropImg(nullptr), cbuf_real(nullptr), cshmap(nullptr), transCrop(nullptr), cieCrop(nullptr), cbuffer(nullptr),
-      updating(false), newUpdatePending(false), skip(10), padding(0),
+      updating(false), newUpdatePending(false), skip(10),
       cropx(0), cropy(0), cropw(-1), croph(-1),
       trafx(0), trafy(0), trafw(-1), trafh(-1),
       rqcropx(0), rqcropy(0), rqcropw(-1), rqcroph(-1),
@@ -111,6 +121,12 @@ void Crop::setEditSubscriber(EditSubscriber* newSubscriber)
     // If oldSubscriber == NULL && newSubscriber != NULL && newSubscriber->getEditingType() == ET_PIPETTE-> the image will be allocated when necessary
 }
 
+bool Crop::hasListener()
+{
+    MyMutex::MyLock cropLock(cropMutex);
+    return cropImageListener;
+}
+
 void Crop::update (int todo)
 {
     MyMutex::MyLock cropLock(cropMutex);
@@ -684,7 +700,7 @@ void Crop::update (int todo)
         }
 
         if (needstransform)
-            parent->ipf.transform (baseCrop, transCrop, cropx / skip, cropy / skip, trafx / skip, trafy / skip, SKIPS(parent->fw, skip), SKIPS(parent->fh, skip), parent->getFullWidth(), parent->getFullHeight(),
+            parent->ipf.transform (baseCrop, transCrop, cropx / skip, cropy / skip, trafx / skip, trafy / skip, skips(parent->fw, skip), skips(parent->fh, skip), parent->getFullWidth(), parent->getFullHeight(),
                                    parent->imgsrc->getMetaData()->getFocalLen(), parent->imgsrc->getMetaData()->getFocalLen35mm(),
                                    parent->imgsrc->getMetaData()->getFocusDist(), parent->imgsrc->getRotateDegree(), false);
         else
@@ -714,7 +730,7 @@ void Crop::update (int todo)
 
     // blurmap for shadow & highlights
     if ((todo & M_BLURMAP) && params.sh.enabled) {
-        double radius = sqrt (double(SKIPS(parent->fw, skip) * SKIPS(parent->fw, skip) + SKIPS(parent->fh, skip) * SKIPS(parent->fh, skip))) / 2.0;
+        double radius = sqrt (double(skips(parent->fw, skip) * skips(parent->fw, skip) + skips(parent->fh, skip) * skips(parent->fh, skip))) / 2.0;
         double shradius = params.sh.radius;
 
         if (!params.sh.hq) {
@@ -1114,11 +1130,11 @@ bool Crop::setCropSizes (int rcx, int rcy, int rcw, int rch, int skip, bool inte
     int orW, orH;
     parent->imgsrc->getSize (cp, orW, orH);
 
-    int cw = SKIPS(bw, skip);
+    int cw = skips(bw, skip);
-    int ch = SKIPS(bh, skip);
+    int ch = skips(bh, skip);
 
-    leftBorder  = SKIPS(rqx1 - bx1, skip);
+    leftBorder  = skips(rqx1 - bx1, skip);
-    upperBorder = SKIPS(rqy1 - by1, skip);
+    upperBorder = skips(rqy1 - by1, skip);
 
     if (settings->verbose) {
         printf ("setsizes starts (%d, %d, %d, %d, %d, %d)\n", orW, orH, trafw, trafh, cw, ch);
@@ -1286,5 +1302,22 @@ void Crop::fullUpdate ()
     parent->updaterThreadStart.unlock ();
 }
 
+int Crop::get_skip()
+{
+    MyMutex::MyLock lock(cropMutex);
+    return skip;
 }
 
+int Crop::getLeftBorder()
+{
+    MyMutex::MyLock lock(cropMutex);
+    return leftBorder;
+}
+
+int Crop::getUpperBorder()
+{
+    MyMutex::MyLock lock(cropMutex);
+    return upperBorder;
+}
+
+}

rtengine/dcrop.h

@@ -16,8 +16,7 @@
  *  You should have received a copy of the GNU General Public License
  *  along with RawTherapee.  If not, see <http://www.gnu.org/licenses/>.
  */
-#ifndef _CROP_H_
+#pragma once
-#define _CROP_H_
 
 #include "improccoordinator.h"
 #include "rtengine.h"
@@ -57,19 +56,18 @@ protected:
     bool updating;         /// Flag telling if an updater thread is currently processing
     bool newUpdatePending; /// Flag telling the updater thread that a new update is pending
     int skip;
-    int padding;           /// Minimum space allowed around image in the display area
     int cropx, cropy, cropw, croph;         /// size of the detail crop image ('skip' taken into account), with border
     int trafx, trafy, trafw, trafh;         /// the size and position to get from the imagesource that is transformed to the requested crop area
     int rqcropx, rqcropy, rqcropw, rqcroph; /// size of the requested detail crop image (the image might be smaller) (without border)
-    int borderRequested;                    /// requested extra border size for image processing
+    const int borderRequested;              /// requested extra border size for image processing
     int upperBorder, leftBorder;            /// extra border size really allocated for image processing
 
     bool cropAllocated;
     DetailedCropListener* cropImageListener;
 
     MyMutex cropMutex;
-    ImProcCoordinator* parent;
+    ImProcCoordinator* const parent;
-    bool isDetailWindow;
+    const bool isDetailWindow;
     EditUniqueID getCurrEditID();
     bool setCropSizes (int cropX, int cropY, int cropW, int cropH, int skip, bool internal);
     void freeAll ();
@@ -78,19 +76,8 @@ public:
     Crop             (ImProcCoordinator* parent, EditDataProvider *editDataProvider, bool isDetailWindow);
     virtual ~Crop    ();
 
-    void mLock       ()
-    {
-        cropMutex.lock();
-    }
-    void mUnlock     ()
-    {
-        cropMutex.lock();
-    }
     void setEditSubscriber(EditSubscriber* newSubscriber);
-    bool hasListener ()
+    bool hasListener();
-    {
-        return cropImageListener;
-    }
     void update      (int todo);
     void setWindow   (int cropX, int cropY, int cropW, int cropH, int skip)
     {
@@ -106,22 +93,8 @@ public:
 
     void setListener    (DetailedCropListener* il);
     void destroy        ();
-    int  get_skip       ()
+    int get_skip();
-    {
+    int getLeftBorder();
-        return skip;
+    int getUpperBorder();
-    }
-    int getPadding      ()
-    {
-        return padding;
-    }
-    int  getLeftBorder  ()
-    {
-        return leftBorder;
-    }
-    int  getUpperBorder ()
-    {
-        return upperBorder;
-    }
 };
 }
-#endif