diff --git a/BGL/include/CGAL/boost/graph/Face_filtered_graph.h b/BGL/include/CGAL/boost/graph/Face_filtered_graph.h index fb6036dc89e..8cf3c7884eb 100644 --- a/BGL/include/CGAL/boost/graph/Face_filtered_graph.h +++ b/BGL/include/CGAL/boost/graph/Face_filtered_graph.h @@ -463,64 +463,57 @@ struct Face_filtered_graph /// returns `true` if around any vertex of a selected face, /// there is at most one connected set of selected faces. - bool is_selection_valid() + bool is_selection_valid() const { - for(vertex_descriptor vd : vertices(*this) ) + typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + + // Non-manifoldness can appear either: + // - if 'pm' is pinched at a vertex. While traversing the incoming halfedges at this vertex, + // we will meet strictly more than one border halfedge. + // - if there are multiple umbrellas around a vertex. In that case, we will find a non-visited + // halfedge that has for target a vertex that is already visited. + + boost::unordered_set vertices_visited; + boost::unordered_set halfedges_handled; + + for(halfedge_descriptor hd : halfedges(*this)) { - face_descriptor first_selected = boost::graph_traits::null_face(); - bool first_unselected_found(false), - second_unselected_found(false); + CGAL_assertion(is_in_cc(hd)); - //find an unselected face, then find the first selected face. - //Find another unselected face, the next selected face must be the first; - //else this is not valid. - halfedge_descriptor hd = halfedge(vd, _graph); - face_descriptor first_tested = boost::graph_traits::null_face(); - while(1) //will break if valid, return false if not valid + if(!halfedges_handled.insert(hd).second) // already treated this halfedge + continue; + + vertex_descriptor vd = target(hd, *this); + CGAL_assertion(is_in_cc(vd)); + + // Check if we have already met this vertex before (necessarily in a different umbrella + // since we have never treated the halfedge 'hd') + if(!vertices_visited.insert(vd).second) + return false; + + std::size_t border_halfedge_counter = 0; + + // Can't simply call halfedges_around_target(vd, *this) because 'halfedge(vd)' is not necessarily 'hd' + halfedge_descriptor ihd = hd; + do { - face_descriptor fd = face(hd, _graph); + halfedges_handled.insert(ihd); + if(is_border(ihd, *this)) + ++border_halfedge_counter; - if(first_tested == boost::graph_traits::null_face()) - first_tested = fd; - else if(fd == first_tested ) + do { - //if there is no unselected face, break - if(selected_faces[get(fimap, fd)] && !first_unselected_found) - break; - //if there is no selected face, break - else if(!selected_faces[get(fimap, fd)] && - first_selected == boost::graph_traits::null_face()) - break; + ihd = prev(opposite(ihd, _graph), _graph); } - - if(fd != boost::graph_traits::null_face()) - { - if(selected_faces[get(fimap, fd)]) - { - if(first_unselected_found && - first_selected == boost::graph_traits::null_face()) - { - first_selected = fd; - } - else if(second_unselected_found) - { - if(fd == first_selected) - break; - else - return false; - } - } - else - { - if(first_selected == boost::graph_traits::null_face()) - first_unselected_found = true; - else - second_unselected_found = true; - } - } - hd = next(opposite(hd, _graph), _graph); + while(!is_in_cc(ihd) && ihd != hd); } + while(ihd != hd); + + if(border_halfedge_counter > 1) + return false; } + return true; } diff --git a/BGL/test/BGL/test_Face_filtered_graph.cpp b/BGL/test/BGL/test_Face_filtered_graph.cpp index 9e234d411f3..a1e20169345 100644 --- a/BGL/test/BGL/test_Face_filtered_graph.cpp +++ b/BGL/test/BGL/test_Face_filtered_graph.cpp @@ -268,17 +268,20 @@ void test_read(const Graph& g) typedef typename boost::graph_traits::face_descriptor g_face_descriptor; typedef CGAL::Face_filtered_graph Adapter; CGAL_GRAPH_TRAITS_MEMBERS(Adapter); + std::map map; CGAL::Polygon_mesh_processing::connected_components(g, boost::make_assoc_property_map(map), CGAL::Polygon_mesh_processing::parameters::all_default()); Adapter fg(g, 0, boost::make_assoc_property_map(map)); + assert(fg.is_selection_valid()); assert(CGAL::is_valid_polygon_mesh(fg)); } template void -test(const std::vector& graphs) +test_graph_range(const std::vector& graphs) { - for(Graph p : graphs){ + for(Graph p : graphs) + { test_read(p); test_vertex_iterators(p); test_out_edges(p); @@ -392,14 +395,75 @@ void test_mesh(Adapter fga) CGAL::copy_face_graph(fga, copy); } - -int -main() +template +void merge_vertices(typename boost::graph_traits::vertex_descriptor v_keep, + typename boost::graph_traits::vertex_descriptor v_rm, + std::vector::face_descriptor>& incident_faces, + PolygonMesh& mesh) { - test(sm_data()); -#ifdef CGAL_USE_OPENMESH - test(omesh_data()); + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + + halfedge_descriptor oh = halfedge(v_keep, mesh), done = oh; + do + { + incident_faces.push_back(face(oh, mesh)); + oh = opposite(next(oh, mesh), mesh); + } + while(oh != done); + + halfedge_descriptor h = halfedge(v_rm, mesh); + halfedge_descriptor start = h; + do + { + set_target(h, v_keep, mesh); + incident_faces.push_back(face(h, mesh)); + h = opposite(next(h, mesh), mesh); + } + while(h != start); + + remove_vertex(v_rm, mesh); +} + +void test_invalid_selections() +{ + // this creates a non-manifold (pinched) vertex + SM mesh; + read_a_mesh(mesh, "data/7_faces_triangle.off"); + + std::vector face_range; + face_range.push_back(SM::Face_index(1)); + face_range.push_back(SM::Face_index(2)); + face_range.push_back(SM::Face_index(3)); + + CGAL::Face_filtered_graph bad_fg(mesh, face_range); + assert(!bad_fg.is_selection_valid()); + + // this creates a non-manifold vertex (multiple umbrellas) + clear(mesh); + read_a_mesh(mesh, "data/genus3.off"); + assert(is_valid_polygon_mesh(mesh)); + + face_range.clear(); + merge_vertices(SM::Vertex_index(1337), SM::Vertex_index(87), face_range, mesh); + + CGAL::Face_filtered_graph bad_fg_2(mesh, face_range); + assert(!bad_fg_2.is_selection_valid()); +} + +int main() +{ + test_graph_range(poly_data()); + +#if defined(CGAL_USE_SURFACE_MESH) + test_graph_range(sm_data()); #endif + +#ifdef CGAL_USE_OPENMESH + test_graph_range(omesh_data()); +#endif + + test_invalid_selections(); + //Make a tetrahedron and test the adapter for a patch that only contains 2 faces typedef CGAL::Face_filtered_graph SM_Adapter; typedef SM::Property_map::face_descriptor , std::size_t> SM_FCCMap;