Modifications to the code for TDS_2.insert_in_hole, the documentation and also the testsuite

TDS_2/doc/TDS_2/Concepts/TriangulationDataStructure_2.h

@@ -553,28 +553,33 @@ void dim_down(Face_handle f, int i);
 /*!
 \cgalModifBegin
 creates a new vertex `v` and uses it to star a hole.
-
+ 
-It takes an iterator range `[edge_begin, edge_end[` of `Edges`, given as pairs `(Face_handle, int)`. 
+It takes an iterator range `[face_begin, face_end[` over a set of faces `F`. The faces 
-The `Face_handles` specify a set of connected faces describing a hole that is a topological disc. Each `Edge` in the iterator range 
+in the set `F` describe a simply connected hole, i.e., a topological disc. 
-is an edge of the boundary of the hole, i.e., if `e = (fh, i)` \f$\in\f$ `[edge_begin, edge_end[`, then `fh`
+Starting from `face_begin`, a heuristic walk through the faces is performed until a
-belongs to the set of faces describing the hole, while `fh->neighbor(i)` does not. The function deletes
+face `fh` is encountered with one edge on the boundary of the hole, i.e., 
-the faces describing the hole, creates a new vertex `v` and for each edge on the boundary of the hole 
+`fh` \f$ \in \f$ `F` and `fh->neighbor(`\f$i\f$`)` \f$ \not\in \f$ `F` for some \f$ i \in \{0, 1, 2\}\f$.
-creates a new face with `v` as an apex. A handle to the vertex `v` is returned.
+The edge (`fh`, \f$i\f$) is then stored, and a walk through the faces on the boundary
-
+is performed to identify all boundary edges in such an order that, for two consecutive
-\pre The set of faces is connected, the set of edges is connected, and the sequence `[edge_begin, edge_end[` is oriented counter-clockwise.
+edges \f$ e_k\f$ `= (f_k, `\f$i_k\f$`)`, \f$e_{k+1}\f$ `= (f_k, `\f$i_{k+1}\f$`)` in the sequence it is true that
+\f$ f_k\f$`->vertex(ccw(`\f$i_k\f$`))` = \f$f_{k+1}\f$`->vertex(cw(`\f$i_{k+1}\f$`))`.
+As a next step, new faces are created by using the edges of the boundary and the vertex `v`.
+Lastly, all faces in the set `F` are deleted and a handle to the vertex `v` is returned. 
+ 
+\pre The set of faces `F` has the topology of a disk.
 \cgalModifEnd
 */
-template< class EdgeIt >
+template< class FaceIt >
-Vertex_handle insert_in_hole(EdgeIt edge_begin, EdgeIt edge_end);
+Vertex_handle insert_in_hole(FaceIt face_begin, FaceIt face_end); 
 
 /*!
 \cgalModifBegin
-same as above, except that `v` will be used as the new vertex, which must have been allocated previously with e.g. 
+same as above, except that `new_v` will be used as the new vertex, which must have been allocated previously with e.g. 
 `create_vertex`.
 \cgalModifEnd
 */
-template< class EdgeIt >
+template< class FaceIt >
-void insert_in_hole(Vertex_handle v, EdgeIt edge_begin, EdgeIt edge_end); 
+void insert_in_hole(Vertex_handle new_v, FaceIt face_begin, FaceIt face_end);  
 
 
 /*!

TDS_2/include/CGAL/Triangulation_data_structure_2.h

@@ -429,94 +429,100 @@ public:
   // template members definition
 public:
 
-  /************* START OF MODIFICATIONS (iiordanov) ***************/
+  /************* START OF MODIFICATIONS ***************/
 
-
+  template< class FaceIt >
-  /* 
+  Vertex_handle insert_in_hole(FaceIt face_begin, FaceIt face_end) 
-   * Creates a new vertex new_v and uses it to star the hole described
-   * by the sequence of edges [edge_begin, edge_end]. The pre-existing
-   * faces in the hole are destroyed.
-   *
-   * Prerequisite: the sequence [edge_begin, edge_end] is oriented
-   * counter-clockwise.
-   */
-
-  template< class EdgeIt >
-  Vertex_handle insert_in_hole(EdgeIt edge_begin, EdgeIt edge_end) 
   {
-    Vertex_handle new_v = create_vertex();
+    Vertex_handle newv = create_vertex();
-    insert_in_hole(new_v, edge_begin, edge_end);
+    insert_in_hole(newv, face_begin, face_end);
-    return new_v;
+    return newv;
   }
 
 
