Merge branch 'houz/inkscape-color-profiles-cleanup'

5 files changed, 120 insertions, 77 deletions

diff --git a/src/color-profile.cpp b/src/color-profile.cpp
index fea2227f7..bbf1f464a 100644
--- a/src/color-profile.cpp
+++ b/src/color-profile.cpp
@@ -179,6 +179,32 @@ cmsHPROFILE ColorProfileImpl::getNULLProfile() {
 
 #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
 
+ColorProfile::FilePlusHome::FilePlusHome(Glib::ustring filename, bool isInHome) : filename(filename), isInHome(isInHome) {
+}
+
+ColorProfile::FilePlusHome::FilePlusHome(const ColorProfile::FilePlusHome &filePlusHome) : FilePlusHome(filePlusHome.filename, filePlusHome.isInHome) {
+}
+
+bool ColorProfile::FilePlusHome::operator<(FilePlusHome const &other) const {
+    // if one is from home folder, other from global folder, sort home folder first. cf bug 1457126
+    bool result;
+    if (this->isInHome != other.isInHome) result = this->isInHome;
+    else                                  result = this->filename < other.filename;
+    return result;
+}
+
+ColorProfile::FilePlusHomeAndName::FilePlusHomeAndName(ColorProfile::FilePlusHome filePlusHome, Glib::ustring name)
+                                                      : FilePlusHome(filePlusHome), name(name) {
+}
+
+bool ColorProfile::FilePlusHomeAndName::operator<(ColorProfile::FilePlusHomeAndName const &other) const {
+    bool result;
+    if (this->isInHome != other.isInHome) result = this->isInHome;
+    else                                  result = this->name < other.name;
+    return result;
+}
+
+
 ColorProfile::ColorProfile() : SPObject() {
     this->impl = new ColorProfileImpl();
 
@@ -192,6 +218,15 @@ ColorProfile::ColorProfile() : SPObject() {
 ColorProfile::~ColorProfile() {
 }
 
+bool ColorProfile::operator<(ColorProfile const &other) const {
+    gchar *a_name_casefold = g_utf8_casefold(this->name, -1 );
+    gchar *b_name_casefold = g_utf8_casefold(other.name, -1 );
+    int result = g_strcmp0(a_name_casefold, b_name_casefold);
+    g_free(a_name_casefold);
+    g_free(b_name_casefold);
+    return result < 0;
+}
+
 /**
  * Callback: free object
  */
@@ -701,16 +736,9 @@ gint Inkscape::CMSSystem::getChannelCount(ColorProfile const *profile)
 
 #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
 
-// sort home dir before the rest, and alphabetically oterhwise
-bool compareProfileBoolPair(const std::pair<Glib::ustring, bool> & a, const std::pair<Glib::ustring, bool> & b)
-{
-    if (a.second != b.second) return a.second; // a comes first iff its home, i.e., second is true
-    return a.first < b.first;
-}
-
 // the bool return value tells if it's a user's directory or a system location
 // note that this will treat places under $HOME as system directories when they are found via $XDG_DATA_DIRS
-std::vector<std::pair<Glib::ustring, bool> > ColorProfile::getBaseProfileDirs() {
+std::set<ColorProfile::FilePlusHome> ColorProfile::getBaseProfileDirs() {
 #if defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
     static bool warnSet = false;
     if (!warnSet) {
@@ -720,28 +748,27 @@ std::vector<std::pair<Glib::ustring, bool> > ColorProfile::getBaseProfileDirs()
         warnSet = true;
     }
 #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
-    std::vector<std::pair<Glib::ustring, bool> > sources;
+    std::set<ColorProfile::FilePlusHome> sources;
 
     // first try user's local dir
     gchar* path = g_build_filename(g_get_user_data_dir(), "color", "icc", NULL);
-    sources.push_back(std::make_pair(path, true));
+    sources.insert(FilePlusHome(path, true));
     g_free(path);
 
     const gchar* const * dataDirs = g_get_system_data_dirs();
     for ( int i = 0; dataDirs[i]; i++ ) {
         gchar* path = g_build_filename(dataDirs[i], "color", "icc", NULL);
-        sources.push_back(std::make_pair(path, false));
+        sources.insert(FilePlusHome(path, false));
         g_free(path);
     }
 
     // On OS X:
     {
-        std::vector<Glib::ustring> possible;
-        sources.push_back(std::make_pair("/System/Library/ColorSync/Profiles", false));
-        sources.push_back(std::make_pair("/Library/ColorSync/Profiles", false));
+        sources.insert(FilePlusHome("/System/Library/ColorSync/Profiles", false));
+        sources.insert(FilePlusHome("/Library/ColorSync/Profiles", false));
 
         gchar *path = g_build_filename(g_get_home_dir(), "Library", "ColorSync", "Profiles", NULL);
-        sources.push_back(std::make_pair(path, true));
+        sources.insert(FilePlusHome(path, true));
         g_free(path);
     }
 
@@ -755,17 +782,12 @@ std::vector<std::pair<Glib::ustring, bool> > ColorProfile::getBaseProfileDirs()
         if ( !g_utf8_validate(utf8Path, -1, NULL) ) {
             g_warning( "GetColorDirectoryW() resulted in invalid UTF-8" );
         } else {
-            sources.push_back(std::make_pair(utf8Path, false));
+            sources.insert(FilePlusHome(utf8Path, false));
         }
         g_free( utf8Path );
     }
 #endif // WIN32
 
-    std::sort(sources.begin(), sources.end(), compareProfileBoolPair);
-    std::vector<std::pair<Glib::ustring, bool> >::iterator last = std::unique(sources.begin(), sources.end());
-    sources.erase(last, sources.end());
-
-
     return sources;
 }
 
@@ -808,50 +830,35 @@ static bool isIccFile( gchar const *filepath )
     return isIccFile;
 }
 
-std::vector<std::pair<Glib::ustring, bool> > ColorProfile::getProfileFiles()
+std::set<ColorProfile::FilePlusHome > ColorProfile::getProfileFiles()
 {
-    std::vector<std::pair<Glib::ustring, bool> > files;
+    std::set<FilePlusHome> files;
     using Inkscape::IO::Resource::get_filenames;
 
     for (auto &path: ColorProfile::getBaseProfileDirs()) {
-        for(auto &filename: get_filenames(path.first, {".icc", ".icm"})) {
+        for(auto &filename: get_filenames(path.filename, {".icc", ".icm"})) {
             if ( isIccFile(filename.c_str()) ) {
-                files.push_back(std::make_pair(filename, path.second));
+                files.insert(FilePlusHome(filename, path.isInHome));
             }
         }
     }
 
-    std::sort(files.begin(), files.end(), compareProfileBoolPair);
-    std::vector<std::pair<Glib::ustring, bool> >::iterator last = std::unique(files.begin(), files.end());
-    files.erase(last, files.end());
-
     return files;
 }
 
-bool compareProfilePairByName(const std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> & a,
-                              const std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> & b)
-{
-    if (a.first.second != b.first.second) return a.first.second;
-    return a.second < b.second;
-}
-
-std::vector<std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> > ColorProfile::getProfileFilesWithNames()
+std::set<ColorProfile::FilePlusHomeAndName> ColorProfile::getProfileFilesWithNames()
 {
-    std::vector<std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> > result;
+    std::set<FilePlusHomeAndName> result;
 
 #if defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
-    std::vector<std::pair<Glib::ustring, bool> > files = getProfileFiles();
-    for ( std::vector<std::pair<Glib::ustring, bool> >::const_iterator it = files.begin(); it != files.end(); ++it ) {
-        cmsHPROFILE hProfile = cmsOpenProfileFromFile(it->first.c_str(), "r");
+    for (auto &profile: getProfileFiles()) {
+        cmsHPROFILE hProfile = cmsOpenProfileFromFile(profile.filename.c_str(), "r");
         if ( hProfile ) {
             Glib::ustring name = getNameFromProfile(hProfile);
-            result.push_back( std::make_pair(*it, name) );
+            result.insert( FilePlusHomeAndName(profile, name) );
             cmsCloseProfile(hProfile);
         }
     }
-
-    std::sort(result.begin(), result.end(), compareProfilePairByName);
-
 #endif // defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
 
     return result;
@@ -931,18 +938,17 @@ void loadProfiles()
     static bool profiles_searched = false;
     if ( !profiles_searched ) {
         knownProfiles.clear();
-        std::vector<std::pair<Glib::ustring, bool> > files = ColorProfile::getProfileFiles();
 
-        for ( std::vector<std::pair<Glib::ustring, bool> >::const_iterator it = files.begin(); it != files.end(); ++it ) {
-            cmsHPROFILE prof = cmsOpenProfileFromFile( it->first.c_str(), "r" );
+        for (auto &profile: ColorProfile::getProfileFiles()) {
+            cmsHPROFILE prof = cmsOpenProfileFromFile( profile.filename.c_str(), "r" );
             if ( prof ) {
-                ProfileInfo info( prof, Glib::filename_to_utf8( it->first.c_str() ) );
+                ProfileInfo info( prof, Glib::filename_to_utf8( profile.filename.c_str() ) );
                 cmsCloseProfile( prof );
                 prof = 0;
 
                 bool sameName = false;
-                for ( std::vector<ProfileInfo>::iterator it = knownProfiles.begin(); it != knownProfiles.end(); ++it ) {
-                    if ( it->getName() == info.getName() ) {
+                for(auto &knownProfile: knownProfiles) {
+                    if ( knownProfile.getName() == info.getName() ) {
                         sameName = true;
                         break;
                     }
diff --git a/src/color-profile.h b/src/color-profile.h
index fed8c8c13..0d27facfb 100644
--- a/src/color-profile.h
+++ b/src/color-profile.h
@@ -1,6 +1,7 @@
 #ifndef SEEN_COLOR_PROFILE_H
 #define SEEN_COLOR_PROFILE_H
 
+#include <set>
 #include <vector>
 #include <sp-object.h>
 #include <glibmm/ustring.h>
@@ -27,15 +28,37 @@ class ColorProfileImpl;
  */
 class ColorProfile : public SPObject {
 public:
-	ColorProfile();
-	virtual ~ColorProfile();
+    ColorProfile();
+    virtual ~ColorProfile();
+
+    bool operator<(ColorProfile const &other) const;
+
+    // we use std::set with pointers to ColorProfile, just having operator< isn't enough to sort these
+    struct pointerComparator {
+        bool operator()(const ColorProfile * const & a, const ColorProfile * const & b) { return (*a) < (*b); };
+    };
 
     friend cmsHPROFILE colorprofile_get_handle( SPDocument*, unsigned int*, char const* );
     friend class CMSSystem;
 
-    static std::vector<std::pair<Glib::ustring, bool> > getBaseProfileDirs();
-    static std::vector<std::pair<Glib::ustring, bool> > getProfileFiles();
-    static std::vector<std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> > getProfileFilesWithNames();
+    class FilePlusHome {
+    public:
+        FilePlusHome(Glib::ustring filename, bool isInHome);
+        FilePlusHome(const FilePlusHome &filePlusHome);
+        bool operator<(FilePlusHome const &other) const;
+        Glib::ustring filename;
+        bool isInHome;
+    };
+    class FilePlusHomeAndName: public FilePlusHome {
+    public:
+        FilePlusHomeAndName(FilePlusHome filePlusHome, Glib::ustring name);
+        bool operator<(FilePlusHomeAndName const &other) const;
+        Glib::ustring name;
+    };
+
+    static std::set<FilePlusHome> getBaseProfileDirs();
+    static std::set<FilePlusHome> getProfileFiles();
+    static std::set<FilePlusHomeAndName> getProfileFilesWithNames();
 #if defined(HAVE_LIBLCMS1) || defined(HAVE_LIBLCMS2)
     //icColorSpaceSignature getColorSpace() const;
     ColorSpaceSig getColorSpace() const;
diff --git a/src/ui/dialog/document-properties.cpp b/src/ui/dialog/document-properties.cpp
index 1b074bb0c..0fff45762 100644
--- a/src/ui/dialog/document-properties.cpp
+++ b/src/ui/dialog/document-properties.cpp
@@ -386,26 +386,25 @@ void DocumentProperties::populate_available_profiles(){
     _AvailableProfilesListStore->clear(); // Clear any existing items in the combo box
 
     // Iterate through the list of profiles and add the name to the combo box.
-    std::vector<std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> > pairs = ColorProfile::getProfileFilesWithNames();
     bool home = true; // initial value doesn't matter, it's just to avoid a compiler warning
-    for ( std::vector<std::pair<std::pair<Glib::ustring, bool>, Glib::ustring> >::const_iterator it = pairs.begin(); it != pairs.end(); ++it ) {
+    bool first = true;
+    for (auto &profile: ColorProfile::getProfileFilesWithNames()) {
         Gtk::TreeModel::Row row;
-        Glib::ustring file = it->first.first;
-        Glib::ustring name = it->second;
 
         // add a separator between profiles from the user's home directory and system profiles
-        if (it != pairs.begin() && it->first.second != home)
+        if (!first && profile.isInHome != home)
         {
           row = *(_AvailableProfilesListStore->append());
           row[_AvailableProfilesListColumns.fileColumn] = "<separator>";
           row[_AvailableProfilesListColumns.nameColumn] = "<separator>";
           row[_AvailableProfilesListColumns.separatorColumn] = true;
         }
-        home = it->first.second;
+        home = profile.isInHome;
+        first = false;
 
         row = *(_AvailableProfilesListStore->append());
-        row[_AvailableProfilesListColumns.fileColumn] = file;
-        row[_AvailableProfilesListColumns.nameColumn] = name;
+        row[_AvailableProfilesListColumns.fileColumn] = profile.filename;
+        row[_AvailableProfilesListColumns.nameColumn] = profile.name;
         row[_AvailableProfilesListColumns.separatorColumn] = false;
     }
 }
@@ -459,10 +458,11 @@ void DocumentProperties::linkSelectedProfile()
             g_warning("No color profile available.");
             return;
         }
-	
+
         // Read the filename and description from the list of available profiles
         Glib::ustring file = (*iter)[_AvailableProfilesListColumns.fileColumn];
         Glib::ustring name = (*iter)[_AvailableProfilesListColumns.nameColumn];
+
         std::vector<SPObject *> current = SP_ACTIVE_DOCUMENT->getResourceList( "iccprofile" );
         for (std::vector<SPObject *>::const_iterator it = current.begin(); it != current.end(); ++it) {
             SPObject* obj = *it;
@@ -514,6 +514,9 @@ struct _cmp {
   }
 };
 
+template <typename From, typename To>
+struct static_caster { To * operator () (From * value) const { return static_cast<To *>(value); } };
+
 void DocumentProperties::populate_linked_profiles_box()
 {
     _LinkedProfilesListStore->clear();
@@ -521,12 +524,16 @@ void DocumentProperties::populate_linked_profiles_box()
     if (! current.empty()) {
         _emb_profiles_observer.set((*(current.begin()))->parent);
     }
-    std::set<SPObject *, _cmp> _current (current.begin(), current.end());
-    for (std::set<SPObject *, _cmp>::const_iterator it = _current.begin(); it != _current.end(); ++it) {
-        SPObject* obj = *it;
-        Inkscape::ColorProfile* prof = reinterpret_cast<Inkscape::ColorProfile*>(obj);
+
+    std::set<Inkscape::ColorProfile *, Inkscape::ColorProfile::pointerComparator> _current;
+    std::transform(current.begin(),
+                   current.end(),
+                   std::inserter(_current, _current.begin()),
+                   static_caster<SPObject, Inkscape::ColorProfile>());
+
+    for (auto &profile: _current) {
         Gtk::TreeModel::Row row = *(_LinkedProfilesListStore->append());
-        row[_LinkedProfilesListColumns.nameColumn] = prof->name;
+        row[_LinkedProfilesListColumns.nameColumn] = profile->name;
 //        row[_LinkedProfilesListColumns.previewColumn] = "Color Preview";
     }
 }
diff --git a/src/ui/dialog/inkscape-preferences.cpp b/src/ui/dialog/inkscape-preferences.cpp
index 1ad44941f..d946bac82 100644
--- a/src/ui/dialog/inkscape-preferences.cpp
+++ b/src/ui/dialog/inkscape-preferences.cpp
@@ -986,9 +986,8 @@ void InkscapePreferences::initPageIO()
     _page_cms.add_group_header( _("Display adjustment"));
 
     Glib::ustring tmpStr;
-    std::vector<std::pair<Glib::ustring, bool> > sources = ColorProfile::getBaseProfileDirs();
-    for ( std::vector<std::pair<Glib::ustring, bool> >::const_iterator it = sources.begin(); it != sources.end(); ++it ) {
-        gchar* part = g_strdup_printf( "\n%s", it->first.c_str() );
+    for (auto &profile: ColorProfile::getBaseProfileDirs()) {
+        gchar* part = g_strdup_printf( "\n%s", profile.filename.c_str() );
         tmpStr += part;
         g_free(part);
     }
diff --git a/src/ui/widget/color-icc-selector.cpp b/src/ui/widget/color-icc-selector.cpp
index 616e9afa8..f1dd92746 100644
--- a/src/ui/widget/color-icc-selector.cpp
+++ b/src/ui/widget/color-icc-selector.cpp
@@ -663,6 +663,9 @@ struct _cmp {
   }
 };
 
+template <typename From, typename To>
+struct static_caster { To * operator () (From * value) const { return static_cast<To *>(value); } };
+
 void ColorICCSelectorImpl::_profilesChanged(std::string const &name)
 {
     GtkComboBox *combo = GTK_COMBO_BOX(_profileSel);
@@ -680,10 +683,15 @@ void ColorICCSelectorImpl::_profilesChanged(std::string const &name)
 
     int index = 1;
     std::vector<SPObject *> current = SP_ACTIVE_DOCUMENT->getResourceList("iccprofile");
-    std::set<SPObject *, _cmp> _current(current.begin(), current.end());
-    for (std::set<SPObject *, _cmp>::const_iterator it = _current.begin(); it != _current.end(); ++it) {
-        SPObject *obj = *it;
-        Inkscape::ColorProfile *prof = reinterpret_cast<Inkscape::ColorProfile *>(obj);
+
+    std::set<Inkscape::ColorProfile *, Inkscape::ColorProfile::pointerComparator> _current;
+    std::transform(current.begin(),
+                   current.end(),
+                   std::inserter(_current, _current.begin()),
+                   static_caster<SPObject, Inkscape::ColorProfile>());
+
+    for (auto &it: _current) {
+        Inkscape::ColorProfile *prof = it;
 
         gtk_list_store_append(store, &iter);
         gtk_list_store_set(store, &iter, 0, gr_ellipsize_text(prof->name, 25).c_str(), 1, prof->name, -1);