Merge pull request #1697 from alicevision/fix/featuresPath

[sfmData] fix relative to absolute path conversion for features/matches folders

pyTests/sfmData/test_sfmdata.py

@@ -316,21 +316,22 @@ def test_sfmdata_get_set_folders():
 
     # Set absolute path
     filename = "internal_folders.sfm"
-    abs_filename = os.path.abspath(os.path.dirname(__file__)) + "/" + filename
+    projectFolder = os.path.abspath(os.path.dirname(__file__))
+    abs_filename = os.path.join(projectFolder, filename)
     data.setAbsolutePath(abs_filename)
 
     # Check that the folders were kept and are now absolute paths
     assert len(data.getFeaturesFolders()) == 1, \
         "The previously added features folder should have remained in the list"
     assert os.path.isabs(data.getFeaturesFolders()[0]), \
         "The absolute path has been set: the features folder's path should be absolute as well"
-    assert os.path.relpath(data.getFeaturesFolders()[0]) == relative_folder, \
+    assert os.path.relpath(data.getFeaturesFolders()[0], projectFolder) == relative_folder, \
         "The absolute path for the features folder should correspond to the provided relative one"
     assert len(data.getMatchesFolders()) == 1, \
         "The previously added matches folder should have remained in the list"
     assert os.path.isabs(data.getMatchesFolders()[0]), \
         "The absolute path has been set: the matches folder's path should be absolute as well"
-    assert os.path.relpath(data.getMatchesFolders()[0]) == relative_folder, \
+    assert os.path.relpath(data.getMatchesFolders()[0], projectFolder) == relative_folder, \
         "The absolute path for the matches folder should correspond to the provided relative one"
 
     # Check that relative folders are still valid
@@ -348,10 +349,10 @@ def test_sfmdata_get_set_folders():
     assert len(data.getMatchesFolders()) == 2, \
         "A second matches folder should have been addded to the list"
 
-    assert os.path.relpath(data.getFeaturesFolders()[0]) == relative_folder
-    assert os.path.relpath(data.getFeaturesFolders()[1]) == other_folder
-    assert os.path.relpath(data.getMatchesFolders()[0]) == relative_folder
-    assert os.path.relpath(data.getMatchesFolders()[1]) == other_folder
+    assert os.path.relpath(data.getFeaturesFolders()[0], projectFolder) == relative_folder
+    assert os.path.relpath(data.getFeaturesFolders()[1], projectFolder) == other_folder
+    assert os.path.relpath(data.getMatchesFolders()[0], projectFolder) == relative_folder
+    assert os.path.relpath(data.getMatchesFolders()[1], projectFolder) == other_folder
 
     # Reset features and matches folders and add new ones
     new_folder1 = "../new"
@@ -365,5 +366,5 @@ def test_sfmdata_get_set_folders():
     assert len(data.getFeaturesFolders()) == 3
     assert len(data.getMatchesFolders()) == 3
     for i in range(len(data.getFeaturesFolders())):
-        assert os.path.relpath(data.getFeaturesFolders()[i]) == new_folders[i]
-        assert os.path.relpath(data.getMatchesFolders()[i]) == new_folders[i]
+        assert os.path.relpath(data.getFeaturesFolders()[i], projectFolder) == new_folders[i]
+        assert os.path.relpath(data.getMatchesFolders()[i], projectFolder) == new_folders[i]

src/aliceVision/sfm/LocalBundleAdjustmentGraph.cpp

