From 2cb82ec9b9fbb930632e8d1ee516eb74a2915272 Mon Sep 17 00:00:00 2001 From: Max Lyon Date: Mon, 3 Feb 2020 09:51:17 +0100 Subject: [PATCH 1/3] add more unittests for subdivision with deleted elements with and without garbage collection --- src/Unittests/unittests_subdivider_uniform.cc | 403 +++++++++++++++++- 1 file changed, 394 insertions(+), 9 deletions(-) diff --git a/src/Unittests/unittests_subdivider_uniform.cc b/src/Unittests/unittests_subdivider_uniform.cc index 52a0cabc..835c89cf 100644 --- a/src/Unittests/unittests_subdivider_uniform.cc +++ b/src/Unittests/unittests_subdivider_uniform.cc @@ -150,6 +150,135 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Sqrt3) { } +TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Sqrt3_delete_vertex) { + + for (bool collect_garbage : { false, true }) + { + mesh_.clear(); + + // Request status flags to use delete and garbage collection + mesh_.request_vertex_status(); + mesh_.request_halfedge_status(); + mesh_.request_edge_status(); + mesh_.request_face_status(); + + // Add some vertices + Mesh::VertexHandle vhandle[9]; + + vhandle[0] = mesh_.add_vertex(Mesh::Point(0, 0, 0)); + vhandle[1] = mesh_.add_vertex(Mesh::Point(0, 1, 0)); + vhandle[2] = mesh_.add_vertex(Mesh::Point(0, 2, 0)); + vhandle[3] = mesh_.add_vertex(Mesh::Point(1, 0, 0)); + vhandle[4] = mesh_.add_vertex(Mesh::Point(1, 1, 0)); + vhandle[5] = mesh_.add_vertex(Mesh::Point(1, 2, 0)); + vhandle[6] = mesh_.add_vertex(Mesh::Point(2, 0, 0)); + vhandle[7] = mesh_.add_vertex(Mesh::Point(2, 1, 0)); + vhandle[8] = mesh_.add_vertex(Mesh::Point(2, 2, 0)); + + // Add eight faces + std::vector face_vhandles; + + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[3]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[4]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[4]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[4]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[3]); + face_vhandles.push_back(vhandle[7]); + face_vhandles.push_back(vhandle[6]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[3]); + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[7]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[8]); + face_vhandles.push_back(vhandle[7]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[8]); + + mesh_.add_face(face_vhandles); + + // Test setup: + // 6 === 7 === 8 + // | / | / | + // | / | / | + // | / | / | + // 3 === 4 === 5 + // | / | \ | + // | / | \ | + // | / | \ | + // 0 === 1 === 2 + + // Delete one vertex + mesh_.delete_vertex(vhandle[1]); + // Check setup + if (collect_garbage) + { + mesh_.garbage_collection(); + EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(6u, mesh_.n_faces() ) << "Wrong number of faces"; + } + else + { + EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(8u, mesh_.n_faces() ) << "Wrong number of faces"; + } + + + // Initialize subdivider + OpenMesh::Subdivider::Uniform::Sqrt3T sqrt3; + + + // Execute 3 subdivision steps + sqrt3.attach(mesh_); + sqrt3( 3 ); + sqrt3.detach(); + + if (!collect_garbage) + mesh_.garbage_collection(); // if we did not collect garbage before, do so now + + // Check setup + EXPECT_EQ(94u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with sqrt3"; + EXPECT_EQ(162u, mesh_.n_faces() ) << "Wrong number of faces after subdivision with sqrt3"; + } +} + + TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop) { mesh_.clear(); @@ -255,6 +384,9 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop) { TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop_delete_vertex) { + + for (bool collect_garbage : { false, true }) + { mesh_.clear(); // Request status flags to use delete and garbage collection @@ -345,26 +477,39 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop_delete_vertex) { // | / | \ | // 0 === 1 === 2 - // Delete one vertex mesh_.delete_vertex(vhandle[1]); - mesh_.garbage_collection(); + if (collect_garbage) + { + mesh_.garbage_collection(); + // Check setup + EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(6u, mesh_.n_faces() ) << "Wrong number of faces"; + } + else + { + // Check setup + EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(8u, mesh_.n_faces() ) << "Wrong number of faces"; + } // Initialize subdivider OpenMesh::Subdivider::Uniform::LoopT loop; - // Check setup - EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices"; - EXPECT_EQ(6u, mesh_.n_faces() ) << "Wrong number of faces"; // Execute 3 subdivision steps loop.attach(mesh_); loop( 3 ); loop.detach(); + if (!collect_garbage) + mesh_.garbage_collection(); // if we did not collect garbage before, do so now + // Check setup EXPECT_EQ(225u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with loop"; EXPECT_EQ(384u, mesh_.n_faces() ) << "Wrong number of faces after subdivision with loop"; + + } } @@ -454,6 +599,108 @@ TEST_F(OpenMeshSubdividerUniform_Poly, Subdivider_CatmullClark) { EXPECT_EQ(256u, mesh_.n_faces() ) << "Wrong number of faces after subdivision with catmull clark"; } + +TEST_F(OpenMeshSubdividerUniform_Poly, Subdivider_CatmullClark_delete_vertex) { + + for (bool collect_garbage : { false, true }) + { + mesh_.clear(); + + // Request status flags to use delete and garbage collection + mesh_.request_vertex_status(); + mesh_.request_halfedge_status(); + mesh_.request_edge_status(); + mesh_.request_face_status(); + + // Add some vertices + Mesh::VertexHandle vhandle[9]; + + vhandle[0] = mesh_.add_vertex(Mesh::Point(0, 0, 0)); + vhandle[1] = mesh_.add_vertex(Mesh::Point(0, 1, 0)); + vhandle[2] = mesh_.add_vertex(Mesh::Point(0, 2, 0)); + vhandle[3] = mesh_.add_vertex(Mesh::Point(1, 0, 0)); + vhandle[4] = mesh_.add_vertex(Mesh::Point(1, 1, 0)); + vhandle[5] = mesh_.add_vertex(Mesh::Point(1, 2, 0)); + vhandle[6] = mesh_.add_vertex(Mesh::Point(2, 0, 0)); + vhandle[7] = mesh_.add_vertex(Mesh::Point(2, 1, 0)); + vhandle[8] = mesh_.add_vertex(Mesh::Point(2, 2, 0)); + + // Add four faces + std::vector face_vhandles; + + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[3]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[4]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[8]); + face_vhandles.push_back(vhandle[7]); + + mesh_.add_face(face_vhandles); + face_vhandles.clear(); + + face_vhandles.push_back(vhandle[3]); + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[7]); + face_vhandles.push_back(vhandle[6]); + + mesh_.add_face(face_vhandles); + + // Test setup: + // 6 === 7 === 8 + // | | | + // | | | + // | | | + // 3 === 4 === 5 + // | | | + // | | | + // | | | + // 0 === 1 === 2 + + + mesh_.delete_vertex(vhandle[1]); + // Check setup + if (collect_garbage) + { + mesh_.garbage_collection(); + EXPECT_EQ(6u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(2u, mesh_.n_faces() ) << "Wrong number of faces"; + } + else + { + EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(4u, mesh_.n_faces() ) << "Wrong number of faces"; + } + + // Initialize subdivider + OpenMesh::Subdivider::Uniform::CatmullClarkT catmull; + + // Execute 3 subdivision steps + catmull.attach(mesh_); + catmull( 3 ); + catmull.detach(); + + if (!collect_garbage) + mesh_.garbage_collection(); // if we did not collect garbage before, do so now + + EXPECT_EQ(153u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with catmull clark"; + EXPECT_EQ(128u, mesh_.n_faces() ) << "Wrong number of faces after subdivision with catmull clark"; + } +} + /* Adds a cube to a polymesh */ TEST_F(OpenMeshSubdividerUniform_Poly, Midpoint) { @@ -552,6 +799,129 @@ TEST_F(OpenMeshSubdividerUniform_Poly, Midpoint) { } +TEST_F(OpenMeshSubdividerUniform_Poly, Midpoint_delete_vertex) { + + for (bool collect_garbage : { false, true }) + { + mesh_.clear(); + + // Request status flags to use delete and garbage collection + mesh_.request_vertex_status(); + mesh_.request_halfedge_status(); + mesh_.request_edge_status(); + mesh_.request_face_status(); + + // Add some vertices + Mesh::VertexHandle vhandle[8]; + vhandle[0] = mesh_.add_vertex(PolyMesh::Point(-1, -1, 1)); + vhandle[1] = mesh_.add_vertex(PolyMesh::Point( 1, -1, 1)); + vhandle[2] = mesh_.add_vertex(PolyMesh::Point( 1, 1, 1)); + vhandle[3] = mesh_.add_vertex(PolyMesh::Point(-1, 1, 1)); + vhandle[4] = mesh_.add_vertex(PolyMesh::Point(-1, -1, -1)); + vhandle[5] = mesh_.add_vertex(PolyMesh::Point( 1, -1, -1)); + vhandle[6] = mesh_.add_vertex(PolyMesh::Point( 1, 1, -1)); + vhandle[7] = mesh_.add_vertex(PolyMesh::Point(-1, 1, -1)); + + // Add six faces to form a cube + std::vector face_vhandles; + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[3]); + mesh_.add_face(face_vhandles); + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[7]); + face_vhandles.push_back(vhandle[6]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[4]); + mesh_.add_face(face_vhandles); + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[4]); + face_vhandles.push_back(vhandle[5]); + mesh_.add_face(face_vhandles); + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[1]); + face_vhandles.push_back(vhandle[5]); + face_vhandles.push_back(vhandle[6]); + mesh_.add_face(face_vhandles); + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[3]); + face_vhandles.push_back(vhandle[2]); + face_vhandles.push_back(vhandle[6]); + face_vhandles.push_back(vhandle[7]); + mesh_.add_face(face_vhandles); + + face_vhandles.clear(); + face_vhandles.push_back(vhandle[0]); + face_vhandles.push_back(vhandle[3]); + face_vhandles.push_back(vhandle[7]); + face_vhandles.push_back(vhandle[4]); + mesh_.add_face(face_vhandles); + + + // Test setup: + // + // + // 3 ======== 2 + // / /| + // / / | z + // 0 ======== 1 | | + // | | | | y + // | 7 | 6 | / + // | | / | / + // | |/ |/ + // 4 ======== 5 -------> x + // + + mesh_.delete_vertex(vhandle[1]); + // Check setup + if (collect_garbage) + { + mesh_.garbage_collection(); + EXPECT_EQ(9u, mesh_.n_edges()) << "Wrong number of Edges"; + EXPECT_EQ(18u, mesh_.n_halfedges()) << "Wrong number of HalfEdges"; + EXPECT_EQ(7u, mesh_.n_vertices()) << "Wrong number of vertices"; + EXPECT_EQ(3u, mesh_.n_faces()) << "Wrong number of faces"; + } + else + { + EXPECT_EQ(12u, mesh_.n_edges()) << "Wrong number of Edges"; + EXPECT_EQ(24u, mesh_.n_halfedges()) << "Wrong number of HalfEdges"; + EXPECT_EQ(8u, mesh_.n_vertices()) << "Wrong number of vertices"; + EXPECT_EQ(6u, mesh_.n_faces()) << "Wrong number of faces"; + } + + // Check setup + + // Initialize subdivider + OpenMesh::Subdivider::Uniform::MidpointT midpoint; + + // Execute 2 subdivision steps + midpoint.attach(mesh_); + midpoint(2); + midpoint.detach(); + + if (!collect_garbage) + mesh_.garbage_collection(); // if we did not collect garbage before, do so now + + // Check Result + EXPECT_EQ(15u, mesh_.n_edges()) << "Wrong number of Edges"; + EXPECT_EQ(30u, mesh_.n_halfedges()) << "Wrong number of HalfEdges"; + EXPECT_EQ(12u, mesh_.n_vertices()) << "Wrong number of vertices"; + EXPECT_EQ(4u, mesh_.n_faces()) << "Wrong number of faces"; + } +} + + TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly) { mesh_.clear(); @@ -658,6 +1028,9 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly) { TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly_delete_vertex) { + + for (bool collect_garbage : { false, true }) + { mesh_.clear(); // Request status flags to use delete and garbage collection @@ -751,23 +1124,35 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly_delete_vertex) { // Delete one vertex mesh_.delete_vertex(vhandle[1]); - mesh_.garbage_collection(); + // Check setup + if (collect_garbage) + { + mesh_.garbage_collection(); + EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(6u, mesh_.n_faces() ) << "Wrong number of faces"; + } + else + { + EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices"; + EXPECT_EQ(8u, mesh_.n_faces() ) << "Wrong number of faces"; + } // Initialize subdivider OpenMesh::Subdivider::Uniform::ModifiedButterflyT butter; - // Check setup - EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices"; - EXPECT_EQ(6u, mesh_.n_faces() ) << "Wrong number of faces"; // Execute 3 subdivision steps butter.attach(mesh_); butter( 3 ); butter.detach(); + if (!collect_garbage) + mesh_.garbage_collection(); // if we did not collect garbage before, do so now + // Check setup EXPECT_EQ(225u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with butter"; EXPECT_EQ(384u, mesh_.n_faces() ) << "Wrong number of faces after subdivision with butter"; + } } From 9ae08da59341e983068cb9a64572d307a7b4b730 Mon Sep 17 00:00:00 2001 From: Max Lyon Date: Mon, 3 Feb 2020 09:51:49 +0100 Subject: [PATCH 2/3] use range based for loops in subdivision algorithms in order to skip deleted elements --- .../Subdivider/Uniform/CatmullClarkT_impl.hh | 35 +++++------- .../Tools/Subdivider/Uniform/LoopT.hh | 10 ++-- .../Tools/Subdivider/Uniform/MidpointT.hh | 24 +++------ .../Subdivider/Uniform/ModifiedButterFlyT.hh | 27 ++++------ .../Tools/Subdivider/Uniform/Sqrt3T.hh | 54 +++++++++---------- 5 files changed, 58 insertions(+), 92 deletions(-) diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT_impl.hh b/src/OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT_impl.hh index 40978930..e74e99a7 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT_impl.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT_impl.hh @@ -100,47 +100,38 @@ CatmullClarkT::subdivide( MeshType& _m , size_t _n , const bo { // Compute face centroid - FaceIter f_itr = _m.faces_begin(); - FaceIter f_end = _m.faces_end(); - for ( ; f_itr != f_end; ++f_itr) + for ( auto fh : _m.faces()) { Point centroid; - _m.calc_face_centroid( *f_itr, centroid); - _m.property( fp_pos_, *f_itr ) = centroid; + _m.calc_face_centroid( fh, centroid); + _m.property( fp_pos_, fh ) = centroid; } // Compute position for new (edge-) vertices and store them in the edge property - EdgeIter e_itr = _m.edges_begin(); - EdgeIter e_end = _m.edges_end(); - for ( ; e_itr != e_end; ++e_itr) - compute_midpoint( _m, *e_itr, _update_points ); + for ( auto eh : _m.edges()) + compute_midpoint( _m, eh, _update_points ); // position updates activated? if(_update_points) { // compute new positions for old vertices - VertexIter v_itr = _m.vertices_begin(); - VertexIter v_end = _m.vertices_end(); - for ( ; v_itr != v_end; ++v_itr) - update_vertex( _m, *v_itr ); + for ( auto vh : _m.vertices()) + update_vertex( _m, vh ); // Commit changes in geometry - v_itr = _m.vertices_begin(); - for ( ; v_itr != v_end; ++v_itr) - _m.set_point(*v_itr, _m.property( vp_pos_, *v_itr ) ); + for ( auto vh : _m.vertices()) + _m.set_point(vh, _m.property( vp_pos_, vh ) ); } // Split each edge at midpoint stored in edge property ep_pos_; // Attention! Creating new edges, hence make sure the loop ends correctly. - e_itr = _m.edges_begin(); - for ( ; e_itr != e_end; ++e_itr) - split_edge( _m, *e_itr ); + for ( auto eh : _m.edges()) + split_edge( _m, eh ); // Commit changes in topology and reconsitute consistency // Attention! Creating new faces, hence make sure the loop ends correctly. - f_itr = _m.faces_begin(); - for ( ; f_itr != f_end; ++f_itr) - split_face( _m, *f_itr); + for ( auto fh : _m.faces()) + split_face( _m, fh); #if defined(_DEBUG) || defined(DEBUG) diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/LoopT.hh b/src/OpenMesh/Tools/Subdivider/Uniform/LoopT.hh index f1d113b8..4601367c 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/LoopT.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/LoopT.hh @@ -174,17 +174,15 @@ protected: // edge property ep_pos_) in the vertex property vp_pos_; // Attention! Creating new edges, hence make sure the loop ends correctly. - e_end = _m.edges_end(); - for (eit=_m.edges_begin(); eit != e_end; ++eit) - split_edge(_m, *eit ); + for (auto eh : _m.edges()) + split_edge(_m, eh ); // Commit changes in topology and reconsitute consistency // Attention! Creating new faces, hence make sure the loop ends correctly. - f_end = _m.faces_end(); - for (fit = _m.faces_begin(); fit != f_end; ++fit) - split_face(_m, *fit ); + for (auto fh : _m.faces()) + split_face(_m, fh ); if(_update_points) { // Commit changes in geometry diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/MidpointT.hh b/src/OpenMesh/Tools/Subdivider/Uniform/MidpointT.hh index b22b7782..f92e2a43 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/MidpointT.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/MidpointT.hh @@ -57,43 +57,33 @@ protected: // SubdividerT interface for (size_t iteration = 0; iteration < _n; ++iteration) { is_original_vertex.set_range(_m.vertices_begin(), _m.vertices_end(), true); // Create vertices on edge midpoints - for (typename mesh_t::EdgeIter it = _m.edges_begin(), end = _m.edges_end(); it != end; ++it) { - EdgeHandle eh = *it; + for (auto eh : _m.edges()) { VertexHandle new_vh = _m.new_vertex(_m.calc_edge_midpoint(eh)); edge_midpoint[eh] = new_vh; is_original_vertex[new_vh] = false; } // Create new faces from original faces - for (typename mesh_t::FaceIter it = _m.faces_begin(), end = _m.faces_end(); it != end; ++it) { - FaceHandle fh = *it; + for (auto fh : _m.faces()) { std::vector new_corners; - for (typename mesh_t::FaceEdgeIter it = _m.fe_begin(fh), end = _m.fe_end(fh); it != end; ++it) { - EdgeHandle eh = *it; + for (auto eh : _m.fe_range(fh)) new_corners.push_back(edge_midpoint[eh]); - } _m.add_face(new_corners); } // Create new faces from original vertices - for (typename mesh_t::VertexIter it = _m.vertices_begin(), end = _m.vertices_end(); it != end; ++it) { - VertexHandle vh = *it; + for (auto vh : _m.vertices()) { if (is_original_vertex[vh]) { if (!_m.is_boundary(vh)) { std::vector new_corners; - for (typename mesh_t::VertexEdgeIter it = _m.ve_begin(vh), end = _m.ve_end(vh); it != end; ++it) { - EdgeHandle eh = *it; + for (auto eh : _m.ve_range(vh)) new_corners.push_back(edge_midpoint[eh]); - } std::reverse(new_corners.begin(), new_corners.end()); _m.add_face(new_corners); } } } - for (typename mesh_t::VertexIter it = _m.vertices_begin(), end = _m.vertices_end(); it != end; ++it) { - VertexHandle vh = *it; - if (is_original_vertex[vh]) { + for (auto vh : _m.vertices()) + if (is_original_vertex[vh]) _m.delete_vertex(vh); - } - } _m.garbage_collection(); } _m.release_face_status(); diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/ModifiedButterFlyT.hh b/src/OpenMesh/Tools/Subdivider/Uniform/ModifiedButterFlyT.hh index a9927f93..78b8f806 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/ModifiedButterFlyT.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/ModifiedButterFlyT.hh @@ -179,45 +179,38 @@ protected: ///TODO:Implement fixed positions - typename mesh_t::FaceIter fit, f_end; - typename mesh_t::EdgeIter eit, e_end; - typename mesh_t::VertexIter vit; - // Do _n subdivisions for (size_t i=0; i < _n; ++i) { // This is an interpolating scheme, old vertices remain the same. typename mesh_t::VertexIter initialVerticesEnd = _m.vertices_end(); - for ( vit = _m.vertices_begin(); vit != initialVerticesEnd; ++vit) - _m.property( vp_pos_, *vit ) = _m.point(*vit); + for ( auto vh : _m.vertices()) + _m.property( vp_pos_, vh ) = _m.point(vh); // Compute position for new vertices and store them in the edge property - for (eit=_m.edges_begin(); eit != _m.edges_end(); ++eit) - compute_midpoint( _m, *eit ); + for (auto eh : _m.edges()) + compute_midpoint( _m, eh); // Split each edge at midpoint and store precomputed positions (stored in // edge property ep_pos_) in the vertex property vp_pos_; // Attention! Creating new edges, hence make sure the loop ends correctly. - e_end = _m.edges_end(); - for (eit=_m.edges_begin(); eit != e_end; ++eit) - split_edge(_m, *eit ); + for (auto eh : _m.edges()) + split_edge(_m, eh ); // Commit changes in topology and reconsitute consistency // Attention! Creating new faces, hence make sure the loop ends correctly. - f_end = _m.faces_end(); - for (fit = _m.faces_begin(); fit != f_end; ++fit) - split_face(_m, *fit ); + for (auto fh : _m.faces()) + split_face(_m, fh ); // Commit changes in geometry - for ( vit = /*initialVerticesEnd;*/_m.vertices_begin(); - vit != _m.vertices_end(); ++vit) - _m.set_point(*vit, _m.property( vp_pos_, *vit ) ); + for ( auto vh : _m.vertices()) + _m.set_point(vh, _m.property( vp_pos_, vh ) ); #if defined(_DEBUG) || defined(DEBUG) // Now we have an consistent mesh! diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh b/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh index 9e793184..7edbc27f 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh @@ -162,11 +162,6 @@ protected: ///TODO:Implement fixed positions - typename MeshType::VertexIter vit; - typename MeshType::VertexVertexIter vvit; - typename MeshType::EdgeIter eit; - typename MeshType::FaceIter fit; - typename MeshType::FaceVertexIter fvit; typename MeshType::VertexHandle vh; typename MeshType::HalfedgeHandle heh; typename MeshType::Point pos(0,0,0), zero(0,0,0); @@ -175,22 +170,22 @@ protected: for (size_t l=0; l<_n; ++l) { // tag existing edges - for (eit=_m.edges_begin(); eit != _m.edges_end();++eit) + for (auto eh : _m.edges()) { - _m.status( *eit ).set_tagged( true ); - if ( (gen%2) && _m.is_boundary(*eit) ) - compute_new_boundary_points( _m, *eit ); // *) creates new vertices + _m.status( eh ).set_tagged( true ); + if ( (gen%2) && _m.is_boundary(eh) ) + compute_new_boundary_points( _m, eh ); // *) creates new vertices } // do relaxation of old vertices, but store new pos in property vp_pos_ - for (vit=_m.vertices_begin(); vit!=_m.vertices_end(); ++vit) + for (auto vh : _m.vertices()) { - if ( _m.is_boundary(*vit) ) + if ( _m.is_boundary(vh) ) { if ( gen%2 ) { - heh = _m.halfedge_handle(*vit); + heh = _m.halfedge_handle(vh); if (heh.is_valid()) // skip isolated newly inserted vertices *) { typename OpenMesh::HalfedgeHandle @@ -203,60 +198,59 @@ protected: pos += _m.point(_m.from_vertex_handle(prev_heh)); pos *= real_t(4.0); - pos += real_t(19.0) * _m.point( *vit ); + pos += real_t(19.0) * _m.point( vh ); pos *= _1over27; - _m.property( vp_pos_, *vit ) = pos; + _m.property( vp_pos_, vh ) = pos; } } else - _m.property( vp_pos_, *vit ) = _m.point( *vit ); + _m.property( vp_pos_, vh ) = _m.point( vh ); } else { size_t valence=0; pos = zero; - for ( vvit = _m.vv_iter(*vit); vvit.is_valid(); ++vvit) + for ( auto vvh : _m.vv_range(vh)) { - pos += _m.point( *vvit ); + pos += _m.point( vvh ); ++valence; } pos *= weights_[ valence ].second; - pos += weights_[ valence ].first * _m.point(*vit); - _m.property( vp_pos_, *vit ) = pos; + pos += weights_[ valence ].first * _m.point(vh); + _m.property( vp_pos_, vh ) = pos; } } // insert new vertices, but store pos in vp_pos_ - typename MeshType::FaceIter fend = _m.faces_end(); - for (fit = _m.faces_begin();fit != fend; ++fit) + for (auto fh : _m.faces()) { - if ( (gen%2) && _m.is_boundary(*fit)) + if ( (gen%2) && _m.is_boundary(fh)) { - boundary_split( _m, *fit ); + boundary_split( _m, fh ); } else { - fvit = _m.fv_iter( *fit ); + auto fvit = _m.fv_iter( fh ); pos = _m.point( *fvit); pos += _m.point(*(++fvit)); pos += _m.point(*(++fvit)); pos *= _1over3; vh = _m.add_vertex( zero ); _m.property( vp_pos_, vh ) = pos; - _m.split( *fit, vh ); + _m.split( fh, vh ); } } // commit new positions (now iterating over all vertices) - for (vit=_m.vertices_begin();vit != _m.vertices_end(); ++vit) - _m.set_point(*vit, _m.property( vp_pos_, *vit ) ); + for (auto vh : _m.vertices()) + _m.set_point(vh, _m.property( vp_pos_, vh ) ); // flip old edges - for (eit=_m.edges_begin(); eit != _m.edges_end(); ++eit) - if ( _m.status( *eit ).tagged() && !_m.is_boundary( *eit ) ) - _m.flip(*eit); + for (auto eh : _m.edges()) + if ( _m.status( eh ).tagged() && !_m.is_boundary( eh ) ) + _m.flip(eh); // Now we have an consistent mesh! ASSERT_CONSISTENCY( MeshType, _m ); From 71698822a15dd3d0669465eeadb13fed9134613e Mon Sep 17 00:00:00 2001 From: Max Lyon Date: Mon, 3 Feb 2020 15:08:00 +0100 Subject: [PATCH 3/3] fix cppcheck warnings --- src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh b/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh index 7edbc27f..229bb692 100644 --- a/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh +++ b/src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh @@ -162,8 +162,6 @@ protected: ///TODO:Implement fixed positions - typename MeshType::VertexHandle vh; - typename MeshType::HalfedgeHandle heh; typename MeshType::Point pos(0,0,0), zero(0,0,0); size_t &gen = _m.property( mp_gen_ ); @@ -185,11 +183,10 @@ protected: { if ( gen%2 ) { - heh = _m.halfedge_handle(vh); + auto heh = _m.halfedge_handle(vh); if (heh.is_valid()) // skip isolated newly inserted vertices *) { - typename OpenMesh::HalfedgeHandle - prev_heh = _m.prev_halfedge_handle(heh); + auto prev_heh = _m.prev_halfedge_handle(heh); assert( _m.is_boundary(heh ) ); assert( _m.is_boundary(prev_heh) ); @@ -237,7 +234,7 @@ protected: pos += _m.point(*(++fvit)); pos += _m.point(*(++fvit)); pos *= _1over3; - vh = _m.add_vertex( zero ); + auto vh = _m.add_vertex( zero ); _m.property( vp_pos_, vh ) = pos; _m.split( fh, vh ); }