merge with dev

CONTRIBUTING.md

@@ -17,4 +17,10 @@ The most useful feedback is based on the latest development code, and in the cas
 - Work in a new branch. Fork if necessary.
 - Keep branches small so that completed and working features can be merged into the "dev" branch often, and so that they can be abandoned if they head in the wrong direction.
 - Use C++11
+- The naming isn't homogeneous throughout the code but here is a rough guideline:
+  - *Types* (classes, structs, enums, typedefs...) should be named with `UpperCamelCase`
+  - *Functions* and *methods* should be named with `lowerCamelCase`
+  - *Variables* should be either named with `lowerCamelCase` or better with `lower_underscores` to avoid conflicts
+  - *Enum values* should be named with `UPPER_UNDERSCORES`
+  - Most important: Be consistent, even when not sticking to the rules
 - Code may be run through astyle version 3 or newer. If using astyle, it is important that the astyle changes go into their own commit, so that style changes are not mixed with actual code changes. Command: `astyle --options=rawtherapee.astylerc code.cc`

rtgui/crophandler.cc

@@ -29,19 +29,35 @@
 
 using namespace rtengine;
 
-CropHandler::CropHandler ()
+CropHandler::CropHandler() :
-    : zoom(100), ww(0), wh(0), cax(-1), cay(-1),
+    zoom(100),
-      cx(0), cy(0), cw(0), ch(0), cropX(0), cropY(0), cropW(0), cropH(0), enabled(false),
+    ww(0),
-      cropimg(nullptr), cropimgtrue(nullptr), cropimg_width(0), cropimg_height(0),
+    wh(0),
-      cix(0), ciy(0), ciw(0), cih(0), cis(1),
+    cax(-1),
-      initial(false), isLowUpdatePriority(false), ipc(nullptr), crop(nullptr),
+    cay(-1),
-      displayHandler(nullptr)
+    cx(0),
+    cy(0),
+    cw(0),
+    ch(0),
+    cropX(0),
+    cropY(0),
+    cropW(0),
+    cropH(0),
+    enabled(false),
+    cropimg_width(0),
+    cropimg_height(0),
+    cix(0),
+    ciy(0),
+    ciw(0),
+    cih(0),
+    cis(1),
+    isLowUpdatePriority(false),
+    ipc(nullptr),
+    crop(nullptr),
+    displayHandler(nullptr),
+    redraw_needed(false),
+    initial(false)
 {
-
-    idle_helper = new IdleHelper;
-    idle_helper->destroyed = false;
-    idle_helper->pending = 0;
-    idle_helper->cropHandler = this;
 }
 
 CropHandler::~CropHandler ()
@@ -59,16 +75,6 @@ CropHandler::~CropHandler ()
         delete crop; // will do the same than destroy, plus delete the object
         crop = nullptr;
     }
-
-    cimg.lock ();
-
-    if (idle_helper->pending) {
-        idle_helper->destroyed = true;
-    } else {
-        delete idle_helper;
-    }
-
-    cimg.unlock ();
 }
 
 void CropHandler::setEditSubscriber (EditSubscriber* newSubscriber)
