Fixed the new updating garbage collection as it produced garbage

git-svn-id: http://www.openmesh.org/svnrepo/OpenMesh/trunk@661 fdac6126-5c0c-442c-9429-916003d36597

src/OpenMesh/Core/Mesh/ArrayKernelT.cc

@@ -72,12 +72,20 @@ void ArrayKernel::garbage_collection(std_API_Container_VHandlePointer& vh_to_upd
   #endif
 #endif
 
+  const bool track_vhandles = ( !vh_to_update.empty() );
+  const bool track_hhandles = ( !hh_to_update.empty() );
+  const bool track_fhandles = ( !fh_to_update.empty() );
+
   int i, i0, i1, nV(n_vertices()), nE(n_edges()), nH(2*n_edges()), nF(n_faces());
 
   std::vector<VertexHandle>    vh_map;
   std::vector<HalfedgeHandle>  hh_map;
   std::vector<FaceHandle>      fh_map;
 
+  std::map <int, int> vertex_inverse_map;
+  std::map <int, int> halfedge_inverse_map;
+  std::map <int, int> face_inverse_map;
+
   // setup handle mapping:
   vh_map.reserve(nV);
   for (i=0; i<nV; ++i) vh_map.push_back(VertexHandle(i));
@@ -100,6 +108,13 @@ void ArrayKernel::garbage_collection(std_API_Container_VHandlePointer& vh_to_upd
       while ( status(VertexHandle(i1)).deleted() && i0 < i1)  --i1;
       if (i0 >= i1) break;
 
+      // If we keep track of the vertex handles for updates,
+      // we need to have the opposite direction
+      if ( track_vhandles ) {
+        vertex_inverse_map[i1] = i0;
+        vertex_inverse_map[i0] = -1;
+      }
+
       // swap
       std::swap(vertices_[i0], vertices_[i1]);
       std::swap(vh_map[i0],  vh_map[i1]);
@@ -150,6 +165,13 @@ void ArrayKernel::garbage_collection(std_API_Container_VHandlePointer& vh_to_upd
       while ( status(FaceHandle(i1)).deleted() && i0 < i1)  --i1;
       if (i0 >= i1) break;
 
+      // If we keep track of the face handles for updates,
+      // we need to have the opposite direction
+      if ( track_fhandles ) {
+        face_inverse_map[i1] = i0;
+        face_inverse_map[i0] = -1;
+      }
+
       // swap
       std::swap(faces_[i0], faces_[i1]);
       std::swap(fh_map[i0], fh_map[i1]);
@@ -223,12 +245,20 @@ void ArrayKernel::garbage_collection(std_API_Container_VHandlePointer& vh_to_upd
   typename std_API_Container_VHandlePointer::iterator v_it(vh_to_update.begin()), v_it_end(vh_to_update.end());
   for(; v_it != v_it_end; ++v_it)
   {
-    if ( (*v_it)->idx() >=  vertexCount )
+
+    // Only changed vertices need to be considered
+    if ( (*v_it)->idx() != vh_map[(*v_it)->idx()].idx() ) {
+      *(*v_it) = VertexHandle(vertex_inverse_map[(*v_it)->idx()]);
+
+      // Vertices above the vertex count have to be already mapped, or they are invalid now!
+    } else if ( ((*v_it)->idx() >= vertexCount) && (vertex_inverse_map.find((*v_it)->idx()) == vertex_inverse_map.end()) ) {
       (*v_it)->invalidate();
-    else
-      *(*v_it) = vh_map[(*v_it)->idx()];
+    }
+
   }
 
+  // TODO :  fix garbage collection for halfedges!!!
+
   // Update the halfedge handles in the halfedge handle vector
   typename std_API_Container_HHandlePointer::iterator hh_it(hh_to_update.begin()), hh_it_end(hh_to_update.end());
   for(; hh_it != hh_it_end; ++hh_it)
@@ -238,14 +268,21 @@ void ArrayKernel::garbage_collection(std_API_Container_VHandlePointer& vh_to_upd
     else
       *(*hh_it) = hh_map[(*hh_it)->idx()];
   }
+
+
   // Update the face handles in the face handle vector
   typename std_API_Container_FHandlePointer::iterator fh_it(fh_to_update.begin()), fh_it_end(fh_to_update.end());
   for(; fh_it != fh_it_end; ++fh_it)
   {
-    if ( (*fh_it)->idx() >=  faceCount )
+
+    // Only changed faces need to be considered
+    if ( (*fh_it)->idx() != fh_map[(*fh_it)->idx()].idx() ) {
+      *(*fh_it) = FaceHandle(face_inverse_map[(*fh_it)->idx()]);
+
+      // Vertices above the face count have to be already mapped, or they are invalid now!
+    } else if ( ((*fh_it)->idx() >= faceCount) && (face_inverse_map.find((*fh_it)->idx()) == face_inverse_map.end()) ) {
       (*fh_it)->invalidate();
-    else
-      *(*fh_it) = fh_map[(*fh_it)->idx()];
+    }
   }
 }
 

src/Unittests/unittests_trimesh_garbage_collection.hh

@@ -337,7 +337,7 @@ TEST_F(OpenMeshTriMeshGarbageCollection, TrackedGarbageCollection) {
     faceHandlesP.push_back(&(faceHandles[i]));
 
 
-//  // REMOVE
+////  // REMOVE
 //  std::cerr << "Vertex   : ";
 //  for ( unsigned int i = 0 ; i < vertexHandles.size() ; ++i)
 //    std::cerr <<  vertexHandles[i].idx() << " ";
@@ -359,6 +359,7 @@ TEST_F(OpenMeshTriMeshGarbageCollection, TrackedGarbageCollection) {
   // -> deletes vertex 0
   // -> deletes faces 0,4,10,11
   mesh_.delete_vertex(vhandle[0]);
+  //mesh_.delete_vertex(vhandle[7]);
 
   // Check setup
   EXPECT_EQ(8u, mesh_.n_vertices() ) << "Wrong number of vertices after deletion";
@@ -374,48 +375,47 @@ TEST_F(OpenMeshTriMeshGarbageCollection, TrackedGarbageCollection) {
   // Check the updated handles
   //================================
 
+//  //  // REMOVE
+//  std::cerr << "Vertex   : ";
+//  for ( unsigned int i = 0 ; i < vertexHandles.size() ; ++i)
+//    std::cerr <<  vertexHandles[i].idx() << " ";
+//  std::cerr << std::endl;
+//
+//  std::cerr << "Halfedge : ";
+//  for ( unsigned int i = 0 ; i < halfedgeHandles.size() ; ++i)
+//    std::cerr <<  halfedgeHandles[i].idx() << " ";
+//  std::cerr << std::endl;
+//
+//  std::cerr << "Face     : ";
+//  for ( unsigned int i = 0 ; i < faceHandles.size() ; ++i)
+//    std::cerr <<  faceHandles[i].idx() << " ";
+//  std::cerr << std::endl;
+//  // REMOVE END
+
   // Check setup of vertices
-  EXPECT_EQ(7 , vertexHandles[0].idx() )  << "Wrong vertex handle after update";
+  EXPECT_EQ(-1, vertexHandles[0].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(1 , vertexHandles[1].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(2 , vertexHandles[2].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(3 , vertexHandles[3].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(4 , vertexHandles[4].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(5 , vertexHandles[5].idx() )  << "Wrong vertex handle after update";
   EXPECT_EQ(6 , vertexHandles[6].idx() )  << "Wrong vertex handle after update";
-  EXPECT_EQ(-1, vertexHandles[7].idx() )  << "Wrong vertex handle after update";
+  EXPECT_EQ(0 , vertexHandles[7].idx() )  << "Wrong vertex handle after update";
 
   // Check setup of faces
-  EXPECT_EQ(9  , faceHandles[0 ].idx() )  << "Wrong face handle after update";
+  EXPECT_EQ(-1 , faceHandles[0 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(1  , faceHandles[1 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(2  , faceHandles[2 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(3  , faceHandles[3 ].idx() )  << "Wrong face handle after update";
-  EXPECT_EQ(8  , faceHandles[4 ].idx() )  << "Wrong face handle after update";
+  EXPECT_EQ(-1 , faceHandles[4 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(5  , faceHandles[5 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(6  , faceHandles[6 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(7  , faceHandles[7 ].idx() )  << "Wrong face handle after update";
-  EXPECT_EQ(-1 , faceHandles[8 ].idx() )  << "Wrong face handle after update";
-  EXPECT_EQ(-1 , faceHandles[9 ].idx() )  << "Wrong face handle after update";
+  EXPECT_EQ(4  , faceHandles[8 ].idx() )  << "Wrong face handle after update";
+  EXPECT_EQ(0  , faceHandles[9 ].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(-1 , faceHandles[10].idx() )  << "Wrong face handle after update";
   EXPECT_EQ(-1 , faceHandles[11].idx() )  << "Wrong face handle after update";
 
-
-//  // REMOVE
-//  std::cerr << "Vertex   : ";
-//  for ( unsigned int i = 0 ; i < vertexHandles.size() ; ++i)
-//    std::cerr <<  vertexHandles[i].idx() << " ";
-//  std::cerr << std::endl;
-//
-//  std::cerr << "Halfedge : ";
-//  for ( unsigned int i = 0 ; i < halfedgeHandles.size() ; ++i)
-//    std::cerr <<  halfedgeHandles[i].idx() << " ";
-//  std::cerr << std::endl;
-//
-//  std::cerr << "Face     : ";
-//  for ( unsigned int i = 0 ; i < faceHandles.size() ; ++i)
-//    std::cerr <<  faceHandles[i].idx() << " ";
-//  std::cerr << std::endl;
-//  // REMOVE END
-
 }