Triangulation_23: Enhance IO

[maxGimeno]

Summary of Changes

Add the possibility to use functions of the cell/face/vertex base, called read/write_data(), and that have access to the maps/vectors internal to the triangulation. This is used in the IO functions.

Release Management

• Affected package(s):Triangulation_2, Triangulation_3
• Issue(s) solved (if any): fix I/O of Triangulation_2 or Triangulation_3, when the vertex or cell types have handles #861, fix Mesh_3: Save/load c3t3 objects correctly #2359
• Feature/Small Feature (if any):
• Link to compiled documentation (obligatory for small feature)
• License and copyright ownership:

[maxGimeno]

@sloriot @lrineau So I tried something to implement the edges in complex IO in .cgal format, but it requiered changing the signature of the Compact_mesh_cell_base_3, which changed the c3t3 type written in the header of the files. In concequences of which the demo is not able to read any of the existing "[.binary].cgal" files... I'd like thoughts on the matter.

[maxGimeno]

@sloriot @lrineau ping.
Should I remake all existing files (in tests and examples) to fit the new type or do you have a different solution to implement the edges in complex IO that does not require changing its type ? Or something about the IO that reads the type, anything ?

[sloriot]

I'm a bit afraid that binary files are conflicting (for size reason). Were those files updated recently?

[maxGimeno]

I did not touch this branch since april 19.

[lrineau]

I'm a bit afraid that binary files are conflicting (for size reason). Were those files updated recently?

That is not really recently. The PR #4397 has changed the order of cells in a triangulation (given the sequence of points). That is why all the files have changed in master. I am surprised that this PR conflicts for the binary files, but not for the two corresponding ASCII files.

[maxGimeno]

I have not yet cupdated all the c3t3 files in this branch, as I was waiting for an aswer to my question.

[maxGimeno]

Succesfully tested in https://cgal.geometryfactory.com/CGAL/testsuite/results-5.1-Ic-112.shtml

[sloriot]

Tested in CGAL-5.1-Ic-114

[sloriot]

@lrineau are you done with your review?

[lrineau]

@lrineau are you done with your review?

No, I have not reviewed the code, yet. And I will not probably do it soon.

From far, I do not really see how this PR solves #2359.

[maxGimeno]

@lrineau are you done with your review?

No, I have not reviewed the code, yet. And I will not probably do it soon.

From far, I do not really see how this PR solves #2359.

Maybe I did not understand the issue then, as I thougt adding the edges in complex in the IO was the whole point.

[MaelRL]

@lrineau @maxGimeno Does this PR just get pushed to next release (that will deal with I/O of triangulations too anyway)?

[maxGimeno]

I don't see why it should be delayed to next release, it already passed the testsuite once, and it is not a small feature. I'm just waiting for @lrineau to do his review to test it again and have it integrated. Unless he explains why it does not work or fix the issue, in which case everything will have to be re thought from the start.

[lrineau]

I disagree strongly with the changes made to Mesh_3. Compact_mesh_cell_base gets that addition of six extra words, and two members functions set_curve_index/get_curve_index that are not in the concept MeshCellBase_3.

The first issue with that is:

• information about edges are stored in each incident cells,
• with the current code, the C3t3 implementation does not maintain the information.

More than ten years ago, I choose to implement the information about features edges
using a special map:

cgal/Mesh_3/include/CGAL/Mesh_complex_3_in_triangulation_3.h

Lines 79 to 88 in d03a023

	private:
	// Type to store the edges:
	// - a set of std::pair<Vertex_handle,Vertex_handle> (ordered at insertion)
	// - which allows fast lookup from one Vertex_handle
	// - each element of the set has an associated info (Curve_index) value
	typedef boost::bimaps::bimap<
	boost::bimaps::multiset_of<Vertex_handle>,
	boost::bimaps::multiset_of<Vertex_handle>,
	boost::bimaps::set_of_relation<>,
	boost::bimaps::with_info<Curve_index> > Edge_map;

and I still think that is the good choice and should not be modified (the type of the map could be discussed and maybe optimized, but not the choice to store the information in a map of the c3t3 object).

So, correct I/O of C3t3 must serialize that map, and not modify the concept of MeshCellBase_3 and its models.

Maybe this PR should be split in two: one that solves the initial issue #861 (I/O of triangulations), and then another one about the I/O of C3t3 (#2359).

[sloriot]

@maxGimeno is this replaced by #5145?

[maxGimeno]

@maxGimeno is this replaced by #5145?

Not entirely, but if #5145 is dropped, then the second part of this PR is impossible to implement, IIRC

[lrineau]

Replaced by #5145.