CGAL / cgal

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

I/O of Triangulation_2 or Triangulation_3, when the vertex or cell types have handles #861

Open lrineau opened 8 years ago

lrineau commented 8 years ago

(Cc @MoniqueTeillaud)

See the discussion in CGAL-develop:

Let's quote my mail:

Let's take Triangulation_3 as an example. The operator<< of Triangulation_3 uses two maps:

// the "map" from indices to Vertex_handle
std::vector<Vertex_handle> TV;
// the map from Vertex_handle to indices
Unique_hash_map<Vertex_handle, std::size_t > V;

The operator<< first displays:

  <dimension>
  <number of vertices>

Then, for each vertices:

  • it fills the TV and V
  • and it calls the operator<< of the vertex

Then:

  • call tds().print_cells(ostream, V);

Then, for each cell, in dimension 3: it calls the operator<< of the cell

Imagine that the vertex base has an extra field of type Vertex_handle (to store for example the "parent vertex" of Steiner vertices, in Shewchuk Delaunay refinement algorithm). To correctly output that triangulation, the operator<< of the vertex should have access to the map V, but it does not have access to it. What could be a solution for that?

sgiraudot commented 7 years ago

@MoniqueTeillaud What is your opinion about this issue?

MoniqueTeillaud commented 7 years ago

Well, I am afraid that I don't have much more to say than what I answered already:

Le Tuesday 31 March 2015 19:23:55 Monique Teillaud a écrit :

I wrote the I/O methods in an early version of CGAL. Sylvain added the rebind mechanism later. I am quite confident that he updated the I/O when he did that. I cannot say more.

Then Laurent explained in more detail why there was a problem (see above), which I can understand. But I don't see how to update the IOs to work when the rebind mechanism is used to add info...

sloriot commented 6 years ago

Just a comment. In the vtk file format, you can associate easily some data to the cells of a mesh. They are stored in a simple array that has the same size as the number of cells. What could be done here (I'll describe it for cells but it would work for vertices as well) is to have for the cell type a virtual function write_data() (read_data()) that would take care of writing (reading) extra data at the end of the usual information written in a file (that part could start by a header for example). That way if you have or not extra data in the file, the fields could be filled if the data type have the correct function. The default implementation being that nothing is written/read.

lrineau commented 6 years ago

Our CGAL version of write_data()/read_data() are operator<< and operator>> for Vertex and Cell.

The issue pointed out here is that if you want to be able to deal with data that are pointers to other Vertex or Cell, then the I/O functions (should they be called write_data or operator<<), need to have access to the map v that maps handles to indices.

sloriot commented 6 years ago

Note what I was trying to say is that you need new functions that would take care only of writing/reading extra data and those functions could be given the index maps needed. As I said the idea is to keep the beginning of the file unchanged and have extra data at the end of the file (so that they can be easily ignored).

lrineau commented 6 years ago

You are right. We could use both operator<</>> and new member functions write_data(v)/read_data(v) where the later would have access to the mapping.

A Cell requiring the extra data could have empty/trivial operator<</>> member functions, and do all the work in write_data(v)/read_data(v).

Good idea. Thanks.

maxGimeno commented 5 years ago

@sloriot I have a question. We don't want to write the "header" if there is no additional data to write, but I am not sure how to decide that. Maybe add another virtual function, something like has_additional_data() that would return false as default and should be overriden with write_data() and read_data(). What did you have in mind ?

lrineau commented 5 years ago

Which header are you talking about?

has_additional_data() cannot be virtual, because there is no base class. write_data and read_data must be detected at compile-time, by meta-programming.

maxGimeno commented 5 years ago

I was talking about

What could be done here (I'll describe it for cells but it would work for vertices as well) is to have for the cell type a virtual function write_data() (read_data()) that would take care of writing (reading) extra data at the end of the usual information written in a file (that part could start by a header for example).

Which talked about a header and virtual functions, so I thought the tds_cell_base_3 could be used as a base class.

lrineau commented 5 years ago
  1. That is a bad idea to add virtual functions in any Vertex or Cell base, because then the class layout is augmented by a pointer to the vtable. And one can do the same at compile time.

  2. I do not think there is any need for a header. The reading function knows exactly the size of the data it needs to read. Any extra data will be... just after in the file, without the need for a header to detect the transition.

maxGimeno commented 5 years ago

The reading function knows exactly the size of the data it needs to read

By detecting the presence of read_data/write_data in its vertex/cell type, like you said ? If there isn't, then it stops after the last cell, if there is it looks for extra_data ?

lrineau commented 5 years ago

@maxGimeno commented on Apr 5, 2019, 10:12 AM GMT+2:

The reading function knows exactly the size of the data it needs to read

By detecting the presence of read_data/write_data in its vertex/cell type, like you said ? If there isn't, then it stops after the last cell, if there is it looks for extra_data ?

That is right. Anyway, if read_data/write_data do not exist, the code will not be able to call them to read or write the extra data.