From afbfa1e3d76f0d8cca96715b857af2b327cfe44b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 14 Mar 2022 16:28:27 +0100 Subject: [PATCH 01/16] add code to avoid creating degenerate triangles --- .../internal/Snapping/snap.h | 97 ++++++++++++++++++- .../internal/Snapping/snap_vertices.h | 39 +++++++- 2 files changed, 131 insertions(+), 5 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index e127b84e8e5..b6384a268c6 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -151,6 +151,28 @@ void simplify_range(HalfedgeRange& halfedge_range, new_tolerance += CGAL::approximate_sqrt(CGAL::squared_distance(new_p, pt)); } + bool call_continue = false; + for (halfedge_descriptor he : halfedges_around_target(h, tm)) + if (he!=h && get(vpm, source(he, tm))==new_p) + { + call_continue = true; + break; + } + if (call_continue) + continue; + for (halfedge_descriptor he : halfedges_around_target(opposite(h,tm), tm)) + if (he!=opposite(h,tm) && get(vpm,source(he, tm))==new_p) + { + call_continue = true; + break; + } + if (call_continue) + continue; + + const halfedge_descriptor opoh = opposite(prev(opposite(h, tm), tm), tm); + if (is_border(opoh, tm)) + edges_to_test.erase( opoh ); + vertex_descriptor v = Euler::collapse_edge(edge(h, tm), tm); put(vpm, v, new_p); put(tolerance_map, v, new_tolerance); @@ -686,6 +708,12 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, (new_position == get(vpm_T, source(h_to_split, tm_T)))) do_split = false; + //in case of self_snapping avoid pinching + if(&tm_T==&tm_S && target(next(opposite(h_to_split, tm_T), tm_T), tm_T)==splitter_v) + { + do_split = false; + } + if(!first_split && new_position == previous_split_position) do_split = false; @@ -697,6 +725,31 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, std::cout << "Actually split? " << do_split << std::endl; #endif + // check the new faces after split are not degenerated + Point p0 = new_position; + Point_ref p1 = get(vpm_T, source(h_to_split, tm_T)); + Point_ref p2 = get(vpm_T, target(next(opposite(h_to_split, tm_T), tm_T), tm_T)); + Point_ref p3 = get(vpm_T, target(h_to_split, tm_T)); + + /* Chooses the diagonal that will split the quad in two triangles that maximize + * the scalar product of of the un-normalized normals of the two triangles. + * The lengths of the un-normalized normals (computed using cross-products of two vectors) + * are proportional to the area of the triangles. + * Maximize the scalar product of the two normals will avoid skinny triangles, + * and will also taken into account the cosine of the angle between the two normals. + * In particular, if the two triangles are oriented in different directions, + * the scalar product will be negative. + */ + auto p1p3 = CGAL::cross_product(p2-p1,p3-p2) * CGAL::cross_product(p0-p3,p1-p0); + auto p0p2 = CGAL::cross_product(p1-p0,p1-p2) * CGAL::cross_product(p3-p2,p3-p0); + + bool is_deg = (p0p2>p1p3) + ? collinear(p0,p1,p2) && collinear(p0,p3,p2) + : collinear(p0,p1,p3) && collinear(p1,p2,p3); + + if (is_deg) + do_split = false; + // Split and update positions vertex_descriptor new_v = boost::graph_traits::null_vertex(); if(do_split) @@ -711,7 +764,19 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, } if(!is_source_mesh_fixed) - put(vpm_S, splitter_v, new_position); + { + bool skip = false; + for (halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) + { + if (!is_border(h,tm_S) && collinear(get(vpm_S, source(h,tm_S)), new_position, get(vpm_S, target(next(h,tm_S),tm_S)))) + { + skip=true; + break; + } + } + if(!skip) + put(vpm_S, splitter_v, new_position); + } first_split = false; previous_split_position = new_position; @@ -722,6 +787,33 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, if(!do_split) continue; +#if 1 + halfedge_descriptor v0, v1, v2, v3; + v0 = opposite(h_to_split, tm_T); + Point_ref p0 = get(vpm_T, target(v0, tm_T)); + v1 = next(v0, tm_T); + Point_ref p1 = get(vpm_T, target(v1, tm_T)); + v2 = next(v1, tm_T); + Point_ref p2 = get(vpm_T, target(v2, tm_T)); + v3 = next(v2, tm_T); + Point_ref p3 = get(vpm_T, target(v3, tm_T)); + + /* Chooses the diagonal that will split the quad in two triangles that maximize + * the scalar product of of the un-normalized normals of the two triangles. + * The lengths of the un-normalized normals (computed using cross-products of two vectors) + * are proportional to the area of the triangles. + * Maximize the scalar product of the two normals will avoid skinny triangles, + * and will also taken into account the cosine of the angle between the two normals. + * In particular, if the two triangles are oriented in different directions, + * the scalar product will be negative. + */ + auto p1p3 = CGAL::cross_product(p2-p1,p3-p2) * CGAL::cross_product(p0-p3,p1-p0); + auto p0p2 = CGAL::cross_product(p1-p0,p1-p2) * CGAL::cross_product(p3-p2,p3-p0); + + halfedge_descriptor res = (p0p2>p1p3) + ? CGAL::Euler::split_face(v0, v2, tm_T) + : CGAL::Euler::split_face(v1, v3, tm_T); +#else /* new_p * / \ * res / \ h_to_split @@ -778,6 +870,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, h_to_split = opposite(next(new_hd, tm_T), tm_T); visitor.after_split_face(opposite(res, tm_T), h2, tm_T); } +#endif } } @@ -1400,6 +1493,8 @@ std::size_t snap_borders(TriangleMesh& tm, true /*self snapping*/, np, np); } +//TODO:add an option to preserve orientation? + } // end namespace experimental } // end namespace Polygon_mesh_processing } // end namespace CGAL diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h index 91f1a51952b..4ab2c0eefb7 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h @@ -955,8 +955,19 @@ std::size_t snap_vertices_two_way(const HalfedgeRange_A& halfedge_range_A, { if(is_second_mesh_fixed) { + const Point& new_p = get(vpm_B, vb); for(const halfedge_descriptor ha : vs_a) - put(vpm_A, target(ha, tm_A), get(vpm_B, vb)); + { + bool skip = false; + for (halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) + if (!is_border(haa,tm_A) && collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) + { + skip=true; + break; + } + if (!skip) + put(vpm_A, target(ha, tm_A), new_p); + } } else { @@ -972,12 +983,32 @@ std::size_t snap_vertices_two_way(const HalfedgeRange_A& halfedge_range_A, #endif for(const halfedge_descriptor ha : vs_a) - put(vpm_A, target(ha, tm_A), new_p); + { + bool skip = false; + for (halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) + if (!is_border(haa,tm_A) && collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) + { + skip=true; + break; + } + if (!skip) + put(vpm_A, target(ha, tm_A), new_p); + } for(const halfedge_descriptor hb : vs_b) - put(vpm_B, target(hb, tm_B), new_p); + { + bool skip = false; + for (halfedge_descriptor hbb : halfedges_around_target(hb, tm_B)) + if (!is_border(hbb,tm_B) && collinear(get(vpm_B, source(hbb,tm_B)), new_p, get(vpm_B, target(next(hbb,tm_B),tm_B)))) + { + skip=true; + break; + } + if (!skip) + put(vpm_B, target(hb, tm_B), new_p); + } } - + //TODO: the counter shall depend on skip? ++counter; } From 9baff6f6d2f90cdbe4ed36e73f63caf29d33f106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Mon, 14 Mar 2022 16:38:20 +0100 Subject: [PATCH 02/16] clean up --- .../internal/Snapping/snap.h | 80 +++---------------- 1 file changed, 9 insertions(+), 71 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index b6384a268c6..5fd979d6210 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -725,7 +725,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, std::cout << "Actually split? " << do_split << std::endl; #endif - // check the new faces after split are not degenerated + // check the new faces after split will not be degenerated Point p0 = new_position; Point_ref p1 = get(vpm_T, source(h_to_split, tm_T)); Point_ref p2 = get(vpm_T, target(next(opposite(h_to_split, tm_T), tm_T), tm_T)); @@ -742,8 +742,9 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, */ auto p1p3 = CGAL::cross_product(p2-p1,p3-p2) * CGAL::cross_product(p0-p3,p1-p0); auto p0p2 = CGAL::cross_product(p1-p0,p1-p2) * CGAL::cross_product(p3-p2,p3-p0); + bool first_split_face = (p0p2>p1p3); - bool is_deg = (p0p2>p1p3) + bool is_deg = first_split_face ? collinear(p0,p1,p2) && collinear(p0,p3,p2) : collinear(p0,p1,p3) && collinear(p1,p2,p3); @@ -787,90 +788,27 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, if(!do_split) continue; -#if 1 halfedge_descriptor v0, v1, v2, v3; v0 = opposite(h_to_split, tm_T); - Point_ref p0 = get(vpm_T, target(v0, tm_T)); v1 = next(v0, tm_T); - Point_ref p1 = get(vpm_T, target(v1, tm_T)); v2 = next(v1, tm_T); - Point_ref p2 = get(vpm_T, target(v2, tm_T)); v3 = next(v2, tm_T); - Point_ref p3 = get(vpm_T, target(v3, tm_T)); - - /* Chooses the diagonal that will split the quad in two triangles that maximize - * the scalar product of of the un-normalized normals of the two triangles. - * The lengths of the un-normalized normals (computed using cross-products of two vectors) - * are proportional to the area of the triangles. - * Maximize the scalar product of the two normals will avoid skinny triangles, - * and will also taken into account the cosine of the angle between the two normals. - * In particular, if the two triangles are oriented in different directions, - * the scalar product will be negative. - */ - auto p1p3 = CGAL::cross_product(p2-p1,p3-p2) * CGAL::cross_product(p0-p3,p1-p0); - auto p0p2 = CGAL::cross_product(p1-p0,p1-p2) * CGAL::cross_product(p3-p2,p3-p0); - - halfedge_descriptor res = (p0p2>p1p3) - ? CGAL::Euler::split_face(v0, v2, tm_T) - : CGAL::Euler::split_face(v1, v3, tm_T); -#else - /* new_p - * / \ - * res / \ h_to_split - * / \ - * / \ - * left right - * | / - * | / - * | / - * | / - * | / - * | / - * opp - */ - - const halfedge_descriptor res = prev(h_to_split, tm_T); - const Point_ref left_pt = get(vpm_T, source(res, tm_T)); - const Point_ref right_pt = get(vpm_T, target(h_to_split, tm_T)); - const Point_ref opp = get(vpm_T, target(next(opposite(res, tm_T), tm_T), tm_T)); - - // Check if 'p' is "visible" from 'opp' (i.e. its projection on the plane 'Pl(left, opp, right)' - // falls in the cone with apex 'opp' and sides given by 'left' and 'right') - const Vector n = gt.construct_orthogonal_vector_3_object()(right_pt, left_pt, opp); - const Point trans_left_pt = gt.construct_translated_point_3_object()(left_pt, n); - const Point trans_right_pt = gt.construct_translated_point_3_object()(right_pt, n); - - const Point_ref new_p = get(vpm_T, new_v); - const bool left_of_left = (gt.orientation_3_object()(trans_left_pt, left_pt, opp, new_p) == CGAL::POSITIVE); - const bool right_of_right = (gt.orientation_3_object()(right_pt, trans_right_pt, opp, new_p) == CGAL::POSITIVE); - - const bool is_visible = (!left_of_left && !right_of_right); - -#ifdef CGAL_PMP_SNAP_DEBUG_PP - std::cout << "Left/Right: " << left_of_left << " " << right_of_right << std::endl; - std::cout << "visible from " << opp << " ? " << is_visible << std::endl; -#endif - - // h_to_split is equal to 'next(res)' after splitting const halfedge_descriptor h_to_split_opp = opposite(h_to_split, tm_T); - - if(is_visible) + halfedge_descriptor h2 = prev(prev(h_to_split_opp, tm_T), tm_T); + const halfedge_descriptor res = prev(h_to_split, tm_T); + halfedge_descriptor new_hd = first_split_face + ? CGAL::Euler::split_face(v0, v2, tm_T) + : CGAL::Euler::split_face(v1, v3, tm_T); + if (first_split_face) { - halfedge_descriptor h2 = prev(prev(h_to_split_opp, tm_T), tm_T); - halfedge_descriptor new_hd = CGAL::Euler::split_face(h_to_split_opp, - h2, tm_T); h_to_split = opposite(prev(new_hd, tm_T), tm_T); visitor.after_split_face(h_to_split_opp, h2, tm_T); } else { - halfedge_descriptor h2 = prev(h_to_split_opp, tm_T); - halfedge_descriptor new_hd = CGAL::Euler::split_face(opposite(res, tm_T), - h2, tm_T); h_to_split = opposite(next(new_hd, tm_T), tm_T); visitor.after_split_face(opposite(res, tm_T), h2, tm_T); } -#endif } } From 43c6c8a6aa93f27294a630528d359f8336634de8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 15 Mar 2022 10:44:42 +0100 Subject: [PATCH 03/16] remove unused typedef --- .../CGAL/Polygon_mesh_processing/internal/Snapping/snap.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 5fd979d6210..a543b6e250d 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -653,7 +653,6 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, typedef typename boost::property_traits::value_type Point; typedef typename boost::property_traits::reference Point_ref; - typedef typename GeomTraits::Vector_3 Vector; typedef std::pair Vertex_with_new_position; typedef std::vector Vertices_with_new_position; From f836b7b06673fa52a9556ac8b79874def22dde42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 11 May 2022 15:06:54 +0200 Subject: [PATCH 04/16] add missing precondition --- .../CGAL/boost/graph/Euler_operations.h | 205 +++++++++--------- 1 file changed, 104 insertions(+), 101 deletions(-) diff --git a/BGL/include/CGAL/boost/graph/Euler_operations.h b/BGL/include/CGAL/boost/graph/Euler_operations.h index 1c3a99dc696..e23db326269 100644 --- a/BGL/include/CGAL/boost/graph/Euler_operations.h +++ b/BGL/include/CGAL/boost/graph/Euler_operations.h @@ -1381,6 +1381,106 @@ add_face_to_border(typename boost::graph_traits::halfedge_descriptor h1, return newh; } +/** + * \returns `true` if `e` satisfies the *link condition* \cgalCite{degn-tpec-98}, which guarantees that the surface is also 2-manifold after the edge collapse. + */ +template +bool +does_satisfy_link_condition(typename boost::graph_traits::edge_descriptor e, + const Graph& g) +{ + typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; + typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; + typedef CGAL::Halfedge_around_source_iterator out_edge_iterator; + + halfedge_descriptor v0_v1 = halfedge(e,g); + halfedge_descriptor v1_v0 = opposite(v0_v1,g); + + vertex_descriptor v0 = target(v1_v0,g), v1 = target(v0_v1,g); + + vertex_descriptor vL = target(next(v0_v1,g),g); + vertex_descriptor vR = target(next(v1_v0,g),g); + + out_edge_iterator eb1, ee1 ; + out_edge_iterator eb2, ee2 ; + + // The following loop checks the link condition for v0_v1. + // Specifically, that for every vertex 'k' adjacent to both 'p and 'q', 'pkq' is a face of the mesh. + // + for ( boost::tie(eb1,ee1) = halfedges_around_source(v0,g) ; eb1 != ee1 ; ++ eb1 ) + { + halfedge_descriptor v0_k = *eb1; + + if ( v0_k != v0_v1 ) + { + vertex_descriptor k = target(v0_k,g); + + for ( boost::tie(eb2,ee2) = halfedges_around_source(k,g) ; eb2 != ee2 ; ++ eb2 ) + { + halfedge_descriptor k_v1 = *eb2; + + if ( target(k_v1,g) == v1 ) + { + // At this point we know p-q-k are connected and we need to determine if this triangle is a face of the mesh. + // + // Since the mesh is known to be triangular there are at most two faces sharing the edge p-q. + // + // If p->q is NOT a border edge, the top face is p->q->t where t is target(next(p->q)) + // If q->p is NOT a border edge, the bottom face is q->p->b where b is target(next(q->p)) + // + // If k is either t or b then p-q-k *might* be a face of the mesh. It won't be if k==t but p->q is border + // or k==b but q->b is a border (because in that case even though there exists triangles p->q->t (or q->p->b) + // they are holes, not faces) + // + + bool lIsFace = ( vL == k && (! is_border(v0_v1,g)) ) + || ( vR == k && (! is_border(v1_v0,g)) ) ; + + if ( !lIsFace ) + { + // CGAL_ECMS_TRACE(3," k=V" << get(Vertex_index_map,k) << " IS NOT in a face with p-q. NON-COLLAPSABLE edge." ) ; + return false ; + } + else + { + //CGAL_ECMS_TRACE(4," k=V" << get(Vertex_index_map,k) << " is in a face with p-q") ; + } + } + } + } + } + + // detect isolated triangle (or triangle attached to a mesh with non-manifold vertices) + if (!is_border(v0_v1,g) && is_border(opposite(next(v0_v1,g), g), g) + && is_border(opposite(prev(v0_v1,g), g), g) ) return false; + if (!is_border(v1_v0,g) && is_border(opposite(next(v1_v0,g), g), g) + && is_border(opposite(prev(v1_v0,g), g), g) ) return false; + + if ( !is_border(v0_v1,g) && !is_border(v1_v0,g) ) + { + if ( is_border(v0,g) && is_border(v1,g) ) + { + //CGAL_ECMS_TRACE(3," both p and q are boundary vertices but p-q is not. NON-COLLAPSABLE edge." ) ; + return false ; + } + else + { + if ( is_tetrahedron(v0_v1,g) ) + { + //CGAL_ECMS_TRACE(3," p-q belongs to a tetrahedron. NON-COLLAPSABLE edge." ) ; + return false ; + } + if ( next(v0_v1, g) == opposite(prev(v1_v0, g), g) && + prev(v0_v1, g) == opposite(next(v1_v0, g), g) ) + { + //CGAL_ECMS_TRACE(3," degenerate volume." ) ; + return false ; + } + } + } + + return true ; +} /** * collapses an edge in a graph. @@ -1408,6 +1508,8 @@ typename boost::graph_traits::vertex_descriptor collapse_edge(typename boost::graph_traits::edge_descriptor e, Graph& g) { + CGAL_precondition(does_satisfy_link_condition(e,g)); + typedef boost::graph_traits< Graph > Traits; typedef typename Traits::vertex_descriptor vertex_descriptor; typedef typename Traits::halfedge_descriptor halfedge_descriptor; @@ -1526,6 +1628,8 @@ collapse_edge(typename boost::graph_traits::edge_descriptor v0v1, Graph& g , EdgeIsConstrainedMap Edge_is_constrained_map) { + CGAL_precondition(does_satisfy_link_condition(v0v1,g)); + typedef boost::graph_traits< Graph > Traits; typedef typename Traits::vertex_descriptor vertex_descriptor; typedef typename Traits::halfedge_descriptor halfedge_descriptor; @@ -1697,107 +1801,6 @@ flip_edge(typename boost::graph_traits::halfedge_descriptor h, set_halfedge(foh,oh,g); } -/** - * \returns `true` if `e` satisfies the *link condition* \cgalCite{degn-tpec-98}, which guarantees that the surface is also 2-manifold after the edge collapse. - */ -template -bool -does_satisfy_link_condition(typename boost::graph_traits::edge_descriptor e, - const Graph& g) -{ - typedef typename boost::graph_traits::vertex_descriptor vertex_descriptor; - typedef typename boost::graph_traits::halfedge_descriptor halfedge_descriptor; - typedef CGAL::Halfedge_around_source_iterator out_edge_iterator; - - halfedge_descriptor v0_v1 = halfedge(e,g); - halfedge_descriptor v1_v0 = opposite(v0_v1,g); - - vertex_descriptor v0 = target(v1_v0,g), v1 = target(v0_v1,g); - - vertex_descriptor vL = target(next(v0_v1,g),g); - vertex_descriptor vR = target(next(v1_v0,g),g); - - out_edge_iterator eb1, ee1 ; - out_edge_iterator eb2, ee2 ; - - // The following loop checks the link condition for v0_v1. - // Specifically, that for every vertex 'k' adjacent to both 'p and 'q', 'pkq' is a face of the mesh. - // - for ( boost::tie(eb1,ee1) = halfedges_around_source(v0,g) ; eb1 != ee1 ; ++ eb1 ) - { - halfedge_descriptor v0_k = *eb1; - - if ( v0_k != v0_v1 ) - { - vertex_descriptor k = target(v0_k,g); - - for ( boost::tie(eb2,ee2) = halfedges_around_source(k,g) ; eb2 != ee2 ; ++ eb2 ) - { - halfedge_descriptor k_v1 = *eb2; - - if ( target(k_v1,g) == v1 ) - { - // At this point we know p-q-k are connected and we need to determine if this triangle is a face of the mesh. - // - // Since the mesh is known to be triangular there are at most two faces sharing the edge p-q. - // - // If p->q is NOT a border edge, the top face is p->q->t where t is target(next(p->q)) - // If q->p is NOT a border edge, the bottom face is q->p->b where b is target(next(q->p)) - // - // If k is either t or b then p-q-k *might* be a face of the mesh. It won't be if k==t but p->q is border - // or k==b but q->b is a border (because in that case even though there exists triangles p->q->t (or q->p->b) - // they are holes, not faces) - // - - bool lIsFace = ( vL == k && (! is_border(v0_v1,g)) ) - || ( vR == k && (! is_border(v1_v0,g)) ) ; - - if ( !lIsFace ) - { - // CGAL_ECMS_TRACE(3," k=V" << get(Vertex_index_map,k) << " IS NOT in a face with p-q. NON-COLLAPSABLE edge." ) ; - return false ; - } - else - { - //CGAL_ECMS_TRACE(4," k=V" << get(Vertex_index_map,k) << " is in a face with p-q") ; - } - } - } - } - } - - // detect isolated triangle (or triangle attached to a mesh with non-manifold vertices) - if (!is_border(v0_v1,g) && is_border(opposite(next(v0_v1,g), g), g) - && is_border(opposite(prev(v0_v1,g), g), g) ) return false; - if (!is_border(v1_v0,g) && is_border(opposite(next(v1_v0,g), g), g) - && is_border(opposite(prev(v1_v0,g), g), g) ) return false; - - if ( !is_border(v0_v1,g) && !is_border(v1_v0,g) ) - { - if ( is_border(v0,g) && is_border(v1,g) ) - { - //CGAL_ECMS_TRACE(3," both p and q are boundary vertices but p-q is not. NON-COLLAPSABLE edge." ) ; - return false ; - } - else - { - if ( is_tetrahedron(v0_v1,g) ) - { - //CGAL_ECMS_TRACE(3," p-q belongs to a tetrahedron. NON-COLLAPSABLE edge." ) ; - return false ; - } - if ( next(v0_v1, g) == opposite(prev(v1_v0, g), g) && - prev(v0_v1, g) == opposite(next(v1_v0, g), g) ) - { - //CGAL_ECMS_TRACE(3," degenerate volume." ) ; - return false ; - } - } - } - - return true ; -} - #ifndef CGAL_NO_DEPRECATED_CODE /// \cond SKIP_IN_MANUAL template From c78c323d089255aba0c4dba14c82d4beb93f4c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 11 May 2022 17:21:59 +0200 Subject: [PATCH 05/16] check for all kinds of degenerate faces --- .../Polygon_mesh_processing/internal/Snapping/snap.h | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index a543b6e250d..92c8c1efdfd 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -151,9 +151,12 @@ void simplify_range(HalfedgeRange& halfedge_range, new_tolerance += CGAL::approximate_sqrt(CGAL::squared_distance(new_p, pt)); } + // check that the collapse does not create a new degenerate face bool call_continue = false; for (halfedge_descriptor he : halfedges_around_target(h, tm)) - if (he!=h && get(vpm, source(he, tm))==new_p) + if ( he!=h && + !is_border(he, tm) && + collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { call_continue = true; break; @@ -161,7 +164,9 @@ void simplify_range(HalfedgeRange& halfedge_range, if (call_continue) continue; for (halfedge_descriptor he : halfedges_around_target(opposite(h,tm), tm)) - if (he!=opposite(h,tm) && get(vpm,source(he, tm))==new_p) + if (he!=opposite(h,tm) && + !is_border(he, tm) && + collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { call_continue = true; break; From a2316e3f2126b1eb80de4ffbfc580103181c9a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Thu, 8 Dec 2022 14:38:36 +0100 Subject: [PATCH 06/16] Minor improvements --- .../internal/Snapping/snap.h | 84 ++++++++++--------- .../internal/Snapping/snap_vertices.h | 39 ++++++--- 2 files changed, 70 insertions(+), 53 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 54fec36d6d2..eb2d0fa2703 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -153,25 +153,32 @@ void simplify_range(HalfedgeRange& halfedge_range, // check that the collapse does not create a new degenerate face bool call_continue = false; - for (halfedge_descriptor he : halfedges_around_target(h, tm)) - if ( he!=h && - !is_border(he, tm) && - collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) + for(halfedge_descriptor he : halfedges_around_target(h, tm)) + { + if(he != h && + !is_border(he, tm) && + collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { call_continue = true; break; } - if (call_continue) + } + + if(call_continue) continue; - for (halfedge_descriptor he : halfedges_around_target(opposite(h,tm), tm)) - if (he!=opposite(h,tm) && - !is_border(he, tm) && - collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) + + for(halfedge_descriptor he : halfedges_around_target(opposite(h,tm), tm)) + { + if(he != opposite(h,tm) && + !is_border(he, tm) && + collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { call_continue = true; break; } - if (call_continue) + } + + if(call_continue) continue; if(!CGAL::Euler::does_satisfy_link_condition(edge(h, tm), tm)) @@ -711,17 +718,16 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, bool do_split = true; - // Some splits can create degenerate faces, avoid that + // In case of self-snapping, avoid degenerate caps + const bool is_same_mesh = (&tm_T == &tm_S); + if(is_same_mesh && target(next(opposite(h_to_split, tm_T), tm_T), tm_T) == splitter_v) + do_split = false; + + // Do not split if it would create a degenerate needle if((new_position == get(vpm_T, target(h_to_split, tm_T))) || (new_position == get(vpm_T, source(h_to_split, tm_T)))) do_split = false; - //in case of self_snapping avoid pinching - if(&tm_T==&tm_S && target(next(opposite(h_to_split, tm_T), tm_T), tm_T)==splitter_v) - { - do_split = false; - } - if(!first_split && new_position == previous_split_position) do_split = false; @@ -733,30 +739,30 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, std::cout << "Actually split? " << do_split << std::endl; #endif - // check the new faces after split will not be degenerated - Point p0 = new_position; + // check if the new faces after split will not be degenerated + const Point& p0 = new_position; Point_ref p1 = get(vpm_T, source(h_to_split, tm_T)); Point_ref p2 = get(vpm_T, target(next(opposite(h_to_split, tm_T), tm_T), tm_T)); Point_ref p3 = get(vpm_T, target(h_to_split, tm_T)); - /* Chooses the diagonal that will split the quad in two triangles that maximize + /* Chooses the diagonal that will split the quad in two triangles that maximizes * the scalar product of of the un-normalized normals of the two triangles. + * * The lengths of the un-normalized normals (computed using cross-products of two vectors) - * are proportional to the area of the triangles. - * Maximize the scalar product of the two normals will avoid skinny triangles, - * and will also taken into account the cosine of the angle between the two normals. + * are proportional to the area of the triangles. + * Maximizing the scalar product of the two normals will avoid skinny triangles, + * and will also take into account the cosine of the angle between the two normals. + * * In particular, if the two triangles are oriented in different directions, * the scalar product will be negative. */ - auto p1p3 = CGAL::cross_product(p2-p1,p3-p2) * CGAL::cross_product(p0-p3,p1-p0); - auto p0p2 = CGAL::cross_product(p1-p0,p1-p2) * CGAL::cross_product(p3-p2,p3-p0); - bool first_split_face = (p0p2>p1p3); + auto p1p3 = CGAL::cross_product(p2-p1, p3-p2) * CGAL::cross_product(p0-p3, p1-p0); + auto p0p2 = CGAL::cross_product(p1-p0, p1-p2) * CGAL::cross_product(p3-p2, p3-p0); + bool first_split_face = (p0p2 > p1p3); - bool is_deg = first_split_face - ? collinear(p0,p1,p2) && collinear(p0,p3,p2) - : collinear(p0,p1,p3) && collinear(p1,p2,p3); - - if (is_deg) + bool is_deg = first_split_face ? collinear(p0,p1,p2) && collinear(p0,p3,p2) + : collinear(p0,p1,p3) && collinear(p1,p2,p3); + if(is_deg) do_split = false; // Split and update positions @@ -775,14 +781,15 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, if(!is_source_mesh_fixed) { bool skip = false; - for (halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) + for(halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) { - if (!is_border(h,tm_S) && collinear(get(vpm_S, source(h,tm_S)), new_position, get(vpm_S, target(next(h,tm_S),tm_S)))) + if(!is_border(h,tm_S) && collinear(get(vpm_S, source(h,tm_S)), new_position, get(vpm_S, target(next(h,tm_S),tm_S)))) { - skip=true; + skip = true; break; } } + if(!skip) put(vpm_S, splitter_v, new_position); } @@ -791,8 +798,6 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, previous_split_position = new_position; ++snapped_n; - // Everything below is choosing the diagonal to triangulate the quad formed by the edge split - // So, it's only relevant if splitting has been performed if(!do_split) continue; @@ -804,10 +809,9 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, const halfedge_descriptor h_to_split_opp = opposite(h_to_split, tm_T); halfedge_descriptor h2 = prev(prev(h_to_split_opp, tm_T), tm_T); const halfedge_descriptor res = prev(h_to_split, tm_T); - halfedge_descriptor new_hd = first_split_face - ? CGAL::Euler::split_face(v0, v2, tm_T) - : CGAL::Euler::split_face(v1, v3, tm_T); - if (first_split_face) + halfedge_descriptor new_hd = first_split_face ? CGAL::Euler::split_face(v0, v2, tm_T) + : CGAL::Euler::split_face(v1, v3, tm_T); + if(first_split_face) { h_to_split = opposite(prev(new_hd, tm_T), tm_T); visitor.after_split_face(h_to_split_opp, h2, tm_T); diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h index 8f0a03d4171..e5460f471ff 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap_vertices.h @@ -706,6 +706,7 @@ std::size_t snap_vertices_two_way(const HalfedgeRange_A& halfedge_range_A, typedef typename GetGeomTraits::type GT; typedef typename GT::FT FT; typedef typename boost::property_traits::value_type Point; + typedef typename boost::property_traits::reference Point_ref; typedef std::vector Vertex_container; typedef std::pair Unique_vertex; @@ -955,17 +956,21 @@ std::size_t snap_vertices_two_way(const HalfedgeRange_A& halfedge_range_A, { if(is_second_mesh_fixed) { - const Point& new_p = get(vpm_B, vb); + const Point_ref new_p = get(vpm_B, vb); for(const halfedge_descriptor ha : vs_a) { bool skip = false; - for (halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) - if (!is_border(haa,tm_A) && collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) + for(halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) + { + if(!is_border(haa,tm_A) && + collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) { - skip=true; + skip = true; break; } - if (!skip) + } + + if(!skip) put(vpm_A, target(ha, tm_A), new_p); } } @@ -985,26 +990,34 @@ std::size_t snap_vertices_two_way(const HalfedgeRange_A& halfedge_range_A, for(const halfedge_descriptor ha : vs_a) { bool skip = false; - for (halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) - if (!is_border(haa,tm_A) && collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) + for(halfedge_descriptor haa : halfedges_around_target(ha, tm_A)) + { + if(!is_border(haa,tm_A) && + collinear(get(vpm_A, source(haa,tm_A)), new_p, get(vpm_A, target(next(haa,tm_A),tm_A)))) { - skip=true; + skip = true; break; } - if (!skip) + } + + if(!skip) put(vpm_A, target(ha, tm_A), new_p); } for(const halfedge_descriptor hb : vs_b) { bool skip = false; - for (halfedge_descriptor hbb : halfedges_around_target(hb, tm_B)) - if (!is_border(hbb,tm_B) && collinear(get(vpm_B, source(hbb,tm_B)), new_p, get(vpm_B, target(next(hbb,tm_B),tm_B)))) + for(halfedge_descriptor hbb : halfedges_around_target(hb, tm_B)) + { + if(!is_border(hbb,tm_B) && + collinear(get(vpm_B, source(hbb,tm_B)), new_p, get(vpm_B, target(next(hbb,tm_B),tm_B)))) { - skip=true; + skip = true; break; } - if (!skip) + } + + if(!skip) put(vpm_B, target(hb, tm_B), new_p); } } From ef41988c644774d9200433ec2f56a009b35541f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 12 Apr 2023 14:59:19 +0200 Subject: [PATCH 07/16] only one deg face is sufficient to cancel the split --- .../CGAL/Polygon_mesh_processing/internal/Snapping/snap.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index eb2d0fa2703..71c55a4d5cc 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -760,8 +760,8 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, auto p0p2 = CGAL::cross_product(p1-p0, p1-p2) * CGAL::cross_product(p3-p2, p3-p0); bool first_split_face = (p0p2 > p1p3); - bool is_deg = first_split_face ? collinear(p0,p1,p2) && collinear(p0,p3,p2) - : collinear(p0,p1,p3) && collinear(p1,p2,p3); + bool is_deg = first_split_face ? collinear(p0,p1,p2) || collinear(p0,p3,p2) + : collinear(p0,p1,p3) || collinear(p1,p2,p3); if(is_deg) do_split = false; From 6c2c6a580bdf94b1f64bb0a98cc129e7a339ace6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 12 Apr 2023 15:07:16 +0200 Subject: [PATCH 08/16] remove useless variable and use existing ones --- .../internal/Snapping/snap.h | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 71c55a4d5cc..27a20fb2d0e 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -806,21 +806,12 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, v1 = next(v0, tm_T); v2 = next(v1, tm_T); v3 = next(v2, tm_T); - const halfedge_descriptor h_to_split_opp = opposite(h_to_split, tm_T); - halfedge_descriptor h2 = prev(prev(h_to_split_opp, tm_T), tm_T); - const halfedge_descriptor res = prev(h_to_split, tm_T); halfedge_descriptor new_hd = first_split_face ? CGAL::Euler::split_face(v0, v2, tm_T) : CGAL::Euler::split_face(v1, v3, tm_T); if(first_split_face) - { - h_to_split = opposite(prev(new_hd, tm_T), tm_T); - visitor.after_split_face(h_to_split_opp, h2, tm_T); - } + visitor.after_split_face(v0, v2, tm_T); else - { - h_to_split = opposite(next(new_hd, tm_T), tm_T); - visitor.after_split_face(opposite(res, tm_T), h2, tm_T); - } + visitor.after_split_face(v1, v3, tm_T); } } From 37361b757b50d09440f6c3b918987403d64af491 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 12 Apr 2023 15:13:15 +0200 Subject: [PATCH 09/16] do not split if after split faces are not consistently oriented --- .../internal/Snapping/snap.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 27a20fb2d0e..111a5db061f 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -746,7 +746,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, Point_ref p3 = get(vpm_T, target(h_to_split, tm_T)); /* Chooses the diagonal that will split the quad in two triangles that maximizes - * the scalar product of of the un-normalized normals of the two triangles. + * the scalar product of the un-normalized normals of the two triangles. * * The lengths of the un-normalized normals (computed using cross-products of two vectors) * are proportional to the area of the triangles. @@ -760,9 +760,14 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, auto p0p2 = CGAL::cross_product(p1-p0, p1-p2) * CGAL::cross_product(p3-p2, p3-p0); bool first_split_face = (p0p2 > p1p3); - bool is_deg = first_split_face ? collinear(p0,p1,p2) || collinear(p0,p3,p2) - : collinear(p0,p1,p3) || collinear(p1,p2,p3); - if(is_deg) + if (p0p2 > 0 && p1p3 >0) + { + bool is_deg = first_split_face ? collinear(p0,p1,p2) || collinear(p0,p3,p2) + : collinear(p0,p1,p3) || collinear(p1,p2,p3); + if(is_deg) + do_split = false; + } + else do_split = false; // Split and update positions From 304c1394e5c1a26884a4dfaa02b8fcb55e44bd40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 12 Apr 2023 15:19:07 +0200 Subject: [PATCH 10/16] check source mesh before doing the split --- .../internal/Snapping/snap.h | 36 +++++++++---------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 111a5db061f..3ce43a03137 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -770,6 +770,21 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, else do_split = false; + if(do_split && !is_source_mesh_fixed) + { + for(halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) + { + if(!is_border(h,tm_S) && collinear(get(vpm_S, source(h,tm_S)), new_position, get(vpm_S, target(next(h,tm_S),tm_S)))) + { + do_split = false; + break; + } + } + + if(do_split) + put(vpm_S, splitter_v, new_position); + } + // Split and update positions vertex_descriptor new_v = boost::graph_traits::null_vertex(); if(do_split) @@ -782,30 +797,13 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, visitor.after_vertex_edge_snap(new_v, tm_T); } - - if(!is_source_mesh_fixed) - { - bool skip = false; - for(halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) - { - if(!is_border(h,tm_S) && collinear(get(vpm_S, source(h,tm_S)), new_position, get(vpm_S, target(next(h,tm_S),tm_S)))) - { - skip = true; - break; - } - } - - if(!skip) - put(vpm_S, splitter_v, new_position); - } + else + continue; first_split = false; previous_split_position = new_position; ++snapped_n; - if(!do_split) - continue; - halfedge_descriptor v0, v1, v2, v3; v0 = opposite(h_to_split, tm_T); v1 = next(v0, tm_T); From f3f6783e8ad9fa684e50c03d9f8efae63da62584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 19 Apr 2023 03:24:01 +0200 Subject: [PATCH 11/16] update split candidate choice in case of presence of deg faces --- .../internal/Snapping/snap.h | 34 ++++++++++++++++--- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 3ce43a03137..425de7dd3ca 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -760,15 +760,39 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, auto p0p2 = CGAL::cross_product(p1-p0, p1-p2) * CGAL::cross_product(p3-p2, p3-p0); bool first_split_face = (p0p2 > p1p3); - if (p0p2 > 0 && p1p3 >0) + if (first_split_face) { - bool is_deg = first_split_face ? collinear(p0,p1,p2) || collinear(p0,p3,p2) - : collinear(p0,p1,p3) || collinear(p1,p2,p3); - if(is_deg) + if(p0p2<0) do_split = false; + else + { + bool is_deg = collinear(p0,p1,p2) || collinear(p0,p3,p2); + if (is_deg) + { + if ( p1p3>0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3)) + first_split_face=false; + else + do_split = false; + } + } } else - do_split = false; + { + if(p1p3<0) + do_split = false; + else + { + bool is_deg = collinear(p0,p1,p3) || collinear(p1,p2,p3); + if (is_deg) + { + if ( p1p3>0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2)) + first_split_face=false; + else + do_split = false; + } + } + } + if(do_split && !is_source_mesh_fixed) { From 3705a4cfefe8806683063df5c42092c08ce927a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Wed, 19 Apr 2023 04:09:46 +0200 Subject: [PATCH 12/16] fix typo --- .../CGAL/Polygon_mesh_processing/internal/Snapping/snap.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 425de7dd3ca..7b62fa19a3e 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -769,7 +769,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, bool is_deg = collinear(p0,p1,p2) || collinear(p0,p3,p2); if (is_deg) { - if ( p1p3>0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3)) + if ( p1p3>0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3))) first_split_face=false; else do_split = false; @@ -785,7 +785,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, bool is_deg = collinear(p0,p1,p3) || collinear(p1,p2,p3); if (is_deg) { - if ( p1p3>0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2)) + if ( p1p3>0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2))) first_split_face=false; else do_split = false; From afd096cb91ac530640d82eeea482c0a829797a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 22 May 2023 12:30:38 +0200 Subject: [PATCH 13/16] Fix indentation --- .../internal/Snapping/snap.h | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 7b62fa19a3e..e54d86a5c71 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -760,17 +760,19 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, auto p0p2 = CGAL::cross_product(p1-p0, p1-p2) * CGAL::cross_product(p3-p2, p3-p0); bool first_split_face = (p0p2 > p1p3); - if (first_split_face) + if(first_split_face) { - if(p0p2<0) + if(p0p2 < 0) + { do_split = false; + } else { bool is_deg = collinear(p0,p1,p2) || collinear(p0,p3,p2); - if (is_deg) + if(is_deg) { - if ( p1p3>0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3))) - first_split_face=false; + if(p1p3 > 0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3))) + first_split_face = false; else do_split = false; } @@ -778,22 +780,23 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, } else { - if(p1p3<0) + if(p1p3 < 0) + { do_split = false; + } else { bool is_deg = collinear(p0,p1,p3) || collinear(p1,p2,p3); - if (is_deg) + if(is_deg) { - if ( p1p3>0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2))) - first_split_face=false; + if(p1p3 > 0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2))) + first_split_face = false; else do_split = false; } } } - if(do_split && !is_source_mesh_fixed) { for(halfedge_descriptor h : halfedges_around_target(splitter_v, tm_S)) @@ -822,7 +825,9 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, visitor.after_vertex_edge_snap(new_v, tm_T); } else + { continue; + } first_split = false; previous_split_position = new_position; From 62ca5751708fe4dd0cc92943b40a1aa6d2be8733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 22 May 2023 14:01:07 +0200 Subject: [PATCH 14/16] Simplify expression --- .../internal/Snapping/snap.h | 30 ++----------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index e54d86a5c71..2bf04753516 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -762,39 +762,13 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, if(first_split_face) { - if(p0p2 < 0) - { + if(p0p2 <= 0 || collinear(p0,p1,p2) || collinear(p0,p2,p3)) do_split = false; - } - else - { - bool is_deg = collinear(p0,p1,p2) || collinear(p0,p3,p2); - if(is_deg) - { - if(p1p3 > 0 && !(collinear(p0,p1,p3) || collinear(p1,p2,p3))) - first_split_face = false; - else - do_split = false; - } - } } else { - if(p1p3 < 0) - { + if(p1p3 <= 0 || collinear(p0,p1,p3) || collinear(p1,p2,p3)) do_split = false; - } - else - { - bool is_deg = collinear(p0,p1,p3) || collinear(p1,p2,p3); - if(is_deg) - { - if(p1p3 > 0 && !(collinear(p0,p1,p2) || collinear(p0,p3,p2))) - first_split_face = false; - else - do_split = false; - } - } } if(do_split && !is_source_mesh_fixed) From 7bf284fafa42028bff3fe7ef540baa7ead0bdd2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mael=20Rouxel-Labb=C3=A9?= Date: Mon, 22 May 2023 14:01:17 +0200 Subject: [PATCH 15/16] Misc minor changes --- .../internal/Snapping/snap.h | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index 2bf04753516..f3444393436 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -152,19 +152,19 @@ void simplify_range(HalfedgeRange& halfedge_range, } // check that the collapse does not create a new degenerate face - bool call_continue = false; + bool do_collapse = true; for(halfedge_descriptor he : halfedges_around_target(h, tm)) { if(he != h && !is_border(he, tm) && collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { - call_continue = true; + do_collapse = false; break; } } - if(call_continue) + if(!do_collapse) continue; for(halfedge_descriptor he : halfedges_around_target(opposite(h,tm), tm)) @@ -173,12 +173,12 @@ void simplify_range(HalfedgeRange& halfedge_range, !is_border(he, tm) && collinear(get(vpm, source(he, tm)), new_p, get(vpm, target(next(he,tm),tm)))) { - call_continue = true; + do_collapse = false; break; } } - if(call_continue) + if(!do_collapse) continue; if(!CGAL::Euler::does_satisfy_link_condition(edge(h, tm), tm)) @@ -731,15 +731,7 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, if(!first_split && new_position == previous_split_position) do_split = false; -#ifdef CGAL_PMP_SNAP_DEBUG_PP - std::cout << " -.-.-. Splitting " << edge(h_to_split, tm_T) << " |||| " - << " Vs " << source(h_to_split, tm_T) << " (" << tm_T.point(source(h_to_split, tm_T)) << ")" - << " --- Vt " << target(h_to_split, tm_T) << " (" << tm_T.point(target(h_to_split, tm_T)) << ")" << std::endl; - std::cout << "With point: " << vnp.second << std::endl; - std::cout << "Actually split? " << do_split << std::endl; -#endif - - // check if the new faces after split will not be degenerated + // check if the new faces after split will not be degenerate const Point& p0 = new_position; Point_ref p1 = get(vpm_T, source(h_to_split, tm_T)); Point_ref p2 = get(vpm_T, target(next(opposite(h_to_split, tm_T), tm_T), tm_T)); @@ -786,6 +778,14 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, put(vpm_S, splitter_v, new_position); } +#ifdef CGAL_PMP_SNAP_DEBUG_PP + std::cout << " -.-.-. Splitting " << edge(h_to_split, tm_T) << " |||| " + << " Vs " << source(h_to_split, tm_T) << " (" << tm_T.point(source(h_to_split, tm_T)) << ")" + << " --- Vt " << target(h_to_split, tm_T) << " (" << tm_T.point(target(h_to_split, tm_T)) << ")" << std::endl; + std::cout << "With point: " << new_position << " (init: " << vnp.second << ")" << std::endl; + std::cout << "Actually split? " << do_split << std::endl; +#endif + // Split and update positions vertex_descriptor new_v = boost::graph_traits::null_vertex(); if(do_split) From ec274dd8e9f7541c300f6410e6bbac810d298a2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Loriot?= Date: Tue, 23 May 2023 18:18:28 +0200 Subject: [PATCH 16/16] hide non used variable --- .../CGAL/Polygon_mesh_processing/internal/Snapping/snap.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h index f3444393436..e30fa8703c3 100644 --- a/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h +++ b/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Snapping/snap.h @@ -812,8 +812,9 @@ std::size_t split_edges(EdgesToSplitContainer& edges_to_split, v1 = next(v0, tm_T); v2 = next(v1, tm_T); v3 = next(v2, tm_T); - halfedge_descriptor new_hd = first_split_face ? CGAL::Euler::split_face(v0, v2, tm_T) - : CGAL::Euler::split_face(v1, v3, tm_T); + // halfedge_descriptor new_hd = + first_split_face ? CGAL::Euler::split_face(v0, v2, tm_T) + : CGAL::Euler::split_face(v1, v3, tm_T); if(first_split_face) visitor.after_split_face(v0, v2, tm_T); else