Prototype for callback: add a callback in Triangulation_3::file_input

[lrineau]

@afabri @sgiraudot

I have implemented a proof of concept for "progress tracker", using a std::function. This PR is for evaluation.

https://cgal.geometryfactory.com/CGAL/Members/wiki/Meetings/Sophia16#.E2.9C.94_Progress_tracker

I Cc also @MoniqueTeillaud.

The undocumented fonction TDS_3::read_cell is modified, so that one can pass a std::function as last argument. The non-documented function T_3::file_input is also modified. That allows to implement a progress tracking of the reading of a huge triangulation (actually a huge Mesh_3 C3t3).

Please review and give feedback during my vacation next week.

[lrineau]

I Cc also @janetournois, that can help you create big c3t3 files, and re-read them.

[lrineau]

For the moment, the callback is this big C++11 lambda function, that:

• uses a timer to return very fastly but every second,
• and displays stats about:
  • the size of the triangulation being read,
  • the size of the portion of the file that has been read, and the full size of the file.

Optionally, this callback can get a string as argument, so that the function force the display of a given string.

[lrineau]

One needs C++11 to evaluation that prototype.

One should:

• bench the code to verify if there is a penalty when the callback parameter is passed nullptr.
• bench the code to verify the overhead of that callback that is supposed to be fast but every second.

[sgiraudot]

From my experience with progress trackers, it is a good idea to only refresh every second, but calling time() in itself can be a bit costly if it is done too often (for example if it's called inside a loop with a large number of very short iterations). I am not sure if this is the case here, but one simple option is to add a simple iterative counter and to only check time every 100 or 1000 iterations for example

static int i = 0;
if (i ++ != 100)
  return;
const double current_time = time.time();

[lrineau]

I agree, in general, because time() can make a syscall. But in my case the loop itself is just a lot of syscalls, that read the file. I do not think that can slow down. ...

-> to bench.

[sloriot]

@lrineau is this PR ready to be tested?

[lrineau]

Yes, I would like to verify how portable my proposal is.

But I do no think it is ready to be integrated in master.

[sloriot]

Locally building the polyhedron demo I got:

  STL_Extension/include/CGAL/callback.h:34:0: warning: "CGAL_CALLBACK" redefined
  #  define CGAL_CALLBACK(f, p1)
  ^
  STL_Extension/include/CGAL/callback.h:33:0: note: this is the location of the previous definition
  #  define CGAL_CALLBACK(f)
  ^

[lrineau]

(In reply to #1036 (comment), by @sloriot)

Oops. I forgot that C++03 not only does not allow variadic macros but it does not allow either to define a macro with several number of arguments.

Probably the lines 33-38 should define macros with different names:

#  define CGAL_CALLBACK0(f)
#  define CGAL_CALLBACK1(f, p1)
#  define CGAL_CALLBACK2(f, p1, p2)
#  define CGAL_CALLBACK3(f, p1, p2, p3)
#  define CGAL_CALLBACK4(f, p1, p2, p3, p4)
#  define CGAL_CALLBACK5(f, p1, p2, p3, p4, p5)

and same for the C++11 part. But the API is not as nice as the previous one.

Or I could use:

 #  define CGAL_CALLBACK(...)

depending on the fact that all compilers we support probably support the variadic macros.

[MoniqueTeillaud]

the link https://cgal.geometryfactory.com/CGAL/Members/wiki/Meeting/Sophia16#.E2.9C.94_Progress_tracker given by Laurent in his initial message does not work.

[lrineau]

Fixed.

https://cgal.geometryfactory.com/CGAL/Members/wiki/Meetings/Sophia16#.E2.9C.94_Progress_tracker

[sgiraudot]

Is there anything we can do to advance on this feature?

(There is still one issue opened #102.)

[sgiraudot]

A callback mechanism similar to this one (but using boost::function when std::function is not available, which avoids the specific tags created here) was introduced in #2695. I don't know if you want to update this PR using the same behavior or if this was just a prototype to test things and that was not expected to be merged at some point.

[lrineau]

@sgiraudot, in #1036 (comment):

A callback mechanism similar to this one (but using boost::function when std::function is not available, which avoids the specific tags created here) was introduced in #2695. I don't know if you want to update this PR using the same behavior or if this was just a prototype to test things and that was not expected to be merged at some point.

Part of this PR was a prototype, that has to be refreshed, to use CGAL::cp11::function, instead of my complicated proposal with macros.

But the example I choose itself (load a big c3t3 file) is a real and useful one. For the moment, I propose that we refresh the PR, and try to display on the console (std::cout, or std::cerr... well std::clog is probably the right one) the progress of the loading. Then the code would be used as well to display a loading progress dialog in the GUI of the demo.

@maxGimeno Would you mind take over this PR, and try to refresh it using the simpler mechanism used in #2695?

[afabri]

Problem with nullptr in travis

[lrineau]

Anyway the code of this PR should be refreshed with the new way, using CGAL::cpp11::function.

@sgiraudot Do you have a documentation of the use of callbacks?

[sgiraudot]

You can refer to the documentation of detect() in the classes of Shape Detection.

[maxGimeno]

@lrineau is it really for 4.14-beta ? It doesn't look ready.