[sfm] Function to split a match file in component openMVG#1135

- Extend the unit test with a corner cases (empty input)
- Simplify the main_plitMatchFileIntoMatchFiles cpp file

Author: pmoulon

src/openMVG/sfm/sfm_data_graph_utils.cpp

@@ -20,35 +20,39 @@
 namespace openMVG {
 namespace sfm {
 
-/// Filter the toFilter iterable sequence into a new sequence without change the source sequence
-/// (keep only the element that share a common index with the provided Ids index list).
-/// See the reference: sfm_filters.hpp
-static void KeepOnlyReferencedElement(
+static void KeepOnlyReferencedElement
+(
   const std::set<IndexT> & set_remainingIds,
-  const matching::PairWiseMatches & map_matches_in,
-  matching::PairWiseMatches & map_matches_out)
+  const matching::PairWiseMatches & matches_in,
+  matching::PairWiseMatches & matches_out
+)
 {
-  matching::PairWiseMatches map_matches_E_infered;
-  for (const auto & iter_pairwise_matches : map_matches_in)
+  matching::PairWiseMatches kept_matches;
+  for (const auto & iter_pairwise_matches : matches_in)
   {
     if (set_remainingIds.count(iter_pairwise_matches.first.first) &&
-      set_remainingIds.count(iter_pairwise_matches.first.second))
+        set_remainingIds.count(iter_pairwise_matches.first.second))
     {
-      map_matches_E_infered.insert(iter_pairwise_matches);
+      kept_matches.insert(iter_pairwise_matches);
     }
   }
-  map_matches_out.swap(map_matches_E_infered);
+  matches_out.swap(kept_matches);
 }
 
-bool PairsToConnectedComponents(const Pair_Set & pairs, bool is_biedge, 
-  int min_nodes, std::map<IndexT, std::set<IndexT>>& subgraphs_ids)
+bool PairsToConnectedComponents
+(
+  const Pair_Set & pairs,
+  bool is_biedge,
+  int min_nodes,
+  std::map<IndexT, std::set<IndexT>>& subgraphs_ids
+)
 {
   subgraphs_ids.clear();
 
   using Graph = graph::indexedGraph::GraphT;
   graph::indexedGraph putativeGraph(pairs);
 
-  // For global SFM, firstly remove the not bi-edge element 
+  // For global SFM, firstly remove the not bi-edge element
   if (is_biedge)
   {
     using EdgeMapAlias = Graph::EdgeMap<bool>;
@@ -72,7 +76,7 @@ bool PairsToConnectedComponents(const Pair_Set & pairs, bool is_biedge,
   const int connectedComponentCount = lemon::countConnectedComponents(putativeGraph.g);
   if (connectedComponentCount >= 1)
   {
-    const std::map<IndexT, std::set<Graph::Node> > map_subgraphs = graph::exportGraphToMapSubgraphs<Graph, IndexT>(putativeGraph.g);
+    const auto map_subgraphs = graph::exportGraphToMapSubgraphs<Graph, IndexT>(putativeGraph.g);
     for (const auto & iter_map_subgraphs : map_subgraphs)
     {
       if (iter_map_subgraphs.second.size() > min_nodes)
@@ -88,27 +92,34 @@ bool PairsToConnectedComponents(const Pair_Set & pairs, bool is_biedge,
       }
     }
   }
-  return true;
+  return !subgraphs_ids.empty();
 }
 
-bool SplitMatchesIntoSubgraphMatches(const Pair_Set & pairs, const matching::PairWiseMatches & matches, 
-  bool is_biedge, int min_nodes, std::vector<matching::PairWiseMatches> & subgraphs_matches)
+bool SplitMatchesIntoSubgraphMatches
+(
+  const Pair_Set & pairs,
+  const matching::PairWiseMatches & matches,
+  bool is_biedge,
+  int min_nodes,
+  std::vector<matching::PairWiseMatches> & subgraphs_matches
+)
 {
   if (pairs.size() == 0 || matches.size() == 0)
   {
     return false;
   }
   subgraphs_matches.clear();
   std::map<IndexT, std::set<IndexT>> subgraphs_ids;
-  PairsToConnectedComponents(pairs, is_biedge, min_nodes, subgraphs_ids);
-
-  for (const auto & iter_subgraphs_ids : subgraphs_ids)
+  if (PairsToConnectedComponents(pairs, is_biedge, min_nodes, subgraphs_ids))
   {
-    matching::PairWiseMatches subgraph_map_matches;
-    KeepOnlyReferencedElement(iter_subgraphs_ids.second, matches, subgraph_map_matches);
-    subgraphs_matches.emplace_back(subgraph_map_matches);
+    for (const auto & iter_subgraphs_ids : subgraphs_ids)
+    {
+      matching::PairWiseMatches component_matches;
+      KeepOnlyReferencedElement(iter_subgraphs_ids.second, matches, component_matches);
+      subgraphs_matches.emplace_back(component_matches);
+    }
   }
-  return true;
+  return !subgraphs_matches.empty();
 }
 
 } // namespace sfm

src/openMVG/sfm/sfm_data_graph_utils.hpp

@@ -8,45 +8,57 @@
 
 #ifndef OPENMVG_SFM_DATA_GRAPH_UTILS_HPP
 #define OPENMVG_SFM_DATA_GRAPH_UTILS_HPP
-#include <string>
+
 #include "openMVG/types.hpp"
 #include "openMVG/matching/indMatch.hpp"
+#include <string>
 
 namespace openMVG {
 namespace sfm {
-///  @brief Split match pairs into connected match pairs 
+///  @brief Split match pairs into connected match pairs
 ///  @param[in]  pairs   The pair sets of the images.
 ///  @param[in]  is_biedge  Set true for global SFM, false for sequential SFM.
-///  @param[in]  min_nodes  The minimum nodes of graph in the output match_file.Note: the value should be larger than 3.
-///  @param[out] subgraps_ids The pairs id of subgraphs splited from the whole graph.
+///  @param[in]  min_nodes  The minimum nodes of graph in the output match_file. Note: the value should be larger than 3.
+///  @param[out] subgraps_ids The pairs id of subgraphs split from the whole graph.
 ///
-///  @return True if the area can be computed
-bool PairsToConnectedComponents(const Pair_Set & pairs, bool is_biedge,
-  int min_nodes, std::map<IndexT, std::set<IndexT> > & subgraphs_ids);
+///  @return True if some components have been kept
+bool PairsToConnectedComponents
+(
+  const Pair_Set & pairs,
+  bool is_biedge,
+  int min_nodes,
+  std::map<IndexT, std::set<IndexT>> & subgraphs_ids
+);
 
 ///  @brief Split match_file into match_files
-///  When handling a scene which may contains many disconnected graphs, 
+///  When handling a scene which may contains many disconnected graphs,
 ///  for the moment OpenMVG consider only the largest connected component for SfM.
-///  Especially, when the GPS data of the images known, the disconnected graphs in a scene should be 
+///  Especially, when the GPS data of the images known, the disconnected graphs in a scene should be
 ///  merged since they all share the same coordinate system.
 ///  Steps handling the above scenario.
-///  Step 1 : Use this function to compute the various connected component and export 
+///  Step 1 : Use this function to compute the various connected component and export
 ///  the matches of the each connected component in a different match file.
 ///  Step 2 : Run the SfM pipeline of your choice on the produced matches file.
-///  Step 3 : Merge all the sfm_data into a single sfm_data and 
+///  Step 3 : Merge all the sfm_data into a single sfm_data and
 ///  triangulate the initial match file, when the GPS data of the images known.
-///         
+///
 ///
 ///  @param[in]  pairs   The pair sets of the images.
 ///  @param[in]  matches The pairwise matches of the images corresponding to pairs.
 ///  @param[in]  is_biedge  Set true for global SFM, false for sequential SFM.
-///  @param[in]  min_nodes  The minimum nodes of graph in the output match_file.Note: the value should be larger than 3.
-///  @param[out] subgraphs_matches The matches of subgraphs splited from the whole graph.
+///  @param[in]  min_nodes  The minimum nodes of graph in the output match_file. Note: the value should be larger than 3.
+///  @param[out] subgraphs_matches The matches of subgraphs split from the whole graph.
 ///
-///  @return True if the area can be computed
+///  @return True if subgraphs_matches is not empty
 ///
-bool SplitMatchesIntoSubgraphMatches(const Pair_Set & pairs, const matching::PairWiseMatches & matches,
-  bool is_biedge, int min_nodes, std::vector<matching::PairWiseMatches> & subgraphs_matches);
+bool SplitMatchesIntoSubgraphMatches
+(
+  const Pair_Set & pairs,
+  const matching::PairWiseMatches & matches,
+  bool is_biedge,
+  int min_nodes,
+  std::vector<matching::PairWiseMatches> & subgraphs_matches
+);
 
 } // namespace sfm
 } // namespace openMVG

src/openMVG/sfm/sfm_data_graph_utils_test.cpp

@@ -17,10 +17,15 @@ using namespace openMVG;
 using namespace openMVG::matching;
 using namespace openMVG::sfm;
 
+TEST(SFM_DATA_GRAPH, PairsToConnectedComponents_EmptyInput)
+{
+  std::map<IndexT, std::set<IndexT>> subgraphs_ids;
+  EXPECT_FALSE(PairsToConnectedComponents({}, true, 1, subgraphs_ids));
+}
 
 TEST(SFM_DATA_GRAPH, PairsToConnectedComponents)
 {
-  Pair_Set pair =
+  const Pair_Set pairs =
   {
     // the first connected graph follows the biEdge condition: 0 -> 1 -> 2 ->3 ->4 -> 0
     {0, 1}, {0, 2}, {0, 3}, {1, 2}, {1, 3}, {2, 3},
@@ -33,39 +38,36 @@ TEST(SFM_DATA_GRAPH, PairsToConnectedComponents)
   std::map<IndexT, std::set<IndexT>> subgraphs_ids;
 
   // test for GlobalSFM with the biEdge condition
-  const bool flag1 = PairsToConnectedComponents(pair, true, 3, subgraphs_ids);
-  EXPECT_TRUE(flag1);
+  EXPECT_TRUE(PairsToConnectedComponents(pairs, true, 3, subgraphs_ids));
   EXPECT_EQ(1, subgraphs_ids.size());
   if (subgraphs_ids.size() == 1)
   {
-    const auto & iter_begin = subgraphs_ids.begin();
-    const std::set<IndexT> & pairs0 = iter_begin->second;
-    EXPECT_EQ(4, pairs0.size());
-    if (pairs0.size() == 4)
+    const auto & iter_begin = subgraphs_ids.cbegin();
+    const std::set<IndexT> & kept_idx = iter_begin->second;
+    EXPECT_EQ(4, kept_idx.size());
     {
-      EXPECT_TRUE(pairs0.find(0) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(1) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(2) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(3) != pairs0.end());
+      EXPECT_TRUE(kept_idx.find(0) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(1) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(2) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(3) != kept_idx.cend());
     }
   }
 
   // test for IncrementalSFM with no biEdge condition
-  const bool flag2 = PairsToConnectedComponents(pair, false, 3, subgraphs_ids);
-  EXPECT_TRUE(flag2);
+  // The smallest graph must disappear since it is too small
+  EXPECT_TRUE(PairsToConnectedComponents(pairs, false, 3, subgraphs_ids));
   EXPECT_EQ(2, subgraphs_ids.size());
   if (subgraphs_ids.size() == 2)
   {
-    auto iter_first = subgraphs_ids.begin();
-    const std::set<IndexT> & pairs0 = iter_first->second;
-    EXPECT_EQ(5, pairs0.size());
-    if (pairs0.size() == 5)
+    const auto iter_first = subgraphs_ids.cbegin();
+    const std::set<IndexT> & kept_idx = iter_first->second;
+    EXPECT_EQ(5, kept_idx.size());
     {
-      EXPECT_TRUE(pairs0.find(4) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(5) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(6) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(7) != pairs0.end());
-      EXPECT_TRUE(pairs0.find(8) != pairs0.end());
+      EXPECT_TRUE(kept_idx.find(4) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(5) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(6) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(7) != kept_idx.cend());
+      EXPECT_TRUE(kept_idx.find(8) != kept_idx.cend());
     }
 
     auto iter_second = subgraphs_ids.begin();

src/software/SfM/main_SplitMatchFileIntoMatchFiles.cpp

@@ -14,17 +14,16 @@
 #include "third_party/stlplus3/filesystemSimplified/file_system.hpp"
 
 #include <cstdlib>
+#include <fstream>
 #include <memory>
 #include <string>
-#include <fstream>
 
 using namespace openMVG;
 using namespace openMVG::sfm;
 
 int main(int argc, char **argv)
 {
   using namespace std;
-  std::cout << std::endl;
 
   CmdLine cmd;
 
@@ -57,23 +56,13 @@ int main(int argc, char **argv)
     return EXIT_FAILURE;
   }
 
-  if (!stlplus::file_exists(sfm_data_filename))
-  {
-    return EXIT_FAILURE;
-  }
-
-  if (!stlplus::file_exists(match_filename))
-  {
-    return EXIT_FAILURE;
-  }
-
   const std::string &match_component_dir = stlplus::folder_part(match_component_filename);
   if (!stlplus::folder_exists(match_component_dir))
   {
     const bool folder_create_flag = stlplus::folder_create(match_component_dir);
     if (!folder_create_flag)
     {
-      std::cerr << "Cannot create the output directory" << std::endl;
+      std::cerr << "Cannot create the output directory: " << match_component_dir << std::endl;
       return EXIT_FAILURE;
     }
   }
@@ -99,9 +88,13 @@ int main(int argc, char **argv)
   std::vector<matching::PairWiseMatches> subgraphs_matches;
 
   // Split match_filename by connected components;
-  const bool success_flag = SplitMatchesIntoSubgraphMatches(matches_provider->getPairs(), matches_provider->pairWise_matches_,
-    is_biedge, min_nodes, subgraphs_matches);
-  if (success_flag)
+  const bool success_flag =
+    SplitMatchesIntoSubgraphMatches(matches_provider->getPairs(),
+                                    matches_provider->pairWise_matches_,
+                                    is_biedge,
+                                    min_nodes,
+                                    subgraphs_matches);
+  if (!success_flag)
   {
     std::cerr << std::endl
       << "Failed to split matches file into subgraph matches." << std::endl;
@@ -118,12 +111,13 @@ int main(int argc, char **argv)
   for (const auto & subgraph : subgraphs_matches)
   {
     std::stringstream strstream_subgraph_match_filename;
-    strstream_subgraph_match_filename << file_basename << "_" << index << "_" << subgraph.size() << "." << match_file_extension;
+    strstream_subgraph_match_filename << file_basename
+      << "_" << index
+      << "_" << subgraph.size() << "." << match_file_extension;
     const std::string &subgraph_match_filename = strstream_subgraph_match_filename.str();
     const std::string &subgraph_match_file = stlplus::create_filespec(output_folder, subgraph_match_filename);
 
-    const bool success_flag = matching::Save(subgraph, subgraph_match_file);
-    if (success_flag)
+    if (matching::Save(subgraph, subgraph_match_file))
     {
       set_filenames.insert(subgraph_match_filename);
     }