Better handle thumbnail generation of extreme aspect ratio images (fixes #3794)

* Backport fix taken from ART
* Enforce minimal thumbnail size of 1x1 px in two places, prevents divison by zero and empty image generation

rtengine/rtthumbnail.cc

@@ -205,11 +205,6 @@ Thumbnail* Thumbnail::loadFromImage (const Glib::ustring& fname, int &w, int &h,
 
     ImageIO* img = imgSrc.getImageIO();
 
-    // agriggio -- hotfix for #3794, to be revised once a proper solution is implemented
-    if (std::max(img->getWidth(), img->getHeight()) / std::min(img->getWidth(), img->getHeight()) >= 10) {
-        return nullptr;
-    }
-    
     Thumbnail* tpp = new Thumbnail ();
 
     unsigned char* data;
@@ -235,15 +230,29 @@ Thumbnail* Thumbnail::loadFromImage (const Glib::ustring& fname, int &w, int &h,
         h = img->getHeight();
         tpp->scale = 1.;
     } else {
-        if (fixwh == 1) {
+        if (fixwh < 0 && w > 0 && h > 0) {
+            const int ww = h * img->getWidth() / img->getHeight();
+            const int hh = w * img->getHeight() / img->getWidth();
+            if (ww <= w) {
+                w = ww;
+                tpp->scale = static_cast<double>(img->getHeight()) / h;
+            } else {
+                h = hh;
+                tpp->scale = static_cast<double>(img->getWidth()) / w;
+            }
+        } else if (fixwh == 1) {
             w = h * img->getWidth() / img->getHeight();
-            tpp->scale = (double)img->getHeight() / h;
+            tpp->scale = static_cast<double>(img->getHeight()) / h;
         } else {
             h = w * img->getHeight() / img->getWidth();
-            tpp->scale = (double)img->getWidth() / w;
+            tpp->scale = static_cast<double>(img->getWidth()) / w;
         }
     }
 
+    // Precaution to prevent division by zero later on
+    if (h < 1) h = 1;
+    if (w < 1) w = 1;
+
     // bilinear interpolation
     if (tpp->thumbImg) {
         delete tpp->thumbImg;
@@ -1178,6 +1187,8 @@ IImage8* Thumbnail::processImage (const procparams::ProcParams& params, eSensorT
         rwidth = int (size_t (thumbImg->getWidth()) * size_t (rheight) / size_t (thumbImg->getHeight()));
     }
 
+    if (rwidth < 1) rwidth = 1;
+    if (rheight < 1) rheight = 1;
 
     Imagefloat* baseImg = resizeTo<Imagefloat> (rwidth, rheight, interp, thumbImg);
 

rtgui/batchqueue.cc

@@ -344,8 +344,9 @@ bool BatchQueue::loadBatchQueue ()
 
             auto job = rtengine::ProcessingJob::create (source, thumb->getType () == FT_Raw, pparams, fast);
 
-            const auto prevh = getMaxThumbnailHeight ();
+            auto prevh = getMaxThumbnailHeight();
-            const auto prevw = thumb->getThumbnailWidth(prevh, &pparams);
+            auto prevw = prevh;
+            thumb->getThumbnailSize(prevw, prevh, &pparams);
 
             auto entry = new BatchQueueEntry (job, pparams, source, prevw, prevh, thumb, options.overwriteOutputFile);
             thumb->decreaseRef ();  // Removing the refCount acquired by cacheMgr->getEntry

rtgui/batchqueueentry.cc

@@ -106,8 +106,12 @@ void BatchQueueEntry::refreshThumbnailImage ()
 
 void BatchQueueEntry::calcThumbnailSize ()
 {
-
     prew = preh * origpw / origph;
+    if (prew > options.maxThumbnailWidth) {
+        const float s = static_cast<float>(options.maxThumbnailWidth) / prew;
+        prew = options.maxThumbnailWidth;
+        preh = std::max<int>(preh * s, 1);
+    }
 }
 
 
@@ -261,9 +265,8 @@ void BatchQueueEntry::_updateImage (guint8* img, int w, int h)
         MYWRITERLOCK(l, lockRW);
 
         prew = w;
-        assert (preview == nullptr);
+        preview.resize(prew * preh * 3);
-        preview = new guint8 [prew * preh * 3];
+        std::copy(img, img + preview.size(), preview.begin());
-        memcpy (preview, img, prew * preh * 3);
 
         if (parent) {
             parent->redrawNeeded (this);

rtgui/filebrowserentry.cc

@@ -116,9 +116,12 @@ void FileBrowserEntry::refreshQuickThumbnailImage ()
 
 void FileBrowserEntry::calcThumbnailSize ()
 {
-
     if (thumbnail) {
-        prew = thumbnail->getThumbnailWidth(preh);
+        int ow = prew, oh = preh;
+        thumbnail->getThumbnailSize(prew, preh);
+        if (ow != prew || oh != preh || preview.size() != static_cast<std::size_t>(prew * preh * 3)) {
+            preview.clear();
+        }
     }
 }
 
@@ -255,22 +258,16 @@ void FileBrowserEntry::_updateImage(rtengine::IImage8* img, double s, const rten
     bool rotated = false;
 
     if (preh == img->getHeight()) {
-        const bool resize = !preview || prew != img->getWidth();
         prew = img->getWidth ();
 
         // Check if image has been rotated since last time
-        rotated = preview && newLandscape != landscape;
+        rotated = !preview.empty() && newLandscape != landscape;
 
-        if (resize) {
+        preview.resize(prew * preh * 3);
-            if (preview) {
+        std::copy(img->getData(), img->getData() + preview.size(), preview.begin());
-                delete [] preview;
-            }
-            preview = new guint8 [prew * preh * 3];
-        }
-        memcpy(preview, img->getData(), prew * preh * 3);
         {
-        GThreadLock lock;
+            GThreadLock lock;
-        updateBackBuffer ();
+            updateBackBuffer ();
         }
     }
 
@@ -601,7 +598,7 @@ bool FileBrowserEntry::onArea (CursorArea a, int x, int y)
 {
 
     MYREADERLOCK(l, lockRW);
-    if (!drawable || !preview) {
+    if (!drawable || preview.empty()) {
         return false;
     }
 

rtgui/filecatalog.cc

@@ -1228,8 +1228,9 @@ void FileCatalog::developRequested(const std::vector<FileBrowserEntry*>& tbe, bo
 
             rtengine::ProcessingJob* pjob = rtengine::ProcessingJob::create (fbe->filename, th->getType() == FT_Raw, params, fastmode && options.fastexport_use_fast_pipeline);
 
-            const int ph = BatchQueue::calcMaxThumbnailHeight();
+            int pw;
-            const int pw = th->getThumbnailWidth(ph);
+            int ph = BatchQueue::calcMaxThumbnailHeight();
+            th->getThumbnailSize (pw, ph);
 
             // processThumbImage is the processing intensive part, but adding to queue must be ordered
             //#pragma omp ordered

rtgui/options.cc

@@ -400,6 +400,7 @@ void Options::setDefaults()
     overwriteOutputFile = false;        // if TRUE, existing output JPGs/PNGs are overwritten, instead of adding ..-1.jpg, -2.jpg etc.
     theme = "RawTherapee";
     maxThumbnailHeight = 250;
+    maxThumbnailWidth = 800;
     maxCacheEntries = 20000;
     thumbInterp = 1;
     autoSuffix = true;
@@ -1019,6 +1020,10 @@ void Options::readFromFile(Glib::ustring fname)
                     maxThumbnailHeight = keyFile.get_integer("File Browser", "MaxPreviewHeight");
                 }
 
+                if (keyFile.has_key("File Browser", "MaxPreviewWidth")) {
+                    maxThumbnailWidth = keyFile.get_integer("File Browser", "MaxPreviewWidth");
+                }
+
                 if (keyFile.has_key("File Browser", "MaxCacheEntries")) {
                     maxCacheEntries = keyFile.get_integer("File Browser", "MaxCacheEntries");
                 }
@@ -2114,6 +2119,7 @@ void Options::saveToFile(Glib::ustring fname)
         keyFile.set_integer("File Browser", "ThumbnailSizeQueue", thumbSizeQueue);
         keyFile.set_integer("File Browser", "SameThumbSize", sameThumbSize);
         keyFile.set_integer("File Browser", "MaxPreviewHeight", maxThumbnailHeight);
+        keyFile.set_integer("File Browser", "MaxPreviewWidth", maxThumbnailWidth);
         keyFile.set_integer("File Browser", "MaxCacheEntries", maxCacheEntries);
         Glib::ArrayHandle<Glib::ustring> pext = parseExtensions;
         keyFile.set_string_list("File Browser", "ParseExtensions", pext);

rtgui/options.h

@@ -281,6 +281,7 @@ public:
     CPBKeyType CPBKeys; // Custom Profile Builder's key type
     int editorToSendTo;
     int maxThumbnailHeight;
+    int maxThumbnailWidth;
     std::size_t maxCacheEntries;
     int thumbInterp; // 0: nearest, 1: bilinear
     std::vector<Glib::ustring> parseExtensions;   // List containing all extensions type

rtgui/thumbbrowserentrybase.cc

@@ -135,7 +135,6 @@ ThumbBrowserEntryBase::ThumbBrowserEntryBase (const Glib::ustring& fname) :
     textGap(6),
     sideMargin(8),
     lowerMargin(8),
-    preview(nullptr),
     dispname(Glib::path_get_basename(fname)),
     buttonSet(nullptr),
     width(0),
@@ -171,7 +170,6 @@ ThumbBrowserEntryBase::ThumbBrowserEntryBase (const Glib::ustring& fname) :
 
 ThumbBrowserEntryBase::~ThumbBrowserEntryBase ()
 {
-    delete[] preview;
     delete buttonSet;
 }
 
@@ -207,7 +205,7 @@ void ThumbBrowserEntryBase::updateBackBuffer ()
 
     bbSelected = selected;
     bbFramed = framed;
-    bbPreview = preview;
+    bbPreview = preview.data();
 
     Cairo::RefPtr<Cairo::Context> cc = Cairo::Context::create(surface);
 
@@ -237,16 +235,20 @@ void ThumbBrowserEntryBase::updateBackBuffer ()
 
     if (buttonSet) {
         int tmp;
-        buttonSet->getAllocatedDimensions (tmp, bsHeight);
+        buttonSet->getAllocatedDimensions(tmp, bsHeight);
     }
 
+    int infow, infoh;
+    getTextSizes(infow, infoh);
+
     // draw preview frame
     //backBuffer->draw_rectangle (cc, false, (exp_width-prew)/2, upperMargin+bsHeight, prew+1, preh+1);
     // draw thumbnail image
-    if (preview) {
+    if (!preview.empty()) {
         prex = borderWidth + (exp_width - prew) / 2;
-        prey = upperMargin + bsHeight + borderWidth;
+        const int hh = exp_height - (upperMargin + bsHeight + borderWidth + infoh + lowerMargin);
-        backBuffer->copyRGBCharData(preview, 0, 0, prew, preh, prew * 3, prex, prey);
+        prey = upperMargin + bsHeight + borderWidth + std::max((hh - preh) / 2, 0);
+        backBuffer->copyRGBCharData(preview.data(), 0, 0, prew, preh, prew * 3, prex, prey);
     }
 
     customBackBufferUpdate (cc);
@@ -255,9 +257,6 @@ void ThumbBrowserEntryBase::updateBackBuffer ()
     bbIcons = getIconsOnImageArea ();
     bbSpecificityIcons = getSpecificityIconsOnImageArea ();
 
-    int infow, infoh;
-    getTextSizes (infow, infoh);
-
     int iofs_x = 4, iofs_y = 4;
     int istartx = prex;
     int istarty = prey;
@@ -356,7 +355,7 @@ void ThumbBrowserEntryBase::updateBackBuffer ()
                 textposx_dt = 0;
             }
 
-            textposy = upperMargin + bsHeight + 2 * borderWidth + preh + borderWidth + textGap;
+            textposy = exp_height - lowerMargin - infoh;
             textw = exp_width - 2 * textGap;
 
             if (selected) {
@@ -556,10 +555,7 @@ void ThumbBrowserEntryBase::resize (int h)
     }
 
     if (preh != old_preh || prew != old_prew) { // if new thumbnail height or new orientation
-        if (preview) {
+        preview.clear();
-            delete [] preview;
-            preview = nullptr;
-        }
         refreshThumbnailImage ();
     } else if (backBuffer) {
         backBuffer->setDirty(true);    // This will force a backBuffer update on queue_draw
@@ -620,7 +616,7 @@ void ThumbBrowserEntryBase::draw (Cairo::RefPtr<Cairo::Context> cc)
         bbHeight = backBuffer->getHeight();
     }
 
-    if (!backBuffer || selected != bbSelected || framed != bbFramed || preview != bbPreview
+    if (!backBuffer || selected != bbSelected || framed != bbFramed || preview.data() != bbPreview
             || exp_width != bbWidth || exp_height != bbHeight || getIconsOnImageArea () != bbIcons
             || getSpecificityIconsOnImageArea() != bbSpecificityIcons || backBuffer->isDirty())
     {
@@ -680,7 +676,7 @@ rtengine::Coord2D ThumbBrowserEntryBase::getPosInImgSpace (int x, int y) const
 {
     rtengine::Coord2D coord(-1., -1.);
 
-    if (preview) {
+    if (!preview.empty()) {
         x -= ofsX + startx;
         y -= ofsY + starty;
 

rtgui/thumbbrowserentrybase.h

@@ -59,7 +59,7 @@ protected:
 
     MyRWMutex lockRW;  // Locks access to all image thumb changing actions
 
-    guint8* preview;  // holds the preview image. used in updateBackBuffer. TODO Olli: Make a cache to reduce mem significantly
+    std::vector<guint8> preview;  // holds the preview image. used in updateBackBuffer.
 
     Glib::ustring dispname;
 

rtgui/thumbnail.cc

@@ -121,7 +121,7 @@ void Thumbnail::_generateThumbnailImage ()
     tpp = nullptr;
     delete [] lastImg;
     lastImg = nullptr;
-    tw = -1;
+    tw = options.maxThumbnailWidth;
     th = options.maxThumbnailHeight;
     imgRatio = -1.;
 
@@ -138,20 +138,20 @@ void Thumbnail::_generateThumbnailImage ()
 
     if (ext == "jpg" || ext == "jpeg") {
         infoFromImage (fname);
-        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, 1, pparams->wb.equal);
+        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, -1, pparams->wb.equal);
 
         if (tpp) {
             cfs.format = FT_Jpeg;
         }
     } else if (ext == "png") {
-        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, 1, pparams->wb.equal);
+        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, -1, pparams->wb.equal);
 
         if (tpp) {
             cfs.format = FT_Png;
         }
     } else if (ext == "tif" || ext == "tiff") {
         infoFromImage (fname);
-        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, 1, pparams->wb.equal);
+        tpp = rtengine::Thumbnail::loadFromImage (fname, tw, th, -1, pparams->wb.equal);
 
         if (tpp) {
             cfs.format = FT_Tiff;
@@ -589,10 +589,13 @@ void Thumbnail::decreaseRef ()
     cachemgr->closeThumbnail (this);
 }
 
-int Thumbnail::getThumbnailWidth (const int h, const rtengine::procparams::ProcParams *pparams) const
+void Thumbnail::getThumbnailSize(int &w, int &h, const rtengine::procparams::ProcParams *pparams)
 {
+    MyMutex::MyLock lock(mutex);
+
     int tw_ = tw;
     int th_ = th;
+
     float imgRatio_ = imgRatio;
 
     if (pparams) {
@@ -617,10 +620,16 @@ int Thumbnail::getThumbnailWidth (const int h, const rtengine::procparams::ProcP
         }
     }
 
-    if (imgRatio_ > 0.f) {
+    if (imgRatio_ > 0.) {
-        return imgRatio_ * h;
+        w = imgRatio_ * static_cast<float>(h);
     } else {
-        return tw_ * h / th_;
+        w = tw_ * h / th_;
+    }
+
+    if (w > options.maxThumbnailWidth) {
+        const float s = static_cast<float>(options.maxThumbnailWidth) / w;
+        w = options.maxThumbnailWidth;
+        h = std::max<int>(h * s, 1);
     }
 }
 

rtgui/thumbnail.h

@@ -119,7 +119,7 @@ public:
 //        unsigned char*  getThumbnailImage (int &w, int &h, int fixwh=1); // fixwh = 0: fix w and calculate h, =1: fix h and calculate w
     rtengine::IImage8* processThumbImage    (const rtengine::procparams::ProcParams& pparams, int h, double& scale);
     rtengine::IImage8* upgradeThumbImage    (const rtengine::procparams::ProcParams& pparams, int h, double& scale);
-    int getThumbnailWidth (int h, const rtengine::procparams::ProcParams *pparams = nullptr) const;
+    void            getThumbnailSize        (int &w, int &h, const rtengine::procparams::ProcParams *pparams = nullptr);
     void            getFinalSize            (const rtengine::procparams::ProcParams& pparams, int& w, int& h);
     void            getOriginalSize         (int& w, int& h);