From 1edbca6509b7abe11b1f6d68402fffbd5bf38b7f Mon Sep 17 00:00:00 2001 From: Dustin Carlino Date: Sat, 15 May 2021 09:05:28 -0700 Subject: [PATCH] Refactor most of the places extracting polygons from geojson files (#644) --- Cargo.lock | 2 - abstutil/src/cli.rs | 2 - abstutil/src/collections.rs | 6 +++ abstutil/src/lib.rs | 3 ++ convert_osm/Cargo.toml | 1 - convert_osm/src/lib.rs | 34 ++------------- game/src/app.rs | 40 ++++------------- geom/src/polygon.rs | 72 ++++++++++++++++++++++++++++++- importer/Cargo.toml | 2 +- importer/src/uk.rs | 81 +++++++---------------------------- popdat/Cargo.toml | 1 - widgetry/src/widgets/panel.rs | 4 +- 12 files changed, 111 insertions(+), 137 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9a732cc9e0..b0bb8c25c9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -622,7 +622,6 @@ dependencies = [ "abstio", "abstutil", "anyhow", - "geojson", "geom", "kml", "log", @@ -3019,7 +3018,6 @@ dependencies = [ "futures", "geo", "geo-booleanop", - "geojson", "geom", "geozero-core", "log", diff --git a/abstutil/src/cli.rs b/abstutil/src/cli.rs index 0f002578b0..02d45de066 100644 --- a/abstutil/src/cli.rs +++ b/abstutil/src/cli.rs @@ -141,8 +141,6 @@ impl CmdArgs { /// between arguments. So for instance "?--dev&--color_scheme=night%20mode" becomes vec!["--dev", /// "--color_scheme=night mode"]. fn parse_args() -> anyhow::Result> { - use anyhow::{anyhow, bail}; - let window = web_sys::window().ok_or(anyhow!("no window?"))?; let url = window.location().href().map_err(|err| { anyhow!(err diff --git a/abstutil/src/collections.rs b/abstutil/src/collections.rs index 7c5746c5b6..57a823f13b 100644 --- a/abstutil/src/collections.rs +++ b/abstutil/src/collections.rs @@ -2,6 +2,8 @@ use std::cmp::Ord; use std::collections::{BTreeMap, BTreeSet}; use std::marker::PhantomData; +use anyhow::Result; + use itertools::Itertools; use serde::{Deserialize, Serialize}; @@ -258,6 +260,10 @@ impl Tags { self.0.get(k) } + pub fn get_result(&self, k: &str) -> Result<&String> { + self.0.get(k).ok_or_else(|| anyhow!("missing {}", k)) + } + pub fn contains_key(&self, k: &str) -> bool { self.0.contains_key(k) } diff --git a/abstutil/src/lib.rs b/abstutil/src/lib.rs index 816f74f17e..b49e2cde31 100644 --- a/abstutil/src/lib.rs +++ b/abstutil/src/lib.rs @@ -6,6 +6,9 @@ #![allow(clippy::ptr_arg)] // very noisy #![allow(clippy::new_without_default)] +#[macro_use] +extern crate anyhow; + // I'm not generally a fan of wildcard exports, but they're more maintable here. pub use crate::serde::*; pub use cli::*; diff --git a/convert_osm/Cargo.toml b/convert_osm/Cargo.toml index f3b95ff162..a464fea448 100644 --- a/convert_osm/Cargo.toml +++ b/convert_osm/Cargo.toml @@ -8,7 +8,6 @@ edition = "2018" abstio = { path = "../abstio" } abstutil = { path = "../abstutil" } anyhow = "1.0.38" -geojson = "0.22" geom = { path = "../geom" } kml = { path = "../kml" } log = "0.4.14" diff --git a/convert_osm/src/lib.rs b/convert_osm/src/lib.rs index b93b7266c6..e09d88814c 100644 --- a/convert_osm/src/lib.rs +++ b/convert_osm/src/lib.rs @@ -7,7 +7,7 @@ use anyhow::Result; use abstio::MapName; use abstutil::{Tags, Timer}; -use geom::{Distance, FindClosest, GPSBounds, LonLat, Pt2D, Ring}; +use geom::{Distance, FindClosest, GPSBounds, LonLat, Polygon, Pt2D, Ring}; use map_model::raw::RawMap; use map_model::{osm, raw, Amenity, MapConfig}; use serde::{Deserialize, Serialize}; @@ -145,35 +145,10 @@ fn use_amenities(map: &mut RawMap, amenities: Vec<(Pt2D, Amenity)>, timer: &mut } fn add_extra_buildings(map: &mut RawMap, path: &str) -> Result<()> { - // TODO Refactor code that just extracts polygons from geojson. - let mut polygons = Vec::new(); - - let bytes = abstio::slurp_file(path)?; - let raw_string = std::str::from_utf8(&bytes)?; - let geojson = raw_string.parse::()?; - - if let geojson::GeoJson::FeatureCollection(collection) = geojson { - for feature in collection.features { - if let Some(geom) = feature.geometry { - if let geojson::Value::Polygon(raw_pts) = geom.value { - // TODO Handle holes, and also, refactor this! - let gps_pts: Vec = raw_pts[0] - .iter() - .map(|pt| LonLat::new(pt[0], pt[1])) - .collect(); - if let Some(pts) = map.gps_bounds.try_convert(&gps_pts) { - if let Ok(ring) = Ring::new(pts) { - polygons.push(ring.into_polygon()); - } - } - } - } - } - } - - // Add these as new buildings, generating a new dummy OSM ID. + let require_in_bounds = true; let mut id = -1; - for polygon in polygons { + for (polygon, _) in Polygon::from_geojson_file(path, &map.gps_bounds, require_in_bounds)? { + // Add these as new buildings, generating a new dummy OSM ID. map.buildings.insert( osm::OsmID::Way(osm::WayID(id)), raw::RawBuilding { @@ -188,6 +163,5 @@ fn add_extra_buildings(map: &mut RawMap, path: &str) -> Result<()> { // new OSM IDs. id -= -1; } - Ok(()) } diff --git a/game/src/app.rs b/game/src/app.rs index f0a56074ab..7b38a4af82 100644 --- a/game/src/app.rs +++ b/game/src/app.rs @@ -6,8 +6,8 @@ use maplit::btreemap; use rand::seq::{IteratorRandom, SliceRandom}; use abstio::MapName; -use abstutil::{Tags, Timer}; -use geom::{Bounds, Circle, Distance, Duration, LonLat, Pt2D, Ring, Time}; +use abstutil::Timer; +use geom::{Bounds, Circle, Distance, Duration, Polygon, Pt2D, Time}; use map_gui::colors::ColorScheme; use map_gui::load::FileLoader; use map_gui::options::Options; @@ -880,35 +880,13 @@ impl SharedAppState for App { /// Load an extra GeoJSON file, and add the area to the map dynamically. fn add_study_area(map: &mut Map, name: &str) -> Result<()> { - let bytes = abstio::slurp_file(abstio::path(format!("system/study_areas/{}.geojson", name)))?; - let raw_string = std::str::from_utf8(&bytes)?; - let geojson = raw_string.parse::()?; - - if let geojson::GeoJson::FeatureCollection(collection) = geojson { - for feature in collection.features { - if let Some(geom) = feature.geometry { - if let geojson::Value::Polygon(raw_pts) = geom.value { - // TODO Handle holes, and also, refactor this! - let gps_pts: Vec = raw_pts[0] - .iter() - .map(|pt| LonLat::new(pt[0], pt[1])) - .collect(); - if let Some(pts) = map.get_gps_bounds().try_convert(&gps_pts) { - let mut tags = Tags::empty(); - if let Some(props) = feature.properties { - for (k, v) in props { - tags.insert(k, v.to_string()); - } - } - - if let Ok(ring) = Ring::new(pts) { - map.hack_add_area(AreaType::StudyArea, ring.into_polygon(), tags); - } - } - } - } - } + let require_in_bounds = true; + for (polygon, tags) in Polygon::from_geojson_file( + abstio::path(format!("system/study_areas/{}.geojson", name)), + map.get_gps_bounds(), + require_in_bounds, + )? { + map.hack_add_area(AreaType::StudyArea, polygon, tags); } - Ok(()) } diff --git a/geom/src/polygon.rs b/geom/src/polygon.rs index 5a0ade6e8e..b2a98971aa 100644 --- a/geom/src/polygon.rs +++ b/geom/src/polygon.rs @@ -1,7 +1,8 @@ use std::convert::TryFrom; use std::fmt; +use std::path::Path; -use anyhow::Result; +use anyhow::{Context, Result}; use geo::algorithm::area::Area; use geo::algorithm::concave_hull::ConcaveHull; use geo::algorithm::convex_hull::ConvexHull; @@ -9,7 +10,11 @@ use geo::algorithm::intersects::Intersects; use geo_booleanop::boolean::BooleanOp; use serde::{Deserialize, Serialize}; -use crate::{Angle, Bounds, CornerRadii, Distance, GPSBounds, HashablePt2D, PolyLine, Pt2D, Ring}; +use abstutil::Tags; + +use crate::{ + Angle, Bounds, CornerRadii, Distance, GPSBounds, HashablePt2D, LonLat, PolyLine, Pt2D, Ring, +}; #[derive(PartialEq, Serialize, Deserialize, Clone, Debug)] pub struct Polygon { @@ -473,6 +478,19 @@ impl Polygon { geojson::Geometry::new(geojson::Value::MultiPolygon(polygons)) } + + /// Extracts all polygons from a GeoJSON file, along with the string key/value properties. Only + /// the first polygon from multipolygons is returned. If `require_in_bounds` is set, then the + /// polygon must completely fit within the `gps_bounds`. + pub fn from_geojson_file>( + path: P, + gps_bounds: &GPSBounds, + require_in_bounds: bool, + ) -> Result> { + let path = path.as_ref(); + from_geojson_file_inner(path, gps_bounds, require_in_bounds) + .with_context(|| format!("polygons from {}", path.display())) + } } impl fmt::Display for Polygon { @@ -603,3 +621,53 @@ fn downsize(input: Vec) -> Vec { } output } + +fn from_geojson_file_inner( + path: &Path, + gps_bounds: &GPSBounds, + require_in_bounds: bool, +) -> Result> { + let raw_string = std::fs::read_to_string(path)?; + let geojson = raw_string.parse::()?; + let features = match geojson { + geojson::GeoJson::Feature(feature) => vec![feature], + geojson::GeoJson::FeatureCollection(collection) => collection.features, + _ => anyhow::bail!("Unexpected geojson: {:?}", geojson), + }; + + let mut results = Vec::new(); + for feature in features { + if let Some(geom) = &feature.geometry { + let raw_pts = match &geom.value { + geojson::Value::Polygon(pts) => pts, + // If there are multiple, just use the first + geojson::Value::MultiPolygon(polygons) => &polygons[0], + _ => { + continue; + } + }; + // TODO Handle holes + let gps_pts: Vec = raw_pts[0] + .iter() + .map(|pt| LonLat::new(pt[0], pt[1])) + .collect(); + let pts = if !require_in_bounds { + gps_bounds.convert(&gps_pts) + } else if let Some(pts) = gps_bounds.try_convert(&gps_pts) { + pts + } else { + continue; + }; + if let Ok(ring) = Ring::new(pts) { + let mut tags = Tags::empty(); + for (key, value) in feature.properties_iter() { + if let Some(value) = value.as_str() { + tags.insert(key, value); + } + } + results.push((ring.into_polygon(), tags)); + } + } + } + Ok(results) +} diff --git a/importer/Cargo.toml b/importer/Cargo.toml index fb6afb39ef..5b9dbff830 100644 --- a/importer/Cargo.toml +++ b/importer/Cargo.toml @@ -17,7 +17,7 @@ collisions = { path = "../collisions" } convert_osm = { path = "../convert_osm" } csv = "1.1.4" geo = "0.17.0" -geojson = "0.22" +geojson = { version = "0.22.0", features = ["geo-types"] } geom = { path = "../geom" } gdal = { version = "0.7.2", optional = true } kml = { path = "../kml" } diff --git a/importer/src/uk.rs b/importer/src/uk.rs index 89575dba36..ca4f1091d2 100644 --- a/importer/src/uk.rs +++ b/importer/src/uk.rs @@ -8,7 +8,7 @@ use serde::Deserialize; use abstio::path_shared_input; use abstutil::{prettyprint_usize, Timer}; -use geom::{GPSBounds, LonLat, Polygon, Ring}; +use geom::{GPSBounds, Polygon}; use map_model::raw::RawMap; use map_model::Map; use popdat::od::DesireLine; @@ -185,74 +185,25 @@ struct Record { // Transforms all zones into the map's coordinate space, no matter how far out-of-bounds they are. fn parse_zones(gps_bounds: &GPSBounds, path: String) -> Result> { let mut zones = HashMap::new(); - - let bytes = abstio::slurp_file(path)?; - let raw_string = std::str::from_utf8(&bytes)?; - let geojson = raw_string.parse::()?; - - if let geojson::GeoJson::FeatureCollection(collection) = geojson { - for feature in collection.features { - let zone = feature - .property("geo_code") - .and_then(|x| x.as_str()) - .ok_or_else(|| anyhow!("no geo_code"))? - .to_string(); - if let Some(geom) = feature.geometry { - if let geojson::Value::MultiPolygon(mut raw_polygons) = geom.value { - if raw_polygons.len() != 1 { - // We'll just one of them arbitrarily - warn!( - "Zone {} has a multipolygon with {} members", - zone, - raw_polygons.len() - ); - } - match parse_polygon(raw_polygons.pop().unwrap(), gps_bounds) { - Ok(polygon) => { - zones.insert(zone, polygon); - } - Err(err) => { - warn!("Zone {} has bad geometry: {}", zone, err); - } - } - } - } - } + let require_in_bounds = false; + for (polygon, tags) in Polygon::from_geojson_file(path, gps_bounds, require_in_bounds)? { + zones.insert(tags.get_result("geo_code")?.to_string(), polygon); } - Ok(zones) } -// TODO Clean up the exploding number of geojson readers everywhere. -fn parse_polygon(input: Vec>>, gps_bounds: &GPSBounds) -> Result { - let mut rings = Vec::new(); - for ring in input { - let gps_pts: Vec = ring - .into_iter() - .map(|pt| LonLat::new(pt[0], pt[1])) - .collect(); - let pts = gps_bounds.convert(&gps_pts); - rings.push(Ring::new(pts)?); - } - Ok(Polygon::from_rings(rings)) -} - fn load_study_area(map: &Map) -> Result { - let bytes = abstio::slurp_file(abstio::path(format!( - "system/study_areas/{}.geojson", - map.get_name().city.city.replace("_", "-") - )))?; - let raw_string = std::str::from_utf8(&bytes)?; - let geojson = raw_string.parse::()?; - - if let geojson::GeoJson::FeatureCollection(collection) = geojson { - for feature in collection.features { - if let Some(geom) = feature.geometry { - if let geojson::Value::Polygon(raw_pts) = geom.value { - return parse_polygon(raw_pts, map.get_gps_bounds()); - } - } - } + let require_in_bounds = true; + let mut list = Polygon::from_geojson_file( + abstio::path(format!( + "system/study_areas/{}.geojson", + map.get_name().city.city.replace("_", "-") + )), + map.get_gps_bounds(), + require_in_bounds, + )?; + if list.len() != 1 { + bail!("study area geojson has {} polygons", list.len()); } - bail!("no study area"); + Ok(list.pop().unwrap().0) } diff --git a/popdat/Cargo.toml b/popdat/Cargo.toml index 70329ffc6a..5644253c91 100644 --- a/popdat/Cargo.toml +++ b/popdat/Cargo.toml @@ -10,7 +10,6 @@ anyhow = "1.0.38" flatgeobuf = { version = "0.4" } futures = "0.3.12" geo = "0.17.0" -geojson = { version = "0.22.0", features = ["geo-types"] } geom = { path = "../geom" } geozero-core = { version = "0.6" } log = "0.4.14" diff --git a/widgetry/src/widgets/panel.rs b/widgetry/src/widgets/panel.rs index 290d523acc..4581d0942b 100644 --- a/widgetry/src/widgets/panel.rs +++ b/widgetry/src/widgets/panel.rs @@ -563,8 +563,8 @@ impl PanelBuilder { }; match panel.dims { Dims::ExactPercent(w, h) => { - // Don't set size, because then scrolling breaks -- the actual size has to be based on - // the contents. + // Don't set size, because then scrolling breaks -- the actual size has to be based + // on the contents. panel.top_level.layout.style.min_size = Size { width: Dimension::Points((w * ctx.canvas.window_width) as f32), height: Dimension::Points((h * ctx.canvas.window_height) as f32),