Skip to content

Commit

Permalink
Update to geo v0.29.0
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
michaelkirk committed Oct 30, 2024
1 parent 065a7d2 commit 07f80c0
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 31 deletions.
94 changes: 92 additions & 2 deletions common/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion common/ferrostar/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
32 changes: 16 additions & 16 deletions common/ferrostar/src/algorithms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ use crate::{
navigation_controller::models::TripProgress,
};
use geo::{
Closest, ClosestPoint, Coord, EuclideanDistance, GeodesicBearing, HaversineDistance,
HaversineLength, LineLocatePoint, LineString, Point,
Bearing, Closest, ClosestPoint, Coord, Distance, Euclidean, Geodesic, Haversine, Length,
LineLocatePoint, LineString, Point,
};

#[cfg(test)]
Expand Down Expand Up @@ -66,8 +66,8 @@ pub fn index_of_closest_segment_origin(location: UserLocation, line: &LineString
// Find the line segment closest to the user's location
.min_by(|(_, line1), (_, line2)| {
// Note: lines don't implement haversine distances
let dist1 = line1.euclidean_distance(&point);
let dist2 = line2.euclidean_distance(&point);
let dist1 = Euclidean::distance(line1, &point);
let dist2 = Euclidean::distance(line2, &point);
dist1.total_cmp(&dist2)
})
.map(|(index, _)| index as u64)
Expand All @@ -86,7 +86,7 @@ fn get_bearing_to_next_point(
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));
let degrees = normalize_bearing(Geodesic::bearing(current, next));

Some(CourseOverGround {
degrees,
Expand Down Expand Up @@ -161,7 +161,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);
}

Expand Down Expand Up @@ -226,7 +226,7 @@ fn snap_point_to_line(point: &Point, line: &LineString) -> Option<Point> {
/// ```
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
Expand All @@ -243,7 +243,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 {
Expand Down Expand Up @@ -310,8 +310,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.
Expand Down Expand Up @@ -366,7 +366,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);
}
Expand All @@ -379,9 +379,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)?;
Expand Down Expand Up @@ -410,7 +410,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)
}
Expand Down Expand Up @@ -536,7 +536,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;
Expand Down Expand Up @@ -635,7 +635,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));
Expand Down
7 changes: 5 additions & 2 deletions common/ferrostar/src/navigation_controller/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
};
Expand Down
4 changes: 2 additions & 2 deletions common/ferrostar/src/navigation_controller/test_helpers.rs
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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(),
Expand Down
14 changes: 7 additions & 7 deletions common/ferrostar/src/simulation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
use crate::algorithms::{normalize_bearing, trunc_float};
use crate::models::{CourseOverGround, GeographicCoordinate, Route, UserLocation};
use geo::{coord, DensifyHaversine, GeodesicBearing, LineString, Point};
use geo::{coord, Bearing, Densify, Geodesic, Haversine, LineString, Point};
use polyline::decode_polyline;

#[cfg(any(test, feature = "wasm-bindgen"))]
Expand Down Expand Up @@ -101,7 +101,7 @@ pub fn location_simulation_from_coordinates(
if let Some(next) = rest.first() {
let current_point = Point::from(*current);
let next_point = Point::from(*next);
let bearing = current_point.geodesic_bearing(next_point);
let bearing = Geodesic::bearing(current_point, next_point);
let current_location = UserLocation {
coordinates: *current,
horizontal_accuracy: 0.0,
Expand All @@ -125,7 +125,7 @@ pub fn location_simulation_from_coordinates(
})
.collect();
let linestring: LineString = coords.into();
let densified_linestring = linestring.densify_haversine(distance);
let densified_linestring = linestring.densify::<Haversine>(distance);
densified_linestring
.points()
.map(|point| GeographicCoordinate {
Expand Down Expand Up @@ -199,7 +199,7 @@ pub fn advance_location_simulation(state: &LocationSimulationState) -> LocationS
if let Some((next_coordinate, rest)) = state.remaining_locations.split_first() {
let current_point = Point::from(state.current_location.coordinates);
let next_point = Point::from(*next_coordinate);
let bearing = normalize_bearing(current_point.geodesic_bearing(next_point));
let bearing = normalize_bearing(Geodesic::bearing(current_point, next_point));

let next_location = UserLocation {
coordinates: *next_coordinate,
Expand Down Expand Up @@ -277,7 +277,7 @@ pub fn js_advance_location_simulation(state: JsValue) -> JsValue {
mod tests {
use super::*;
use crate::algorithms::snap_user_location_to_line;
use geo::HaversineDistance;
use geo::{Distance, Haversine};
use rstest::rstest;

#[rstest]
Expand Down Expand Up @@ -348,7 +348,7 @@ mod tests {
// The distance between each point in the simulation should be <= max_distance
let current_point: Point = state.current_location.into();
let next_point: Point = new_state.current_location.into();
let distance = current_point.haversine_distance(&next_point);
let distance = Haversine::distance(current_point, next_point);
// I'm actually not 100% sure why this extra fudge is needed, but it's not a concern for today.
assert!(
distance <= max_distance + 7.0,
Expand All @@ -358,7 +358,7 @@ mod tests {
let snapped =
snap_user_location_to_line(new_state.current_location, &original_linestring);
let snapped_point: Point = snapped.coordinates.into();
let distance = next_point.haversine_distance(&snapped_point);
let distance = Haversine::distance(next_point, snapped_point);
assert!(
distance <= max_distance,
"Expected snapped point to be on the line; was {distance}m away"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: ferrostar/src/simulation.rs
assertion_line: 329
expression: state
---
current_location:
Expand All @@ -8,7 +9,7 @@ current_location:
lng: -149.543469
horizontal_accuracy: 0
course_over_ground:
degrees: 0
degrees: 288
accuracy: ~
speed: ~
remaining_locations:
Expand Down

0 comments on commit 07f80c0

Please sign in to comment.