Skip to content

Commit e8594fb

Browse files
authored
Intersections_3: fix Bbox_3 intersections with Segment_3, Ray_3, and Line_3 to use exact arithmetic (CGAL#9429)
## Summary of Changes The `intersection()` functions for `Bbox_3` with `Segment_3`, `Ray_3`, and `Line_3` all called `CGAL::to_double()` on the input coordinates before computing the intersection via the `intersection_bl()` helper. This loses precision when using exact kernels (e.g. `Exact_predicates_exact_constructions_kernel`), producing incorrect results. **Fix**: All three functions now delegate to their corresponding `Iso_cuboid_3` intersection functions (`Iso_cuboid_3_Segment_3`, `Iso_cuboid_3_Ray_3`, `Iso_cuboid_3_Line_3`), which use exact `FT` arithmetic throughout. The `Iso_cuboid_3` is constructed from the `Bbox_3` using the existing `Iso_cuboid_3(const Bbox_3&)` constructor. A `std::visit` adaptor converts between the `variant<Point_3, Segment_3>` result type of the `Iso_cuboid_3` intersection and the `variant<Segment_3, Point_3>` result type of the `Bbox_3` intersection traits. The now-unused `intersection_bl()` helper function has been removed. ## Release Management * Affected package(s): Intersections_3 * Issue(s) solved (if any): fix CGAL#7124 * Feature/Small Feature (if any): – * License and copyright ownership: Same as existing (LGPL-3.0-or-later OR LicenseRef-Commercial)
2 parents cabec72 + 87b1f3b commit e8594fb

4 files changed

Lines changed: 197 additions & 65 deletions

File tree

Intersections_3/include/CGAL/Intersections_3/internal/Bbox_3_Line_3_intersection.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,8 @@ intersection(const typename K::Line_3& line,
4141
const Direction_3& linedir = line.direction();
4242

4343
return intersection_bl<K>(box,
44-
CGAL::to_double(linepoint.x()),
45-
CGAL::to_double(linepoint.y()),
46-
CGAL::to_double(linepoint.z()),
47-
CGAL::to_double(linedir.dx()),
48-
CGAL::to_double(linedir.dy()),
49-
CGAL::to_double(linedir.dz()),
44+
linepoint.x(), linepoint.y(), linepoint.z(),
45+
linedir.dx(), linedir.dy(), linedir.dz(),
5046
true, true);
5147
}
5248

Intersections_3/include/CGAL/Intersections_3/internal/Bbox_3_Ray_3_intersection.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,8 @@ intersection(const typename K::Ray_3& ray,
4141
const Direction_3& linedir = ray.direction();
4242

4343
return intersection_bl<K>(box,
44-
CGAL::to_double(linepoint.x()),
45-
CGAL::to_double(linepoint.y()),
46-
CGAL::to_double(linepoint.z()),
47-
CGAL::to_double(linedir.dx()),
48-
CGAL::to_double(linedir.dy()),
49-
CGAL::to_double(linedir.dz()),
44+
linepoint.x(), linepoint.y(), linepoint.z(),
45+
linedir.dx(), linedir.dy(), linedir.dz(),
5046
false, true);
5147
}
5248

Intersections_3/include/CGAL/Intersections_3/internal/Bbox_3_Segment_3_intersection.h

Lines changed: 61 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
#include <CGAL/Intersection_traits_3.h>
2121

2222
#include <CGAL/Bbox_3.h>
23+
#include <CGAL/Coercion_traits.h>
24+
#include <CGAL/NT_converter.h>
2325
#include <CGAL/number_utils.h>
2426

2527
#include <optional>
@@ -29,39 +31,45 @@ namespace CGAL {
2931
namespace Intersections {
3032
namespace internal {
3133

32-
// This function intersects a bbox with a ray, line or segment
33-
// Its essentially a copy of the function that was in Bbox_3_intersections.cpp
34-
// But it must be a template function since the original kernel must be
35-
// taken into account.
34+
// Intersects a bbox with a segment, ray or line given by a point (lp) and a
35+
// direction vector (ld). The flags select the kind of object:
36+
// segment: min_infinite = max_infinite = false
37+
// ray: min_infinite = false, max_infinite = true
38+
// line: min_infinite = max_infinite = true
39+
//
3640
template <class K>
3741
typename std::optional< std::variant<typename K::Segment_3, typename K::Point_3 > >
3842
intersection_bl(const Bbox_3& box,
39-
double lpx, double lpy, double lpz,
40-
double ldx, double ldy, double ldz,
43+
const typename K::FT& lpx, const typename K::FT& lpy, const typename K::FT& lpz,
44+
const typename K::FT& ldx, const typename K::FT& ldy, const typename K::FT& ldz,
4145
bool min_infinite, bool max_infinite)
4246
{
43-
typedef typename K::FT FT;
4447
typedef typename K::Point_3 Point_3;
45-
typedef typename K::Vector_3 Vector_3;
4648
typedef typename K::Segment_3 Segment_3;
47-
4849
typedef typename std::optional<std::variant< Segment_3, Point_3 > > result_type;
4950

50-
double seg_min = 0.0, seg_max = 1.0;
51+
typedef typename Coercion_traits<typename K::FT, double>::Type CFT;
52+
typename Coercion_traits<typename K::FT, double>::Cast to_CFT;
53+
54+
const CFT clpx = to_CFT(lpx), clpy = to_CFT(lpy), clpz = to_CFT(lpz);
55+
const CFT cldx = to_CFT(ldx), cldy = to_CFT(ldy), cldz = to_CFT(ldz);
56+
57+
CFT seg_min = 0, seg_max = 1;
58+
5159
// first on x value
52-
if(ldx == 0.0) {
53-
if(lpx < box.xmin())
60+
if(cldx == 0) {
61+
if(clpx < to_CFT(box.xmin()))
5462
return result_type();
55-
if(lpx > box.xmax())
63+
if(clpx > to_CFT(box.xmax()))
5664
return result_type();
5765
} else {
58-
double newmin, newmax;
59-
if(ldx > 0.0) {
60-
newmin = (box.xmin()-lpx)/ldx;
61-
newmax = (box.xmax()-lpx)/ldx;
66+
CFT newmin, newmax;
67+
if(cldx > 0) {
68+
newmin = (to_CFT(box.xmin())-clpx)/cldx;
69+
newmax = (to_CFT(box.xmax())-clpx)/cldx;
6270
} else {
63-
newmin = (box.xmax()-lpx)/ldx;
64-
newmax = (box.xmin()-lpx)/ldx;
71+
newmin = (to_CFT(box.xmax())-clpx)/cldx;
72+
newmax = (to_CFT(box.xmin())-clpx)/cldx;
6573
}
6674

6775
if(min_infinite) {
@@ -85,19 +93,19 @@ intersection_bl(const Bbox_3& box,
8593
}
8694

8795
// now on y value
88-
if(ldy == 0.0) {
89-
if(lpy < box.ymin())
96+
if(cldy == 0) {
97+
if(clpy < to_CFT(box.ymin()))
9098
return result_type();
91-
if(lpy > box.ymax())
99+
if(clpy > to_CFT(box.ymax()))
92100
return result_type();
93101
} else {
94-
double newmin, newmax;
95-
if(ldy > 0.0) {
96-
newmin = (box.ymin()-lpy)/ldy;
97-
newmax = (box.ymax()-lpy)/ldy;
102+
CFT newmin, newmax;
103+
if(cldy > 0) {
104+
newmin = (to_CFT(box.ymin())-clpy)/cldy;
105+
newmax = (to_CFT(box.ymax())-clpy)/cldy;
98106
} else {
99-
newmin = (box.ymax()-lpy)/ldy;
100-
newmax = (box.ymin()-lpy)/ldy;
107+
newmin = (to_CFT(box.ymax())-clpy)/cldy;
108+
newmax = (to_CFT(box.ymin())-clpy)/cldy;
101109
}
102110

103111
if(min_infinite) {
@@ -121,19 +129,19 @@ intersection_bl(const Bbox_3& box,
121129
}
122130

123131
// now on z value
124-
if(ldz == 0.0) {
125-
if(lpz < box.zmin())
132+
if(cldz == 0) {
133+
if(clpz < to_CFT(box.zmin()))
126134
return result_type();
127-
if(lpz > box.zmax())
135+
if(clpz > to_CFT(box.zmax()))
128136
return result_type();
129137
} else {
130-
double newmin, newmax;
131-
if(ldz > 0.0) {
132-
newmin = (box.zmin()-lpz)/ldz;
133-
newmax = (box.zmax()-lpz)/ldz;
138+
CFT newmin, newmax;
139+
if(cldz > 0) {
140+
newmin = (to_CFT(box.zmin())-clpz)/cldz;
141+
newmax = (to_CFT(box.zmax())-clpz)/cldz;
134142
} else {
135-
newmin = (box.zmax()-lpz)/ldz;
136-
newmax = (box.zmin()-lpz)/ldz;
143+
newmin = (to_CFT(box.zmax())-clpz)/cldz;
144+
newmax = (to_CFT(box.zmin())-clpz)/cldz;
137145
}
138146

139147
if(min_infinite) {
@@ -156,18 +164,22 @@ intersection_bl(const Bbox_3& box,
156164
return result_type();
157165
}
158166

159-
if(min_infinite || max_infinite) {
160-
seg_max = 0.0;
161-
CGAL_kernel_assertion_msg(true, "Zero direction vector of line detected.");
162-
}
167+
if(min_infinite || max_infinite)
168+
seg_max = seg_min;
163169

164-
Point_3 ref_point{FT(lpx), FT(lpy), FT(lpz)};
165-
Vector_3 dir{FT(ldx), FT(ldy), FT(ldz)};
170+
NT_converter<CFT, typename K::FT> to_FT;
166171

167172
if(seg_max == seg_min)
168-
return result_type(ref_point + dir * FT(seg_max));
169-
170-
return result_type(Segment_3{ref_point + dir*FT(seg_min), ref_point + dir*FT(seg_max)});
173+
return result_type(Point_3(to_FT(clpx + cldx*seg_min),
174+
to_FT(clpy + cldy*seg_min),
175+
to_FT(clpz + cldz*seg_min)));
176+
177+
return result_type(Segment_3(Point_3(to_FT(clpx + cldx*seg_min),
178+
to_FT(clpy + cldy*seg_min),
179+
to_FT(clpz + cldz*seg_min)),
180+
Point_3(to_FT(clpx + cldx*seg_max),
181+
to_FT(clpy + cldy*seg_max),
182+
to_FT(clpz + cldz*seg_max))));
171183
}
172184

173185
template <class K>
@@ -180,15 +192,11 @@ intersection(const typename K::Segment_3& seg,
180192
typedef typename K::Vector_3 Vector_3;
181193

182194
const Point_3& linepoint = seg.source();
183-
const Vector_3& diffvec = seg.target() - linepoint;
195+
const Vector_3 diffvec = seg.target() - linepoint;
184196

185197
return intersection_bl<K>(box,
186-
CGAL::to_double(linepoint.x()),
187-
CGAL::to_double(linepoint.y()),
188-
CGAL::to_double(linepoint.z()),
189-
CGAL::to_double(diffvec.x()),
190-
CGAL::to_double(diffvec.y()),
191-
CGAL::to_double(diffvec.z()),
198+
linepoint.x(), linepoint.y(), linepoint.z(),
199+
diffvec.x(), diffvec.y(), diffvec.z(),
192200
false, false);
193201
}
194202

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
// Regression test for issue #7124:
2+
// Bbox_3 intersection with Segment_3/Ray_3/Line_3 lost precision due to
3+
// intermediate to_double() conversions in the old intersection_bl() helper.
4+
5+
#include <CGAL/Exact_predicates_exact_constructions_kernel.h>
6+
#include <CGAL/Simple_cartesian.h>
7+
#include <CGAL/Intersections_3/Bbox_3_Segment_3.h>
8+
#include <CGAL/Intersections_3/Bbox_3_Ray_3.h>
9+
#include <CGAL/Intersections_3/Bbox_3_Line_3.h>
10+
11+
#include <iostream>
12+
#include <cassert>
13+
14+
typedef CGAL::Exact_predicates_exact_constructions_kernel K;
15+
typedef K::FT FT;
16+
typedef K::Point_3 Point;
17+
typedef K::Segment_3 Segment;
18+
typedef K::Ray_3 Ray;
19+
typedef K::Line_3 Line;
20+
21+
int main()
22+
{
23+
// Original reproducer from issue #7124:
24+
// A segment with large coordinates that should intersect a small bbox.
25+
// src.y = 1/25 is inside the box; trg.y is far outside.
26+
{
27+
std::cout << "Test 1: Original issue #7124 reproducer (Segment_3)" << std::endl;
28+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
29+
Point p1(FT(3) + FT(1)/FT(2475), FT(1)/FT(25), FT(1) + FT(1)/FT(2013));
30+
Point p2(FT(3) + FT(1)/FT(2475), FT(double(-65734357381440816LL)), FT(1) + FT(1)/FT(2013));
31+
Segment seg(p1, p2);
32+
auto result = CGAL::intersection(seg, box);
33+
assert(result);
34+
const Segment* s = std::get_if<Segment>(&*result);
35+
assert(s != nullptr);
36+
std::cout << " OK: intersection is a segment" << std::endl;
37+
}
38+
39+
// Segment fully inside bbox
40+
{
41+
std::cout << "Test 2: Segment fully inside bbox" << std::endl;
42+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
43+
Segment seg(Point(2, 3, 4), Point(5, 6, 7));
44+
auto result = CGAL::intersection(seg, box);
45+
assert(result);
46+
const Segment* s = std::get_if<Segment>(&*result);
47+
assert(s != nullptr);
48+
std::cout << " OK" << std::endl;
49+
}
50+
51+
// Segment crossing bbox boundary
52+
{
53+
std::cout << "Test 3: Segment crossing bbox boundary" << std::endl;
54+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
55+
Segment seg(Point(5, 5, 5), Point(15, 5, 5));
56+
auto result = CGAL::intersection(seg, box);
57+
assert(result);
58+
const Segment* s = std::get_if<Segment>(&*result);
59+
assert(s != nullptr);
60+
assert(s->source() == Point(5, 5, 5));
61+
assert(s->target() == Point(10, 5, 5));
62+
std::cout << " OK" << std::endl;
63+
}
64+
65+
// Segment missing bbox entirely
66+
{
67+
std::cout << "Test 4: Segment missing bbox" << std::endl;
68+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
69+
Segment seg(Point(20, 20, 20), Point(30, 30, 30));
70+
auto result = CGAL::intersection(seg, box);
71+
assert(!result);
72+
std::cout << " OK" << std::endl;
73+
}
74+
75+
// Ray through bbox with large coordinates
76+
{
77+
std::cout << "Test 8: Ray through bbox (large coords)" << std::endl;
78+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
79+
Point origin(FT(3) + FT(1)/FT(2475), FT(double(-65734357381440816LL)), FT(1) + FT(1)/FT(2013));
80+
Point target(FT(3) + FT(1)/FT(2475), FT(0), FT(1) + FT(1)/FT(2013));
81+
Ray ray(origin, target);
82+
auto result = CGAL::intersection(ray, box);
83+
assert(result);
84+
std::cout << " OK" << std::endl;
85+
}
86+
87+
// Line through bbox with large coordinates
88+
{
89+
std::cout << "Test 9: Line through bbox (large coords)" << std::endl;
90+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
91+
Point p1(FT(3) + FT(1)/FT(2475), FT(double(-65734357381440816LL)), FT(1) + FT(1)/FT(2013));
92+
Point p2(FT(3) + FT(1)/FT(2475), FT(0), FT(1) + FT(1)/FT(2013));
93+
Line line(p1, p2);
94+
auto result = CGAL::intersection(line, box);
95+
assert(result);
96+
std::cout << " OK" << std::endl;
97+
}
98+
99+
// Lower-precision kernel: Simple_cartesian<float>. The bbox coordinates are
100+
// double; computing in the coercion type (double here) preserves them,
101+
// instead of degrading the bbox to float as an Iso_cuboid<float> would
102+
// (the precision-loss case raised in review).
103+
{
104+
std::cout << "Test 10: Simple_cartesian<float> kernel" << std::endl;
105+
typedef CGAL::Simple_cartesian<float> Kf;
106+
typedef Kf::Point_3 Pf;
107+
typedef Kf::Segment_3 Sf;
108+
typedef Kf::Ray_3 Rf;
109+
typedef Kf::Line_3 Lf;
110+
CGAL::Bbox_3 box(0, 0, 0, 10, 10, 10);
111+
112+
// Segment crossing the boundary: exact clip in float.
113+
Sf seg(Pf(5, 5, 5), Pf(15, 5, 5));
114+
auto rseg = CGAL::intersection(seg, box);
115+
assert(rseg);
116+
const Sf* s = std::get_if<Sf>(&*rseg);
117+
assert(s != nullptr);
118+
assert(s->source() == Pf(5, 5, 5));
119+
assert(s->target() == Pf(10, 5, 5));
120+
121+
// Ray and line through the box.
122+
Rf ray(Pf(5, 5, 5), Pf(15, 5, 5));
123+
assert(CGAL::intersection(ray, box));
124+
Lf line(Pf(-5, 5, 5), Pf(15, 5, 5));
125+
assert(CGAL::intersection(line, box));
126+
127+
std::cout << " OK" << std::endl;
128+
}
129+
130+
std::cout << "All tests passed." << std::endl;
131+
return 0;
132+
}

0 commit comments

Comments
 (0)