From 1318935a870a32610f5a06e29b673aaa472e6170 Mon Sep 17 00:00:00 2001 From: Floessie Date: Sun, 25 Oct 2020 18:24:30 +0100 Subject: [PATCH] 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 | 27 +++++++++++++++++++-------- rtgui/batchqueue.cc | 5 +++-- rtgui/batchqueueentry.cc | 11 +++++++---- rtgui/filebrowserentry.cc | 25 +++++++++++-------------- rtgui/filecatalog.cc | 5 +++-- rtgui/options.cc | 6 ++++++ rtgui/options.h | 1 + rtgui/thumbbrowserentrybase.cc | 30 +++++++++++++----------------- rtgui/thumbbrowserentrybase.h | 2 +- rtgui/thumbnail.cc | 25 +++++++++++++++++-------- rtgui/thumbnail.h | 2 +- 11 files changed, 82 insertions(+), 57 deletions(-) diff --git a/rtengine/rtthumbnail.cc b/rtengine/rtthumbnail.cc index 357ad3a69c..7d0f40f70e 100644 --- a/rtengine/rtthumbnail.cc +++ b/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(img->getHeight()) / h; + } else { + h = hh; + tpp->scale = static_cast(img->getWidth()) / w; + } + } else if (fixwh == 1) { w = h * img->getWidth() / img->getHeight(); - tpp->scale = (double)img->getHeight() / h; + tpp->scale = static_cast(img->getHeight()) / h; } else { h = w * img->getHeight() / img->getWidth(); - tpp->scale = (double)img->getWidth() / w; + tpp->scale = static_cast(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 (rwidth, rheight, interp, thumbImg); diff --git a/rtgui/batchqueue.cc b/rtgui/batchqueue.cc index fc1fc855ee..19da96fb52 100644 --- a/rtgui/batchqueue.cc +++ b/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 (); - const auto prevw = thumb->getThumbnailWidth(prevh, &pparams); + auto prevh = getMaxThumbnailHeight(); + 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 diff --git a/rtgui/batchqueueentry.cc b/rtgui/batchqueueentry.cc index 90079b2cc3..31a6f40c7d 100644 --- a/rtgui/batchqueueentry.cc +++ b/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(options.maxThumbnailWidth) / prew; + prew = options.maxThumbnailWidth; + preh = std::max(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 = new guint8 [prew * preh * 3]; - memcpy (preview, img, prew * preh * 3); + preview.resize(prew * preh * 3); + std::copy(img, img + preview.size(), preview.begin()); if (parent) { parent->redrawNeeded (this); diff --git a/rtgui/filebrowserentry.cc b/rtgui/filebrowserentry.cc index cbe31726d8..432296f389 100644 --- a/rtgui/filebrowserentry.cc +++ b/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(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) { - if (preview) { - delete [] preview; - } - preview = new guint8 [prew * preh * 3]; - } - memcpy(preview, img->getData(), prew * preh * 3); + preview.resize(prew * preh * 3); + std::copy(img->getData(), img->getData() + preview.size(), preview.begin()); { - GThreadLock lock; - updateBackBuffer (); + GThreadLock lock; + 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; } diff --git a/rtgui/filecatalog.cc b/rtgui/filecatalog.cc index 34d6c8aa39..08bcc276b1 100644 --- a/rtgui/filecatalog.cc +++ b/rtgui/filecatalog.cc @@ -1228,8 +1228,9 @@ void FileCatalog::developRequested(const std::vector& 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(); - const int pw = th->getThumbnailWidth(ph); + int pw; + int ph = BatchQueue::calcMaxThumbnailHeight(); + th->getThumbnailSize (pw, ph); // processThumbImage is the processing intensive part, but adding to queue must be ordered //#pragma omp ordered diff --git a/rtgui/options.cc b/rtgui/options.cc index 46d9b9ca5f..06ca5420f7 100644 --- a/rtgui/options.cc +++ b/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 pext = parseExtensions; keyFile.set_string_list("File Browser", "ParseExtensions", pext); diff --git a/rtgui/options.h b/rtgui/options.h index d2e0c05433..f7bf80a987 100644 --- a/rtgui/options.h +++ b/rtgui/options.h @@ -281,6 +281,7 @@ class Options 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 parseExtensions; // List containing all extensions type diff --git a/rtgui/thumbbrowserentrybase.cc b/rtgui/thumbbrowserentrybase.cc index 3840c8bf98..0c71cea2c5 100644 --- a/rtgui/thumbbrowserentrybase.cc +++ b/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 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; - backBuffer->copyRGBCharData(preview, 0, 0, prew, preh, prew * 3, prex, prey); + const int hh = exp_height - (upperMargin + bsHeight + borderWidth + infoh + lowerMargin); + 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) { - delete [] preview; - preview = nullptr; - } + preview.clear(); 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 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; diff --git a/rtgui/thumbbrowserentrybase.h b/rtgui/thumbbrowserentrybase.h index dbc6cf73ea..764f806fdb 100644 --- a/rtgui/thumbbrowserentrybase.h +++ b/rtgui/thumbbrowserentrybase.h @@ -59,7 +59,7 @@ class ThumbBrowserEntryBase 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 preview; // holds the preview image. used in updateBackBuffer. Glib::ustring dispname; diff --git a/rtgui/thumbnail.cc b/rtgui/thumbnail.cc index c1884edeb7..324a0c0c17 100644 --- a/rtgui/thumbnail.cc +++ b/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) { - return imgRatio_ * h; + if (imgRatio_ > 0.) { + w = imgRatio_ * static_cast(h); } else { - return tw_ * h / th_; + w = tw_ * h / th_; + } + + if (w > options.maxThumbnailWidth) { + const float s = static_cast(options.maxThumbnailWidth) / w; + w = options.maxThumbnailWidth; + h = std::max(h * s, 1); } } diff --git a/rtgui/thumbnail.h b/rtgui/thumbnail.h index c22c80cea2..aee5ee0a68 100644 --- a/rtgui/thumbnail.h +++ b/rtgui/thumbnail.h @@ -119,7 +119,7 @@ class Thumbnail // 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);