Skip to content

Commit

Permalink
fix: bound calculation for negative values
Browse files Browse the repository at this point in the history
Co-authored-by: Pete Gadomski <pete.gadomski@gmail.com>
  • Loading branch information
wischi-chr and gadomski committed May 13, 2024
1 parent 3cdf4e7 commit 0ab652d
Show file tree
Hide file tree
Showing 3 changed files with 95 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Fixed

- Deny more things ([#78](https://github.com/gadomski/las-rs/pull/78))
- Bounds calculation for negative values ([#77](https://github.com/gadomski/las-rs/pull/77))

## [0.8.6] - 2024-05-06

Expand Down
83 changes: 74 additions & 9 deletions src/bounds.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::f64;

use crate::{Point, Result, Transform, Vector};
use crate::{transform::RoundingMode, Point, Result, Transform, Vector};

/// Minimum and maximum bounds in three dimensions.
#[derive(Clone, Copy, Debug, PartialEq)]
Expand Down Expand Up @@ -69,24 +69,26 @@ impl Bounds {
/// assert_eq!(new_bounds.max.z, -0.094);
/// ```
pub fn adapt(&self, transform: &Vector<Transform>) -> Result<Self> {
fn inner_convert(value: f64, transform: &Transform) -> Result<f64> {
use RoundingMode::*;

fn inner_convert(value: f64, transform: &Transform, rounding: RoundingMode) -> Result<f64> {
// During saving, an instance with +-inf is saved. We must consider for this corner case.
if value.is_infinite() {
return Ok(value);
}
Ok(transform.direct(transform.inverse(value)?))
Ok(transform.direct(transform.inverse_with_rounding_mode(value, rounding)?))
}

Ok(Self {
min: Vector {
x: inner_convert(self.min.x, &transform.x)?,
y: inner_convert(self.min.y, &transform.y)?,
z: inner_convert(self.min.z, &transform.z)?,
x: inner_convert(self.min.x, &transform.x, Floor)?,
y: inner_convert(self.min.y, &transform.y, Floor)?,
z: inner_convert(self.min.z, &transform.z, Floor)?,
},
max: Vector {
x: inner_convert(self.max.x, &transform.x)?,
y: inner_convert(self.max.y, &transform.y)?,
z: inner_convert(self.max.z, &transform.z)?,
x: inner_convert(self.max.x, &transform.x, Ceil)?,
y: inner_convert(self.max.y, &transform.y, Ceil)?,
z: inner_convert(self.max.z, &transform.z, Ceil)?,
},
})
}
Expand Down Expand Up @@ -157,4 +159,67 @@ mod tests {
assert_eq!(2., bounds.min.z);
assert_eq!(4., bounds.max.z);
}

const EPSILON: f64 = 0.00000001;

#[test]
fn bounds_adapt_neg_tick_above() {
assert_bounds_adapt_to_value(-1.0 + EPSILON);
}

#[test]
fn bounds_adapt_pos_tick_above() {
assert_bounds_adapt_to_value(1.0 + EPSILON);
}

#[test]
fn bounds_adapt_neg_tick_below() {
assert_bounds_adapt_to_value(-1.0 - EPSILON);
}

#[test]
fn bounds_adapt_pos_tick_below() {
assert_bounds_adapt_to_value(1.0 - EPSILON);
}

fn assert_bounds_adapt_to_value(n: f64) {
let mut bounds = Bounds {
..Default::default()
};

let point = Point {
x: n,
y: n,
z: n,
..Default::default()
};

bounds.grow(&point);

let rounded_box = bounds.adapt(&Default::default()).unwrap();

assert_point_inside_bounds(&point, &rounded_box);
}

fn assert_point_inside_bounds(p: &Point, b: &Bounds) {
let x_inside = b.max.x >= p.x && b.min.x <= p.x;
let y_inside = b.max.y >= p.y && b.min.y <= p.y;
let z_inside = b.max.z >= p.z && b.min.z <= p.z;

let inside = x_inside && y_inside && z_inside;

if !inside {
let bounds = b;
let point = Vector::<f64> {
x: p.x,
y: p.y,
z: p.z,
};

panic!(
"Point was outside the calculated bounds:\n{:#?}\n\n",
(point, bounds)
);
}
}
}
21 changes: 20 additions & 1 deletion src/transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,23 @@ impl Transform {
/// assert_eq!(1, transform.inverse(2.9).unwrap());
/// ```
pub fn inverse(&self, n: f64) -> Result<i32> {
self.inverse_with_rounding_mode(n, RoundingMode::Round)
}

pub(crate) fn inverse_with_rounding_mode(&self, n: f64, r: RoundingMode) -> Result<i32> {
use crate::Error;
use std::i32;

let n = ((n - self.offset) / self.scale).round();
fn round(n: f64, r: RoundingMode) -> f64 {
match r {
RoundingMode::Round => n.round(),
RoundingMode::Ceil => n.ceil(),
RoundingMode::Floor => n.floor(),
}
}

let n = round((n - self.offset) / self.scale, r);

if n > f64::from(i32::MAX) || n < f64::from(i32::MIN) {
Err(Error::InverseTransform {
n,
Expand All @@ -66,6 +79,12 @@ impl fmt::Display for Transform {
}
}

pub(crate) enum RoundingMode {
Round,
Ceil,
Floor,
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down

0 comments on commit 0ab652d

Please sign in to comment.