Merge pull request #6872 from MaelRL/PMP-Fix_keep_LCC-GF

Fix dry-run of keep_largest_CCs dumping the wrong faces

Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/connected_components.h

@@ -418,11 +418,12 @@ std::size_t keep_largest_connected_components(PolygonMesh& pmesh,
 
   // vector_property_map
   boost::vector_property_map<std::size_t, FaceIndexMap> face_cc(static_cast<unsigned>(num_faces(pmesh)), fimap);
-  std::size_t num = connected_components(pmesh, face_cc, np);
 
   // Even if we do not want to keep anything we need to first
   // calculate the number of existing connected_components to get the
   // correct return value.
+  const std::size_t num = connected_components(pmesh, face_cc, np);
+
   if(nb_components_to_keep == 0)
   {
     CGAL::clear(pmesh);
@@ -445,12 +446,12 @@ std::size_t keep_largest_connected_components(PolygonMesh& pmesh,
 
   if(dry_run)
   {
-    std::vector<bool> is_to_be_removed(num, false);
+    std::vector<bool> is_to_be_kept(num, false);
     for(std::size_t i=0; i<nb_components_to_keep; ++i)
-      is_to_be_removed[component_size[i].first] = true;
+      is_to_be_kept[component_size[i].first] = true;
 
     for(face_descriptor f : faces(pmesh))
-      if(is_to_be_removed[face_cc[f]])
+      if(!is_to_be_kept[face_cc[f]])
         *out++ = f;
   }
   else

Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/repair.h

@@ -218,7 +218,7 @@ std::size_t remove_connected_components_of_negligible_size(TriangleMesh& tmesh,
     area_threshold = CGAL::square(threshold_value);
 
   if(is_default_volume_threshold)
-    volume_threshold = CGAL::square(threshold_value);
+    volume_threshold = CGAL::square(threshold_value) * threshold_value;
 
   const bool use_areas = (is_default_area_threshold || area_threshold > 0);
   const bool use_volumes = (is_default_volume_threshold || volume_threshold > 0);

Polygon_mesh_processing/test/Polygon_mesh_processing/connected_component_polyhedron.cpp

@@ -40,28 +40,53 @@ void mesh_with_id(const std::string argv1, const bool save_output)
     boost::property_map<Mesh_with_id, CGAL::face_index_t>::type>
       fccmap(static_cast<unsigned>(num_faces(sm)), get(CGAL::face_index,sm));
 
-  std::size_t num = PMP::connected_components(sm, fccmap);
+  const std::size_t num = PMP::connected_components(sm, fccmap);
+
   if(argv1 == CGAL::data_file_path("meshes/blobby_3cc.off"))
+  {
     assert(num == 3);
+  }
 
   std::cerr << "The graph has " << num << " connected components (face connectivity)" << std::endl;
-  std::size_t nb_faces = num_faces(sm);
-  std::vector<face_descriptor> faces_to_remove; // faces that would be removed but are not because we're doing a dry run
-  std::size_t nb_to_remove =   PMP::keep_large_connected_components(sm, 1000,
-                                                                      CGAL::parameters::face_size_map(
-                                                                        CGAL::Constant_property_map<face_descriptor, std::size_t>(1))
+  const std::size_t nb_faces = num_faces(sm);
+
+  std::vector<face_descriptor> faces_to_remove;
+  std::size_t nb_to_remove = PMP::keep_large_connected_components(
+                               sm, 1000,
+                               CGAL::parameters::face_size_map(CGAL::Constant_property_map<face_descriptor, std::size_t>(1))
                                                 .dry_run(true)
                                                 .output_iterator(std::back_inserter(faces_to_remove)));
-  assert(!faces_to_remove.empty());
+
   if (argv1 == CGAL::data_file_path("meshes/blobby_3cc.off"))
   {
     assert(nb_to_remove == 1);
+    assert(faces_to_remove.size() == 680);
     assert(num_faces(sm) == nb_faces);
   }
 
-  PMP::keep_largest_connected_components(sm, 2,
-                                         CGAL::parameters::face_size_map(
-                                           CGAL::Constant_property_map<face_descriptor, std::size_t>(1)));
+  faces_to_remove.clear();
+  nb_to_remove = PMP::keep_largest_connected_components(
+                   sm, 2,
+                   CGAL::parameters::face_size_map(CGAL::Constant_property_map<face_descriptor, std::size_t>(1))
+                                    .dry_run(true)
+                                    .output_iterator(std::back_inserter(faces_to_remove)));
+
+  if (argv1 == CGAL::data_file_path("meshes/blobby_3cc.off"))
+  {
+    assert(nb_to_remove == 1);
+    assert(faces_to_remove.size() == 680);
+    assert(num_faces(sm) == nb_faces);
+  }
+
+  nb_to_remove = PMP::keep_largest_connected_components(
+                   sm, 2,
+                   CGAL::parameters::face_size_map(CGAL::Constant_property_map<face_descriptor, std::size_t>(1)));
+
+  if (argv1 == CGAL::data_file_path("meshes/blobby_3cc.off"))
+  {
+    assert(nb_to_remove == 1);
+    assert(faces(sm).size() == 2737);
+  }
 
   if (!save_output)
     return;
@@ -188,7 +213,7 @@ void keep_nothing(const std::string argv1)
 int main(int argc, char* argv[])
 {
   const std::string filename = (argc > 1) ? argv[1] : CGAL::data_file_path("meshes/blobby_3cc.off");
-  const bool save_output = (argc > 2) ? true : false;
+  const bool save_output = (argc > 2);
 
   mesh_with_id(filename, save_output);
   mesh_no_id(filename, save_output);

Polygon_mesh_processing/test/Polygon_mesh_processing/connected_component_surface_mesh.cpp

@@ -180,6 +180,7 @@ void test_CC_with_area_size_map(Mesh sm,
   // didn't actually remove anything
   assert(PMP::internal::number_of_connected_components(sm) == num);
   assert(num_vertices(sm) == nv);
+  assert(faces_to_remove.size() == 680);
 
   if(num > 2)
   {