CGAL
Re-add Epick_without_intervals
Summary of Changes
New attempt for Epick_without_intervals. First attempt was #3939, reverted by commit bb640d2883be715e8e5727333ad54ef6b1fc6808 in #4096.
This time, I made it with the right kernel base, I think.
Release Management
- Affected package(s): all
Mael had suggestions in https://github.com/CGAL/cgal/pull/3939#issuecomment-512822185.
https://github.com/MaelRL/cgal/commit/88ee987eeda18b0c62a164900320c5d197aceba0 is a proof of concept of what I meant. I only modified Orientation_3.
Running 'Kernel_23/test/Kernel_23/Cartesian' gives:
typeid k: N4CGAL5EpickE
static call in orientation_3
Calling base of static...
Calling a filtered predicate
Could not resolve with filtered predicate, calling base of filtered
typeid kiwi: N4CGAL23Epick_without_intervalsE
static call in orientation_3
Calling base of static...
MaelRL@88ee987 is a proof of concept of what I meant.
Makes sense, but you will need a (trivial) functor for all remaining predicates to replace them with an exact version, not just a fallback for those that have a static filter. An alternative would be to decompose it into 2 wrappers: one that replaces all predicates with their exact counterpart, and one that interposes static filters on known predicates, but this split is probably not necessary.
One simple way to get something like this is to modify Filtered_predicate and put everything but the last line of the body between #ifndef CGAL_NO_INTERVALS and #endif. Then you can just use Epick. (ok, probably this needs to be done in a few more places as well)
PR #4495 would also benefit from the same mechanism of Static_filters<CK> not automatically wrapping with Filtered_kernel_base. It uses a special type of filtering and cannot use CGAL::Filtered_predicate. For now, we have added (commit https://github.com/CGAL/cgal/pull/4495/commits/46876c8395ba13535126a7dfd4fda848b7f0b522) another class that omits this wrapping, but if changes from above were to be used it could be simply Static_filters<CK>.
@mglisse Since CGAL::Filtered_predicate is its own class, I don't know if we can handle everything with macros. Maybe just a template parameter to Filtered_kernel_base? But then wouldn't it look a bit weird with CGAL::Filtered_kernel_base<CK, DONT_FILTER>?
@mglisse Since
CGAL::Filtered_predicateis its own class, I don't know if we can handle everything with macros.
The idea of a macro was first as a quick proof of concept, and second that the only point of Epick_without_interval (for users) is as a replacement for Epick on platforms where we cannot use intervals (no rounded operations), so it might as well be called Epick, controlled by some macros.
Maybe just a template parameter to
Filtered_kernel_base? But then wouldn't it look a bit weird withCGAL::Filtered_kernel_base<CK, DONT_FILTER>?
You can call it CGAL::Filtered_kernel_base<CK, NO_INTERVAL> if that looks less weird. In software pipelines, I have seen a few times functions that take an argument enable=true/false, so that the pipeline is structurally constant, but the behavior can be controlled. So I don't find it shocking anymore...
(I haven't looked at this since October, so I don't remember much, and I haven't looked at your other branch either, so don't take my comment as something insightful that needs to be analyzed)
The implementation is wrong. If I have time, I will enhance internal::Static_filters like I said above (I did part of it in the Filtered rational kernel), and it can be finished that way.