From f1724b3f4c9a4967d76f185f9927c320d69d77c0 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 16 Mar 2017 11:18:02 +0100 Subject: [PATCH 01/29] Make collapse_edge always keep the target vertex. --- .../CGAL/boost/graph/Euler_operations.h | 58 +++---------------- .../Scene_polyhedron_selection_item.cpp | 2 +- 2 files changed, 9 insertions(+), 51 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 2472d30f220..3dff7f0d0e6 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1068,49 +1068,7 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, vertex_descriptor q = target(pq, g); vertex_descriptor p = source(pq, g); -#if 0 - if(lTopLeftFaceExists && lBottomRightFaceExists){ - std::cerr << " // do it low level" << std::endl; - halfedge_descriptor qt = next(pq,g); - halfedge_descriptor pb = next(qp,g); - halfedge_descriptor ppt = prev(pt,g); - halfedge_descriptor pqb = prev(qb,g); - if(halfedge(q,g) == pq){ - set_halfedge(q, pqb,g); - } - vertex_descriptor t = target(qt,g); - if(halfedge(t,g) == pt){ - set_halfedge(t, qt,g); - } - vertex_descriptor b = target(pb,g); - if(halfedge(b,g) == qb){ - set_halfedge(t, pb,g); - } - set_face(qt, face(pt,g),g); - set_halfedge(face(qt,g),qt,g); - set_face(pb, face(qb,g),g); - set_halfedge(face(pb,g),pb,g); - set_next(qt, next(pt,g),g); - set_next(pb, next(qb,g),g); - set_next(ppt, qt,g); - set_next(pqb,pb,g); - remove_face(face(pq,g),g); - remove_face(face(qp,g),g); - remove_edge(v0v1,g); - remove_edge(edge(pt,g),g); - remove_edge(edge(qb,g),g); - remove_vertex(p,g); - Halfedge_around_target_circulator beg(ppt,g), end(pqb,g); - while(beg != end){ - CGAL_assertion(target(*beg,g) == p); - set_target(*beg,q,g); - --beg; - } - return q; - // return the vertex kept - } -#endif bool lP_Erased = false, lQ_Erased = false ; @@ -1137,7 +1095,7 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, //CGAL_ECMS_TRACE(3, "Bottom face doesn't exist so vertex P already removed" ) ; lP_Erased = true ; - } + } } } @@ -1157,14 +1115,14 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, //CGAL_ECMS_TRACE(3, "Removing q-b E" << qb.idx() << " (V" // << q.idx() << "->V" << target(qb, g).idx() // << ") by erasing bottom face" ) ; - - remove_face(opposite(qb, g),g); - - if ( !lTopFaceExists ) + if ( lTopFaceExists ) { - //CGAL_ECMS_TRACE(3, "Top face doesn't exist so vertex Q already removed" ) ; - lQ_Erased = true ; - } + remove_face(opposite(qb, g),g); + } + else + { + join_face(opposite(next(qp, g), g), g); + } } } diff --git a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp index c89b80cd686..89183cfca00 100644 --- a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp +++ b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp @@ -1229,7 +1229,7 @@ bool Scene_polyhedron_selection_item:: treat_selection(const std::setpoint() = T; compute_normal_maps(); polyhedron_item()->invalidateOpenGLBuffers(); From a779db591c7cfa162c3390c1a95f7f1ae8164c8e Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 16 Mar 2017 11:18:32 +0100 Subject: [PATCH 02/29] Make Isotropic_remeshing keep the constrained vertices when collapsing an edge. --- .../Isotropic_remeshing/remesh_impl.h | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 97a400388ce..f1bf2bf385f 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -752,27 +752,26 @@ namespace internal { if (!mesh_border_case_opp) halfedge_and_opp_removed(prev(opposite(he, mesh_), mesh_)); + //constrained case bool constrained_case = is_constrained(va) || is_constrained(vb); - if (constrained_case) - { - CGAL_assertion(is_constrained(va) ^ is_constrained(vb));//XOR - set_constrained(va, false); - set_constrained(vb, false); - } - //perform collapse - Point target_point = get(vpmap_, vb); - vertex_descriptor vkept = CGAL::Euler::collapse_edge(e, mesh_); - put(vpmap_, vkept, target_point); + //Take vb as the target of the collapse_edge so it is kept + vertex_descriptor vkept; + if(target(halfedge(e, mesh_), mesh_) == vb) + vkept = CGAL::Euler::collapse_edge(e, mesh_); + else + vkept = CGAL::Euler::collapse_edge(edge(opposite(halfedge(e, mesh_), mesh_), mesh_), mesh_); + CGAL_assertion(vkept == vb);//is the constrained point still here ++nb_collapses; //fix constrained case if (constrained_case)//we have made sure that collapse goes to constrained vertex - set_constrained(vkept, true); - + CGAL_assertion(is_constrained(vkept)); fix_degenerate_faces(vkept, short_edges, sq_low); + + #ifdef CGAL_PMP_REMESHING_DEBUG debug_status_map(); CGAL_assertion(!incident_to_degenerate(halfedge(vkept, mesh_))); From 4e036377305472fbfc887236305437aba28462c4 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 15 Jan 2018 11:59:15 +0100 Subject: [PATCH 03/29] Fix after rebase --- Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp index 89183cfca00..dff50bacb14 100644 --- a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp +++ b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp @@ -1229,7 +1229,7 @@ bool Scene_polyhedron_selection_item:: treat_selection(const std::setpoint() = T; + get(vpm, CGAL::Euler::collapse_edge(ed, *polyhedron())) = T; compute_normal_maps(); polyhedron_item()->invalidateOpenGLBuffers(); From cbbf244849c709dff790c4f00c4b569a2075bc4a Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 15 Jan 2018 12:14:20 +0100 Subject: [PATCH 04/29] updates changes.md --- Installation/CHANGES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index 78fc891eea6..495ed26295a 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -36,6 +36,14 @@ Release date: September 2018 - Added a function to apply a transformation to a mesh : - `CGAL::Polygon_mesh_processing::transform()` +### CGAL and the Boost Graph Library (BGL) + +- Change the function `CGAL::Euler::collapse_edge` so that the target + vertex stays the same after the collapsing. + +- Guarantee that constrained vertices are kept in the remeshed mesh, + and not only constrained points like before. + Release 4.12 ------------ From 558662282917ad7318c7fa64d3120cb36475aa37 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 15 Jan 2018 12:18:48 +0100 Subject: [PATCH 05/29] Update doc for edge_collapse --- BGL/include/CGAL/boost/graph/Euler_operations.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 3dff7f0d0e6..f8aa92961b7 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1033,14 +1033,13 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, *
    *
  • The edge `v0v1` is no longer in `g`. *
  • The faces incident to edge `v0v1` are no longer in `g`. - *
  • Either `v0`, or `v1` is no longer in `g` while the other remains. - * Let `vgone` be the removed vertex and `vkept` be the remaining vertex. + *
  • `v0` is no longer in `g`. *
  • If `e` was a border halfedge, that is `is_border(e, g) == true`, then `next(ep,g) == en`, and `prev(en,g) == ep`. *
  • If `e` was not a border halfedge, that is `is_border(e, g) == false`, then `ep` and `epo` are no longer in `g` while `en` and `eno` are kept in `g`. - *
  • For all halfedges `hv` in `halfedges_around_target(vgone, g)`, `target(*hv, g) == vkept` and `source(opposite(*hv, g), g) == vkept`. + *
  • For all halfedges `hv` in `halfedges_around_target(v0, g)`, `target(*hv, g) == v1` and `source(opposite(*hv, g), g) == v1`. *
  • No other incidence information has changed in `g`. *