@@ -108,9 +108,10 @@ void LocalBundleAdjustmentGraph::saveIntrinsicsToHistory(const sfmData::SfMData&
 
 void LocalBundleAdjustmentGraph::exportIntrinsicsHistory(const std::string& folder, const std::string& filename)
 {
-    ALICEVISION_LOG_DEBUG("Exporting intrinsics history...");
+    const std::string filepath = (fs::path(folder) / filename).string();
+    ALICEVISION_LOG_DEBUG("Exporting intrinsics history: " << filepath);
     std::ofstream os;
-    os.open((fs::path(folder) / filename).string(), std::ios::app);
+    os.open(filepath, std::ios::app);
     os.seekp(0, std::ios::end);  // put the cursor at the end
 
     for (const auto& intrinsicHistoryPair : _intrinsicsHistory)

src/aliceVision/sfm/pipeline/regionsIO.cpp

@@ -10,6 +10,8 @@
 #include <aliceVision/system/ProgressDisplay.hpp>
 #include <aliceVision/utils/filesIO.hpp>
 
+#include <boost/algorithm/string/join.hpp>
+
 #include <atomic>
 #include <cassert>
 #include <filesystem>
@@ -44,7 +46,10 @@ std::unique_ptr<feature::Regions> loadRegions(const std::vector<std::string>& fo
     }
 
     if (featFilename.empty() || descFilename.empty())
-        throw std::runtime_error("Can't find view " + basename + " region files");
+    {
+        const std::string foldersStr = boost::algorithm::join(folders, ", ");
+        throw std::runtime_error("Can't find view " + basename + " region files in folders " + foldersStr);
+    }
 
     ALICEVISION_LOG_TRACE("Features filename: " << featFilename);
     ALICEVISION_LOG_TRACE("Descriptors filename: " << descFilename);
@@ -65,7 +70,7 @@ std::unique_ptr<feature::Regions> loadRegions(const std::vector<std::string>& fo
         ss << "\t  " << e.what() << "\n";
         ALICEVISION_LOG_ERROR(ss.str());
 
-        throw std::runtime_error(e.what());
+        throw;
     }
 
     ALICEVISION_LOG_TRACE("Region count: " << regionsPtr->RegionCount());
@@ -99,7 +104,11 @@ std::unique_ptr<feature::Regions> loadFeatures(const std::vector<std::string>& f
     }
 
     if (featFilename.empty())
-        throw std::runtime_error("Can't find view " + basename + " features file");
+    {
+        const std::vector<std::string> folders(foldersSet.begin(), foldersSet.end());
+        const std::string foldersStr = boost::algorithm::join(folders, ", ");
+        throw std::runtime_error("Can't find view " + basename + " features files in folders " + foldersStr);
+    }
 
     ALICEVISION_LOG_DEBUG("Features filename: " << featFilename);
 
@@ -118,7 +127,7 @@ std::unique_ptr<feature::Regions> loadFeatures(const std::vector<std::string>& f
         ss << "\t  " << e.what() << "\n";
         ALICEVISION_LOG_ERROR(ss.str());
 
-        throw std::runtime_error(e.what());
+        throw;
     }
 
     ALICEVISION_LOG_TRACE("Feature count: " << regionsPtr->RegionCount());
@@ -170,6 +179,13 @@ bool loadFeaturesPerDescPerView(std::vector<std::vector<std::unique_ptr<feature:
             }
             catch (const std::exception& e)
             {
+                const feature::EImageDescriberType d = imageDescribers.at(descIdx)->getDescriberType();
+
+#pragma omp critical
+                {
+                    ALICEVISION_LOG_ERROR("Cannot load features for View " << viewIds.at(viewIdx) << " for describer type " << feature::EImageDescriberType_enumToString(d) << ".");
+                    ALICEVISION_LOG_ERROR(e.what());
+                }
                 loadingSuccess = false;
             }
         }
@@ -214,9 +230,13 @@ bool loadRegionsPerView(feature::RegionsPerView& regionsPerView,
                     {
                         regionsPtr = loadRegions(featuresFolders, iter->second.get()->getViewId(), *(imageDescribers.at(i)));
                     }
-                    catch (const std::exception&)
+                    catch (const std::exception& e)
                     {
                         invalid = true;
+#pragma omp critical
+                        {
+                            ALICEVISION_LOG_ERROR(e.what());
+                        }
                         continue;
                     }
 #pragma omp critical
@@ -243,7 +263,7 @@ bool loadFeaturesPerView(feature::FeaturesPerView& featuresPerView,
     for (auto it = folders.begin(); it != folders.end(); ++it)
         ALICEVISION_LOG_DEBUG("\t - " << *it);
 
-    ALICEVISION_LOG_DEBUG("List of feature folders to load:");
+    ALICEVISION_LOG_DEBUG("List of feature folders (from sfmData) to load:");
     for (auto it = featuresFolders.begin(); it != featuresFolders.end(); ++it)
         ALICEVISION_LOG_DEBUG("\t - " << *it);
 
@@ -270,9 +290,13 @@ bool loadFeaturesPerView(feature::FeaturesPerView& featuresPerView,
                 {
                     regionsPtr = loadFeatures(featuresFolders, iter->second.get()->getViewId(), *imageDescribers.at(i));
                 }
-                catch (const std::exception&)
+                catch (const std::exception& e)
                 {
                     invalid = true;
+#pragma omp critical
+                    {
+                        ALICEVISION_LOG_ERROR(e.what());
+                    }
                     continue;
                 }
 #pragma omp critical

src/aliceVision/sfmData/SfMData.cpp

@@ -148,21 +148,26 @@ std::vector<std::string> toAbsoluteFolders(const std::vector<std::string>& folde
     // If absolute path is not set, return input folders
     if (absolutePath.empty())
         return folders;
+    // project folder from project filepath
+    const fs::path projectFolder = fs::path(absolutePath).parent_path();
     // Else, convert relative paths to absolute paths
     std::vector<std::string> absolutePaths;
     absolutePaths.reserve(folders.size());
     for (const auto& folder : folders)
     {
-        const fs::path f = fs::absolute(folder);
-        if (utils::exists(f))
+        fs::path f(folder);
+        if(f.is_relative())
         {
-            // fs::canonical can only be used if the path exists
-            absolutePaths.push_back(fs::canonical(f).string());
+            // convert to absolute path
+            f = projectFolder / folder;
         }
-        else
+        if (fs::exists(f))
         {
-            absolutePaths.push_back(f.string());
+            // simplify the path to avoid things like "../.."
+            // fs::canonical can only be used if the path exists
+            f = fs::canonical(f);
         }
+        absolutePaths.push_back(f.string());
     }
     return absolutePaths;
 }