CGAL / cgal

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

Kernel_23d: Add a Bbox class with dimension as parameter #8258

Open afabri opened 4 weeks ago

afabri commented 4 weeks ago

Summary of Changes

For the upcoming Frechet Distance package we need a dD bounding box. We do not have that in Kernel_d, and there is such a class in ign's Distributed Delaunay Triangulation package to come (link). This PR copies it to the Kernel package (with the ok of @mbredif), but in the CGAL namespace.

This draft pull request is to get a discussion started.

Release Management

afabri commented 2 weeks ago

@mglisse I am also wondering why there is no bounding box class in Epick_d etc. Apparently there is no need for it so far, but would you like to see it integrated? With functors etc.

afabri commented 2 weeks ago

@mbredif do I have to make you collaborator, or is this already the case?

mglisse commented 2 weeks ago

@mglisse I am also wondering why there is no bounding box class in Epick_d etc.

I remember thinking about it, maybe discussing it, but I don't remember the content...

Apparently there is no need for it so far,

That may have played a big role :wink: (it doesn't seem to be part of the Kernel_d concept)

but would you like to see it integrated? With functors etc.

If you have a use for it, we probably should. To be sure I understand: the current bbox_23 always use double, whatever the kernel, making them not really kernel objects. Is the plan the same for the new bbox, i.e. ignore the generality offered by the template parameter and only use it with double (otherwise it becomes Iso_rectangle_2/Iso_cuboid_3)? It looks like the kernel integration can be done later in a separate PR (this one adds the class), which gives me a bit of time to think about how that fits with the design of NewKernel_d.

afabri commented 1 week ago

We also need a constructor taking an iterator range of coordinates. And maybe also for a range of pairs of coordinates. The former is needed when constructing a bbox from a point with floating point coordinates. The latter when constructing it from exact coordinates transformed to a std::pair<double,double> using CGAL::to_double().

I just realize that this would correspond more to what you do for the I/O operators. But do we want a constructor for a range of low and a second range for high? Seems not intuitive.

afabri commented 1 week ago

As Epick_d may have a dynamic and a static dimension, depending on a template parameter, the functor Epick_d::Construct_bbox_d() must return either a bbox type with dynamic and static dimension, respectively.