From b8aa69c8d545e389783c858c3379ecf43506b1d8 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 11 Jan 2016 18:57:45 +0100 Subject: [PATCH 1/6] Fix SIGSEGV on startup if ClutsDirectory is empty by always creating a model. --- rtgui/filmsimulation.cc | 15 +++++++-------- rtgui/filmsimulation.h | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index 907f5578a..e1cbd4966 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -143,16 +143,15 @@ ClutComboBox::ClutColumns::ClutColumns() add( clutFilename ); } -int ClutComboBox::fillFromDir( Glib::ustring path ) +int ClutComboBox::fillFromDir (const Glib::ustring& path) { - int result = 0; + m_model = Gtk::TreeStore::create (m_columns); + set_model (m_model); - if ( !path.empty() ) { - m_model.clear(); - m_model = Gtk::TreeStore::create( m_columns ); - set_model( m_model ); - result = parseDir( path, 0 ); - pack_start( m_columns.label, false ); + const auto result = parseDir (path, nullptr); + + if (result > 0) { + pack_start (m_columns.label, false); } return result; diff --git a/rtgui/filmsimulation.h b/rtgui/filmsimulation.h index 89e9f6fa3..a463bdf60 100644 --- a/rtgui/filmsimulation.h +++ b/rtgui/filmsimulation.h @@ -11,7 +11,7 @@ class ClutComboBox : public MyComboBox { public: - int fillFromDir( Glib::ustring path ); + int fillFromDir (const Glib::ustring& path); Glib::ustring getSelectedClut(); void setSelectedClut( Glib::ustring filename ); void addUnchangedEntry(); From cc585058abdb003a338865a8807dc3275fc1b110 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 11 Jan 2016 20:58:36 +0100 Subject: [PATCH 2/6] Limit the recusrion depth, direction and file count of the search for CLUT files. --- rtgui/filmsimulation.cc | 135 ++++++++++++++++++++++++++-------------- rtgui/filmsimulation.h | 2 +- 2 files changed, 91 insertions(+), 46 deletions(-) diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index e1cbd4966..feed0ba35 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -148,7 +148,7 @@ int ClutComboBox::fillFromDir (const Glib::ustring& path) m_model = Gtk::TreeStore::create (m_columns); set_model (m_model); - const auto result = parseDir (path, nullptr); + const auto result = parseDir (path); if (result > 0) { pack_start (m_columns.label, false); @@ -157,68 +157,113 @@ int ClutComboBox::fillFromDir (const Glib::ustring& path) return result; } -Gtk::TreeIter appendToModel( Glib::RefPtr model, Gtk::TreeModel::Row *parent ) +int ClutComboBox::parseDir (const Glib::ustring& path) { - Gtk::TreeIter result; - - if ( parent ) { - result = model->append( parent->children() ); - - } else { - result = model->append(); + if (path.empty () || !Glib::file_test (path, Glib::FILE_TEST_IS_DIR)) { + return 0; } - return result; -} + // Build menu of limited directory structure using breadth-first search + using Dirs = std::vector>; + Dirs dirs; -int ClutComboBox::parseDir( Glib::ustring path, Gtk::TreeModel::Row *parentRow ) -{ - int result = 0; + { + Dirs currDirs; + Dirs nextDirs; - if ( path.empty() || !safe_file_test( path, Glib::FILE_TEST_EXISTS ) || !safe_file_test ( path, Glib::FILE_TEST_IS_DIR ) ) { - return result; - } + constexpr auto maxDirCount = 128, maxDirDepth = 4; + auto dirCount = 0, dirDepth = 0; - Glib::Dir* dir = new Glib::Dir( path ); + currDirs.emplace_back (path, Gtk::TreeModel::Row ()); - Strings names; + while (!currDirs.empty ()) { - for( Glib::DirIterator it = dir->begin(); it != dir->end(); ++it ) { - Glib::ustring current = *it; + for (auto& dir : currDirs) { - if ( current != "." && current != ".." ) { - names.push_back( current ); - } - } + const auto& path = dir.first; + const auto& row = dir.second; - std::sort( names.begin(), names.end() ); + try { + for (const auto& entry : Glib::Dir (path)) { - for ( Strings::iterator it = names.begin(); it != names.end(); ++it ) { - Glib::ustring current = *it; - Glib::ustring fullname = Glib::build_filename( path, current ); + const auto entryPath = Glib::build_filename (path, entry); - if ( safe_file_test( fullname, Glib::FILE_TEST_IS_DIR ) ) { + if (!Glib::file_test (entryPath, Glib::FILE_TEST_IS_DIR)) { + continue; + } - Gtk::TreeModel::Row newFolderMenu = *appendToModel( m_model, parentRow ); - newFolderMenu[ m_columns.label ] = current; - result += parseDir( fullname, &newFolderMenu ); - } else { - Glib::ustring name, extension, profileName; - splitClutFilename( current, name, extension, profileName ); + auto newRow = row ? *m_model->append (row.children ()) : *m_model->append (); + newRow[m_columns.label] = entry; - if ( extension == "tif" || - extension == "TIF" || - extension == "png" || - extension == "PNG" ) { - Gtk::TreeModel::Row newClut = *appendToModel( m_model, parentRow ); - newClut[ m_columns.label ] = name; - newClut[ m_columns.clutFilename ] = fullname; - ++result; + nextDirs.emplace_back (entryPath, newRow); + } + } catch (Glib::Exception&) {} + + dirs.push_back (std::move (dir)); + if (++dirCount > maxDirCount) { + m_model->clear (); + return 0; + } + } + + currDirs.clear (); + currDirs.swap (nextDirs); + if (++dirDepth > maxDirDepth) { + m_model->clear (); + return 0; } } } - return result; + // Fill menu structure with CLUT files + Strings entries; + + constexpr auto maxFileCount = 4096; + auto fileCount = 0; + + for (const auto& dir : dirs) { + + const auto& path = dir.first; + const auto& row = dir.second; + + entries.clear (); + + try { + for (const auto& entry : Glib::Dir (path)) { + entries.push_back (entry); + } + } catch (Glib::Exception&) {} + + std::sort (entries.begin (), entries.end ()); + + for (const auto& entry : entries) { + + const auto entryPath = Glib::build_filename (path, entry); + + if (!Glib::file_test (entryPath, Glib::FILE_TEST_IS_REGULAR)) { + continue; + } + + Glib::ustring name, extension, profileName; + splitClutFilename (entry, name, extension, profileName); + + extension = extension.casefold (); + if (extension.compare ("tif") == 0 && extension.compare ("png") == 0) { + continue; + } + + auto newRow = row ? *m_model->append (row.children ()) : *m_model->append (); + newRow[m_columns.label] = name; + newRow[m_columns.clutFilename] = entryPath; + + if (++fileCount > maxFileCount) { + m_model->clear (); + return 0; + } + } + } + + return fileCount; } Glib::ustring ClutComboBox::getSelectedClut() diff --git a/rtgui/filmsimulation.h b/rtgui/filmsimulation.h index a463bdf60..440bf0c9e 100644 --- a/rtgui/filmsimulation.h +++ b/rtgui/filmsimulation.h @@ -25,7 +25,7 @@ private: ClutColumns(); }; - int parseDir( Glib::ustring path, Gtk::TreeModel::Row *parentRow ); + int parseDir (const Glib::ustring& path); Gtk::TreeIter findRowByClutFilename( Gtk::TreeModel::Children childs, Glib::ustring filename ); Glib::RefPtr m_model; From 93d497573a7243297810867d3e8009119d643681 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 11 Jan 2016 22:55:04 +0100 Subject: [PATCH 3/6] Only sort those CLUT file names which are actually regular files. --- rtgui/filmsimulation.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index feed0ba35..da765ef7a 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -230,7 +230,14 @@ int ClutComboBox::parseDir (const Glib::ustring& path) try { for (const auto& entry : Glib::Dir (path)) { - entries.push_back (entry); + + const auto entryPath = Glib::build_filename (path, entry); + + if (!Glib::file_test (entryPath, Glib::FILE_TEST_IS_REGULAR)) { + continue; + } + + entries.push_back (entryPath); } } catch (Glib::Exception&) {} @@ -238,12 +245,6 @@ int ClutComboBox::parseDir (const Glib::ustring& path) for (const auto& entry : entries) { - const auto entryPath = Glib::build_filename (path, entry); - - if (!Glib::file_test (entryPath, Glib::FILE_TEST_IS_REGULAR)) { - continue; - } - Glib::ustring name, extension, profileName; splitClutFilename (entry, name, extension, profileName); @@ -254,7 +255,7 @@ int ClutComboBox::parseDir (const Glib::ustring& path) auto newRow = row ? *m_model->append (row.children ()) : *m_model->append (); newRow[m_columns.label] = name; - newRow[m_columns.clutFilename] = entryPath; + newRow[m_columns.clutFilename] = entry; if (++fileCount > maxFileCount) { m_model->clear (); From bf2c24a48fe54874766d450061f3a78b0927373a Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 11 Jan 2016 23:42:59 +0100 Subject: [PATCH 4/6] Limit CLUT file search by wall time instead of number of directories and files. --- rtgui/filmsimulation.cc | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index da765ef7a..d20638fd2 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -1,4 +1,7 @@ #include "filmsimulation.h" + +#include + #include "options.h" #include "../rtengine/clutstore.h" #include "../rtengine/safegtk.h" @@ -6,6 +9,31 @@ using namespace rtengine; using namespace rtengine::procparams; +namespace +{ + +bool confirmToContinue (const std::chrono::system_clock::time_point& startedAt, + std::chrono::system_clock::time_point& continuedAt) +{ + const auto now = std::chrono::system_clock::now (); + const auto searchingFor = std::chrono::duration_cast (now-continuedAt); + + if (searchingFor >= std::chrono::seconds (5)) { + + const auto message = Glib::ustring::compose ("Searching CLUT files for %1 seconds now...\nDo you wish to continue?", searchingFor.count ()); + + Gtk::MessageDialog dialog (message, false, Gtk::MESSAGE_QUESTION, Gtk::BUTTONS_YES_NO, true); + if (dialog.run () != Gtk::RESPONSE_YES) { + return false; + } + } + + continuedAt = now; + return true; +} + +} + typedef std::vector Strings; FilmSimulation::FilmSimulation() @@ -163,6 +191,9 @@ int ClutComboBox::parseDir (const Glib::ustring& path) return 0; } + const auto startedAt = std::chrono::system_clock::now (); + auto continuedAt = startedAt; + // Build menu of limited directory structure using breadth-first search using Dirs = std::vector>; Dirs dirs; @@ -171,9 +202,6 @@ int ClutComboBox::parseDir (const Glib::ustring& path) Dirs currDirs; Dirs nextDirs; - constexpr auto maxDirCount = 128, maxDirDepth = 4; - auto dirCount = 0, dirDepth = 0; - currDirs.emplace_back (path, Gtk::TreeModel::Row ()); while (!currDirs.empty ()) { @@ -200,7 +228,8 @@ int ClutComboBox::parseDir (const Glib::ustring& path) } catch (Glib::Exception&) {} dirs.push_back (std::move (dir)); - if (++dirCount > maxDirCount) { + + if (!confirmToContinue (startedAt, continuedAt)) { m_model->clear (); return 0; } @@ -208,17 +237,12 @@ int ClutComboBox::parseDir (const Glib::ustring& path) currDirs.clear (); currDirs.swap (nextDirs); - if (++dirDepth > maxDirDepth) { - m_model->clear (); - return 0; - } } } // Fill menu structure with CLUT files Strings entries; - constexpr auto maxFileCount = 4096; auto fileCount = 0; for (const auto& dir : dirs) { @@ -257,7 +281,9 @@ int ClutComboBox::parseDir (const Glib::ustring& path) newRow[m_columns.label] = name; newRow[m_columns.clutFilename] = entry; - if (++fileCount > maxFileCount) { + ++fileCount; + + if (!confirmToContinue (startedAt, continuedAt)) { m_model->clear (); return 0; } From 250e06884d28fc1db5a047df29a496f3e0df9531 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Tue, 12 Jan 2016 17:31:13 +0100 Subject: [PATCH 5/6] Only notify the user once if the parseDir method of the film simulation is slow. --- rtdata/languages/default | 1 + rtgui/filmsimulation.cc | 38 +++++++++++++++----------------------- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/rtdata/languages/default b/rtdata/languages/default index 5f589b0f3..6af289209 100644 --- a/rtdata/languages/default +++ b/rtdata/languages/default @@ -1466,6 +1466,7 @@ TP_EXPOS_WHITEPOINT_LABEL;Raw White Points TP_FILMSIMULATION_LABEL;Film Simulation TP_FILMSIMULATION_STRENGTH;Strength TP_FILMSIMULATION_ZEROCLUTSFOUND;Set HaldCLUT directory in Preferences +TP_FILMSIMULATION_SLOWPARSEDIR;The Film Simulation HaldCLUT folder you pointed RawTherapee to is taking too long to load. Reduce the number of files in that folder or point RawTherapee to an empty one instead.\n\nSee Preferences > Image Processing > Film Simulation TP_FLATFIELD_AUTOSELECT;Auto-selection TP_FLATFIELD_BLURRADIUS;Blur radius TP_FLATFIELD_BLURTYPE;Blur type diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index d20638fd2..b195cc238 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -12,24 +12,23 @@ using namespace rtengine::procparams; namespace { -bool confirmToContinue (const std::chrono::system_clock::time_point& startedAt, - std::chrono::system_clock::time_point& continuedAt) +void notifySlowParseDir (const std::chrono::system_clock::time_point& startedAt) { - const auto now = std::chrono::system_clock::now (); - const auto searchingFor = std::chrono::duration_cast (now-continuedAt); + static bool alreadyNotified = false; - if (searchingFor >= std::chrono::seconds (5)) { - - const auto message = Glib::ustring::compose ("Searching CLUT files for %1 seconds now...\nDo you wish to continue?", searchingFor.count ()); - - Gtk::MessageDialog dialog (message, false, Gtk::MESSAGE_QUESTION, Gtk::BUTTONS_YES_NO, true); - if (dialog.run () != Gtk::RESPONSE_YES) { - return false; - } + if (alreadyNotified) { + return; } - continuedAt = now; - return true; + const auto now = std::chrono::system_clock::now (); + if (now - startedAt < std::chrono::seconds (10)) { + return; + } + + Gtk::MessageDialog dialog (M ("TP_FILMSIMULATION_SLOWPARSEDIR"), false, Gtk::MESSAGE_WARNING, Gtk::BUTTONS_OK, true); + dialog.run (); + + alreadyNotified = true; } } @@ -192,7 +191,6 @@ int ClutComboBox::parseDir (const Glib::ustring& path) } const auto startedAt = std::chrono::system_clock::now (); - auto continuedAt = startedAt; // Build menu of limited directory structure using breadth-first search using Dirs = std::vector>; @@ -229,10 +227,7 @@ int ClutComboBox::parseDir (const Glib::ustring& path) dirs.push_back (std::move (dir)); - if (!confirmToContinue (startedAt, continuedAt)) { - m_model->clear (); - return 0; - } + notifySlowParseDir (startedAt); } currDirs.clear (); @@ -283,10 +278,7 @@ int ClutComboBox::parseDir (const Glib::ustring& path) ++fileCount; - if (!confirmToContinue (startedAt, continuedAt)) { - m_model->clear (); - return 0; - } + notifySlowParseDir (startedAt); } } From 75e605012a4d910b3ad3a068c2167b3e558baf82 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sun, 17 Jan 2016 16:21:57 +0100 Subject: [PATCH 6/6] Fix logic error in extension checks for loading ICC and CLUT files. --- rtengine/iccstore.cc | 2 +- rtgui/filmsimulation.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rtengine/iccstore.cc b/rtengine/iccstore.cc index b77da03dd..ff29c72ab 100644 --- a/rtengine/iccstore.cc +++ b/rtengine/iccstore.cc @@ -54,7 +54,7 @@ void loadProfiles (const Glib::ustring& dirName, const Glib::ustring extension = fileName.substr (fileName.size () - 4).casefold (); - if (extension.compare(".icc") == 0 && extension.compare(".icm") == 0) + if (extension.compare (".icc") != 0 && extension.compare (".icm") != 0) continue; const Glib::ustring filePath = Glib::build_filename (dirName, fileName); diff --git a/rtgui/filmsimulation.cc b/rtgui/filmsimulation.cc index b195cc238..6052fdc87 100644 --- a/rtgui/filmsimulation.cc +++ b/rtgui/filmsimulation.cc @@ -268,7 +268,7 @@ int ClutComboBox::parseDir (const Glib::ustring& path) splitClutFilename (entry, name, extension, profileName); extension = extension.casefold (); - if (extension.compare ("tif") == 0 && extension.compare ("png") == 0) { + if (extension.compare ("tif") != 0 && extension.compare ("png") != 0) { continue; }