- * \returns vertex `vkept` (which can be either `v0` or `v1`). + * \returns vertex `v1`. * \pre g must be a triangulated graph * \pre `does_satisfy_link_condition(v0v1,g) == true`. */ From 34a66797b595394d0d2fb678ef79022f2bb8adca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 14:58:02 +0100 Subject: [PATCH 06/29] the corner status of a vertex is already taken into account in the code it is meaningful for collapse and smooth that are using is_corner function --- .../Isotropic_remeshing/remesh_impl.h | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index f1bf2bf385f..4ce162d5b45 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -331,8 +331,6 @@ namespace internal { { tag_halfedges_status(face_range); //called first - constrain_patch_corners(face_range); - BOOST_FOREACH(face_descriptor f, face_range) { if (is_degenerate_triangle_face(halfedge(f,mesh_),mesh_,vpmap_,GeomTraits())){ @@ -1368,10 +1366,6 @@ private: { return get(vcmap_, v); } - void set_constrained(const vertex_descriptor& v, const bool b) - { - put(vcmap_, v, b); - } bool is_isolated(const vertex_descriptor& v) const { return halfedges_around_target(v, mesh_).empty(); @@ -1414,43 +1408,6 @@ private: return PMP::compute_face_normal(f, mesh_, parameters::vertex_point_map(vpmap_)); } - template - void constrain_patch_corners(const FaceRange& face_range) - { - boost::container::flat_set visited; - - BOOST_FOREACH(face_descriptor f, face_range) - { - BOOST_FOREACH(halfedge_descriptor h, - halfedges_around_face(halfedge(f, mesh_), mesh_)) - { - vertex_descriptor vt = target(h, mesh_); - //treat target(h, mesh_) - if (visited.find(vt) != visited.end()) - continue;//already treated - - if (status(h) == PATCH)//h not on patch boundary - continue; //so neither is target(h, mesh_) - - //count incident MESH_BORDER edges - unsigned int nb_incident_borders = 0; - BOOST_FOREACH(halfedge_descriptor hv, - halfedges_around_target(h, mesh_)) - { - CGAL_assertion(vt == target(hv, mesh_)); - if ( (status(hv) == PATCH_BORDER && status(opposite(hv, mesh_)) == MESH_BORDER) - || (status(hv) == MESH_BORDER && status(opposite(hv, mesh_)) == PATCH_BORDER)) - nb_incident_borders++; - } - - if (nb_incident_borders == 1) //this is a special corner - set_constrained(vt, true); - - visited.insert(vt); - } - } - } - template void tag_halfedges_status(const FaceRange& face_range) { From d3caff29dc367cffe1d8568c3c9a8a7600ee36f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 15:02:26 +0100 Subject: [PATCH 07/29] fix indentation --- .../internal/Isotropic_remeshing/remesh_impl.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 4ce162d5b45..a3a36e22425 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -941,10 +941,10 @@ namespace internal { else if (is_on_patch(v)) { - Vector_3 vn = PMP::compute_vertex_normal(v, mesh_ - , PMP::parameters::vertex_point_map(vpmap_) - .geom_traits(GeomTraits())); - put(propmap_normals, v, vn); + Vector_3 vn = PMP::compute_vertex_normal(v, mesh_ + , PMP::parameters::vertex_point_map(vpmap_) + .geom_traits(GeomTraits())); + put(propmap_normals, v, vn); Vector_3 move = CGAL::NULL_VECTOR; unsigned int star_size = 0; From 53164edc5a90516e30ef617f6fdf0a8c99c81da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 15:03:32 +0100 Subject: [PATCH 08/29] a vertex of degree less than 3 is on the boundary collapsing an edge incident to it should be controlled only by the constrained status of the vertices --- .../internal/Isotropic_remeshing/remesh_impl.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index a3a36e22425..4356e5d83c5 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -691,9 +691,7 @@ namespace internal { CGAL_assertion(collapse_does_not_invert_face(he)); CGAL_assertion(is_collapse_allowed(e)); - if (degree(va, mesh_) < 3 - || degree(vb, mesh_) < 3 - || !CGAL::Euler::does_satisfy_link_condition(e, mesh_))//necessary to collapse + if (!CGAL::Euler::does_satisfy_link_condition(e, mesh_))//necessary to collapse continue; //check that collapse would not create an edge with length > high From 0695f30c17b3ec2276f89ac3603c99ebfdb90220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 20:52:01 +0100 Subject: [PATCH 09/29] always preserve q at collapse by swapping p and q if q is of degree 2 the advantage is that we have an easy way to document which halfedges are removed --- .../CGAL/boost/graph/Euler_operations.h | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index f8aa92961b7..01845e106f2 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1069,7 +1069,7 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, vertex_descriptor p = source(pq, g); - bool lP_Erased = false, lQ_Erased = false ; + bool lP_Erased = false; if ( lTopFaceExists ) { @@ -1114,20 +1114,28 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, //CGAL_ECMS_TRACE(3, "Removing q-b E" << qb.idx() << " (V" // << q.idx() << "->V" << target(qb, g).idx() // << ") by erasing bottom face" ) ; - if ( lTopFaceExists ) + + if ( !lTopFaceExists ) { - remove_face(opposite(qb, g),g); - } - else - { - join_face(opposite(next(qp, g), g), g); + //CGAL_ECMS_TRACE(3, "Top face doesn't exist so vertex Q already removed" ) ; + lP_Erased = true ; + + // q will be removed, swap p and q + halfedge_descriptor hq=halfedge(q, g); + halfedge_descriptor hp=halfedge(p, g); + BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hq, g)) + set_target(h, p, g); + BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hp, g)) + set_target(h, q, g); + set_halfedge(p, hq, g); + set_halfedge(q, hp, g); } + + remove_face(opposite(qb, g),g); } } - CGAL_assertion( !lP_Erased || !lQ_Erased ) ; - - if ( !lP_Erased && !lQ_Erased ) + if ( !lP_Erased ) { //CGAL_ECMS_TRACE(3, "Removing vertex P by joining pQ" ) ; @@ -1137,7 +1145,7 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, CGAL_expensive_assertion(is_valid_polygon_mesh(g)); - return lP_Erased ? q : p ; + return q; } /** From 4e580a6450d23c8facab15c18fac59f7bafa2a3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 21:20:45 +0100 Subject: [PATCH 10/29] update documentation --- .../CGAL/boost/graph/Euler_operations.h | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 01845e106f2..67348a609bf 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1020,39 +1020,33 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, * collapses an edge in a graph. * * \tparam Graph must be a model of `MutableFaceGraph` - * Let `v0` and `v1` be the source and target vertices, and let `e` and `e'` be the halfedges of edge `v0v1`. + * Let `h` be the halfedge of `e`, and let `v0` and `v1` be the source and target vertices of `h`. + * Let `ep` and `e'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. + * Let `en` and `e'n` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. * - * For `e`, let `en` and `ep` be the next and previous - * halfedges, that is `en = next(e, g)`, `ep = prev(e, g)`, and let - * `eno` and `epo` be their opposite halfedges, that is - * `eno = opposite(en, g)` and `epo = opposite(ep, g)`. - * Analoguously, for `e'` define `en'`, `ep'`, `eno'`, and `epo'`. + * After the collapse of edge `e` the following holds: + * - The edge `e` is no longer in `g`. + * - The faces incident to edge `v0v1` are no longer in `g`. + * - `v0` is no longer in `g`. + * - If `h` is not a border halfedge, `ep` is no longer in `g` and is replaced by `en`. + * - If the opposite of `h` is not a border halfedge, `e'p` is no longer in `g` and is replaced by `e'n`. + * - The halfedges kept in `g` that had `v0` as target now have `v1` as target, and similarly for the source. + * - No other incidence information is changed in `g`. * - * Then, after the collapse of edge `v0v1` the following holds for `e` (and analoguously for `e'`) - * - *
    - *
  • The edge `v0v1` is no longer in `g`. - *
  • The faces incident to edge `v0v1` are no longer in `g`. - *
  • `v0` is no longer in `g`. - *
  • If `e` was a border halfedge, that is `is_border(e, g) == true`, then `next(ep,g) == en`, and `prev(en,g) == ep`. - *
  • If `e` was not a border halfedge, that is `is_border(e, g) == false`, then `ep` and `epo` are no longer in `g` while `en` and `eno` are kept in `g`. - *
  • For all halfedges `hv` in `halfedges_around_target(v0, g)`, `target(*hv, g) == v1` and `source(opposite(*hv, g), g) == v1`. - *
  • No other incidence information has changed in `g`. - *
* \returns vertex `v1`. * \pre g must be a triangulated graph * \pre `does_satisfy_link_condition(v0v1,g) == true`. */ template typename boost::graph_traits::vertex_descriptor -collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, +collapse_edge(typename boost::graph_traits::edge_descriptor e, Graph& g) { typedef boost::graph_traits< Graph > Traits; typedef typename Traits::vertex_descriptor vertex_descriptor; typedef typename Traits::halfedge_descriptor halfedge_descriptor; - halfedge_descriptor pq = halfedge(v0v1,g); + halfedge_descriptor pq = halfedge(e,g); halfedge_descriptor qp = opposite(pq, g); halfedge_descriptor pt = opposite(prev(pq, g), g); halfedge_descriptor qb = opposite(prev(qp, g), g); From de3d654a72f1ddf2aef2320c8c5e034449b8648f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 21:27:31 +0100 Subject: [PATCH 11/29] simplify the code collapsing edges handles corner and constrained vertices --- .../Isotropic_remeshing/remesh_impl.h | 52 ++++++++----------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 4356e5d83c5..1e320daf713 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -661,29 +661,29 @@ namespace internal { vertex_descriptor va = source(he, mesh_); vertex_descriptor vb = target(he, mesh_); - bool swap_done = false; - if (is_on_patch_border(va) && !is_on_patch_border(vb)) + bool is_va_constrained = is_constrained(va) || is_corner(va); + bool is_vb_constrained = is_constrained(vb) || is_corner(vb); + + + // do not collapse edge with two constrained vertices + if (is_va_constrained && is_vb_constrained) continue; + + bool can_swap = !is_va_constrained || !is_vb_constrained; + if (is_va_constrained) { he = opposite(he, mesh_); - va = source(he, mesh_); - vb = target(he, mesh_); - swap_done = true; - CGAL_assertion(is_on_patch_border(vb) && !is_on_patch_border(va)); + e=edge(he, mesh_); + std::swap(va, vb); } if(!collapse_does_not_invert_face(he)) { - if (!swap_done//if swap has already been done, don't re-swap - && collapse_does_not_invert_face(opposite(he, mesh_))) + if (can_swap//if swap allowed (no constrained vertices) + && collapse_does_not_invert_face(opposite(he, mesh_))) { he = opposite(he, mesh_); - va = source(he, mesh_); - vb = target(he, mesh_); - - if (is_on_patch_border(va) && !is_on_patch_border(vb)) - continue;//we cannot swap again. It would lead to a face inversion - else if (is_corner(va) && !is_corner(vb)) - continue;//idem + e=edge(he, mesh_); + std::swap(va, vb); } else continue;//both directions invert a face @@ -706,9 +706,9 @@ namespace internal { break; } } - //before collapsing va into vb, check that it does not break a corner - //if it is allowed, perform the collapse - if (collapse_ok && !is_constrained(va) && !is_corner(va)) + // before collapsing va into vb, check that it does not break a corner + // or a constrained vertex + if (collapse_ok) { //"collapse va into vb along e" // remove edges incident to va and vb, because their lengths will change @@ -748,26 +748,16 @@ namespace internal { if (!mesh_border_case_opp) halfedge_and_opp_removed(prev(opposite(he, mesh_), mesh_)); - - //constrained case - bool constrained_case = is_constrained(va) || is_constrained(vb); //perform collapse - //Take vb as the target of the collapse_edge so it is kept - vertex_descriptor vkept; - if(target(halfedge(e, mesh_), mesh_) == vb) - vkept = CGAL::Euler::collapse_edge(e, mesh_); - else - vkept = CGAL::Euler::collapse_edge(edge(opposite(halfedge(e, mesh_), mesh_), mesh_), mesh_); + CGAL_assertion(target(halfedge(e, mesh_), mesh_) == vb); + vertex_descriptor vkept = CGAL::Euler::collapse_edge(e, mesh_); CGAL_assertion(vkept == vb);//is the constrained point still here ++nb_collapses; //fix constrained case - if (constrained_case)//we have made sure that collapse goes to constrained vertex - CGAL_assertion(is_constrained(vkept)); + CGAL_assertion(is_constrained(vkept) == (is_va_constrained || is_vb_constrained)); fix_degenerate_faces(vkept, short_edges, sq_low); - - #ifdef CGAL_PMP_REMESHING_DEBUG debug_status_map(); CGAL_assertion(!incident_to_degenerate(halfedge(vkept, mesh_))); From 0549e39af49954fc11b3a50a5372fd040bc3b705 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 15 Jan 2018 21:32:55 +0100 Subject: [PATCH 12/29] update changes --- Installation/CHANGES.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index 495ed26295a..79dd631cb10 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -36,10 +36,12 @@ Release date: September 2018 - Added a function to apply a transformation to a mesh : - `CGAL::Polygon_mesh_processing::transform()` +- Fix a bug in `isotropic_remeshing()` making constrained vertices missing in the output + ### CGAL and the Boost Graph Library (BGL) -- Change the function `CGAL::Euler::collapse_edge` so that the target - vertex stays the same after the collapsing. +- Update the function `CGAL::Euler::collapse_edge` so that the target + vertex of the edge collapsed is always kept after the collapse. - Guarantee that constrained vertices are kept in the remeshed mesh, and not only constrained points like before. From abe346f8d21f9a80929d6dcfe11e4ad233211f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 16 Jan 2018 11:47:22 +0100 Subject: [PATCH 13/29] consider any vertex incident to a constrained as constrained --- .../internal/Isotropic_remeshing/remesh_impl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 1e320daf713..51d6df06697 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -661,8 +661,8 @@ namespace internal { vertex_descriptor va = source(he, mesh_); vertex_descriptor vb = target(he, mesh_); - bool is_va_constrained = is_constrained(va) || is_corner(va); - bool is_vb_constrained = is_constrained(vb) || is_corner(vb); + bool is_va_constrained = is_constrained(va) || is_on_patch_border(va); + bool is_vb_constrained = is_constrained(vb) || is_on_patch_border(vb); // do not collapse edge with two constrained vertices From 5738a11841e6a4205a6c051a09be343370ca396a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 16 Jan 2018 17:46:15 +0100 Subject: [PATCH 14/29] swap halfedges to guarantee that constrained halfedges are kept this is only requires in case prev(opposite(he)) is constrained since if there is a constrained vertex it is vb. --- .../Isotropic_remeshing/remesh_impl.h | 46 ++++++++++++++++--- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 51d6df06697..a9ba94bcfb7 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -724,12 +724,14 @@ namespace internal { } //before collapse + halfedge_descriptor he_opp= opposite(he, mesh_); bool mesh_border_case = is_on_border(he); - bool mesh_border_case_opp = is_on_border(opposite(he, mesh_)); - halfedge_descriptor ep_p = prev(opposite(he, mesh_), mesh_); + bool mesh_border_case_opp = is_on_border(he_opp); + halfedge_descriptor ep_p = prev(he_opp, mesh_); halfedge_descriptor epo_p = opposite(ep_p, mesh_); halfedge_descriptor en = next(he, mesh_); - halfedge_descriptor en_p = next(opposite(he, mesh_), mesh_); + halfedge_descriptor en_p = next(he_opp, mesh_); + halfedge_descriptor eno_p = opposite(en_p, mesh_); Halfedge_status s_ep_p = status(ep_p); Halfedge_status s_epo_p = status(epo_p); Halfedge_status s_ep = status(prev(he, mesh_)); @@ -739,23 +741,55 @@ namespace internal { //do it before collapse is performed to be sure everything is valid if (!mesh_border_case) merge_status(en, s_epo, s_ep); - if (!mesh_border_case_opp) + if (!mesh_border_case_opp && s_ep_p!=PATCH_BORDER) merge_status(en_p, s_epo_p, s_ep_p); halfedge_and_opp_removed(he); if (!mesh_border_case) halfedge_and_opp_removed(prev(he, mesh_)); if (!mesh_border_case_opp) - halfedge_and_opp_removed(prev(opposite(he, mesh_), mesh_)); + { + if (s_ep_p!=PATCH_BORDER) + halfedge_and_opp_removed(prev(he_opp, mesh_)); + else{ + // swap edges so as to keep constained edges: + // replace en_p by epo_p and ep_p by eno_p + // (1) exchange next pointer + set_next(prev(epo_p, mesh_), en_p, mesh_); + set_next(en_p, next(epo_p, mesh_), mesh_); + set_next(prev(eno_p, mesh_), ep_p, mesh_); + set_next(ep_p, next(eno_p, mesh_), mesh_); + set_next(epo_p, eno_p, mesh_); + set_next(he_opp, epo_p, mesh_); + set_next(eno_p, he_opp, mesh_); + // (2) exchange vertex-halfedge pointer + set_target(ep_p, va, mesh_); + set_target(eno_p, vb, mesh_); + set_halfedge(va, ep_p, mesh_); + set_halfedge(vb, eno_p, mesh_); + // (3) exchange face-halfedge pointer + set_face(ep_p, face(eno_p, mesh_), mesh_); + set_face(en_p, face(epo_p, mesh_), mesh_); + set_face(epo_p, face(he_opp, mesh_), mesh_); + set_face(eno_p, face(he_opp, mesh_), mesh_); + set_halfedge(face(he_opp, mesh_), he_opp, mesh_); + if (face(ep_p, mesh_) != boost::graph_traits::null_face()) + set_halfedge(face(ep_p, mesh_), ep_p, mesh_); + if (face(en_p, mesh_) != boost::graph_traits::null_face()) + set_halfedge(face(en_p, mesh_), en_p, mesh_); + CGAL_assertion(mesh_.is_valid()); + } + } //perform collapse CGAL_assertion(target(halfedge(e, mesh_), mesh_) == vb); vertex_descriptor vkept = CGAL::Euler::collapse_edge(e, mesh_); + CGAL_assertion(mesh_.is_valid()); CGAL_assertion(vkept == vb);//is the constrained point still here ++nb_collapses; //fix constrained case - CGAL_assertion(is_constrained(vkept) == (is_va_constrained || is_vb_constrained)); + CGAL_assertion((is_constrained(vkept) || is_on_patch_border(vb)) == (is_va_constrained || is_vb_constrained)); fix_degenerate_faces(vkept, short_edges, sq_low); #ifdef CGAL_PMP_REMESHING_DEBUG From a929253ae5c443a0effbab01306a0d05612b802e Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 18 Jan 2018 11:27:31 +0100 Subject: [PATCH 15/29] WIP testsuite collapse_edge --- BGL/test/BGL/CMakeLists.txt | 3 + BGL/test/BGL/data/flat_hexahedron.off | 26 +++++ BGL/test/BGL/test_Collapse_edge.cpp | 159 ++++++++++++++++++++++++++ 3 files changed, 188 insertions(+) create mode 100644 BGL/test/BGL/data/flat_hexahedron.off create mode 100644 BGL/test/BGL/test_Collapse_edge.cpp diff --git a/BGL/test/BGL/CMakeLists.txt b/BGL/test/BGL/CMakeLists.txt index e3938cdd9c6..74efdb21fc3 100644 --- a/BGL/test/BGL/CMakeLists.txt +++ b/BGL/test/BGL/CMakeLists.txt @@ -98,6 +98,8 @@ create_single_source_cgal_program( "test_Face_filtered_graph.cpp" ) create_single_source_cgal_program( "test_Euler_operations.cpp" ) +create_single_source_cgal_program( "test_Collapse_edge.cpp" ) + create_single_source_cgal_program( "test_graph_traits.cpp" ) create_single_source_cgal_program( "test_Properties.cpp" ) @@ -105,6 +107,7 @@ create_single_source_cgal_program( "test_Properties.cpp" ) if(OpenMesh_FOUND) target_link_libraries( test_clear PRIVATE ${OPENMESH_LIBRARIES}) target_link_libraries( test_Euler_operations PRIVATE ${OPENMESH_LIBRARIES}) + target_link_libraries( test_Collapse_edge PRIVATE ${OPENMESH_LIBRARIES}) target_link_libraries( test_Face_filtered_graph PRIVATE ${OPENMESH_LIBRARIES}) target_link_libraries( test_graph_traits PRIVATE ${OPENMESH_LIBRARIES} ) target_link_libraries( test_Properties PRIVATE ${OPENMESH_LIBRARIES}) diff --git a/BGL/test/BGL/data/flat_hexahedron.off b/BGL/test/BGL/data/flat_hexahedron.off new file mode 100644 index 00000000000..58c23e12764 --- /dev/null +++ b/BGL/test/BGL/data/flat_hexahedron.off @@ -0,0 +1,26 @@ +OFF +10 10 0 +#Vertices +-1.5 0 0 #0 +-0.5 0 0 #1 +0.5 0 0 #2 +1.5 0 0 #3 +-0.75 -0.5 0 #4 +0 -0.5 0 #5 +0.75 -0.5 0 #6 +-0.75 0.5 0 #7 +0 0.5 0 #8 +0.75 0.5 0 #9 + +#Facets +3 0 1 7 +3 7 1 8 +3 8 1 2 +3 2 9 8 +3 9 2 3 + +3 0 4 1 +3 1 4 5 +3 1 5 2 +3 2 5 6 +3 2 6 3 diff --git a/BGL/test/BGL/test_Collapse_edge.cpp b/BGL/test/BGL/test_Collapse_edge.cpp new file mode 100644 index 00000000000..8887678d2dd --- /dev/null +++ b/BGL/test/BGL/test_Collapse_edge.cpp @@ -0,0 +1,159 @@ +#include "test_Prefix.h" +#include +#include + +template < typename Mesh> +typename boost::graph_traits:: +halfedge_descriptor find_halfedge(float x1, float y1, + float x2, float y2, + Mesh& m, + bool is_border = false) +{ + typedef typename boost::property_map::type VPMAP; + typedef typename boost::property_traits::value_type Point; + + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + VPMAP vpmap = get(CGAL::vertex_point, m); + BOOST_FOREACH(halfedge_descriptor h, halfedges(m)) + { + if(get(vpmap, source(h, m)) == Point(x1,y1,0) + && get(vpmap, target(h, m)) == Point(x2,y2,0)) + { + if(is_border == CGAL::is_border(h, m)) + return h; + else + return opposite(h, m); + } + } + return boost::graph_traits::null_halfedge(); +} + +template +void +collapse_edge_test() +{ + CGAL_GRAPH_TRAITS_MEMBERS(Mesh); + typedef typename boost::graph_traits:: vertex_descriptor vertex_descriptor; + typedef typename boost::graph_traits:: halfedge_descriptor halfedge_descriptor; + + const std::string fname = "data/flat_hexahedron.off"; + Mesh m; + if(!CGAL::read_off(fname, m)) { + std::cout << "Error reading file: " << fname << std::endl; + } + assert(CGAL::is_valid(m)); + + Mesh test_mesh; + CGAL::copy_face_graph(m, test_mesh); + assert(CGAL::is_valid(test_mesh)); + + //case 1 + { + halfedge_descriptor e = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + halfedge_descriptor en = next(e, test_mesh); + halfedge_descriptor eno = opposite(nesxt(e, test_mesh), test_mesh); + vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); + char found =0; + BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) + { + + } + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + CGAL::clear(test_mesh); + + } + //case 2 + { + CGAL::copy_face_graph(m, test_mesh); + assert(CGAL::is_valid(test_mesh)); + halfedge_descriptor e = find_halfedge(0,0.5, + -0.75,0.5, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + + e = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + CGAL::clear(test_mesh); + } + //case 3 + { + CGAL::copy_face_graph(m, test_mesh); + assert(CGAL::is_valid(test_mesh)); + halfedge_descriptor e = find_halfedge(1.5,0, + 0.75,0.5, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + + e = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + CGAL::clear(test_mesh); + } + //case 4 + { + CGAL::copy_face_graph(m, test_mesh); + assert(CGAL::is_valid(test_mesh)); + halfedge_descriptor e = find_halfedge(-0.5, 0, + 0, -0.5, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + e = find_halfedge(0, -0.5, + -0.5, 0, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + e = find_halfedge(0, -0.5, + 0.75, -0.5, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + + + e = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + CGAL::clear(test_mesh); + } + //case 5 + { + CGAL::copy_face_graph(m, test_mesh); + assert(CGAL::is_valid(test_mesh)); + halfedge_descriptor e = find_halfedge(0.75,0.5, + 1.5,0, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + e = find_halfedge(0.75,-0.5, + 1.5,0, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + + e = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + CGAL::clear(test_mesh); + } +} + + +int main() +{ + + collapse_edge_test(); + collapse_edge_test(); + +#ifdef CGAL_USE_OPENMESH + collapse_edge_test(); +#endif + + std::cerr << "done\n"; + return 0; +} From a72c71318486ecfc99310952b7c25770b8b88d73 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 18 Jan 2018 12:12:43 +0100 Subject: [PATCH 16/29] test halfedges in test_Collapse_edge.cpp --- BGL/test/BGL/test_Collapse_edge.cpp | 72 +++++++++++++++++++++++++++-- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/BGL/test/BGL/test_Collapse_edge.cpp b/BGL/test/BGL/test_Collapse_edge.cpp index 8887678d2dd..a787ffebc4d 100644 --- a/BGL/test/BGL/test_Collapse_edge.cpp +++ b/BGL/test/BGL/test_Collapse_edge.cpp @@ -53,14 +53,19 @@ collapse_edge_test() 0.5,0, test_mesh); halfedge_descriptor en = next(e, test_mesh); - halfedge_descriptor eno = opposite(nesxt(e, test_mesh), test_mesh); + halfedge_descriptor eno = opposite(en, test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - char found =0; + assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { - + if(it == eno + || it == eno_prime){ + ++found; + } } - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + assert(found == 2); CGAL::clear(test_mesh); } @@ -76,8 +81,20 @@ collapse_edge_test() e = find_halfedge(-0.5,0, 0.5,0, test_mesh); + halfedge_descriptor en = next(e, test_mesh); + halfedge_descriptor eno = opposite(en, test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + char found = 0; + BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) + { + if(it == eno + || it == eno_prime){ + ++found; + } + } + assert(found == 2); CGAL::clear(test_mesh); } //case 3 @@ -92,8 +109,20 @@ collapse_edge_test() e = find_halfedge(-0.5,0, 0.5,0, test_mesh); + halfedge_descriptor en = next(e, test_mesh); + halfedge_descriptor eno = opposite(en, test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + char found = 0; + BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) + { + if(it == eno + || it == eno_prime){ + ++found; + } + } + assert(found == 2); CGAL::clear(test_mesh); } //case 4 @@ -117,8 +146,22 @@ collapse_edge_test() e = find_halfedge(-0.5,0, 0.5,0, test_mesh); + halfedge_descriptor en = next(e, test_mesh); + halfedge_descriptor eno = opposite(en, test_mesh); + halfedge_descriptor ep_prime = prev(opposite(e, test_mesh), test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + char found = 0; + BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) + { + if(it == eno + || it == eno_prime + || it == ep_prime){ + ++found; + } + } + assert(found == 3); CGAL::clear(test_mesh); } //case 5 @@ -133,12 +176,33 @@ collapse_edge_test() 1.5,0, test_mesh); CGAL::Euler::remove_face(e, test_mesh); + e = find_halfedge(0,0.5, + 0.5,0, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + e = find_halfedge(0.5,0, + 0,-0.5, + test_mesh); + CGAL::Euler::remove_face(e, test_mesh); e = find_halfedge(-0.5,0, 0.5,0, test_mesh); + CGAL::Euler::remove_face(e, test_mesh); + halfedge_descriptor ep = prev(e, test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + char found = 0; + BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) + { + if(it == ep) + ++found; + else if( it == eno_prime){ + ++found; + } + } + assert(found == 2); CGAL::clear(test_mesh); } } From d35781d698b0e1dd45f4f970d187eb4deea2c891 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 18 Jan 2018 14:30:07 +0100 Subject: [PATCH 17/29] Change float to double in the arguments of find_halfedge --- BGL/test/BGL/test_Collapse_edge.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/BGL/test/BGL/test_Collapse_edge.cpp b/BGL/test/BGL/test_Collapse_edge.cpp index a787ffebc4d..7ebfc85c0c2 100644 --- a/BGL/test/BGL/test_Collapse_edge.cpp +++ b/BGL/test/BGL/test_Collapse_edge.cpp @@ -4,8 +4,8 @@ template < typename Mesh> typename boost::graph_traits:: -halfedge_descriptor find_halfedge(float x1, float y1, - float x2, float y2, +halfedge_descriptor find_halfedge(double x1, double y1, + double x2, double y2, Mesh& m, bool is_border = false) { From 4618632629bfd263ae009c4aec9962b6f2dc1fa6 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Thu, 18 Jan 2018 14:54:44 +0100 Subject: [PATCH 18/29] add comments on cases and remove code from asserts --- BGL/test/BGL/test_Collapse_edge.cpp | 45 +++++++++++++++-------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/BGL/test/BGL/test_Collapse_edge.cpp b/BGL/test/BGL/test_Collapse_edge.cpp index 7ebfc85c0c2..efbb461017c 100644 --- a/BGL/test/BGL/test_Collapse_edge.cpp +++ b/BGL/test/BGL/test_Collapse_edge.cpp @@ -41,13 +41,15 @@ collapse_edge_test() if(!CGAL::read_off(fname, m)) { std::cout << "Error reading file: " << fname << std::endl; } - assert(CGAL::is_valid(m)); + bool m_is_valid = CGAL::is_valid(m); + assert(m_is_valid); Mesh test_mesh; CGAL::copy_face_graph(m, test_mesh); - assert(CGAL::is_valid(test_mesh)); + m_is_valid = CGAL::is_valid(m); + assert(m_is_valid); - //case 1 + //case 1: General Case. { halfedge_descriptor e = find_halfedge(-0.5,0, 0.5,0, @@ -55,8 +57,9 @@ collapse_edge_test() halfedge_descriptor en = next(e, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + vertex_descriptor v1 = target(e, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { @@ -69,10 +72,9 @@ collapse_edge_test() CGAL::clear(test_mesh); } - //case 2 + //case 2: collapsing edge is not itself a border, but is incident upon a border edge that is removed. { CGAL::copy_face_graph(m, test_mesh); - assert(CGAL::is_valid(test_mesh)); halfedge_descriptor e = find_halfedge(0,0.5, -0.75,0.5, test_mesh); @@ -84,8 +86,9 @@ collapse_edge_test() halfedge_descriptor en = next(e, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + vertex_descriptor v1 = target(e, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { @@ -97,10 +100,9 @@ collapse_edge_test() assert(found == 2); CGAL::clear(test_mesh); } - //case 3 + //case 3: collapsing edge is not itself a border, but is incident upon a border edge that is not removed { CGAL::copy_face_graph(m, test_mesh); - assert(CGAL::is_valid(test_mesh)); halfedge_descriptor e = find_halfedge(1.5,0, 0.75,0.5, test_mesh); @@ -112,8 +114,9 @@ collapse_edge_test() halfedge_descriptor en = next(e, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + vertex_descriptor v1 = target(e, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { @@ -125,10 +128,9 @@ collapse_edge_test() assert(found == 2); CGAL::clear(test_mesh); } - //case 4 + //case 4: collapsing edge is itself a border { CGAL::copy_face_graph(m, test_mesh); - assert(CGAL::is_valid(test_mesh)); halfedge_descriptor e = find_halfedge(-0.5, 0, 0, -0.5, test_mesh); @@ -150,8 +152,9 @@ collapse_edge_test() halfedge_descriptor eno = opposite(en, test_mesh); halfedge_descriptor ep_prime = prev(opposite(e, test_mesh), test_mesh); halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + vertex_descriptor v1 = target(e, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { @@ -164,10 +167,9 @@ collapse_edge_test() assert(found == 3); CGAL::clear(test_mesh); } - //case 5 + //case 5 singular case. { CGAL::copy_face_graph(m, test_mesh); - assert(CGAL::is_valid(test_mesh)); halfedge_descriptor e = find_halfedge(0.75,0.5, 1.5,0, test_mesh); @@ -191,8 +193,9 @@ collapse_edge_test() CGAL::Euler::remove_face(e, test_mesh); halfedge_descriptor ep = prev(e, test_mesh); halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(edge(e, test_mesh), test_mesh); - assert(CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1); + vertex_descriptor v1 = target(e, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) { From b81453868bc1d4b49d930e7b66157ee5ddcd8b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 4 May 2018 10:09:33 +0200 Subject: [PATCH 19/29] fix the handling of constrained vertices --- .../internal/Isotropic_remeshing/remesh_impl.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index a9ba94bcfb7..e11067c17b5 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -668,12 +668,13 @@ namespace internal { // do not collapse edge with two constrained vertices if (is_va_constrained && is_vb_constrained) continue; - bool can_swap = !is_va_constrained || !is_vb_constrained; + bool can_swap = !is_vb_constrained; if (is_va_constrained) { he = opposite(he, mesh_); e=edge(he, mesh_); std::swap(va, vb); + can_swap=false; } if(!collapse_does_not_invert_face(he)) From 957eb36667d33ce08faa69dc928eb9c415b28307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Fri, 4 May 2018 10:15:13 +0200 Subject: [PATCH 20/29] restore non-wanted change --- Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp index dff50bacb14..c89b80cd686 100644 --- a/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp +++ b/Polyhedron/demo/Polyhedron/Scene_polyhedron_selection_item.cpp @@ -1229,7 +1229,7 @@ bool Scene_polyhedron_selection_item:: treat_selection(const std::setinvalidateOpenGLBuffers(); From 0b2436d0624bdb90ac02dfa54921e8b80befd8f1 Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Mon, 7 May 2018 12:45:29 +0200 Subject: [PATCH 21/29] Clean-up after review --- .../CGAL/boost/graph/Euler_operations.h | 10 +- BGL/test/BGL/test_Collapse_edge.cpp | 146 +++++++++--------- Installation/CHANGES.md | 8 +- .../Isotropic_remeshing/remesh_impl.h | 4 +- 4 files changed, 84 insertions(+), 84 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 67348a609bf..7e90d70c057 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1021,21 +1021,21 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, * * \tparam Graph must be a model of `MutableFaceGraph` * Let `h` be the halfedge of `e`, and let `v0` and `v1` be the source and target vertices of `h`. - * Let `ep` and `e'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. + * Let `hp` and `e'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. * Let `en` and `e'n` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. * * After the collapse of edge `e` the following holds: * - The edge `e` is no longer in `g`. - * - The faces incident to edge `v0v1` are no longer in `g`. + * - The faces incident to edge `e` are no longer in `g`. * - `v0` is no longer in `g`. - * - If `h` is not a border halfedge, `ep` is no longer in `g` and is replaced by `en`. + * - If `h` is not a border halfedge, `hp` is no longer in `g` and is replaced by `en`. * - If the opposite of `h` is not a border halfedge, `e'p` is no longer in `g` and is replaced by `e'n`. - * - The halfedges kept in `g` that had `v0` as target now have `v1` as target, and similarly for the source. + * - The halfedges kept in `g` that had that had v0 as target (resp. source) now have v1 as target (resp. source). * - No other incidence information is changed in `g`. * * \returns vertex `v1`. * \pre g must be a triangulated graph - * \pre `does_satisfy_link_condition(v0v1,g) == true`. + * \pre `does_satisfy_link_condition(e,g) == true`. */ template typename boost::graph_traits::vertex_descriptor diff --git a/BGL/test/BGL/test_Collapse_edge.cpp b/BGL/test/BGL/test_Collapse_edge.cpp index efbb461017c..d1f9e0e22d8 100644 --- a/BGL/test/BGL/test_Collapse_edge.cpp +++ b/BGL/test/BGL/test_Collapse_edge.cpp @@ -51,14 +51,14 @@ collapse_edge_test() //case 1: General Case. { - halfedge_descriptor e = find_halfedge(-0.5,0, - 0.5,0, - test_mesh); - halfedge_descriptor en = next(e, test_mesh); + halfedge_descriptor he = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + halfedge_descriptor en = next(he, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); - halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(e, test_mesh); - bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + halfedge_descriptor eno_prime = opposite(next(opposite(he, test_mesh), test_mesh), test_mesh); + vertex_descriptor v1 = target(he, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(he, test_mesh), test_mesh) == v1; assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) @@ -75,19 +75,19 @@ collapse_edge_test() //case 2: collapsing edge is not itself a border, but is incident upon a border edge that is removed. { CGAL::copy_face_graph(m, test_mesh); - halfedge_descriptor e = find_halfedge(0,0.5, - -0.75,0.5, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); + halfedge_descriptor he = find_halfedge(0,0.5, + -0.75,0.5, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); - e = find_halfedge(-0.5,0, - 0.5,0, - test_mesh); - halfedge_descriptor en = next(e, test_mesh); + he = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + halfedge_descriptor en = next(he, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); - halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(e, test_mesh); - bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + halfedge_descriptor eno_prime = opposite(next(opposite(he, test_mesh), test_mesh), test_mesh); + vertex_descriptor v1 = target(he, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(he, test_mesh), test_mesh) == v1; assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) @@ -103,19 +103,19 @@ collapse_edge_test() //case 3: collapsing edge is not itself a border, but is incident upon a border edge that is not removed { CGAL::copy_face_graph(m, test_mesh); - halfedge_descriptor e = find_halfedge(1.5,0, - 0.75,0.5, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); + halfedge_descriptor he = find_halfedge(1.5,0, + 0.75,0.5, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); - e = find_halfedge(-0.5,0, - 0.5,0, - test_mesh); - halfedge_descriptor en = next(e, test_mesh); + he = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + halfedge_descriptor en = next(he, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); - halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(e, test_mesh); - bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + halfedge_descriptor eno_prime = opposite(next(opposite(he, test_mesh), test_mesh), test_mesh); + vertex_descriptor v1 = target(he, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(he, test_mesh), test_mesh) == v1; assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) @@ -131,29 +131,29 @@ collapse_edge_test() //case 4: collapsing edge is itself a border { CGAL::copy_face_graph(m, test_mesh); - halfedge_descriptor e = find_halfedge(-0.5, 0, - 0, -0.5, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - e = find_halfedge(0, -0.5, - -0.5, 0, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - e = find_halfedge(0, -0.5, - 0.75, -0.5, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); + halfedge_descriptor he = find_halfedge(-0.5, 0, + 0, -0.5, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + he = find_halfedge(0, -0.5, + -0.5, 0, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + he = find_halfedge(0, -0.5, + 0.75, -0.5, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); - e = find_halfedge(-0.5,0, - 0.5,0, - test_mesh); - halfedge_descriptor en = next(e, test_mesh); + he = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + halfedge_descriptor en = next(he, test_mesh); halfedge_descriptor eno = opposite(en, test_mesh); - halfedge_descriptor ep_prime = prev(opposite(e, test_mesh), test_mesh); - halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(e, test_mesh); - bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + halfedge_descriptor ep_prime = prev(opposite(he, test_mesh), test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(he, test_mesh), test_mesh), test_mesh); + vertex_descriptor v1 = target(he, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(he, test_mesh), test_mesh) == v1; assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) @@ -170,31 +170,31 @@ collapse_edge_test() //case 5 singular case. { CGAL::copy_face_graph(m, test_mesh); - halfedge_descriptor e = find_halfedge(0.75,0.5, - 1.5,0, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - e = find_halfedge(0.75,-0.5, - 1.5,0, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - e = find_halfedge(0,0.5, - 0.5,0, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - e = find_halfedge(0.5,0, - 0,-0.5, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); + halfedge_descriptor he = find_halfedge(0.75,0.5, + 1.5,0, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + he = find_halfedge(0.75,-0.5, + 1.5,0, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + he = find_halfedge(0,0.5, + 0.5,0, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + he = find_halfedge(0.5,0, + 0,-0.5, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); - e = find_halfedge(-0.5,0, - 0.5,0, - test_mesh); - CGAL::Euler::remove_face(e, test_mesh); - halfedge_descriptor ep = prev(e, test_mesh); - halfedge_descriptor eno_prime = opposite(next(opposite(e, test_mesh), test_mesh), test_mesh); - vertex_descriptor v1 = target(e, test_mesh); - bool ok = CGAL::Euler::collapse_edge(edge(e, test_mesh), test_mesh) == v1; + he = find_halfedge(-0.5,0, + 0.5,0, + test_mesh); + CGAL::Euler::remove_face(he, test_mesh); + halfedge_descriptor ep = prev(he, test_mesh); + halfedge_descriptor eno_prime = opposite(next(opposite(he, test_mesh), test_mesh), test_mesh); + vertex_descriptor v1 = target(he, test_mesh); + bool ok = CGAL::Euler::collapse_edge(edge(he, test_mesh), test_mesh) == v1; assert(ok); char found = 0; BOOST_FOREACH(halfedge_descriptor it, CGAL::halfedges_around_target(v1,test_mesh)) diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index 79dd631cb10..ded709dc7b2 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -37,14 +37,14 @@ Release date: September 2018 - `CGAL::Polygon_mesh_processing::transform()` - Fix a bug in `isotropic_remeshing()` making constrained vertices missing in the output +- Guarantee that constrained vertices are kept in the remeshed mesh, + and not only constrained points like before. ### CGAL and the Boost Graph Library (BGL) -- Update the function `CGAL::Euler::collapse_edge` so that the target - vertex of the edge collapsed is always kept after the collapse. +- Improve the function `CGAL::Euler::collapse_edge` so that the target + vertex of the collapsed edge is always kept after the collapse. -- Guarantee that constrained vertices are kept in the remeshed mesh, - and not only constrained points like before. Release 4.12 ------------ diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index e11067c17b5..4dda66dda8e 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -778,14 +778,14 @@ namespace internal { set_halfedge(face(ep_p, mesh_), ep_p, mesh_); if (face(en_p, mesh_) != boost::graph_traits::null_face()) set_halfedge(face(en_p, mesh_), en_p, mesh_); - CGAL_assertion(mesh_.is_valid()); + CGAL_assertion(is_valid(mesh_)); } } //perform collapse CGAL_assertion(target(halfedge(e, mesh_), mesh_) == vb); vertex_descriptor vkept = CGAL::Euler::collapse_edge(e, mesh_); - CGAL_assertion(mesh_.is_valid()); + CGAL_assertion(is_valid(mesh_)); CGAL_assertion(vkept == vb);//is the constrained point still here ++nb_collapses; From 127b0c68d58a1bcd1d2e07fa94fb9413ec8dd8ab Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Wed, 9 May 2018 10:38:21 +0200 Subject: [PATCH 22/29] Fix doc --- BGL/include/CGAL/boost/graph/Euler_operations.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 7e90d70c057..f8fed4bde98 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1021,16 +1021,16 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, * * \tparam Graph must be a model of `MutableFaceGraph` * Let `h` be the halfedge of `e`, and let `v0` and `v1` be the source and target vertices of `h`. - * Let `hp` and `e'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. - * Let `en` and `e'n` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. + * Let `hp` and `h'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. + * Let `hn'` and `h'n'` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. * * After the collapse of edge `e` the following holds: * - The edge `e` is no longer in `g`. * - The faces incident to edge `e` are no longer in `g`. * - `v0` is no longer in `g`. - * - If `h` is not a border halfedge, `hp` is no longer in `g` and is replaced by `en`. - * - If the opposite of `h` is not a border halfedge, `e'p` is no longer in `g` and is replaced by `e'n`. - * - The halfedges kept in `g` that had that had v0 as target (resp. source) now have v1 as target (resp. source). + * - If `h` is not a border halfedge, `hp` is no longer in `g` and is replaced by `hn'`. + * - If the opposite of `h` is not a border halfedge, `h'p` is no longer in `g` and is replaced by `h'n'`. + * - The halfedges kept in `g` that had `v0` as target (resp. source) now have `v1` as target (resp. source). * - No other incidence information is changed in `g`. * * \returns vertex `v1`. From acf969560dea732987571ed4630660b44111d018 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 14 May 2018 10:07:03 +0200 Subject: [PATCH 23/29] try improving phrasing --- BGL/include/CGAL/boost/graph/Euler_operations.h | 10 +++++----- Installation/CHANGES.md | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index f8fed4bde98..8303b08c691 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1021,16 +1021,16 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, * * \tparam Graph must be a model of `MutableFaceGraph` * Let `h` be the halfedge of `e`, and let `v0` and `v1` be the source and target vertices of `h`. - * Let `hp` and `h'p` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. - * Let `hn'` and `h'n'` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. + * Let `p_h` and `p_o_h` be respectively the edges of `prev(h,g)` and `prev(opposite(h, g), g)`. + * Let `o_n_h` and `o_n_o_h` be respectively the edges of `opposite(next(h,g))` and `opposite(next(opposite(h, g), g))`. * * After the collapse of edge `e` the following holds: * - The edge `e` is no longer in `g`. * - The faces incident to edge `e` are no longer in `g`. * - `v0` is no longer in `g`. - * - If `h` is not a border halfedge, `hp` is no longer in `g` and is replaced by `hn'`. - * - If the opposite of `h` is not a border halfedge, `h'p` is no longer in `g` and is replaced by `h'n'`. - * - The halfedges kept in `g` that had `v0` as target (resp. source) now have `v1` as target (resp. source). + * - If `h` is not a border halfedge, `p_h` is no longer in `g` and is replaced by `o_n_h`. + * - If the opposite of `h` is not a border halfedge, `p_o_h` is no longer in `g` and is replaced by `o_n_o_h`. + * - The halfedges kept in `g` that had `v0` as target and source now have `v1` as target and source, respectively. * - No other incidence information is changed in `g`. * * \returns vertex `v1`. diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index ded709dc7b2..c61cfbca535 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -37,8 +37,8 @@ Release date: September 2018 - `CGAL::Polygon_mesh_processing::transform()` - Fix a bug in `isotropic_remeshing()` making constrained vertices missing in the output -- Guarantee that constrained vertices are kept in the remeshed mesh, - and not only constrained points like before. +- Guarantee that constrained vertices are kept in the mesh after calling `isotropic_remeshing()` + (and not only constrained points like it was before). ### CGAL and the Boost Graph Library (BGL) From 2e7ceb07ff53d6c30ebce84be7b4a2867718c870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 15 May 2018 10:34:48 +0200 Subject: [PATCH 24/29] improve phrasing --- Installation/CHANGES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Installation/CHANGES.md b/Installation/CHANGES.md index c61cfbca535..9ae04070cfe 100644 --- a/Installation/CHANGES.md +++ b/Installation/CHANGES.md @@ -38,7 +38,7 @@ Release date: September 2018 - Fix a bug in `isotropic_remeshing()` making constrained vertices missing in the output - Guarantee that constrained vertices are kept in the mesh after calling `isotropic_remeshing()` - (and not only constrained points like it was before). + (and not only the points associated to constrained vertices as it was before). ### CGAL and the Boost Graph Library (BGL) From fe25b6e6b2d94e0fe063785ee8edde8eed5152b0 Mon Sep 17 00:00:00 2001 From: Andreas Fabri Date: Mon, 21 May 2018 11:17:24 +0100 Subject: [PATCH 25/29] fix off file so that OpenMesh can read it too --- BGL/test/BGL/data/flat_hexahedron.off | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/BGL/test/BGL/data/flat_hexahedron.off b/BGL/test/BGL/data/flat_hexahedron.off index 58c23e12764..b7c3562356d 100644 --- a/BGL/test/BGL/data/flat_hexahedron.off +++ b/BGL/test/BGL/data/flat_hexahedron.off @@ -1,18 +1,16 @@ OFF 10 10 0 -#Vertices --1.5 0 0 #0 --0.5 0 0 #1 -0.5 0 0 #2 -1.5 0 0 #3 --0.75 -0.5 0 #4 -0 -0.5 0 #5 -0.75 -0.5 0 #6 --0.75 0.5 0 #7 -0 0.5 0 #8 -0.75 0.5 0 #9 +-1.5 0 0 +-0.5 0 0 +0.5 0 0 +1.5 0 0 +-0.75 -0.5 0 +0 -0.5 0 +0.75 -0.5 0 +-0.75 0.5 0 +0 0.5 0 +0.75 0.5 0 -#Facets 3 0 1 7 3 7 1 8 3 8 1 2 From 1f1c97050ba4519b1b38cf75e232ca3db9a61111 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 29 May 2018 08:39:04 +0200 Subject: [PATCH 26/29] move code swapping vertices in an internal helper function --- .../CGAL/boost/graph/Euler_operations.h | 9 +-------- BGL/include/CGAL/boost/graph/helpers.h | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 8303b08c691..e4ec6913681 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1115,14 +1115,7 @@ collapse_edge(typename boost::graph_traits::edge_descriptor e, lP_Erased = true ; // q will be removed, swap p and q - halfedge_descriptor hq=halfedge(q, g); - halfedge_descriptor hp=halfedge(p, g); - BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hq, g)) - set_target(h, p, g); - BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hp, g)) - set_target(h, q, g); - set_halfedge(p, hq, g); - set_halfedge(q, hp, g); + internal::swap_vertices(p, q, g); } remove_face(opposite(qb, g),g); diff --git a/BGL/include/CGAL/boost/graph/helpers.h b/BGL/include/CGAL/boost/graph/helpers.h index 5e2c03e95e9..f8a84fe11c2 100644 --- a/BGL/include/CGAL/boost/graph/helpers.h +++ b/BGL/include/CGAL/boost/graph/helpers.h @@ -1397,6 +1397,24 @@ clear_impl(FaceGraph& g) } } +template +void swap_vertices( + typename boost::graph_traits::vertex_descriptor& p, + typename boost::graph_traits::vertex_descriptor& q, + FaceGraph& g) +{ + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + + halfedge_descriptor hq=halfedge(q, g); + halfedge_descriptor hp=halfedge(p, g); + BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hq, g)) + set_target(h, p, g); + BOOST_FOREACH(halfedge_descriptor h, halfedges_around_target(hp, g)) + set_target(h, q, g); + set_halfedge(p, hq, g); + set_halfedge(q, hp, g); +} + } //end of internal namespace /** From 460f49d64ff6ea5fd3312fb429d5120f67baad74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 29 May 2018 15:36:15 +0200 Subject: [PATCH 27/29] move code to swap edges in a function --- BGL/include/CGAL/boost/graph/helpers.h | 77 +++++++++++++++++++ BGL/test/BGL/test_Euler_operations.cpp | 23 +++++- .../Isotropic_remeshing/remesh_impl.h | 25 +----- 3 files changed, 100 insertions(+), 25 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/helpers.h b/BGL/include/CGAL/boost/graph/helpers.h index f8a84fe11c2..f34e24b7687 100644 --- a/BGL/include/CGAL/boost/graph/helpers.h +++ b/BGL/include/CGAL/boost/graph/helpers.h @@ -1415,6 +1415,83 @@ void swap_vertices( set_halfedge(q, hp, g); } +template +void swap_edges( + const typename boost::graph_traits::halfedge_descriptor& h1, + const typename boost::graph_traits::halfedge_descriptor& h2, + FaceGraph& g) +{ + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + typedef typename boost::graph_traits::face_descriptor face_descriptor; + typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; + const halfedge_descriptor oh1 = opposite(h1, g), oh2 = opposite(h2, g); + + // backup vertex pointers + vertex_descriptor s1 = target(oh1, g), s2 = target(oh2, g); + vertex_descriptor t1 = target(h1, g), t2 = target(h2, g); + + // backup face pointers + face_descriptor f1 = face(h1, g), f2 = face(h2, g); + face_descriptor fo1 = face(oh1, g), fo2 = face(oh2, g); + + // backup next prev pointers + halfedge_descriptor nh1 = next(h1, g), nh2 = next(h2, g); + halfedge_descriptor ph1 = prev(h1, g), ph2 = prev(h2, g); + halfedge_descriptor noh1 = next(oh1, g), noh2 = next(oh2, g); + halfedge_descriptor poh1 = prev(oh1, g), poh2 = prev(oh2, g); + + // handle particular cases where next/prev are halfedges to be swapt + if (nh1 == oh2) nh1 = oh1; + if (nh1 == h2) nh1 = h1; + if (nh2 == oh1) nh2 = oh2; + if (nh2 == h1) nh2 = h2; + if (ph1 == oh2) ph1 = oh1; + if (ph1 == h2) ph1 = h1; + if (ph2 == oh1) ph2 = oh2; + if (ph2 == h1) ph2 = h2; + if (noh1 == oh2) noh1 = oh1; + if (noh1 == h2) noh1 = h1; + if (noh2 == oh1) noh2 = oh2; + if (noh2 == h1) noh2 = h2; + if (poh1 == oh2) poh1 = oh1; + if (poh1 == h2) poh1 = h1; + if (poh2 == oh1) poh2 = oh2; + if (poh2 == h1) poh2 = h2; + + // (1) exchange next pointers + set_next(h1, nh2, g); + set_next(h2, nh1, g); + set_next(ph1, h2, g); + set_next(ph2, h1, g); + set_next(oh1, noh2, g); + set_next(oh2, noh1, g); + set_next(poh1, oh2, g); + set_next(poh2, oh1, g); + + // (2) exchange vertex-halfedge pointers + set_target(h1, t2, g); + set_target(h2, t1, g); + set_target(oh1, s2, g); + set_target(oh2, s1, g); + if (halfedge(t1, g)==h1) set_halfedge(t1, h2, g); + if (halfedge(t2, g)==h2) set_halfedge(t2, h1, g); + if (halfedge(s1, g)==oh1) set_halfedge(s1, oh2, g); + if (halfedge(s2, g)==oh2) set_halfedge(s2, oh1, g); + + // (3) exchange face-halfedge pointers + set_face(h1, f2, g); + set_face(h2, f1, g); + set_face(oh1, fo2, g); + set_face(oh2, fo1, g); + + face_descriptor nf = boost::graph_traits::null_face(); + if (f1 != nf && halfedge(f1, g)==h1) set_halfedge(f1, h2, g); + if (f2 != nf && halfedge(f2, g)==h2) set_halfedge(f2, h1, g); + if (fo1 != nf && halfedge(fo1, g)==oh1) set_halfedge(fo1, oh2, g); + if (fo2 != nf && halfedge(fo2, g)==oh2) set_halfedge(fo2, oh1, g); +} + + } //end of internal namespace /** diff --git a/BGL/test/BGL/test_Euler_operations.cpp b/BGL/test/BGL/test_Euler_operations.cpp index cdf53ec5f4c..5099749380f 100644 --- a/BGL/test/BGL/test_Euler_operations.cpp +++ b/BGL/test/BGL/test_Euler_operations.cpp @@ -380,7 +380,27 @@ does_satisfy_link_condition() assert(CGAL::Euler::does_satisfy_link_condition(*edges(f.m).first,f.m)); } - +template +void +test_swap_edges() +{ + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + std::size_t nbh=12; + Kernel::Point_3 pt(0,0,0); + // test all possible pairs of halfedges + for (std::size_t i=0; i void @@ -400,6 +420,7 @@ test_Euler_operations() remove_center_vertex_test(); join_split_inverse(); does_satisfy_link_condition(); + test_swap_edges(); } int main() diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h index 4dda66dda8e..acf556b71e3 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h @@ -732,7 +732,6 @@ namespace internal { halfedge_descriptor epo_p = opposite(ep_p, mesh_); halfedge_descriptor en = next(he, mesh_); halfedge_descriptor en_p = next(he_opp, mesh_); - halfedge_descriptor eno_p = opposite(en_p, mesh_); Halfedge_status s_ep_p = status(ep_p); Halfedge_status s_epo_p = status(epo_p); Halfedge_status s_ep = status(prev(he, mesh_)); @@ -755,29 +754,7 @@ namespace internal { else{ // swap edges so as to keep constained edges: // replace en_p by epo_p and ep_p by eno_p - // (1) exchange next pointer - set_next(prev(epo_p, mesh_), en_p, mesh_); - set_next(en_p, next(epo_p, mesh_), mesh_); - set_next(prev(eno_p, mesh_), ep_p, mesh_); - set_next(ep_p, next(eno_p, mesh_), mesh_); - set_next(epo_p, eno_p, mesh_); - set_next(he_opp, epo_p, mesh_); - set_next(eno_p, he_opp, mesh_); - // (2) exchange vertex-halfedge pointer - set_target(ep_p, va, mesh_); - set_target(eno_p, vb, mesh_); - set_halfedge(va, ep_p, mesh_); - set_halfedge(vb, eno_p, mesh_); - // (3) exchange face-halfedge pointer - set_face(ep_p, face(eno_p, mesh_), mesh_); - set_face(en_p, face(epo_p, mesh_), mesh_); - set_face(epo_p, face(he_opp, mesh_), mesh_); - set_face(eno_p, face(he_opp, mesh_), mesh_); - set_halfedge(face(he_opp, mesh_), he_opp, mesh_); - if (face(ep_p, mesh_) != boost::graph_traits::null_face()) - set_halfedge(face(ep_p, mesh_), ep_p, mesh_); - if (face(en_p, mesh_) != boost::graph_traits::null_face()) - set_halfedge(face(en_p, mesh_), en_p, mesh_); + ::CGAL::internal::swap_edges(en_p, epo_p, mesh_); CGAL_assertion(is_valid(mesh_)); } } From 13cb7d250ce14ace103bc513e41d7c9349668177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 29 May 2018 15:49:32 +0200 Subject: [PATCH 28/29] make the function collapsing edges with constraints always keep the target --- .../CGAL/boost/graph/Euler_operations.h | 36 +++++++++++++------ BGL/test/BGL/test_Euler_operations.cpp | 2 +- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index e4ec6913681..1548cbe5e84 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1136,15 +1136,18 @@ collapse_edge(typename boost::graph_traits::edge_descriptor e, } /** - * Collapses the edge `v0v1` replacing it with v0 or v1, as described in the paragraph above + * collapses an edge in a graph having non-collapsable edges. + * + * Let `h` be the halfedge of `e`, and let `v0` and `v1` be the source and target vertices of `h`. + * Collapses the edge `e` replacing it with `v1`, as described in the paragraph above * and guarantees that an edge `e2`, for which `get(edge_is_constrained_map, e2)==true`, * is not removed after the collapse. * - * * \tparam Graph must be a model of `MutableFaceGraph` * \tparam EdgeIsConstrainedMap mut be a model of `ReadablePropertyMap` with the edge descriptor of `Graph` * as key type and a Boolean as value type. It indicates if an edge is constrained or not. * + * \returns vertex `v1`. * \pre This function requires `g` to be an oriented 2-manifold with or without boundaries. * Furthermore, the edge `v0v1` must satisfy the link condition, which guarantees that the surface mesh is also 2-manifold after the edge collapse. * \pre `get(edge_is_constrained_map, v0v1)==false`. @@ -1218,10 +1221,13 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, halfedge_descriptor edge = next(edges_to_erase[0],g) == edges_to_erase[1]? edges_to_erase[0]:edges_to_erase[1]; - if (target(edge,g) == p) - lP_Erased = true; + if (target(edge,g) != p) + { + // q will be removed, swap it with p + internal::swap_vertices(p, q, g); + } remove_center_vertex(edge,g); - return lP_Erased? q : p; + return q; } else { @@ -1246,19 +1252,29 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, join_vertex(pq,g); return q; } - bool lQ_Erased = is_border(opposite(next(pq,g),g),g); + if( is_border(opposite(next(pq,g),g),g) ) + { + // q will be removed, swap it with p + internal::swap_vertices(p, q, g); + } remove_face(opposite(edges_to_erase[0],g),g); - return lQ_Erased?p:q; + return q; } if (! (is_border(edges_to_erase[0],g))){ + // q will be removed, swap it with p + internal::swap_vertices(p, q, g); join_face(edges_to_erase[0],g); join_vertex(qp,g); - return p; + return q; + } + if(!is_border(opposite(next(qp,g),g),g)) + { + // q will be removed, swap it with p + internal::swap_vertices(p, q, g); } - bool lP_Erased= is_border(opposite(next(qp,g),g),g); remove_face(opposite(edges_to_erase[0],g),g); - return lP_Erased?q:p; + return q; }; } diff --git a/BGL/test/BGL/test_Euler_operations.cpp b/BGL/test/BGL/test_Euler_operations.cpp index 5099749380f..3ae73d6c38d 100644 --- a/BGL/test/BGL/test_Euler_operations.cpp +++ b/BGL/test/BGL/test_Euler_operations.cpp @@ -397,7 +397,7 @@ test_swap_edges() halfedge_descriptor h1 = *CGAL::cpp11::next(boost::begin(halfedges(g)), i); halfedge_descriptor h2 = *CGAL::cpp11::next(boost::begin(halfedges(g)), j); CGAL::internal::swap_edges(h1, h2, g); - CGAL_assertion(is_valid(g)); + CGAL_assertion(CGAL::is_valid_polygon_mesh(g)); } } } From b18f5e554607c40952a9d025fbdff93383dd518a Mon Sep 17 00:00:00 2001 From: Maxime Gimeno Date: Tue, 5 Jun 2018 09:30:58 +0200 Subject: [PATCH 29/29] Fix warning. --- BGL/include/CGAL/boost/graph/Euler_operations.h | 1 - 1 file changed, 1 deletion(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 1548cbe5e84..109de182736 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1217,7 +1217,6 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, { // the vertex is of valence 3 and we simply need to remove the vertex // and its indicent edges - bool lP_Erased = false; halfedge_descriptor edge = next(edges_to_erase[0],g) == edges_to_erase[1]? edges_to_erase[0]:edges_to_erase[1];