[common] Update to georust/geo v0.29.0 (#328)

* Update to geo v0.29.0

A lot of the line measure traits were re-worked.

Mostly this is just moving code around in hopes of similar methods
across metric spaces being easier to find now that they are uniform.

It also enabled replacing some mostly copy/pasted implementations in geo
with generic implementations, which enabled some new functionality -
e.g. we can now `.densify::<Geodesic>`

One notable behavioral change: the output of the new `Bearing` trait is
now uniformly 0...360.  Previously GeodesicBearing and HaversineBearing
returned -180..180 while RhumbBearing returned 0...360.

georust/geo#1210

This actually uncovered a bug in ferrostar, reflected in the new output
of
ferrostar/src/snapshots/ferrostar__simulation__tests__state_from_polyline.snap

Since previously we were casting a potentially negative number to u16 —
now it's always positive (0...360).

* adapt to new geo::Bearing output

* Clarifying comments for the index_of_closest_segment_origin function

* Fix debug assert range

* Use haversine_closest_ponit

---------

Co-authored-by: Ian Wagner <[email protected]>

common/ferrostar/Cargo.toml

@@ -28,7 +28,7 @@ wasm_js = [
 ]
 
 [dependencies]
-geo = "0.28.0"
+geo = "0.29.0"
 polyline = "0.11.0"
 serde = { version = "1.0.210", features = ["derive"] }
 serde_json = { version = "1.0.128", default-features = false }

common/ferrostar/src/algorithms.rs

@@ -12,8 +12,8 @@ use crate::{
     navigation_controller::models::TripProgress,
 };
 use geo::{
-    Closest, ClosestPoint, Coord, EuclideanDistance, GeodesicBearing, HaversineDistance,
-    HaversineLength, LineLocatePoint, LineString, Point,
+    Bearing, Closest, Coord, Distance, Euclidean, Geodesic, Haversine, HaversineClosestPoint,
+    Length, LineLocatePoint, LineString, Point,
 };
 
 #[cfg(test)]
@@ -28,23 +28,6 @@ use std::time::SystemTime;
 #[cfg(all(test, feature = "web-time"))]
 use web_time::SystemTime;
 
-/// Normalizes a bearing returned from several `geo` crate functions,
-/// which may be negative, into a positive unsigned integer.
-///
-/// NOTE: This function assumes that the input values are in the range -360 to +360,
-/// and does not check the inputs for validity.
-pub(crate) fn normalize_bearing(degrees: f64) -> u16 {
-    let rounded = degrees.round();
-    let normalized = if rounded < 0.0 {
-        rounded + 360.0
-    } else if rounded >= 360.0 {
-        rounded - 360.0
-    } else {
-        rounded
-    };
-    normalized.round() as u16
-}
-
 /// Get the index of the closest *segment* to the user's location within a [`LineString`].
 ///
 /// A [`LineString`] is a set of points (ex: representing the geometry of a maneuver),
@@ -64,10 +47,13 @@ pub fn index_of_closest_segment_origin(location: UserLocation, line: &LineString
         // Iterate through all segments of the line
         .enumerate()
         // Find the line segment closest to the user's location
-        .min_by(|(_, line1), (_, line2)| {
+        .min_by(|(_, line_segment_1), (_, line_segment_2)| {
             // Note: lines don't implement haversine distances
-            let dist1 = line1.euclidean_distance(&point);
-            let dist2 = line2.euclidean_distance(&point);
+            // In case you're tempted to say that this looks like cross track distance,
+            // note that the Line type here is actually a line *segment*,
+            // and we actually want to find the closest segment, not the closest mathematical line.
+            let dist1 = Euclidean::distance(line_segment_1, &point);
+            let dist2 = Euclidean::distance(line_segment_2, &point);
             dist1.total_cmp(&dist2)
         })
         .map(|(index, _)| index as u64)
@@ -85,13 +71,8 @@ fn get_bearing_to_next_point(
     let current = points.next()?;
     let next = points.next()?;
 
-    // This function may return negative bearing values, but we want to always normalize to [0, 360)
-    let degrees = normalize_bearing(current.geodesic_bearing(next));
-
-    Some(CourseOverGround {
-        degrees,
-        accuracy: None,
-    })
+    let degrees = Geodesic::bearing(current, next);
+    Some(CourseOverGround::new(degrees, None))
 }
 
 /// Apply a snapped course to a user location.
@@ -161,7 +142,7 @@ fn snap_point_to_line(point: &Point, line: &LineString) -> Option<Point> {
     // Bail early when we have two essentially identical points.
     // This can cause some issues with edge cases (captured in proptest regressions)
     // with the underlying libraries.
-    if line.euclidean_distance(point) < 0.000_001 {
+    if Euclidean::distance(line, point) < 0.000_001 {
         return Some(*point);
     }
 
@@ -170,8 +151,7 @@ fn snap_point_to_line(point: &Point, line: &LineString) -> Option<Point> {
         return None;
     }
 
-    // TODO: Use haversine_closest_point once a new release is cut which doesn't panic on intersections
-    match line.closest_point(point) {
+    match line.haversine_closest_point(point) {
         Closest::Intersection(snapped) | Closest::SinglePoint(snapped) => {
             let (x, y) = (snapped.x(), snapped.y());
             if is_valid_float(x) && is_valid_float(y) {
@@ -210,7 +190,7 @@ fn snap_point_to_line(point: &Point, line: &LineString) -> Option<Point> {
 /// };
 /// let off_line = point! {
 ///     x: 1.0,
-/// y: 0.5,
+///     y: 0.5,
 /// };
 ///
 /// // The origin is directly on the line
@@ -220,13 +200,14 @@ fn snap_point_to_line(point: &Point, line: &LineString) -> Option<Point> {
 /// assert_eq!(deviation_from_line(&midpoint, &linestring), Some(0.0));
 ///
 /// // This point, however is off the line.
-/// // That's a huge number, because we're dealing with degrees ;)
+/// // That's a huge number, because we're dealing with points jumping by degrees ;)
+/// println!("{:?}", deviation_from_line(&off_line, &linestring));
 /// assert!(deviation_from_line(&off_line, &linestring)
-///     .map_or(false, |deviation| deviation - 39312.21257675703 < f64::EPSILON));
+///     .map_or(false, |deviation| deviation - 39316.14208341989 < f64::EPSILON));
 /// ```
 pub fn deviation_from_line(point: &Point, line: &LineString) -> Option<f64> {
     snap_point_to_line(point, line).and_then(|snapped| {
-        let distance = snapped.haversine_distance(point);
+        let distance = Haversine::distance(snapped, *point);
 
         if distance.is_nan() || distance.is_infinite() {
             None
@@ -243,7 +224,7 @@ fn is_close_enough_to_end_of_linestring(
 ) -> bool {
     if let Some(end_coord) = current_step_linestring.coords().last() {
         let end_point = Point::from(*end_coord);
-        let distance_to_end = end_point.haversine_distance(current_position);
+        let distance_to_end = Haversine::distance(end_point, *current_position);
 
         distance_to_end <= threshold
     } else {
@@ -310,8 +291,8 @@ pub fn should_advance_to_next_step(
                         // If the user's distance to the snapped location on the *next* step is <=
                         // the user's distance to the snapped location on the *current* step,
                         // advance to the next step
-                        current_position.haversine_distance(&next_step_closest_point)
-                            <= current_position.haversine_distance(&current_step_closest_point)
+                        Haversine::distance(current_position, next_step_closest_point)
+                            <= Haversine::distance(current_position, current_step_closest_point)
                     } else {
                         // The user's location couldn't be mapped to a single point on both the current and next step.
                         // Fall back to the distance to end of step mode, which has some graceful fallbacks.
@@ -366,7 +347,7 @@ pub(crate) fn advance_step(remaining_steps: &[RouteStep]) -> StepAdvanceStatus {
 /// The result is given in meters.
 /// The result may be [`None`] in case of invalid input such as infinite floats.
 fn distance_along(point: &Point, linestring: &LineString) -> Option<f64> {
-    let total_length = linestring.haversine_length();
+    let total_length = linestring.length::<Haversine>();
     if total_length == 0.0 {
         return Some(0.0);
     }
@@ -379,9 +360,9 @@ fn distance_along(point: &Point, linestring: &LineString) -> Option<f64> {
 
             // Compute distance to the line (sadly Euclidean only; no haversine_distance in GeoRust
             // but this is probably OK for now)
-            let segment_distance_to_point = segment.euclidean_distance(point);
+            let segment_distance_to_point = Euclidean::distance(&segment, point);
             // Compute total segment length in meters
-            let segment_length = segment_linestring.haversine_length();
+            let segment_length = segment_linestring.length::<Haversine>();
 
             if segment_distance_to_point < closest_dist_to_point {
                 let segment_fraction = segment.line_locate_point(point)?;
@@ -410,7 +391,7 @@ fn distance_to_end_of_step(
     snapped_location: &Point,
     current_step_linestring: &LineString,
 ) -> Option<f64> {
-    let step_length = current_step_linestring.haversine_length();
+    let step_length = current_step_linestring.length::<Haversine>();
     distance_along(snapped_location, current_step_linestring)
         .map(|traversed| step_length - traversed)
 }
@@ -536,7 +517,7 @@ proptest! {
             prop_assert!(is_valid_float(x) || (!is_valid_float(x1) && x == x1));
             prop_assert!(is_valid_float(y) || (!is_valid_float(y1) && y == y1));
 
-            prop_assert!(line.euclidean_distance(&snapped) < 0.000001);
+            prop_assert!(Euclidean::distance(&line, &snapped) < 0.000001);
         } else {
             // Edge case 1: extremely small differences in values
             let is_miniscule_difference = (x1 - x2).abs() < 0.00000001 || (y1 - y2).abs() < 0.00000001;
@@ -635,7 +616,7 @@ proptest! {
             speed: None
         };
         let user_location_point = Point::from(user_location);
-        let distance_from_end_of_current_step = user_location_point.haversine_distance(&end_of_step.into());
+        let distance_from_end_of_current_step = Haversine::distance(user_location_point, end_of_step.into());
 
         // Never advance to the next step when StepAdvanceMode is Manual
         prop_assert!(!should_advance_to_next_step(&current_route_step.get_linestring(), next_route_step.as_ref(), &user_location, StepAdvanceMode::Manual));
@@ -730,12 +711,6 @@ proptest! {
         prop_assert!(index < (coord_len - 1) as u64);
     }
 
-    #[test]
-    fn test_bearing_correction_valid_range(bearing in -360f64..360f64) {
-        let result = normalize_bearing(bearing);
-        prop_assert!(result < 360);
-    }
-
     #[test]
     fn test_bearing_fuzz(coords in vec(arb_coord(), 2..500), index in 0usize..1_000usize) {
         let line = LineString::new(coords);

common/ferrostar/src/models.rs

@@ -171,8 +171,18 @@ pub struct CourseOverGround {
 }
 
 impl CourseOverGround {
-    pub fn new(degrees: u16, accuracy: Option<u16>) -> Self {
-        Self { degrees, accuracy }
+    /// # Arguments
+    ///
+    /// - degrees: The direction in which the user's device is traveling, measured in clockwise degrees from
+    ///            true north (N = 0, E = 90, S = 180, W = 270).
+    ///            NOTE: Input values must lie in the range [0, 360).
+    /// - accuracy: the accuracy of the course value, measured in degrees.
+    pub fn new(degrees: f64, accuracy: Option<u16>) -> Self {
+        debug_assert!(degrees >= 0.0 && degrees < 360.0);
+        Self {
+            degrees: degrees.round() as u16,
+            accuracy,
+        }
     }
 }
 

common/ferrostar/src/navigation_controller/mod.rs

@@ -12,7 +12,10 @@ use crate::{
     },
     models::{Route, UserLocation},
 };
-use geo::{HaversineDistance, LineString, Point};
+use geo::{
+    algorithm::{Distance, Haversine},
+    geometry::{LineString, Point},
+};
 use models::{NavigationControllerConfig, StepAdvanceStatus, TripState};
 use std::clone::Clone;
 
@@ -125,7 +128,7 @@ impl NavigationController {
                             let next_waypoint: Point = waypoint.coordinate.into();
                             // TODO: This is just a hard-coded threshold for the time being.
                             // More sophisticated behavior will take some time and use cases, so punting on this for now.
-                            current_location.haversine_distance(&next_waypoint) < 100.0
+                            Haversine::distance(current_location, next_waypoint) < 100.0
                         } else {
                             false
                         };

common/ferrostar/src/navigation_controller/test_helpers.rs

@@ -1,7 +1,7 @@
 use crate::models::{BoundingBox, GeographicCoordinate, Route, RouteStep, Waypoint, WaypointKind};
 #[cfg(feature = "alloc")]
 use alloc::string::ToString;
-use geo::{line_string, BoundingRect, HaversineLength, LineString, Point};
+use geo::{line_string, BoundingRect, Haversine, Length, LineString, Point};
 
 pub fn gen_dummy_route_step(
     start_lng: f64,
@@ -24,7 +24,7 @@ pub fn gen_dummy_route_step(
             (x: start_lng, y: start_lat),
             (x: end_lng, y: end_lat)
         ]
-        .haversine_length(),
+        .length::<Haversine>(),
         duration: 0.0,
         road_name: None,
         instruction: "".to_string(),