@@ -308,110 +314,94 @@ void CropHandler::setDetailedCrop (IImage8* im, IImage8* imtrue, rtengine::procp
 
     cropPixbuf.clear ();
 
-    if (cropimg) {
+    if (!cropimg.empty()) {
-        delete [] cropimg;
+        cropimg.clear();
     }
 
-    cropimg = nullptr;
+    if (!cropimgtrue.empty()) {
-
+        cropimgtrue.clear();
-    if (cropimgtrue) {
-        delete [] cropimgtrue;
     }
 
-    cropimgtrue = nullptr;
-
     if (ax == cropX && ay == cropY && aw == cropW && ah == cropH && askip == (zoom >= 1000 ? 1 : zoom / 10)) {
         cropimg_width = im->getWidth ();
         cropimg_height = im->getHeight ();
-        cropimg = new unsigned char [3 * cropimg_width * cropimg_height];
+        const std::size_t cropimg_size = 3 * cropimg_width * cropimg_height;
-        cropimgtrue = new unsigned char [3 * cropimg_width * cropimg_height];
+        cropimg.assign(im->getData(), im->getData() + cropimg_size);
-        memcpy (cropimg, im->getData(), 3 * cropimg_width * cropimg_height);
+        cropimgtrue.assign(imtrue->getData(), imtrue->getData() + cropimg_size);
-        memcpy (cropimgtrue, imtrue->getData(), 3 * cropimg_width * cropimg_height);
         cix = ax;
         ciy = ay;
         ciw = aw;
         cih = ah;
         cis = askip;
-        idle_helper->pending++;
 
+        bool expected = false;
+
+        if (redraw_needed.compare_exchange_strong(expected, true)) {
             const auto func = [](gpointer data) -> gboolean {
-            IdleHelper* const idle_helper = static_cast<IdleHelper*>(data);
+                CropHandler* const self = static_cast<CropHandler*>(data);
 
-            if (idle_helper->destroyed) {
+                self->cimg.lock ();
-                if (idle_helper->pending == 1) {
-                    delete idle_helper;
-                } else {
-                    idle_helper->pending--;
-                }
 
+                if (self->redraw_needed.exchange(false)) {
+                    self->cropPixbuf.clear ();
+
+                    if (!self->enabled) {
+                        self->cropimg.clear();
+                        self->cropimgtrue.clear();
+                        self->cimg.unlock ();
                         return FALSE;
                     }
 
-            CropHandler* ch = idle_helper->cropHandler;
+                    if (!self->cropimg.empty()) {
-
+                        if (self->cix == self->cropX && self->ciy == self->cropY && self->ciw == self->cropW && self->cih == self->cropH && self->cis == (self->zoom >= 1000 ? 1 : self->zoom / 10)) {
-            ch->cimg.lock ();
-            ch->cropPixbuf.clear ();
-
-            if (!ch->enabled) {
-                delete [] ch->cropimg;
-                ch->cropimg = nullptr;
-                delete [] ch->cropimgtrue;
-                ch->cropimgtrue = nullptr;
-                ch->cimg.unlock ();
-                return FALSE;
-            }
-
-            if (ch->cropimg) {
-                if (ch->cix == ch->cropX && ch->ciy == ch->cropY && ch->ciw == ch->cropW && ch->cih == ch->cropH && ch->cis == (ch->zoom >= 1000 ? 1 : ch->zoom / 10)) {
                             // calculate final image size
-                    float czoom = ch->zoom >= 1000 ?
+                            float czoom = self->zoom >= 1000 ?
-                        ch->zoom / 1000.f :
+                                self->zoom / 1000.f :
-                        float((ch->zoom/10) * 10) / float(ch->zoom);
+                                float((self->zoom/10) * 10) / float(self->zoom);
-                    int imw = ch->cropimg_width * czoom;
+                            int imw = self->cropimg_width * czoom;
-                    int imh = ch->cropimg_height * czoom;
+                            int imh = self->cropimg_height * czoom;
 
-                    if (imw > ch->ww) {
+                            if (imw > self->ww) {
-                        imw = ch->ww;
+                                imw = self->ww;
                             }
 
-                    if (imh > ch->wh) {
+                            if (imh > self->wh) {
-                        imh = ch->wh;
+                                imh = self->wh;
                             }
 
-                    Glib::RefPtr<Gdk::Pixbuf> tmpPixbuf = Gdk::Pixbuf::create_from_data (ch->cropimg, Gdk::COLORSPACE_RGB, false, 8, ch->cropimg_width, ch->cropimg_height, 3 * ch->cropimg_width);
+                            Glib::RefPtr<Gdk::Pixbuf> tmpPixbuf = Gdk::Pixbuf::create_from_data (self->cropimg.data(), Gdk::COLORSPACE_RGB, false, 8, self->cropimg_width, self->cropimg_height, 3 * self->cropimg_width);
-                    ch->cropPixbuf = Gdk::Pixbuf::create (Gdk::COLORSPACE_RGB, false, 8, imw, imh);
+                            self->cropPixbuf = Gdk::Pixbuf::create (Gdk::COLORSPACE_RGB, false, 8, imw, imh);
-                    tmpPixbuf->scale (ch->cropPixbuf, 0, 0, imw, imh, 0, 0, czoom, czoom, Gdk::INTERP_TILES);
+                            tmpPixbuf->scale (self->cropPixbuf, 0, 0, imw, imh, 0, 0, czoom, czoom, Gdk::INTERP_TILES);
                             tmpPixbuf.clear ();
 
-                    Glib::RefPtr<Gdk::Pixbuf> tmpPixbuftrue = Gdk::Pixbuf::create_from_data (ch->cropimgtrue, Gdk::COLORSPACE_RGB, false, 8, ch->cropimg_width, ch->cropimg_height, 3 * ch->cropimg_width);
+                            Glib::RefPtr<Gdk::Pixbuf> tmpPixbuftrue = Gdk::Pixbuf::create_from_data (self->cropimgtrue.data(), Gdk::COLORSPACE_RGB, false, 8, self->cropimg_width, self->cropimg_height, 3 * self->cropimg_width);
-                    ch->cropPixbuftrue = Gdk::Pixbuf::create (Gdk::COLORSPACE_RGB, false, 8, imw, imh);
+                            self->cropPixbuftrue = Gdk::Pixbuf::create (Gdk::COLORSPACE_RGB, false, 8, imw, imh);
-                    tmpPixbuftrue->scale (ch->cropPixbuftrue, 0, 0, imw, imh, 0, 0, czoom, czoom, Gdk::INTERP_TILES);
+                            tmpPixbuftrue->scale (self->cropPixbuftrue, 0, 0, imw, imh, 0, 0, czoom, czoom, Gdk::INTERP_TILES);
                             tmpPixbuftrue.clear ();
                         }
 
-                delete [] ch->cropimg;
+                        self->cropimg.clear();
-                ch->cropimg = nullptr;
+                        self->cropimgtrue.clear();
-                delete [] ch->cropimgtrue;
-                ch->cropimgtrue = nullptr;
                     }
 
-            ch->cimg.unlock ();
+                    self->cimg.unlock ();
 
-            if (ch->displayHandler) {
+                    if (self->displayHandler) {
-                ch->displayHandler->cropImageUpdated ();
+                        self->displayHandler->cropImageUpdated ();
 
-                if (ch->initial) {
+                        if (self->initial.exchange(false)) {
-                    ch->displayHandler->initialImageArrived ();
+                            self->displayHandler->initialImageArrived ();
-                    ch->initial = false;
                         }
                     }
-
+                } else {
-            idle_helper->pending--;
+                    self->cimg.unlock();
+                }
 
                 return FALSE;
             };
 
-        idle_register.add(func, idle_helper, G_PRIORITY_HIGH_IDLE);
+            idle_register.add(func, this/*, G_PRIORITY_HIGH_IDLE*/);
+        }
     }
 
     cimg.unlock ();
@@ -469,10 +459,8 @@ void CropHandler::setEnabled (bool e)
         }
 
         cimg.lock();
-        delete [] cropimg;
+        cropimg.clear();
-        cropimg = nullptr;
+        cropimgtrue.clear();
-        delete [] cropimgtrue;
-        cropimgtrue = nullptr;
         cropPixbuf.clear();
         cimg.unlock();
     } else {

rtgui/crophandler.h

@@ -19,11 +19,16 @@
 #ifndef __CROPHANDLER__
 #define __CROPHANDLER__
 
+#include <atomic>
+#include <vector>
+
+#include <gtkmm.h>
+
 #include "../rtengine/rtengine.h"
-#include "threadutils.h"
+
 #include "edit.h"
 #include "lockablecolorpicker.h"
-#include <gtkmm.h>
+#include "threadutils.h"
 
 class CropDisplayHandler
 {
@@ -98,12 +103,6 @@ public:
     MyMutex cimg;
 
 private:
-    struct IdleHelper {
-        CropHandler* cropHandler;
-        bool destroyed;
-        int pending;
-    };
-
     void    compDim ();
 
     int zoom;               // scale factor (e.g. 5 if 1:5 scale) ; if 1:1 scale and bigger, factor is multiplied by 1000  (i.e. 1000 for 1:1 scale, 2000 for 2:1, etc...)
@@ -112,17 +111,18 @@ private:
     int cx, cy, cw, ch;     // position and size of the requested crop ; position expressed in image coordinates, so cx and cy might be negative and cw and ch higher than the image's 1:1 size
     int cropX, cropY, cropW, cropH; // cropPixbuf's displayed area (position and size), i.e. coordinates in 1:1 scale, i.e. cx, cy, cw & ch trimmed to the image's bounds
     bool enabled;
-    unsigned char* cropimg;
+    std::vector<unsigned char> cropimg;
-    unsigned char* cropimgtrue;
+    std::vector<unsigned char> cropimgtrue;
     int cropimg_width, cropimg_height, cix, ciy, ciw, cih, cis;
-    bool initial;
     bool isLowUpdatePriority;
 
     rtengine::StagedImageProcessor* ipc;
     rtengine::DetailedCrop* crop;
 
     CropDisplayHandler* displayHandler;
-    IdleHelper* idle_helper;
+
+    std::atomic<bool> redraw_needed;
+    std::atomic<bool> initial;
 
     IdleRegister idle_register;
 };

rtgui/toolpanelcoord.cc

@@ -265,6 +265,7 @@ void ToolPanelCoordinator::addPanel(Gtk::Box* where, FoldableToolPanel* panel, i
 
 ToolPanelCoordinator::~ToolPanelCoordinator()
 {
+    idle_register.destroy();
 
     closeImage();
 
@@ -274,29 +275,58 @@ ToolPanelCoordinator::~ToolPanelCoordinator()
 
 void ToolPanelCoordinator::imageTypeChanged(bool isRaw, bool isBayer, bool isXtrans)
 {
-    GThreadLock lock;
-
     if (isRaw) {
-        rawPanelSW->set_sensitive(true);
-
         if (isBayer) {
-            sensorxtrans->FoldableToolPanel::hide();
+            const auto func = [](gpointer data) -> gboolean {
-            sensorbayer->FoldableToolPanel::show();
+                ToolPanelCoordinator* const self = static_cast<ToolPanelCoordinator*>(data);
-            preprocess->FoldableToolPanel::show();
+
-            flatfield->FoldableToolPanel::show();
+                self->rawPanelSW->set_sensitive (true);
-        } else if (isXtrans) {
+                self->sensorxtrans->FoldableToolPanel::hide();
-            sensorxtrans->FoldableToolPanel::show();
+                self->sensorbayer->FoldableToolPanel::show();
-            sensorbayer->FoldableToolPanel::hide();
+                self->preprocess->FoldableToolPanel::show();
-            preprocess->FoldableToolPanel::show();
+                self->flatfield->FoldableToolPanel::show();
-            flatfield->FoldableToolPanel::show();
+
-        } else {
+                return FALSE;
-            sensorbayer->FoldableToolPanel::hide();
+            };
-            sensorxtrans->FoldableToolPanel::hide();
+            idle_register.add(func, this);
-            preprocess->FoldableToolPanel::hide();
+        }
-            flatfield->FoldableToolPanel::hide();
+        else if (isXtrans) {
+            const auto func = [](gpointer data) -> gboolean {
+                ToolPanelCoordinator* const self = static_cast<ToolPanelCoordinator*>(data);
+
+                self->rawPanelSW->set_sensitive (true);
+                self->sensorxtrans->FoldableToolPanel::show();
+                self->sensorbayer->FoldableToolPanel::hide();
+                self->preprocess->FoldableToolPanel::show();
+                self->flatfield->FoldableToolPanel::show();
+
+                return FALSE;
+            };
+            idle_register.add(func, this);
+        }
+        else {
+            const auto func = [](gpointer data) -> gboolean {
+                ToolPanelCoordinator* const self = static_cast<ToolPanelCoordinator*>(data);
+
+                self->rawPanelSW->set_sensitive (true);
+                self->sensorbayer->FoldableToolPanel::hide();
+                self->sensorxtrans->FoldableToolPanel::hide();
+                self->preprocess->FoldableToolPanel::hide();
+                self->flatfield->FoldableToolPanel::hide();
+
+                return FALSE;
+            };
+            idle_register.add(func, this);
         }
     } else {
-        rawPanelSW->set_sensitive(false);
+        const auto func = [](gpointer data) -> gboolean {
+            ToolPanelCoordinator* const self = static_cast<ToolPanelCoordinator*>(data);
+
+            self->rawPanelSW->set_sensitive (false);
+
+            return FALSE;
+        };
+        idle_register.add(func, this);
     }
 
 }
@@ -503,7 +533,6 @@ void ToolPanelCoordinator::initImage(rtengine::StagedImageProcessor* ipc_, bool
 
 
     toneCurve->setRaw(raw);
-//>>>>>>> dev
     hasChanged = true;
 }
 

rtgui/toolpanelcoord.h

@@ -163,13 +163,7 @@ protected:
     ToolVBox* transformPanel;
     ToolVBox* rawPanel;
     ToolVBox* advancedPanel;
-//<<<<<<< HEAD
     ToolVBox* locallabPanel;
-//    Gtk::Notebook* metadataPanel;
-//    ExifPanel* exifpanel;
-//    IPTCPanel* iptcpanel;
-//=======
-//>>>>>>> dev
     ToolBar* toolBar;
 
     TextOrIcon* toiE;
@@ -312,6 +306,9 @@ public:
     void editModeSwitchedOff();
 
     void setEditProvider (EditDataProvider *provider);
+
+private:
+    IdleRegister idle_register;
 };
 
 #endif