Skip to content

Commit

Permalink
New TotalOrd trait avoids exploding when working with NaN.
Browse files Browse the repository at this point in the history
This fixes a new lint introduced in rust 1.75

Fixes #1088
  • Loading branch information
michaelkirk committed Jan 2, 2024
1 parent 36a8840 commit ac35b65
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 16 deletions.
11 changes: 11 additions & 0 deletions geo/CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
# Changes

## Unreleased
* POSSIBLY BREAKING: New `TotalOrd` trait is required for our base numeric
trait GeoNum, this avoids some potential crashes when working with geometries
that contain NaN points. This shouldn't break for any common numeric types,
but if you are using something exotic you'll need to manually implement
TotalOrd for your numeric type.
* <https://github.com/georust/geo/pull/1134>
* POSSIBLY BREAKING: Visvalingam trait implementation moved from
`geo_types::CoordNum` to `geo::GeoNum` as fallout of introducing the
`TotalOrd` constraint. This shouldn't break anything for common numeric
types, but if you are using something exotic you'll need to manually
implement TotalOrd for your numeric type.
* Implement ChaikinSmoothing to work on Geometry types
* <https://github.com/georust/geo/pull/1116>
* Fix a panic when calculating the haversine closest point to a point intersecting the geometry
Expand Down
18 changes: 9 additions & 9 deletions geo/src/algorithm/simplify_vw.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::prelude::*;
use crate::{
Coord, CoordFloat, HasKernel, Line, LineString, MultiLineString, MultiPolygon, Point, Polygon,
Triangle,
Coord, CoordFloat, GeoFloat, HasKernel, Line, LineString, MultiLineString, MultiPolygon, Point,
Polygon, Triangle,
};
use std::cmp::Ordering;
use std::collections::BinaryHeap;
Expand Down Expand Up @@ -241,7 +241,7 @@ fn vwp_wrapper<T, const INITIAL_MIN: usize, const MIN_POINTS: usize>(
epsilon: &T,
) -> Vec<Vec<Coord<T>>>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
let mut rings = vec![];
// Populate R* tree with exterior and interior samples, if any
Expand Down Expand Up @@ -291,7 +291,7 @@ fn visvalingam_preserve<T, const INITIAL_MIN: usize, const MIN_POINTS: usize>(
tree: &mut RTree<CachedEnvelope<Line<T>>>,
) -> Vec<Coord<T>>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
if orig.0.len() < 3 || *epsilon <= T::zero() {
return orig.0.to_vec();
Expand Down Expand Up @@ -400,7 +400,7 @@ fn tree_intersect<T>(
orig: &[Coord<T>],
) -> bool
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
let new_segment_start = orig[triangle.left];
let new_segment_end = orig[triangle.right];
Expand Down Expand Up @@ -581,7 +581,7 @@ pub trait SimplifyVwPreserve<T, Epsilon = T> {

impl<T> SimplifyVwPreserve<T> for LineString<T>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
fn simplify_vw_preserve(&self, epsilon: &T) -> LineString<T> {
let mut simplified = vwp_wrapper::<_, 2, 4>(self, None, epsilon);
Expand All @@ -591,7 +591,7 @@ where

impl<T> SimplifyVwPreserve<T> for MultiLineString<T>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
fn simplify_vw_preserve(&self, epsilon: &T) -> MultiLineString<T> {
MultiLineString::new(
Expand All @@ -605,7 +605,7 @@ where

impl<T> SimplifyVwPreserve<T> for Polygon<T>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
fn simplify_vw_preserve(&self, epsilon: &T) -> Polygon<T> {
let mut simplified =
Expand All @@ -619,7 +619,7 @@ where

impl<T> SimplifyVwPreserve<T> for MultiPolygon<T>
where
T: CoordFloat + RTreeNum + HasKernel,
T: GeoFloat + RTreeNum + HasKernel,
{
fn simplify_vw_preserve(&self, epsilon: &T) -> MultiPolygon<T> {
MultiPolygon::new(
Expand Down
10 changes: 5 additions & 5 deletions geo/src/algorithm/sweep/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,17 @@ impl<T: GeoNum> std::fmt::Debug for SweepPoint<T> {
/// coordinate.
impl<T: GeoNum> PartialOrd for SweepPoint<T> {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
match self.0.x.partial_cmp(&other.0.x) {
Some(Ordering::Equal) => self.0.y.partial_cmp(&other.0.y),
o => o,
}
Some(self.cmp(other))
}
}

/// Derive `Ord` from `PartialOrd` and expect to not fail.
impl<T: GeoNum> Ord for SweepPoint<T> {
fn cmp(&self, other: &Self) -> Ordering {
self.partial_cmp(other).unwrap()
match self.0.x.total_cmp(&other.0.x) {
Ordering::Equal => self.0.y.total_cmp(&other.0.y),
o => o,
}
}
}

Expand Down
5 changes: 3 additions & 2 deletions geo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ pub mod algorithm;
mod geometry_cow;
mod types;
mod utils;
use crate::types::TotalOrd;
pub(crate) use geometry_cow::GeometryCow;

#[cfg(test)]
Expand Down Expand Up @@ -302,5 +303,5 @@ impl<T> GeoFloat for T where
}

/// A trait for methods which work for both integers **and** floating point
pub trait GeoNum: CoordNum + HasKernel {}
impl<T> GeoNum for T where T: CoordNum + HasKernel {}
pub trait GeoNum: CoordNum + HasKernel + TotalOrd {}
impl<T> GeoNum for T where T: CoordNum + HasKernel + TotalOrd {}
39 changes: 39 additions & 0 deletions geo/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,42 @@ macro_rules! __geometry_delegate_impl_helper {
)+
};
}

use std::cmp::Ordering;
pub trait TotalOrd {
fn total_cmp(&self, other: &Self) -> Ordering;
}

macro_rules! impl_total_ord_for_float {
($t: ident) => {
impl TotalOrd for $t {
fn total_cmp(&self, other: &Self) -> Ordering {
self.total_cmp(other)
}
}
};
}

macro_rules! impl_total_ord_for_int {
($t: ident) => {
impl TotalOrd for $t {
fn total_cmp(&self, other: &Self) -> Ordering {
self.cmp(other)
}
}
};
}

impl_total_ord_for_float!(f32);
impl_total_ord_for_float!(f64);

impl_total_ord_for_int!(u8);
impl_total_ord_for_int!(u16);
impl_total_ord_for_int!(u32);
impl_total_ord_for_int!(u64);
impl_total_ord_for_int!(u128);
impl_total_ord_for_int!(i8);
impl_total_ord_for_int!(i16);
impl_total_ord_for_int!(i32);
impl_total_ord_for_int!(i64);
impl_total_ord_for_int!(i128);

0 comments on commit ac35b65

Please sign in to comment.