[sfmData] manage features/matches folders internally

* handle conversions to relative and absolute paths when _absolutePath is defined
* update internal relative folder paths when absolutePath is modified
* ensure features/matches folders contains no duplicates
* [tests] add 'sfmData_test' module + test internal folders management
* [software] update global/incrementalSfm

src/aliceVision/sfmData/CMakeLists.txt

@@ -30,3 +30,9 @@ alicevision_add_library(aliceVision_sfmData
 )
 
 # Unit tests
+
+alicevision_add_test(sfmData_test.cpp
+  NAME "sfmData"
+  LINKS aliceVision_sfmData
+        aliceVision_system
+)

src/aliceVision/sfmData/SfMData.cpp

@@ -89,22 +89,79 @@ bool SfMData::operator==(const SfMData& other) const {
 
 }
 
-std::vector<std::string> SfMData::getFeaturesFolders() const
+/**
+ * @brief Convert paths in \p folders to absolute paths using \p absolutePath parent folder as base.
+ * @param[in] folders list of paths to convert
+ * @param[in] absolutePath filepath which parent folder should be used as base for absolute path conversion
+ * @return the list of converted absolute paths or input folder if absolutePath is empty
+ */
+std::vector<std::string> toAbsoluteFolders(const std::vector<std::string>& folders, const std::string& absolutePath)
 {
-  fs::path sfmFolder = fs::path(_absolutePath).parent_path();
-  std::vector<std::string> absolutePaths(_featuresFolders.size());
+  // if absolute path is not set, return input folders
+  if(absolutePath.empty())
+    return folders;
+  // else, convert relative paths to absolute paths
+  std::vector<std::string> absolutePaths(folders.size());
   for(int i = 0; i < absolutePaths.size(); ++i)
-    absolutePaths.at(i) = fs::canonical(fs::path(_featuresFolders.at(i)), sfmFolder).string();
+    absolutePaths.at(i) = fs::canonical(folders.at(i), fs::path(absolutePath).parent_path()).string();
   return absolutePaths;
 }
 
+/** 
+ * @brief Add paths contained in \p folders to \p dst as relative paths to \p absolutePath.
+ *        Paths already present in \p dst are omitted.
+ * @param[in] dst list in which paths should be added
+ * @param[in] folders paths to add to \p dst as relative folders
+ * @param[in] absolutePath filepath which parent folder should be used as base for relative path conversions
+ */
+void addAsRelativeFolders(std::vector<std::string>& dst, const std::vector<std::string>& folders, const std::string& absolutePath)
+{
+  for(auto folderPath: folders) 
+  {
+    // if absolutePath is set, convert to relative path
+    if(fs::path(folderPath).is_absolute() && !absolutePath.empty())
+    {
+      folderPath = fs::relative(folderPath, fs::path(absolutePath).parent_path()).string();
+    }
+    // add path only if not already in dst
+    if(std::find(dst.begin(), dst.end(), folderPath) == dst.end())
+    {
+      dst.emplace_back(folderPath);
+    }
+  }
+}
+
+std::vector<std::string> SfMData::getFeaturesFolders() const
+{
+  return toAbsoluteFolders(_featuresFolders, _absolutePath);
+}
+
 std::vector<std::string> SfMData::getMatchesFolders() const
 {
-  fs::path sfmFolder = fs::path(_absolutePath).parent_path();
-  std::vector<std::string> absolutePaths(_matchesFolders.size());
-  for(int i = 0; i < absolutePaths.size(); ++i)
-    absolutePaths.at(i) = fs::canonical(fs::path(_matchesFolders.at(i)), sfmFolder).string();
-  return absolutePaths;
+  return toAbsoluteFolders(_matchesFolders, _absolutePath);
+}
+
+void SfMData::addFeaturesFolders(const std::vector<std::string>& folders)
+{
+  addAsRelativeFolders(_featuresFolders, folders, _absolutePath);
+}
+
+void SfMData::addMatchesFolders(const std::vector<std::string>& folders)
+{
+  addAsRelativeFolders(_matchesFolders, folders, _absolutePath);
+}
+
+void SfMData::setAbsolutePath(const std::string& path)
+{
+  // get absolute path to features/matches folders
+  const std::vector<std::string> featuresFolders = getFeaturesFolders();
+  const std::vector<std::string> matchesFolders = getMatchesFolders();
+  // change internal absolute path
+  _absolutePath = path;
+  // re-set features/matches folders
+  // they will be converted back to relative paths based on updated _absolutePath
+  setFeaturesFolders(featuresFolders);
+  setMatchesFolders(matchesFolders);
 }
 
 std::set<IndexT> SfMData::getValidViews() const
