From 9bf457ffcc6a3e763172a1d2fffbba66cf683e8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 5 Aug 2019 15:38:30 +0200 Subject: [PATCH 1/4] Fix duplicate nm vertices There was an issue with keeping a valid incident halfedge for vertices post-duplication --- .../CGAL/Polygon_mesh_processing/repair.h | 78 ++++++++++++++----- 1 file changed, 59 insertions(+), 19 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/repair.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/repair.h index b0fae9cf46a..940653a1244 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/repair.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/repair.h @@ -1717,6 +1717,11 @@ struct Vertex_collector typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; bool has_old_vertex(const vertex_descriptor v) const { return collections.count(v) != 0; } + void tag_old_vertex(const vertex_descriptor v) + { + CGAL_precondition(!has_old_vertex(v)); + collections[v]; + } void collect_vertices(vertex_descriptor v1, vertex_descriptor v2) { @@ -1820,15 +1825,25 @@ std::size_t make_umbrella_manifold(typename boost::graph_traits::ha bool is_non_manifold_within_umbrella = (border_counter > 1); - // if there is a single sector, then simply move the full umbrella to a new vertex, and we're done if(!is_non_manifold_within_umbrella) { - // note that since this is marked as a non-manifold vertex, we necessarily need to create - // a new vertex for this umbrella (the main umbrella is not marked as non-manifold) - halfedge_descriptor last_h = opposite(next(h, pm), pm); - vertex_descriptor new_v = create_new_vertex_for_sector(h, last_h, pm, vpm, cmap); - dmap.collect_vertices(old_v, new_v); - nb_new_vertices = 1; + const bool first_time_meeting_v = !dmap.has_old_vertex(old_v); + if(first_time_meeting_v) + { + // The star is manifold, so if it is the first time we have met that vertex, + // there is nothing to do, we just keep the same vertex. + set_halfedge(old_v, h, pm); // to ensure halfedge(old_v, pm) stays valid + dmap.tag_old_vertex(old_v); // so that we know we have met old_v already, next time, we'll have to duplicate + } + else + { + // This is not the canonical star associated to 'v'. + // Create a new vertex, and move the whole star to that new vertex + halfedge_descriptor last_h = opposite(next(h, pm), pm); + vertex_descriptor new_v = create_new_vertex_for_sector(h, last_h, pm, vpm, cmap); + dmap.collect_vertices(old_v, new_v); + nb_new_vertices = 1; + } } // if there is more than one sector, look at each sector and split them away from the main one else @@ -1861,8 +1876,8 @@ std::size_t make_umbrella_manifold(typename boost::graph_traits::ha halfedge_descriptor next_start_h = prev(opposite(sector_last_h, pm), pm); // there are multiple CCs incident to this particular vertex, and we should create a new vertex - // if it's not the first umbrella around 'old_v' or not the first sector, but not if it's - // the first umbrella and first sector. + // if it's not the first umbrella around 'old_v' or not the first sector, but only not if it's + // both the first umbrella and first sector. bool must_create_new_vertex = (!is_main_sector || dmap.has_old_vertex(old_v)); // In any case, we must set up the next pointer correctly @@ -1874,6 +1889,11 @@ std::size_t make_umbrella_manifold(typename boost::graph_traits::ha dmap.collect_vertices(old_v, new_v); ++nb_new_vertices; } + else + { + // Ensure that halfedge(old_v, pm) stays valid + set_halfedge(old_v, sector_start_h, pm); + } is_main_sector = false; sector_start_h = next_start_h; @@ -1934,17 +1954,25 @@ std::size_t duplicate_non_manifold_vertices(PolygonMesh& pm, internal::Vertex_collector dmap; - typedef CGAL::dynamic_vertex_property_t Vertex_property_tag; - typedef typename boost::property_map::type Visited_vertex_map; - typedef CGAL::dynamic_halfedge_property_t Halfedge_property_tag; - typedef typename boost::property_map::type Visited_halfedge_map; + typedef CGAL::dynamic_vertex_property_t Vertex_bool_tag; + typedef typename boost::property_map::type Known_manifold_vertex_map; + typedef CGAL::dynamic_vertex_property_t Vertex_halfedge_tag; + typedef typename boost::property_map::type Visited_vertex_map; + typedef CGAL::dynamic_halfedge_property_t Halfedge_property_tag; + typedef typename boost::property_map::type Visited_halfedge_map; - Visited_vertex_map visited_vertices = get(Vertex_property_tag(), pm); + Known_manifold_vertex_map known_nm_vertices = get(Vertex_bool_tag(), pm); + Visited_vertex_map visited_vertices = get(Vertex_halfedge_tag(), pm); Visited_halfedge_map visited_halfedges = get(Halfedge_property_tag(), pm); + halfedge_descriptor null_h = boost::graph_traits::null_halfedge(); + // Dynamic pmaps do not have default initialization values (yet) BOOST_FOREACH(vertex_descriptor v, vertices(pm)) - put(visited_vertices, v, false); + { + put(known_nm_vertices, v, false); + put(visited_vertices, v, null_h); + } BOOST_FOREACH(halfedge_descriptor h, halfedges(pm)) put(visited_halfedges, h, false); @@ -1961,11 +1989,20 @@ std::size_t duplicate_non_manifold_vertices(PolygonMesh& pm, put(visited_halfedges, h, true); bool is_non_manifold = false; - vertex_descriptor vd = target(h, pm); - if(get(visited_vertices, vd)) // already seen this vertex, but not from this star + vertex_descriptor v = target(h, pm); + if(get(visited_vertices, v) != null_h) // already seen this vertex, but not from this star + { is_non_manifold = true; - - put(visited_vertices, vd, true); + // if this is the second time we visit that vertex and the first star was manifold, we have + // never reported the first star, but we must now + if(!get(known_nm_vertices, v)) + non_manifold_cones.push_back(get(visited_vertices, v)); // that's a halfedge of the first star we've seen 'v' in + } + else + { + // first time we meet this vertex, just mark it so, and keep the halfedge we found the vertex with in memory + put(visited_vertices, v, h); + } // While walking the star of this halfedge, if we meet a border halfedge more than once, // it means the mesh is pinched and we are also in the case of a non-manifold situation @@ -1985,7 +2022,10 @@ std::size_t duplicate_non_manifold_vertices(PolygonMesh& pm, is_non_manifold = true; if(is_non_manifold) + { non_manifold_cones.push_back(h); + put(known_nm_vertices, v, true); + } } } From a1039b9d33a0c2f949a0fc052528e2431e4168a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Wed, 21 Aug 2019 16:51:58 +0200 Subject: [PATCH 2/4] Fix boundary cycle stitching when nm vertices are present --- .../Polygon_mesh_processing/stitch_borders.h | 202 ++++++++++++------ 1 file changed, 142 insertions(+), 60 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h index 85e1334fc75..5788528b62d 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -400,8 +401,8 @@ void run_stitch_borders(PM& pmesh, } template -void stitch_borders_impl(PM& pmesh, - const HalfedgePairsRange& to_stitch) +std::size_t stitch_borders_impl(PM& pmesh, + const HalfedgePairsRange& to_stitch) { typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; @@ -526,9 +527,147 @@ void stitch_borders_impl(PM& pmesh, } run_stitch_borders(pmesh, to_stitch_filtered, uf_vertices, uf_handles); + return to_stitch_filtered.size(); } else + { run_stitch_borders(pmesh, to_stitch, uf_vertices, uf_handles); + return to_stitch.size(); + } +} + +template +std::size_t stitch_boundary_cycle(const typename boost::graph_traits::halfedge_descriptor h, + PolygonMesh& pm, + const CGAL_PMP_NP_CLASS& np) +{ + using parameters::choose_parameter; + using parameters::get_parameter; + + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + + typedef typename GetVertexPointMap::const_type VPMap; + VPMap vpm = choose_parameter(get_parameter(np, internal_np::vertex_point), + get_const_property_map(vertex_point, pm)); + + std::size_t stitched_boundary_cycles_n = 0; + + // A boundary cycle might need to be stitched starting from different extremities + // + // v11 ------ v10 + // | | + // v0 --- v1(v13) === v2(v12) v5(v9) === v6(v8) --- v7 + // | | + // v3 ------- v4 + // + // As long as we find vertices on the boundary with both halfedges being geometrically equal, + // we zip it up as much as we can. + + // not everything is always stitchable + std::set unstitchable_halfedges; + + halfedge_descriptor null_h = boost::graph_traits::null_halfedge(); + halfedge_descriptor bh = h; + for(;;) // until there is nothing to stitch anymore + { + if(bh == null_h) // the complete border is stitched + break; + + CGAL_assertion(is_border(bh, pm)); + + halfedge_descriptor hn = next(bh, pm), start_h = null_h; + do + { + halfedge_descriptor hnn = next(hn, pm); + CGAL_assertion(get(vpm, target(hn, pm)) == get(vpm, source(hnn, pm))); + + if(get(vpm, source(hn, pm)) == get(vpm, target(hnn, pm)) && + !is_degenerate_edge(edge(hn, pm), pm, parameters::vertex_point_map(vpm))) + { + if(unstitchable_halfedges.count(hn) == 0) + { + start_h = hn; + break; + } + } + + hn = hnn; + } + while(hn != bh); + + if(start_h == null_h) // nothing to be stitched on this boundary cycle + break; + + CGAL_assertion(is_border(start_h, pm)); + + // Associate as many consecutive halfedge pairs as possible ("zipping") + std::vector > hedges_to_stitch; + + halfedge_descriptor curr_h = start_h; + halfedge_descriptor curr_hn = next(curr_h, pm); + for(;;) // while we can expand the zipping range + { + // Don't create an invalid polygon mesh, even if the geometry allows it + if(face(opposite(curr_h, pm), pm) == face(opposite(curr_hn, pm), pm)) + { + unstitchable_halfedges.insert(curr_h); + bh = curr_hn; + break; + } + + CGAL_assertion(is_border(curr_h, pm)); + CGAL_assertion(is_border(curr_hn, pm)); + + hedges_to_stitch.push_back(std::make_pair(curr_h, curr_hn)); + + // check if we have reached the end of the cycle + if(curr_h == curr_hn || curr_h == next(curr_hn, pm)) + { + bh = null_h; + break; + } + + curr_h = prev(curr_h, pm); + curr_hn = next(curr_hn, pm); + + // check if the next two halfedges are no longer geometrically compatible + if(get(vpm, source(curr_h, pm)) != get(vpm, target(curr_hn, pm)) || + is_degenerate_edge(edge(curr_hn, pm), pm, parameters::vertex_point_map(vpm))) + { + bh = curr_hn; + break; + } + } + + // bh must be a boundary halfedge on the border that will not be impacted by any stitching + typedef std::pair Halfedge_pair; + + CGAL_assertion_code(if(bh != null_h) {) + CGAL_assertion_code( BOOST_FOREACH(const Halfedge_pair& hp, hedges_to_stitch) {) + CGAL_assertion( bh != hp.first && bh != hp.second); + CGAL_assertion_code(}}) + + if(!hedges_to_stitch.empty()) + { + std::size_t local_stitches = internal::stitch_borders_impl(pm, hedges_to_stitch); + stitched_boundary_cycles_n += local_stitches; + + if(local_stitches == 0) // refused to stitch this halfedge pair range due to manifold issue + { + BOOST_FOREACH(const Halfedge_pair& hp, hedges_to_stitch) + unstitchable_halfedges.insert(hp.first); + } + } + } + + return stitched_boundary_cycles_n; +} + +template +std::size_t stitch_boundary_cycle(const typename boost::graph_traits::halfedge_descriptor h, + PolygonMesh& pm) +{ + return stitch_boundary_cycle(h, pm, CGAL::parameters::all_default()); } // \ingroup PMP_repairing_grp @@ -556,69 +695,12 @@ std::size_t stitch_boundary_cycles(PolygonMesh& pm, typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; - typedef typename GetVertexPointMap::const_type VPMap; - VPMap vpm = choose_parameter(get_parameter(np, internal_np::vertex_point), - get_const_property_map(vertex_point, pm)); - std::vector boundary_cycles; extract_boundary_cycles(pm, std::back_inserter(boundary_cycles)); std::size_t stitched_boundary_cycles_n = 0; - - // A boundary cycle might need to be stitched starting from different extremities - // - // v11 ------ v10 - // | | - // v0 --- v1(v13) === v2(v12) v5(v9) === v6(v8) --- v7 - // | | - // v3 ------- v4 - // so we mark which edges have been stitched - cpp11::unordered_set stitched_hedges; - BOOST_FOREACH(halfedge_descriptor h, boundary_cycles) - { - std::vector stitching_starting_points; - halfedge_descriptor hn = next(h, pm); - while(hn != h) - { - if(get(vpm, source(hn, pm)) == get(vpm, target(next(hn, pm), pm))) - stitching_starting_points.push_back(hn); - - hn = next(hn, pm); - } - - for(std::size_t i=0, end=stitching_starting_points.size(); i 0) // already treated - continue; - - std::vector > hedges_to_stitch; - - halfedge_descriptor hn = next(h, pm); - bool do_stitching = true; - do - { - hedges_to_stitch.push_back(std::make_pair(h, hn)); - stitched_hedges.insert(h); - stitched_hedges.insert(hn); - - if(next(hn, pm) == h) - break; - - h = prev(h, pm); - hn = next(hn, pm); - - if(get(vpm, source(h, pm)) != get(vpm, target(hn, pm))) - do_stitching = false; - } - while(do_stitching); - - internal::stitch_borders_impl(pm, hedges_to_stitch); - ++stitched_boundary_cycles_n; - } - } + stitched_boundary_cycles_n += stitch_boundary_cycle(h, pm, np); return stitched_boundary_cycles_n; } From 799916d7c50bcf1f002b5d2a2e8b46052156e091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Wed, 21 Aug 2019 17:04:39 +0200 Subject: [PATCH 3/4] Rephrase end condition --- .../include/CGAL/Polygon_mesh_processing/stitch_borders.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h index 5788528b62d..65f76299f05 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h @@ -621,7 +621,7 @@ std::size_t stitch_boundary_cycle(const typename boost::graph_traits Date: Thu, 22 Aug 2019 09:03:01 +0200 Subject: [PATCH 4/4] Add const marker Co-Authored-By: Sebastien Loriot --- .../include/CGAL/Polygon_mesh_processing/stitch_borders.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h index 65f76299f05..cd511f16913 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/stitch_borders.h @@ -566,7 +566,7 @@ std::size_t stitch_boundary_cycle(const typename boost::graph_traits unstitchable_halfedges; - halfedge_descriptor null_h = boost::graph_traits::null_halfedge(); + const halfedge_descriptor null_h = boost::graph_traits::null_halfedge(); halfedge_descriptor bh = h; for(;;) // until there is nothing to stitch anymore {