-  /* 
+  template< class FaceIt >
-   * Uses the vertex v to star the hole described by the sequence 
+  void insert_in_hole(Vertex_handle v, FaceIt face_begin, FaceIt face_end) 
-   * of edges [edge_begin, edge_end]. The pre-existing faces in 
-   * the hole are destroyed.
-   *
-   * Prerequisite: the sequence [edge_begin, edge_end] is oriented
-   * counter-clockwise.
-   */
-  template< class EdgeIt >
-  void insert_in_hole(Vertex_handle v, EdgeIt edge_begin, EdgeIt edge_end) 
   {
 
-    // Keep new faces in a vector
+    std::vector<Face_handle>  new_faces;
-    std::vector<Face_handle> new_faces;
+    std::vector<Edge>         bdry_edges;
 
-    // Exploit std::set functionality to keep unique old faces
+    Face_handle fh = *face_begin;
-    std::set<Face_handle> old;
+    int ii = 0;
+    bool found_boundary = false;
+    do {
+      if (std::find(face_begin, face_end, fh->neighbor(ii)) == face_end) {
+        bdry_edges.push_back(Edge(fh, ii));
+        found_boundary = true;
+      } else {
+        int newi = fh->neighbor(ii)->index(fh->vertex(ccw(ii)));
+        fh = fh->neighbor(ii);
+        ii = newi;
+      }
+    } while(!found_boundary);
+    // Now we have found ONE edge on the boundary. 
+    // From that one edge we must walk on the boundary 
+    // of the hole until we've covered the whole thing.
 
-    for (EdgeIt it = edge_begin; it != edge_end; it++) {
+    bool complete_walk = false;
-      Face_handle fh = (*it).first;
+    do {
-      int i = (*it).second;
+      Face_handle nh = fh->neighbor(ccw(ii));
+      if (std::find(face_begin, face_end, nh) == face_end) {
+        ii = ccw(ii);
+        Edge new_edge(fh, ii);
+        if (std::find(bdry_edges.begin(), bdry_edges.end(), new_edge) == bdry_edges.end()) {
+          bdry_edges.push_back(Edge(fh, ii));
+        } else {
+          complete_walk = true;
+        }
+      } else {
+        int newi = cw(nh->index(fh->vertex(ii)));
+        fh = nh;
+        ii = newi;
+      }
+    } while (!complete_walk);
+    // At this point, bdry_edges contains the edges that define
+    // the boundary of the hole with a specific ordering: for any
+    // two consecutive edges in the vector e1 = (f1, i1), 
+    // e2 = (f2, i2) it holds that 
+    //      f1->vertex(cw(i1)) == f2->vertex(ccw(i2))
 
-      old.insert(fh);
+    for (int jj = 0; jj < bdry_edges.size(); jj++) {
+      Face_handle fh = bdry_edges[jj].first;
+      int idx = bdry_edges[jj].second;
+      
+      Vertex_handle v1 = fh->vertex(ccw(idx));
+      Vertex_handle v2 = fh->vertex(cw(idx));
 
-      //          v
+      Face_handle nf = fh->neighbor(idx);
-      //          .
+      int jdx = mirror_index(fh, idx);
-      //         / \
-      //        /   \
-      //       /     \
-      //      / new_f \
-      //  v1 /_________\ v2
-      //     \         /
-      //      \  nf   /
-      //       \     /
-      //        \   /
-      //         \ /
-      //          *
-      //     nf->vertex(j)     
-  
-      Vertex_handle v1 = fh->vertex(ccw(i));
-      Vertex_handle v2 = fh->vertex(cw(i));
-
-      Face_handle nf = fh->neighbor(i);
-      int j = mirror_index(fh, i);
 
       Face_handle new_f = create_face(v, v1, v2);
-      set_adjacency(new_f, 0, nf, j);
+      set_adjacency(new_f, 0, nf, jdx);
       new_faces.push_back(new_f);
     }
+    // At this point we have created all the new faces of the triangulation,
+    // and we have set adjacency relationships with the faces on the border
+    // of the hole.
 
-    // Set adjacency for the new faces
     for (int i = 0; i < new_faces.size() - 1; i++) {
       set_adjacency(new_faces[i], 1, new_faces[i+1], 2);
     }
-    // The last one has to be treated separately
     set_adjacency(new_faces[0], 2, new_faces[new_faces.size()-1], 1);
+    // Now we have also set adjacency relationships between the new faces.
 
-    // Delete the old faces
+    for (FaceIt it = face_begin; it != face_end; it++) {
-    for (typename std::set<Face_handle>::iterator it = old.begin(); it != old.end(); it++) {
       delete_face(*it);
     }
+    // The old faces that were in conflict are now deleted.
 
-    // Set the new vertex to point at the first new face (arbitrarily)
     v->set_face(new_faces[0]);
+    // Set the pointer of the new vertex to one of the new faces.
   }
 
-  /************* END OF MODIFICATIONS (iiordanov) ***************/
+
+
+  /************* END OF MODIFICATIONS ***************/
 
 
   template< class EdgeIt>

TDS_2/test/TDS_2/include/CGAL/_test_cls_tds_2.h

@@ -210,16 +210,97 @@ _test_cls_tds_2( const Tds &)
 
   assert(td45.is_valid() && td45.number_of_vertices() == 4 && td45.number_of_faces() == 4);
 
-  Face_iterator fi = td45.faces_begin();
+  Face_handle f0_0 = td45.faces_begin();
-  std::vector<Edge> vhole;
+  Face_handle f0_1 = f0_0++;
-  vhole.push_back( Edge(  fi, 2) );
+
-  vhole.push_back( Edge(  fi, 0) );
+  Vertex_handle v445 = td45.insert_in_face(f0_0);
-  vhole.push_back( Edge(++fi, 1) );
+  Vertex_handle v545 = td45.insert_in_face(f0_1);
-  vhole.push_back( Edge(  fi, 2) );
+
+  Face_handle f1_0, f1_1, f1_2, f1_3, fcf_0, fcf_1;
+  Face_circulator fc1 = td45.incident_faces(v445), fc1e(fc1);
+  do {
+    int i = fc1->index(v445);
+    if (fc1->vertex((i+1)%3) == v345) {
+      f1_0 = fc1;
+    }
+    if (fc1->vertex((i+1)%3) == v145) {
+      f1_1 = fc1;
+    }
+    if (fc1->vertex((i+1)%3) == v245) {
+      fcf_0 = fc1;
+    }
+  } while(++fc1 != fc1e);
+
+
+  Face_circulator fc2 = td45.incident_faces(v545), fc2e(fc2);
+  do {
+    int i = fc2->index(v545);
+    if (fc2->vertex((i+1)%3) == v045) {
+      f1_2 = fc2;
+    }
+    if (fc2->vertex((i+1)%3) == v245) {
+      f1_3 = fc2;
+    }
+    if (fc2->vertex((i+1)%3) == v345) {
+      fcf_1 = fc2;
+    }
+  } while(++fc2 != fc2e);
+
+  Vertex_handle v645 = td45.insert_in_face(f1_0);
+  Vertex_handle v745 = td45.insert_in_face(f1_1);
+  Vertex_handle v845 = td45.insert_in_face(f1_2);
+  Vertex_handle v945 = td45.insert_in_face(f1_3);
+
+  Face_handle fcf_2, fcf_3, fcf_4, fcf_5;
+  Face_circulator fc6 = td45.incident_faces(v645), fc6e(fc6);
+  do {
+    int i = fc6->index(v645);
+    if (fc6->vertex((i+1)%3) == v445) {
+      fcf_2 = fc6;
+      break;
+    }
+  } while(++fc6 != fc6e);
+
+  Face_circulator fc7 = td45.incident_faces(v745), fc7e(fc7);
+  do {
+    int i = fc7->index(v745);
+    if (fc7->vertex((i+1)%3) == v245) {
+      fcf_3 = fc7;
+      break;
+    }
+  } while(++fc7 != fc7e);
+
+  Face_circulator fc8 = td45.incident_faces(v845), fc8e(fc8);
+  do {
+    int i = fc8->index(v845);
+    if (fc8->vertex((i+1)%3) == v345) {
+      fcf_4 = fc8;
+      break;
+    }
+  } while(++fc8 != fc8e);
+
+   Face_circulator fc9 = td45.incident_faces(v945), fc9e(fc9);
+  do {
+    int i = fc9->index(v945);
+    if (fc9->vertex((i+1)%3) == v545) {
+      fcf_5 = fc9;
+      break;
+    }
+  } while(++fc9 != fc9e);
+
+  std::vector<Face_handle> vhole;
+  vhole.push_back( fcf_0 );
+  vhole.push_back( fcf_1 );
+  vhole.push_back( fcf_2 );
+  vhole.push_back( fcf_3 );
+  vhole.push_back( fcf_4 );
+  vhole.push_back( fcf_5 );
+
+  assert(td45.is_valid() && td45.number_of_vertices() == 10 && td45.number_of_faces() == 16);
 
   Vertex_handle nv45 = td45.insert_in_hole(vhole.begin(), vhole.end());
 
-  assert(td45.is_valid() && td45.number_of_vertices() == 5 && td45.number_of_faces() == 6);
+  assert(td45.is_valid() && td45.number_of_vertices() == 11 && td45.number_of_faces() == 18);
 
   // dim_down
   std::cout << "    dim_down" << std::endl;