@@ -169,10 +226,10 @@ void SfMData::combine(const SfMData& sfmData)
     throw std::runtime_error("Can't combine two SfMData with rigs");
 
   // feature folder
-  _featuresFolders.insert(_featuresFolders.end(), sfmData._featuresFolders.begin(), sfmData._featuresFolders.end());
+  addFeaturesFolders(sfmData.getFeaturesFolders());
 
   // matching folder
-  _matchesFolders.insert(_matchesFolders.end(), sfmData._matchesFolders.begin(), sfmData._matchesFolders.end());
+  addMatchesFolders(sfmData.getMatchesFolders());
 
   // views
   views.insert(sfmData.views.begin(), sfmData.views.end());

src/aliceVision/sfmData/SfMData.hpp

@@ -298,49 +298,74 @@ class SfMData
   }
 
   /**
-   * @brief Add the given features Folder
-   * @param[in] featuresFolder The given features folder
+   * @brief Add the given \p folder to features folders.
+   * @note If SfmData's absolutePath has been set, 
+   *       an absolute path will be converted to a relative one.
+   * @param[in] folder path to a folder containing features
    */
-  void addFeaturesFolder(const std::string& featuresFolder)
+  inline void addFeaturesFolder(const std::string& folder)
   {
-    _featuresFolders.emplace_back(featuresFolder);
+    addFeaturesFolders({folder});
   }
 
   /**
-   * @brief A the given matches Folder
-   * @param[in] matchesFolder The given mathes folder
+   * @brief Add the given \p folders to features folders.
+   * @note If SfmData's absolutePath has been set, 
+   *       absolute paths will be converted to relative ones.
+   * @param[in] folders paths to folders containing features
    */
-  void addMatchesFolder(const std::string& matchesFolder)
+  void addFeaturesFolders(const std::vector<std::string>& folders);
+
+  /**
+   * @brief Add the given \p folder to matches folders.
+   * @note If SfmData's absolutePath has been set, 
+   *       an absolute path will be converted to a relative one.
+   * @param[in] folder path to a folder containing matches
+   */
+  inline void addMatchesFolder(const std::string& folder)
   {
-    _matchesFolders.emplace_back(matchesFolder);
+    addMatchesFolders({folder});
   }
 
   /**
-   * @brief Set the given features folders
-   * @param[in] featuresFolders The given features folders
+   * @brief Add the given \p folders to matches folders.
+   * @note If SfmData's absolutePath has been set, 
+   *       absolute paths will be converted to relative ones.
+   * @param[in] folders paths to folders containing matches
+   */
+  void addMatchesFolders(const std::vector<std::string>& folders);
+
+  /**
+   * @brief Replace the current features folders by the given ones.
+   * @note If SfmData's absolutePath has been set, 
+   *       absolute paths will be converted to relative ones.
+   * @param[in] folders paths to folders containing features
    */
-  void setFeaturesFolders(const std::vector<std::string>& featuresFolders)
+  inline void setFeaturesFolders(const std::vector<std::string>& folders)
   {
-    _featuresFolders = featuresFolders;
+    _featuresFolders.clear();
+    addFeaturesFolders(folders);
   }
 
   /**
-   * @brief Set the given mathes folders
-   * @param[in] matchesFolders The given mathes folders
+   * @brief Replace the current matches folders by the given ones.
+   * @note If SfmData's absolutePath has been set, 
+   *       absolute paths will be converted to relative ones.
+   * @param[in] folders paths to folders containing matches
    */
-  void setMatchesFolders(const std::vector<std::string>& matchesFolders)
+  inline void setMatchesFolders(const std::vector<std::string>& folders)
   {
-    _matchesFolders = matchesFolders;
+    _matchesFolders.clear();
+    addMatchesFolders(folders);
   }
 
   /**
-   * @brief Set the SfMData file folder absolute path
+   * @brief Set the SfMData file absolute path.
+   * @note Internal relative features/matches folders will be remapped 
+   *       to be relative to the new absolute \p path.
    * @param[in] path The absolute path to the SfMData file folder
    */
