CGAL / cgal

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

Intersect_3 Bbox_3/Segment_3 invalid implementation #7124

Open sloriot opened 1 year ago

sloriot commented 1 year ago

The code is using some calls to to_double which implies that the result cannot be exact. Why don't we use Iso_cuboid_3 implementation?

I'm not sure it is related but the following code fails (works with Kernel::Isocuboid_3(bb) instead of bb).

#include <CGAL/Exact_predicates_inexact_constructions_kernel.h>
#include <CGAL/Mesh_3/Robust_intersection_traits_3.h>

using Kernel = CGAL::Exact_predicates_inexact_constructions_kernel;

int main()
{
  CGAL::Bbox_3 bb(0, 0, 0, 4, 4, 5);

  Kernel::Segment_3 s(Kernel::Point_3(3.0403999999959956, -65734357381440816, 1.0410400227272694),
                      Kernel::Point_3(3.0403999999999995, 0.040399999999999207, 1.4972194341340495));

  auto inter = CGAL::Mesh_3::Robust_intersection_3<Kernel>()(bb, s);

}
sloriot commented 1 year ago

Screenshot from 2022-12-16 15-13-42

afabri commented 1 year ago

It probably makes sense to add an overload for Bbox_3 here which constructs an exact iso cuboid. Would that solve your problem?

afabri commented 1 year ago

Is the bbox segment intersection computation done in the Mesh_3 code?

sloriot commented 1 year ago

No that's the Kernel construction.

MaelRL commented 1 year ago

The code is using some calls to to_double which implies that the result cannot be exact. Why don't we use Iso_cuboid_3 implementation?

You need to use Coercion traits in case the kernel is something strange like FT = float, it's done properly in the do_intersect() (e.g. Bbox_3, Plane_3).

afabri commented 1 year ago

You mean Bbox_3/Line_3, right?

MaelRL commented 1 year ago

You mean Bbox_3/Line_3, right?

Yes, sorry.

afabri commented 1 year ago

Do we speak about the predicate or the construction?

This comment for the bbox/isocuboid predicate mentions coercion traits.