Merge branch 'delete_vertex_crash_subdivider' into 'master'

Delete vertex crash subdivider

Closes #65

See merge request OpenMesh/OpenMesh!219

Doc/changelog.docu

@@ -8,6 +8,13 @@
 
 <tr valign=top><td><b>8.1</b> (?/?/?)</td><td>
 
+<b>Tools</b>
+<ul>
+<li>Subdivider: Fixed crash in Loop subdivider</li>
+<li>Subdivider: Fixed crash in ModifiedButterfly subdivider</li>
+</ul>
+
+
 <b>Utils</b>
 <ul>
 <li>Change PropertyManager::operator* to access the property value for mesh properties</li>

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

@@ -321,7 +321,7 @@ private: // topological modifiers
     typename mesh_t::VertexHandle   vh1(_m.to_vertex_handle(heh));
     typename mesh_t::Point          midP(_m.point(_m.to_vertex_handle(heh)));
     midP += _m.point(_m.to_vertex_handle(opp_heh));
-    midP *= static_cast<typename mesh_t::Point::value_type>(0.5);
+    midP *= static_cast<RealType>(0.5);
 
     // new vertex
     vh                = _m.new_vertex( midP );
@@ -363,7 +363,11 @@ private: // topological modifiers
 
     _m.set_face_handle( new_heh, _m.face_handle(heh) );
     _m.set_halfedge_handle( vh, new_heh);
-    _m.set_halfedge_handle( _m.face_handle(heh), heh );
+
+    // We cant reconnect a non existing face, so we skip this here if necessary
+    if ( !_m.is_boundary(heh) )
+      _m.set_halfedge_handle( _m.face_handle(heh), heh );
+
     _m.set_halfedge_handle( vh1, opp_new_heh );
 
     // Never forget this, when playing with the topology
@@ -389,7 +393,7 @@ private: // geometry helper
     // boundary edge: just average vertex positions
     if (_m.is_boundary(_eh) )
     {
-      pos *= static_cast<typename MeshType::Point::value_type>(0.5);
+      pos *= static_cast<RealType>(0.5);
     }
     else // inner edge: add neighbouring Vertices to sum
     {

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

@@ -356,7 +356,11 @@ private: // topological modifiers
 
     _m.set_face_handle( new_heh, _m.face_handle(heh) );
     _m.set_halfedge_handle( vh, new_heh);
-    _m.set_halfedge_handle( _m.face_handle(heh), heh );
+
+    // We cant reconnect a non existing face, so we skip this here if necessary
+    if ( !_m.is_boundary(heh) )
+      _m.set_halfedge_handle( _m.face_handle(heh), heh );
+
     _m.set_halfedge_handle( vh1, opp_new_heh );
 
     // Never forget this, when playing with the topology
@@ -383,7 +387,7 @@ private: // geometry helper
     {
         pos = _m.point(a_0);
         pos += _m.point(a_1);
-        pos *= static_cast<typename mesh_t::Point::value_type>(9.0/16.0);
+        pos *= static_cast<RealType>(9.0/16.0);
         typename mesh_t::Point tpos;
         if(_m.is_boundary(heh))
         {
@@ -396,7 +400,7 @@ private: // geometry helper
             tpos = _m.point(_m.to_vertex_handle(_m.next_halfedge_handle(opp_heh)));
             tpos += _m.point(_m.to_vertex_handle(_m.opposite_halfedge_handle(_m.prev_halfedge_handle(opp_heh))));
         }
-        tpos *= static_cast<typename mesh_t::Point::value_type>(-1.0/16.0);
+        tpos *= static_cast<RealType>(-1.0/16.0);
         pos += tpos;
     }
     else
@@ -499,7 +503,7 @@ private: // geometry helper
         }
         else //at least one endpoint is [irregular and not in boundary]
         {
-            typename mesh_t::Point::value_type normFactor = static_cast<typename mesh_t::Point::value_type>(0.0);
+          RealType normFactor = static_cast<RealType>(0.0);
 
             if(valence_a_0!=6 && !_m.is_boundary(a_0))
             {

src/Unittests/unittests_subdivider_uniform.cc

@@ -1,7 +1,9 @@
 
 #include <gtest/gtest.h>
 #include <Unittests/unittests_common.hh>
+#include <OpenMesh/Tools/Subdivider/Uniform/ModifiedButterFlyT.hh>
 #include <OpenMesh/Tools/Subdivider/Uniform/CatmullClarkT.hh>
+#include <OpenMesh/Tools/Subdivider/Uniform/LoopT.hh>
 #include <OpenMesh/Tools/Subdivider/Uniform/Sqrt3T.hh>
 #include <OpenMesh/Tools/Subdivider/Uniform/MidpointT.hh>
 
@@ -147,6 +149,226 @@ TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Sqrt3) {
     EXPECT_EQ(216u, mesh_.n_faces() )    << "Wrong number of faces after subdivision with sqrt3";
 }
 
+
+TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop) {
+    mesh_.clear();
+
+    // 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
+
+    // Initialize subdivider
+    OpenMesh::Subdivider::Uniform::LoopT<Mesh> loop;
+
+    // Check setup
+    EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices";
+    EXPECT_EQ(8u, mesh_.n_faces() )    << "Wrong number of faces";
+
+    // Execute 3 subdivision steps
+    loop.attach(mesh_);
+    loop( 3 );
+    loop.detach();
+
+    // Check setup
+    EXPECT_EQ(289u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with loop";
+    EXPECT_EQ(512u, mesh_.n_faces() )    << "Wrong number of faces after subdivision with loop";
+}
+
+
+
+TEST_F(OpenMeshSubdividerUniform_Triangle, Subdivider_Loop_delete_vertex) {
+    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]);
+    mesh_.garbage_collection();
+
+    // 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();
+
+    // 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";
+}
+
+
+
 /*
  * ====================================================================
  * Define tests below
@@ -329,4 +551,226 @@ TEST_F(OpenMeshSubdividerUniform_Poly, Midpoint) {
     EXPECT_EQ(26u, mesh_.n_faces())     << "Wrong number of faces";
 }
 
+
+
+TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly) {
+    mesh_.clear();
+
+    // 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
+
+    // Initialize subdivider
+    OpenMesh::Subdivider::Uniform::ModifiedButterflyT<Mesh> butter;
+
+    // Check setup
+    EXPECT_EQ(9u, mesh_.n_vertices() ) << "Wrong number of vertices";
+    EXPECT_EQ(8u, mesh_.n_faces() )    << "Wrong number of faces";
+
+    // Execute 3 subdivision steps
+    butter.attach(mesh_);
+    butter( 3 );
+    butter.detach();
+
+    // Check setup
+    EXPECT_EQ(289u, mesh_.n_vertices() ) << "Wrong number of vertices after subdivision with loop";
+    EXPECT_EQ(512u, mesh_.n_faces() )    << "Wrong number of faces after subdivision with loop";
+}
+
+
+
+TEST_F(OpenMeshSubdividerUniform_Triangle, Modified_Butterfly_delete_vertex) {
+    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]);
+    mesh_.garbage_collection();
+
+    // Initialize subdivider
+    OpenMesh::Subdivider::Uniform::ModifiedButterflyT<Mesh> 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();
+
+    // 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";
+}
+
+
+
+
 }