From 17da1bbf22e4cf473ad925c7e31e7d3810f6c8c5 Mon Sep 17 00:00:00 2001
From: Dustin Carlino <dabreegster@gmail.com>
Date: Fri, 25 Oct 2024 10:04:08 +0100
Subject: [PATCH] Upfront filter out roads with no access for any profile. #17

---
 graph/src/muv_profiles.rs | 36 +++++++++++++++++++++++-------------
 graph/src/scrape.rs       | 31 ++++++++++++++++++++++---------
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/graph/src/muv_profiles.rs b/graph/src/muv_profiles.rs
index 0c544e9..19c253d 100644
--- a/graph/src/muv_profiles.rs
+++ b/graph/src/muv_profiles.rs
@@ -1,47 +1,57 @@
 use std::time::Duration;
 
+use geo::{EuclideanLength, LineString};
 use muv_osm::{AccessLevel, TMode};
 use utils::Tags;
 
-use crate::{Direction, Road};
+use crate::Direction;
 
 // TODO Separate profiles like this will repeat work parsing!
 
-pub fn muv_car_profile() -> (String, Box<dyn Fn(&Road) -> (Direction, Duration)>) {
+pub fn muv_car_profile() -> (
+    String,
+    Box<dyn Fn(&Tags, &LineString) -> (Direction, Duration)>,
+) {
     (
         "car".to_string(),
-        Box::new(|road| {
-            let access = calculate_access(&road.osm_tags, TMode::Motorcar);
+        Box::new(|tags, linestring| {
+            let access = calculate_access(tags, TMode::Motorcar);
             let cost =
-                Duration::from_secs_f64(road.length_meters / calculate_max_speed(&road.osm_tags));
+                Duration::from_secs_f64(linestring.euclidean_length() / calculate_max_speed(tags));
             (access, cost)
         }),
     )
 }
 
-pub fn muv_bicycle_profile() -> (String, Box<dyn Fn(&Road) -> (Direction, Duration)>) {
+pub fn muv_bicycle_profile() -> (
+    String,
+    Box<dyn Fn(&Tags, &LineString) -> (Direction, Duration)>,
+) {
     (
         "bicycle".to_string(),
-        Box::new(|road| {
-            let access = calculate_access(&road.osm_tags, TMode::Bicycle);
+        Box::new(|tags, linestring| {
+            let access = calculate_access(tags, TMode::Bicycle);
             // TODO Use elevation and other more detailed things
             // 10 mph
             let max_bicycle_speed = 4.4704;
-            let cost = Duration::from_secs_f64(road.length_meters / max_bicycle_speed);
+            let cost = Duration::from_secs_f64(linestring.euclidean_length() / max_bicycle_speed);
             (access, cost)
         }),
     )
 }
 
-pub fn muv_pedestrian_profile() -> (String, Box<dyn Fn(&Road) -> (Direction, Duration)>) {
+pub fn muv_pedestrian_profile() -> (
+    String,
+    Box<dyn Fn(&Tags, &LineString) -> (Direction, Duration)>,
+) {
     (
         "foot".to_string(),
-        Box::new(|road| {
-            let access = calculate_access(&road.osm_tags, TMode::Foot);
+        Box::new(|tags, linestring| {
+            let access = calculate_access(tags, TMode::Foot);
             // TODO Use elevation and other more detailed things
             // 3 mph
             let max_foot_speed = 1.34112;
-            let cost = Duration::from_secs_f64(road.length_meters / max_foot_speed);
+            let cost = Duration::from_secs_f64(linestring.euclidean_length() / max_foot_speed);
             (access, cost)
         }),
     )
diff --git a/graph/src/scrape.rs b/graph/src/scrape.rs
index 926e084..e161930 100644
--- a/graph/src/scrape.rs
+++ b/graph/src/scrape.rs
@@ -2,7 +2,8 @@ use std::collections::BTreeMap;
 use std::time::Duration;
 
 use anyhow::Result;
-use geo::EuclideanLength;
+use geo::{EuclideanLength, LineString};
+use utils::Tags;
 
 use crate::gtfs::GtfsModel;
 use crate::route::Router;
@@ -13,23 +14,34 @@ impl Graph {
     ///
     /// - `input_bytes`: Bytes of an osm.pbf or osm.xml file
     /// - `osm_reader`: A callback for every OSM element read, to extract non-graph data
-    /// - `profiles`: A list of named profiles. Each one assigns an access direction and cost to
-    ///    each Road.
+    /// - `profiles`: A list of named profiles. Each one assigns an access direction and cost,
+    ///   given OSM tags and a Euclidean center-line. If every profile assigns `Direction::None`,
+    ///   then the Road is completely excluded from the graph.
     pub fn new<R: utils::osm2graph::OsmReader>(
         input_bytes: &[u8],
         osm_reader: &mut R,
-        profiles: Vec<(String, Box<dyn Fn(&Road) -> (Direction, Duration)>)>,
+        profiles: Vec<(
+            String,
+            Box<dyn Fn(&Tags, &LineString) -> (Direction, Duration)>,
+        )>,
         timer: &mut Timer,
     ) -> Result<Graph> {
         timer.step("parse OSM and split graph");
 
         let graph = utils::osm2graph::Graph::new(
             input_bytes,
-            // Don't do any filtering by profile yet
-            // TODO Actually, see if any profile accepts it. But can we avoid calling the profiles
-            // twice?
             |tags| {
-                tags.has("highway") && !tags.is("highway", "proposed") && !tags.is("area", "yes")
+                if !tags.has("highway") || tags.is("highway", "proposed") || tags.is("area", "yes")
+                {
+                    return false;
+                }
+                // Make sure at least one profile allows access
+                // TODO It's weird to pass in an empty linestring
+                // TODO It's inefficient to call the profiles twice
+                let empty = LineString::new(Vec::new());
+                profiles
+                    .iter()
+                    .any(|(_, profile)| profile(tags, &empty).0 != Direction::None)
             },
             osm_reader,
         )?;
@@ -73,7 +85,7 @@ impl Graph {
             let mut access = Vec::new();
             let mut cost = Vec::new();
             for (_, profile) in &profiles {
-                let (dir, c) = profile(road);
+                let (dir, c) = profile(&road.osm_tags, &road.linestring);
                 access.push(dir);
                 cost.push(c);
             }
@@ -90,6 +102,7 @@ impl Graph {
 
             profile_names.insert(name, ProfileID(idx));
         }
+        timer.pop();
 
         Ok(Graph {
             roads,