-  void setAbsolutePath(const std::string& path)
-  {
-    _absolutePath = path;
-  }
+  void setAbsolutePath(const std::string& path);
 
   /**
    * @brief Set the given pose for the given view

src/aliceVision/sfmData/sfmData_test.cpp

@@ -0,0 +1,52 @@
+
+#include <boost/filesystem.hpp>
+#include <aliceVision/sfmData/SfMData.hpp>
+
+#define BOOST_TEST_MODULE sfmData
+#include <boost/test/included/unit_test.hpp>
+
+using namespace aliceVision;
+namespace fs = boost::filesystem;
+
+BOOST_AUTO_TEST_CASE(SfMData_InternalFolders)
+{
+  const std::string filename = "InternalFolders.sfm";
+  sfmData::SfMData sfmData;
+
+  // add relative features/matches folders with duplicates
+  std::string refFolder("..");
+  sfmData.addFeaturesFolders({refFolder, refFolder});
+  sfmData.addMatchesFolders({refFolder, refFolder});
+  auto featuresFolders = sfmData.getFeaturesFolders();
+  auto matchesFolders = sfmData.getMatchesFolders();
+  // ensure duplicates were removed
+  BOOST_CHECK_EQUAL(featuresFolders.size(), 1);
+  BOOST_CHECK_EQUAL(matchesFolders.size(), 1);
+  // sfmData has no absolute path set, folders are still in relative form
+  BOOST_CHECK_EQUAL(featuresFolders[0], refFolder);
+  BOOST_CHECK_EQUAL(matchesFolders[0], refFolder);
+
+  // set absolutePath to current filename
+  sfmData.setAbsolutePath(fs::absolute(filename).string());
+  featuresFolders = sfmData.getFeaturesFolders();
+  matchesFolders = sfmData.getMatchesFolders();
+  // internal folders were kept...
+  BOOST_CHECK_EQUAL(featuresFolders.size(), 1);
+  BOOST_CHECK_EQUAL(matchesFolders.size(), 1);
+  // ... and are now absolute paths
+  BOOST_CHECK(fs::path(featuresFolders[0]).is_absolute());
+  BOOST_CHECK(fs::equivalent(featuresFolders[0], refFolder));
+  BOOST_CHECK(fs::path(matchesFolders[0]).is_absolute());
+  BOOST_CHECK(fs::equivalent(matchesFolders[0], refFolder));
+
+  // update sfm absolute path to be in parent/parent folder
+  fs::path otherFolder = fs::path("../..");
+  std::string updatedFilename = ( otherFolder / filename).string();
+  sfmData.setAbsolutePath(updatedFilename);
+  // internal folders still reference the same folder as before
+  BOOST_CHECK(fs::equivalent(featuresFolders[0], refFolder));
+  BOOST_CHECK(fs::equivalent(matchesFolders[0], refFolder));
+  BOOST_CHECK_EQUAL(sfmData.getRelativeFeaturesFolders()[0], fs::relative(refFolder, otherFolder));
+  BOOST_CHECK_EQUAL(sfmData.getRelativeMatchesFolders()[0], fs::relative(refFolder, otherFolder));
+}
+

src/software/pipeline/main_globalSfM.cpp

@@ -206,12 +206,8 @@ int main(int argc, char **argv)
 
   // set featuresFolders and matchesFolders relative paths
   {
-    for(const std::string& featuresFolder : featuresFolders)
-       sfmEngine.getSfMData().addFeaturesFolder(fs::relative(fs::path(featuresFolder), outDirectory).string());
-
-    for(const std::string& matchesFolder : matchesFolders)
-       sfmEngine.getSfMData().addMatchesFolder(fs::relative(fs::path(matchesFolder), outDirectory).string());
-
+    sfmEngine.getSfMData().addFeaturesFolders(featuresFolders);
+    sfmEngine.getSfMData().addMatchesFolders(matchesFolders);
     sfmEngine.getSfMData().setAbsolutePath(outDirectory);
   }
 

src/software/pipeline/main_incrementalSfM.cpp

@@ -303,14 +303,8 @@ int main(int argc, char **argv)
 
   // set featuresFolders and matchesFolders relative paths
   {
-    const fs::path sfmFolder = fs::path(outputSfM).remove_filename();
-
-    for(const std::string& featuresFolder : featuresFolders)
-       sfmEngine.getSfMData().addFeaturesFolder(fs::relative(fs::path(featuresFolder), sfmFolder).string());
-
-    for(const std::string& matchesFolder : matchesFolders)
-       sfmEngine.getSfMData().addMatchesFolder(fs::relative(fs::path(matchesFolder), sfmFolder).string());
-
+    sfmEngine.getSfMData().addFeaturesFolders(featuresFolders);
+    sfmEngine.getSfMData().addMatchesFolders(matchesFolders);
     sfmEngine.getSfMData().setAbsolutePath(outputSfM);
   }