Merge branch 'fix_subdivision' into 'master'

Fix subdivision

See merge request !248

src/OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT_impl.hh

@@ -100,47 +100,38 @@ CatmullClarkT<MeshType,RealType>::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)

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

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<typename mesh_t::VertexHandle> 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<typename mesh_t::VertexHandle> 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();

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!

src/OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh

@@ -162,39 +162,31 @@ 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);
     size_t                            &gen = _m.property( mp_gen_ );
 
     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);
+            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) );
@@ -203,60 +195,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 );
+          auto 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 );

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<Mesh::VertexHandle> 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<Mesh> 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<Mesh> 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<Mesh::VertexHandle> 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