mirror of https://github.com/CGAL/cgal
132 lines
7.2 KiB
Plaintext
132 lines
7.2 KiB
Plaintext
Stuff to look at, as time permits:
|
|
----------------------------------
|
|
|
|
- include/CGAL/smallest_radiusC2.h should use the kernel code.
|
|
Check if it's ok (correct and as efficient), and tell Frank to do so.
|
|
The name of this file doesn't have anything to do with what is inside anyway.
|
|
- Write a Checker for full kernels, like Pm_traits_checker.h ?
|
|
- SphereC3 should use Handle_for<>.
|
|
- Use CGAL_TYPENAME_MSVC_NULL instead of "#define typename".
|
|
Only in the template parameters, right ?
|
|
- Rename CGAL_CFG_NO_ADVANCED_KERNEL to CGAL_USE_ADVANCED_KERNEL, definable on
|
|
the command line, and not in config.h...
|
|
- Aff_tranformations can't use Handle_for<> yet.
|
|
- Merging Cartesian and SimpleCartesian : mail to Stefan [+ the list ?]
|
|
Currently, there are 10000 lines of code in SimpleCartesian kernel, mostly
|
|
copy-pasted and adapted from the Cartesian kernel. This is evil in itself
|
|
from the maintainance point of view. I think only the low level (FT-based)
|
|
predicates and constructions are common (and this is already diverging).
|
|
Moreover, I think it would be good for the Cartesian kernel to somehow
|
|
have access to raw points/objects, in order to, for example:
|
|
- easy to have Iso_rectangleC2 being a twotuple of non ref-counted points,
|
|
whereas now it's a twotuple of ref-counted points, which is a pure lost.
|
|
(all local, temporary objects don't need any ref-counting)
|
|
- still my big plans of a kernel offering filtered constructions would be
|
|
far easier and cleaner if based on the primitives of such a kernel.
|
|
So, I propose to merge much more of these two kernels : I propose that the
|
|
ref-counted kernel becomes just a set of wrappers around non ref-counted
|
|
kernel objects and associated predicates/constructions.
|
|
So basically, I would like to offer another non-ref counted Cartesian kernel
|
|
(thus replacing the current SimpleCartesian in the end).
|
|
Having a central place for the code and a single maintainer for those 2
|
|
kernels seems a better solution than the current one (which will quickly
|
|
diverge, like homogeneous and Cartesian did in the past).
|
|
What do you think ?
|
|
[ Note : another possibility would have been to have a template parameter
|
|
to the kernel specifying the ref-countability, and having partial
|
|
specialization of some rep classes. But I fear a well-known "compiler"
|
|
would have problems with this... ]
|
|
- Maybe easier : in Cartesian<>, define :
|
|
typedef Handle_for <twotuple<FT> > Handled_Point_2;
|
|
|
|
And derive :
|
|
template <class R>
|
|
PointC2 : public R::Handled_Point_2
|
|
And similarly for SimpleCartesian.
|
|
Try it...
|
|
- ::bbox() robustness issues : it's not robust, since basically, we use
|
|
to_double().
|
|
The homogeneous kernel uses an epsilon to get this right, in the case
|
|
to_double() returns an error < 1 ulp().
|
|
I would propose to use the intervals, and require the NTs to have a
|
|
to_interval(). For most of the current ones, it's already done, so...
|
|
Ask Stefan and the list about that. For PointH2::bbox(), we would have:
|
|
{
|
|
Interval_base x = CGAL::to_interval(hx());
|
|
Interval_base y = CGAL::to_interval(hy());
|
|
Interval_base w = CGAL::to_interval(hw());
|
|
// The following can be slightly optimized using the advanced class.
|
|
return Bbox_2(Interval_nt<>(x)/Interval_nt<>(w),
|
|
Interval_nt<>(y)/Interval_nt<>(w));
|
|
}
|
|
And PointC2::bbox():
|
|
{
|
|
return Bbox_2(CGAL::to_interval(x()), CGAL::to_interval(y()));
|
|
}
|
|
- Why can't we simply have : typedef Iso_rectangleC2<double> Bbox_2; ?
|
|
- Iso_rectangleC2 stores a Twotuple<Point_2>, which means they are
|
|
ref-counted, which is sub-optimal... See above.
|
|
- Have a separate kernel for the advanced cartesian ?
|
|
- Getting rid of the partial kernels Cartesian_2 and Cartesian_3 ? This is a
|
|
lot (1Kloc) of redundant code, and this is something only aimed at reducing
|
|
compile time, for which I have some serious doubts anyway. So:
|
|
- XXX: Make sure it's not needed. Cartesian seems built on top of it...
|
|
- Make a compile time benchmark between Cartesian_2 vs Cartesian, say, with
|
|
Triangulation_2.
|
|
- Herve's opinion (original author) is it's not important.
|
|
- Check it's not documented. Grep it's not used.
|
|
- Ask on cgal-develop if anyone uses/needs this.
|
|
- Remove.
|
|
- Can I get rid of the empty destructors of the kernel classes ?
|
|
The homogeneous kernel defines them too. Why ?
|
|
Was it useful when they derived from a virtual base having a virtual
|
|
destructor, and thus needing this destructor ? It's not the case anymore,
|
|
so maybe it's time to gain a few lines :)
|
|
- Eventually merge C2, C3, Cartesian_basic (Cd ?). Ask if it's a problem for
|
|
others. If not, merge them inside CVS, so that history won't be lost.
|
|
- Factorize all the new kernel traits stuff of the different kernels in a
|
|
separate .def file, included by all kernels ?
|
|
- Orientation of Circle_2 and Sphere_3. I wonder if it's used anywhere.
|
|
Wouldn't it have been better if the kernel circles and spheres were not
|
|
oriented, and if someone needs an orientation, he will be able to build one
|
|
using the kernel's as a base or something. Because right now, Weighted_point
|
|
need to be defined in regular triangulations (check if it's the same
|
|
representation, square_radius wise...)... It seems even more strange in 3D.
|
|
Maybe it's too late for a change, but maybe not ?
|
|
Or maybe worth having a separate Non_oriented_Circle in the kernel ?
|
|
(the oriented one using code from the non-oriented one...)
|
|
- How does the test-suite runs on the kernel ? I never figured that out ;)
|
|
Run GCOV on it, I think it's _desperately_ needed...
|
|
- Test Advanced Kernel, Cd.
|
|
- Put the predicates that are marked /*CGAL_NO_FILTER*/ in a separate file,
|
|
say <predicates/no_filter_kernel_ftC2.h> ? This will simplify the script
|
|
somehow, but how will it work for those damn static filters ?
|
|
They have to be filtered for the filtered constructions kernel though.
|
|
And they do not depend on the other predicates... yes some do !
|
|
[ why compare_angle_with_x_axisC2() is not filtered,
|
|
while orientationC2() is ???!!!??? ]
|
|
Currently, there are 10/27 like that for C2, and 10/21 for C3.
|
|
This defiitely must be cleaned up. Maybe after some work on the filtered
|
|
kernel ?
|
|
- There are still a few predicates and constructions that are not FT-based,
|
|
but mainly they are the simple ones, just needing to compute an opposite or
|
|
something. I need to think about what to do with these, it's painful to have
|
|
a 3-stage calling sequence for not that much...
|
|
- Merge Intersection package.
|
|
- line_get_pointC2() should take "i" as "const FT&" directly, not "int", it
|
|
will avoid several constructions. Also, propagating that to the higher
|
|
level will also bring more functionality at the top level...
|
|
Seems like a stupid design choice at the beginning ?
|
|
Or did I miss something ?
|
|
Do we want to allow that for other implementations ?
|
|
Maybe there are other similar places.
|
|
- Replace "xxx == FT(0)" by "is_zero(xxx)", and similar for FT(1), is_one().
|
|
It can be faster for some NTs.
|
|
- Homogeneous' Point_2 has member functions :
|
|
// and for efficiency in the predicates:
|
|
const RT& hx_ref() const;
|
|
const RT& hy_ref() const;
|
|
const RT& hw_ref() const;
|
|
Probably a good idea ? Or is it worse with doubles ? Let's try.
|
|
Why do Point.x(), y(), hx(), hy(), hw() return FT instead of a "const FT &" ?
|