Intersections: Consistent behavior of Circle_2 and Sphere_3 #9096
Conversation
added filtered traits for 3d AABB tree changed behavior of Sphere_3 Iso_cuboid_3 intersection
|
documentation still needs to be updated, but can be tested already |
AABB_tree/include/CGAL/AABB_tree/internal/AABB_filtered_traits_2.h
Outdated
Show resolved
Hide resolved
Intersections_3/include/CGAL/Intersections_3/internal/Iso_cuboid_3_Sphere_3_do_intersect.h
Outdated
Show resolved
Hide resolved
Filtered_kernel/include/CGAL/Filtered_kernel/internal/Static_filters/Do_intersect_3.h
Show resolved
Hide resolved
|
Looks in CGAL-6.2-Ic-30 but the PR is still under review. |
sloriot
added
the
Not yet approved
|
Edit: The timeouts are due to another PR. I suspect this PR to be responsible for the timeouts . However, the timeouts only concern debug compilations and could not be reproduced locally (msvc). |
fixing static filter for Sphere_3 Iso_cuboid_3 intersection
| assert( !CGAL::Epick::Has_static_filters); | ||
| #endif | ||
|
|
||
| #ifdef CGAL_DONT_USE_LAZY_KERNEL |
There was a problem hiding this comment.
CGAL_LAZY_KERNEL_USE_STATIC_FILTERS_BY_DEFAULT is not used or set anywhere and it was conflicting with the change in Lazy_kernel.h
I can put it back and replace the macro from above.
Mael suggested to remove it.
| return recursive_descendant(child(node, i), remaining_indices...); | ||
| } | ||
|
|
||
| bool do_intersect(Node_index n, const Sphere& sphere) const { |
There was a problem hiding this comment.
why not renaming the function and keep the rest as is? I'm not sure to understand the changes
There was a problem hiding this comment.
also for intersected_nodes(), shouldn't we have version with the intersection functor provided as third parameter?
There was a problem hiding this comment.
do_intersect(Node_index n, const Sphere& sphere) was a private method that was used in the recursive nearest neighbors method. However, the nodes are sorted with respect to the distance. Thus, most of the work for calculating an intersection is already done. The new version benchmarked a bit faster in the tests I did.
The change for intersected_nodes makes sense. I can add it.
There was a problem hiding this comment.
I added intersected_nodes using a functor. However, I don't have a solution for d dimensional so the method without provided functor is broken in case of dimension>3
|
Ping @soesau |
3 participants
Summary of Changes
Circle_2 and Sphere_3 intersections are not fully consistent. The Circle_2 - Segment_2 intersection does consider inclusion as intersection. The same is true for Sphere_3 and Iso_cuboid_3.
This PR changes the behavior of those two intersection methods and adds specific intersection methods with inclusion for Circle_2 and Iso_rectangle_2 to AABB_traits_2 with static filter. The same will be done for Sphere_3, Iso_cuboid_3 and AABB_traits_3.
Release Management