From 921832bc720a8ecefeb82baf539cc1211d828e19 Mon Sep 17 00:00:00 2001 From: Johannes0021 Date: Tue, 4 Mar 2025 20:02:42 +0100 Subject: [PATCH 1/5] Fix #310: Matrix index out of bounds. --- src/query/clip/clip_aabb_line.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/query/clip/clip_aabb_line.rs b/src/query/clip/clip_aabb_line.rs index 724d3e74..681a93b2 100644 --- a/src/query/clip/clip_aabb_line.rs +++ b/src/query/clip/clip_aabb_line.rs @@ -137,8 +137,10 @@ pub fn clip_aabb_line( if near_side < 0 { normal[(-near_side - 1) as usize] = 1.0; - } else { + } else if near_side > 0 { normal[(near_side - 1) as usize] = -1.0; + } else { + return None; } (tmin, normal, near_side) @@ -151,8 +153,10 @@ pub fn clip_aabb_line( if far_side < 0 { normal[(-far_side - 1) as usize] = -1.0; - } else { + } else if far_side > 0 { normal[(far_side - 1) as usize] = 1.0; + } else { + return None; } (tmax, normal, far_side) From c1685992f3911f30659d74879b98a7ebe2f8ece4 Mon Sep 17 00:00:00 2001 From: Johannes0021 Date: Wed, 19 Mar 2025 21:36:23 +0100 Subject: [PATCH 2/5] Fix #310: Matrix index out of bounds. + Clippy --- src/query/clip/clip_aabb_line.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/query/clip/clip_aabb_line.rs b/src/query/clip/clip_aabb_line.rs index 681a93b2..16cf0efc 100644 --- a/src/query/clip/clip_aabb_line.rs +++ b/src/query/clip/clip_aabb_line.rs @@ -133,14 +133,17 @@ pub fn clip_aabb_line( let near = if near_diag { (tmin, -dir.normalize(), near_side) } else { + // If near_side is 0, the index calculation would be invalid (-1). + if near_side == 0 { + return None; + } + let mut normal = Vector::zeros(); if near_side < 0 { normal[(-near_side - 1) as usize] = 1.0; - } else if near_side > 0 { - normal[(near_side - 1) as usize] = -1.0; } else { - return None; + normal[(near_side - 1) as usize] = -1.0; } (tmin, normal, near_side) @@ -149,14 +152,17 @@ pub fn clip_aabb_line( let far = if far_diag { (tmax, -dir.normalize(), far_side) } else { + // If far_side is 0, the index calculation would be invalid (-1). + if far_side == 0 { + return None; + } + let mut normal = Vector::zeros(); if far_side < 0 { normal[(-far_side - 1) as usize] = -1.0; - } else if far_side > 0 { - normal[(far_side - 1) as usize] = 1.0; } else { - return None; + normal[(far_side - 1) as usize] = 1.0; } (tmax, normal, far_side) From 8940a19bfb0f57cf6f50b30299dd5b9d00ad153a Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 25 Jun 2025 17:37:58 +0200 Subject: [PATCH 3/5] add test + changelog --- CHANGELOG.md | 4 +++ src/query/clip/clip_aabb_line.rs | 43 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 51b836cf..8bc35d41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Change Log +### Fixed + +- Fix `clip_aabb_line` crashing when given incorrect inputs (zero length direction or NAN). + ## 0.21.1 ### Added diff --git a/src/query/clip/clip_aabb_line.rs b/src/query/clip/clip_aabb_line.rs index 276f32f3..249c3adc 100644 --- a/src/query/clip/clip_aabb_line.rs +++ b/src/query/clip/clip_aabb_line.rs @@ -170,3 +170,46 @@ pub fn clip_aabb_line( Some((near, far)) } +#[cfg(test)] +mod test { + use super::*; + + #[test] + pub fn clip_empty_aabb_line() { + assert!(clip_aabb_line( + &Aabb::new(Point::origin(), Point::origin()), + &Point::origin(), + &[ + 0.0, + 0.0, + #[cfg(feature = "dim3")] + 0.0, + ] + .into(), + ) + .is_none()); + } + + #[test] + pub fn clip_empty_aabb_segment() { + let aabb_empty = Aabb::new(Point::origin(), Point::origin()); + assert!(aabb_empty + .clip_segment( + &[ + 0.0, + 0.0, + #[cfg(feature = "dim3")] + 0.0, + ] + .into(), + &[ + Real::NAN, + Real::NAN, + #[cfg(feature = "dim3")] + Real::NAN, + ] + .into(), + ) + .is_none()); + } +} From 8ebb3712a5e87b372dff8980086bf6fa215c5f03 Mon Sep 17 00:00:00 2001 From: Thierry Berger Date: Wed, 2 Jul 2025 08:55:40 +0200 Subject: [PATCH 4/5] simpler tests api used --- src/query/clip/clip_aabb_line.rs | 25 ++----------------------- 1 file changed, 2 insertions(+), 23 deletions(-) diff --git a/src/query/clip/clip_aabb_line.rs b/src/query/clip/clip_aabb_line.rs index 249c3adc..d8e0f905 100644 --- a/src/query/clip/clip_aabb_line.rs +++ b/src/query/clip/clip_aabb_line.rs @@ -179,13 +179,7 @@ mod test { assert!(clip_aabb_line( &Aabb::new(Point::origin(), Point::origin()), &Point::origin(), - &[ - 0.0, - 0.0, - #[cfg(feature = "dim3")] - 0.0, - ] - .into(), + &Vector::zeros(), ) .is_none()); } @@ -194,22 +188,7 @@ mod test { pub fn clip_empty_aabb_segment() { let aabb_empty = Aabb::new(Point::origin(), Point::origin()); assert!(aabb_empty - .clip_segment( - &[ - 0.0, - 0.0, - #[cfg(feature = "dim3")] - 0.0, - ] - .into(), - &[ - Real::NAN, - Real::NAN, - #[cfg(feature = "dim3")] - Real::NAN, - ] - .into(), - ) + .clip_segment(&Point::origin(), &Point::from(Vector::repeat(Real::NAN))) .is_none()); } } From b65f1cae47ade14cbbf13e1ca22096d7fc83ceb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Crozet?= Date: Fri, 11 Jul 2025 14:10:02 +0200 Subject: [PATCH 5/5] =?UTF-8?q?fix:=20make=20clip=5Faabb=5Fline=20return?= =?UTF-8?q?=20a=20result=20if=20the=20line=E2=80=99s=20starting=20point=20?= =?UTF-8?q?is=20inside=20the=20AABB=E2=80=AFbut=20its=20direction=20is=20z?= =?UTF-8?q?ero.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows other functions like segment intersection to return a valid result if the segment degenerates to a point. --- src/query/clip/clip_aabb_line.rs | 45 +++++++++++++++++++++++++------- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/src/query/clip/clip_aabb_line.rs b/src/query/clip/clip_aabb_line.rs index d8e0f905..966600b9 100644 --- a/src/query/clip/clip_aabb_line.rs +++ b/src/query/clip/clip_aabb_line.rs @@ -7,7 +7,7 @@ use num::{Bounded, Zero}; impl Aabb { /// Computes the intersection of a segment with this Aabb. /// - /// Returns `None` if there is no intersection. + /// Returns `None` if there is no intersection or if `pa` is invalid (contains `NaN`). #[inline] pub fn clip_segment(&self, pa: &Point, pb: &Point) -> Option { let ab = pb - pa; @@ -18,7 +18,7 @@ impl Aabb { /// Computes the parameters of the two intersection points between a line and this Aabb. /// /// The parameters are such that the point are given by `orig + dir * parameter`. - /// Returns `None` if there is no intersection. + /// Returns `None` if there is no intersection or if `orig` is invalid (contains `NaN`). #[inline] pub fn clip_line_parameters( &self, @@ -30,7 +30,7 @@ impl Aabb { /// Computes the intersection segment between a line and this Aabb. /// - /// Returns `None` if there is no intersection. + /// Returns `None` if there is no intersection or if `orig` is invalid (contains `NaN`). #[inline] pub fn clip_line(&self, orig: &Point, dir: &Vector) -> Option { clip_aabb_line(self, orig, dir) @@ -40,7 +40,7 @@ impl Aabb { /// Computes the parameters of the two intersection points between a ray and this Aabb. /// /// The parameters are such that the point are given by `ray.orig + ray.dir * parameter`. - /// Returns `None` if there is no intersection. + /// Returns `None` if there is no intersection or if `ray.orig` is invalid (contains `NaN`). #[inline] pub fn clip_ray_parameters(&self, ray: &Ray) -> Option<(Real, Real)> { self.clip_line_parameters(&ray.origin, &ray.dir) @@ -67,6 +67,15 @@ impl Aabb { } /// Computes the segment given by the intersection of a line and an Aabb. +/// +/// Returns the two intersections represented as `(t, normal, side)` such that: +/// - `origin + dir * t` gives the intersection points. +/// - `normal` is the face normal at the intersection. This is equal to the zero vector if `dir` +/// is invalid (a zero vector or NaN) and `origin` is inside the AABB. +/// - `side` is the side of the AABB that was hit. This is an integer in [-3, 3] where `1` represents +/// the `+X` axis, `2` the `+Y` axis, etc., and negative values represent the corresponding +/// negative axis. The special value of `0` indicates that the provided `dir` is zero or `NaN` +/// and the line origin is inside the AABB. pub fn clip_aabb_line( aabb: &Aabb, origin: &Point, @@ -133,9 +142,12 @@ pub fn clip_aabb_line( let near = if near_diag { (tmin, -dir.normalize(), near_side) } else { - // If near_side is 0, the index calculation would be invalid (-1). + // If near_side is 0, we are in a special case where `dir` is + // zero or NaN. Return `Some` only if the ray starts inside the + // aabb. if near_side == 0 { - return None; + let zero = (0.0, Vector::zeros(), 0); + return aabb.contains_local_point(origin).then_some((zero, zero)); } let mut normal = Vector::zeros(); @@ -152,9 +164,12 @@ pub fn clip_aabb_line( let far = if far_diag { (tmax, -dir.normalize(), far_side) } else { - // If far_side is 0, the index calculation would be invalid (-1). + // If far_side is 0, we are in a special case where `dir` is + // zero or NaN. Return `Some` only if the ray starts inside the + // aabb. if far_side == 0 { - return None; + let zero = (0.0, Vector::zeros(), 0); + return aabb.contains_local_point(origin).then_some((zero, zero)); } let mut normal = Vector::zeros(); @@ -181,13 +196,23 @@ mod test { &Point::origin(), &Vector::zeros(), ) + .is_some()); + assert!(clip_aabb_line( + &Aabb::new(Vector::repeat(1.0).into(), Vector::repeat(2.0).into()), + &Point::origin(), + &Vector::zeros(), + ) .is_none()); } #[test] pub fn clip_empty_aabb_segment() { - let aabb_empty = Aabb::new(Point::origin(), Point::origin()); - assert!(aabb_empty + let aabb_origin = Aabb::new(Point::origin(), Point::origin()); + let aabb_shifted = Aabb::new(Vector::repeat(1.0).into(), Vector::repeat(2.0).into()); + assert!(aabb_origin + .clip_segment(&Point::origin(), &Point::from(Vector::repeat(Real::NAN))) + .is_some()); + assert!(aabb_shifted .clip_segment(&Point::origin(), &Point::from(Vector::repeat(Real::NAN))) .is_none()); }