CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
4.98k stars 1.39k forks source link

Store userdata in edges in Triangulation #2024

Open flm8620 opened 7 years ago

flm8620 commented 7 years ago

Problem

I want to store userdata in edges. But CGAL::TriangulationDataStructure_2 Concept only has a implicit representations for edge, which is attached to a face.

So I can create my face class and plug it into CGAL::TriangulationDataStructure_2. But then I found this function: TriangulationDataStructure_2::insert_in_face(Face_handle f)

It add a vertex in a face and then split it in three faces. But actually the old face is reused and two new faces are created. So in my case, my userdata in the old face wasn't aware of this change, and the edge to which my userdata correspond doesn't exist any more. Which leads to unexpected result in my codes.

Question

  1. Why the design of insert_in_face is like this ? Why not just destroy the old face and create three new one ? The latter way looks more "symmetric" in the sense that none of new faces are related to old one, they are all fresh and "equal"
  2. What is the right way to store userdata in edges ?
lrineau commented 7 years ago

I agree with the reporter. For the face that is recycled, we should call the destructor ~Face(), and then a placement new to call the default constructor again. And do the same consistently in Triangulation_2 and Triangulation_3. That consistent behavior would have saved a lot of trouble in the Mesh_2 and Mesh_3 package.

sloriot commented 7 years ago

A solution to your problem would be to identify an edge as a sorted pair of vertex handles. Then you can use a hash map or a map to access to the edge.

MoniqueTeillaud commented 7 years ago

Just a historical comment about one of the questions

  1. Why the design of insert_in_face is like this ? Why not just destroy the old face and create three new one ? At the beginning I guess that the goal was to be more efficient: instead of calling 1 destructor and 3 constructors, in this way only 2 constructors are called. If I remember correctly, at that early time there was no possibility offered to add information in faces. Then somebody added such a possibility but missed the reported problem...
flm8620 commented 7 years ago

I found a better way to implement a real Edge object. I use shared_ptr in Face to point to Edge. Edge will be automatically created, destroyed and shared properly. See my github CGAL_edge If you are interested.

Besides, I think I might know the reason for this design. CGAL use CGAL::Compact_Container to hold objects like faces, vertices. This container use lazy deletion, which means the memory of deleted objects is not freed, their destructor is called but their memory is marked "freed". So the programmer want to minimize the creation of new object.