From 29c4547d6e7bcc27e163792b928a21df3d5ef0cf Mon Sep 17 00:00:00 2001 From: Michael Droogleever Fortuyn Date: Sun, 29 May 2022 23:04:06 +0200 Subject: [PATCH 1/2] Add Support For cycleway=opposite Covers part of #162 --- data/tests.yml | 9 ++- osm-tags/src/access.rs | 2 +- osm2lanes/src/test.rs | 14 +++-- osm2lanes/src/transform/lanes_to_tags/mod.rs | 4 ++ .../transform/tags_to_lanes/modes/bicycle.rs | 59 +++++++++++++------ .../tags_to_lanes/modes/non_motorized.rs | 16 +++-- osm2lanes/src/transform/tags_to_lanes/road.rs | 30 ++++------ 7 files changed, 83 insertions(+), 51 deletions(-) diff --git a/data/tests.yml b/data/tests.yml index 957977da..9f7741bc 100644 --- a/data/tests.yml +++ b/data/tests.yml @@ -710,10 +710,8 @@ designated: motor_vehicle - description: cycleway=opposite oneway=yes oneway:bicycle=no - rust: false # https://github.com/a-b-street/osm2lanes/issues/162 tags: highway: "road" - lanes: "1" oneway: "yes" oneway:bicycle: "no" cycleway: "opposite" @@ -723,12 +721,13 @@ road: highway: road lanes: - - type: travel - direction: backward - designated: bicycle - type: travel direction: forward designated: motor_vehicle + access: + bicycle: + access: "yes" + direction: "both" - description: "cycleway:BACKWARD:lane=advisory oneway=yes oneway:bicycle=no" way_id: 25745877 diff --git a/osm-tags/src/access.rs b/osm-tags/src/access.rs index f3a226d2..be32f357 100644 --- a/osm-tags/src/access.rs +++ b/osm-tags/src/access.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Serialize}; /// Access variants from #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +#[serde(rename_all = "snake_case")] pub enum Access { Yes, No, diff --git a/osm2lanes/src/test.rs b/osm2lanes/src/test.rs index 47109ee1..44afcd4a 100644 --- a/osm2lanes/src/test.rs +++ b/osm2lanes/src/test.rs @@ -28,11 +28,14 @@ pub struct TestCase { // Metadata /// The OSM way unique identifier pub way_id: Option, + /// Relevant link pub link: Option, + /// Comment on test case pub comment: Option, + /// Description of test case pub description: Option, - // List as a named example in the web app + /// List as a named example in the web app, with the given name example: Option, // Config and Locale @@ -196,20 +199,21 @@ mod tests { direction: actual_direction, width: actual_width, max_speed: actual_max_speed, - access: _actual_access, + access: actual_access, }, Lane::Travel { designated: expected_designated, direction: expected_direction, width: expected_width, max_speed: expected_max_speed, - access: _expected_access, + access: expected_access, }, ) => { actual_designated == expected_designated && actual_direction == expected_direction && approx_eq(actual_width, expected_width) && approx_eq(actual_max_speed, expected_max_speed) + && approx_eq(actual_access, expected_access) }, ( Lane::Parking { @@ -242,7 +246,6 @@ mod tests { impl Marking { /// Eq where None is treaty as always equal - #[allow(clippy::unnested_or_patterns)] fn approx_eq(&self, expected: &Self) -> bool { self.style == expected.style && approx_eq(&self.color, &expected.color) @@ -508,7 +511,7 @@ mod tests { }, ) .unwrap(); - let (output_road, _warnings) = output_lanes.into_filtered_road(test); + let (output_road, warnings) = output_lanes.into_filtered_road(test); if !output_road.approx_eq(&input_road) { test.print(); println!("From:"); @@ -521,6 +524,7 @@ mod tests { println!("Got:"); println!(" {}", stringify_lane_types(&output_road)); println!(" {}", stringify_directions(&output_road)); + println!("{}", warnings); if stringify_lane_types(&input_road) == stringify_lane_types(&output_road) || stringify_directions(&input_road) == stringify_directions(&output_road) { diff --git a/osm2lanes/src/transform/lanes_to_tags/mod.rs b/osm2lanes/src/transform/lanes_to_tags/mod.rs index f1aad11a..1a7475f8 100644 --- a/osm2lanes/src/transform/lanes_to_tags/mod.rs +++ b/osm2lanes/src/transform/lanes_to_tags/mod.rs @@ -356,6 +356,7 @@ fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), Lan { tags.checked_insert("oneway:bicycle", "no")?; } + // indicate cycling traffic direction relative to the direction the osm way is oriented // yes: same direction // -1: contraflow @@ -394,6 +395,9 @@ fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), Lan tags.checked_insert("cycleway:right:width", width.val().to_string())?; } + // Handle shared lanes + if lanes.forward_inside() // TODO: this needs to exist... + Ok(()) } diff --git a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs index 83189313..2d541a27 100644 --- a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs +++ b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs @@ -2,11 +2,11 @@ use std::borrow::Borrow; use std::fmt::Display; use std::hash::Hash; -use osm_tags::TagKey; +use osm_tags::{Access, TagKey}; use crate::locale::Locale; use crate::metric::Metre; -use crate::road::{Designated, Direction}; +use crate::road::{AccessAndDirection, Designated, Direction}; use crate::tag::Tags; use crate::transform::tags::CYCLEWAY; use crate::transform::tags_to_lanes::oneway::Oneway; @@ -150,7 +150,7 @@ impl Scheme { road_oneway: Oneway, warnings: &mut RoadWarnings, ) -> Result { - let scheme_cycleway = Self::from_tags_cycleway(tags, locale, road_oneway, warnings); + let scheme_cycleway = Self::from_tags_cycleway(tags, locale, road_oneway, warnings)?; let scheme_cycleway_both = Self::from_tags_cycleway_both(tags, locale, road_oneway, warnings); let scheme_cycleway_forward = @@ -187,16 +187,16 @@ impl Scheme { locale: &Locale, road_oneway: Oneway, warnings: &mut RoadWarnings, - ) -> Option { + ) -> Result, TagsToLanesMsg> { match cycleway_variant(tags, None) { Ok(OptionNo::Some((variant, opposite))) => { if road_oneway.into() { if opposite.is_none() { - Some(Self(Location::Forward(Way { + Ok(Some(Self(Location::Forward(Way { variant, direction: Direction::Forward, width: None, - }))) + })))) } else { if let Variant::Lane | Variant::Track = variant { warnings.push(TagsToLanesMsg::deprecated( @@ -214,19 +214,19 @@ impl Scheme { .unwrap(), )); } - Some(Self(Location::Backward(Way { + Ok(Some(Self(Location::Backward(Way { variant, direction: Direction::Backward, width: None, - }))) + })))) } } else { if opposite.is_some() { - warnings.push(TagsToLanesMsg::unsupported_tags( + return Err(TagsToLanesMsg::unsupported_tags( tags.subset(["oneway", "cycleway"]), )); } - Some(Self(Location::Both { + Ok(Some(Self(Location::Both { forward: Way { variant, direction: Direction::Forward, @@ -237,14 +237,14 @@ impl Scheme { direction: Direction::Backward, width: None, }, - })) + }))) } }, - Ok(OptionNo::No) => Some(Self(Location::None)), - Ok(OptionNo::None) => None, + Ok(OptionNo::No) => Ok(Some(Self(Location::None))), + Ok(OptionNo::None) => Ok(None), Err(e) => { warnings.push(e.into()); - None + Ok(None) }, } } @@ -447,10 +447,22 @@ pub(in crate::transform::tags_to_lanes) fn bicycle( match scheme.0 { Location::None => {}, Location::Forward(way) => { - road.push_forward_outside(lane(way)); + if let Variant::Lane | Variant::Track = way.variant { + road.push_forward_outside(lane(way)); + } + // TODO: Do nothing if forward sharing the lane? What if we are on a bus-only road? }, - Location::Backward(way) => { - road.push_backward_outside(lane(way)); + Location::Backward(way) => match way.variant { + Variant::Lane | Variant::Track => road.push_backward_outside(lane(way)), + Variant::SharedMotor => { + road.forward_outside_mut() + .ok_or_else(|| TagsToLanesMsg::unsupported_str("no forward lanes for busway"))? + .access + .bicycle = Infer::Direct(AccessAndDirection { + access: Access::Yes, + direction: Some(Direction::Both), + }) + }, }, Location::Both { forward, backward } => { road.push_forward_outside(lane(forward)); @@ -605,6 +617,7 @@ mod tests { ); } + // cycleway=opposite #[test] fn opposite() { let mut warnings = RoadWarnings::default(); @@ -626,6 +639,18 @@ mod tests { ); } + // cycleway=opposite only applies to oneway + #[test] + fn err_opposite_twoway() { + let scheme = Scheme::from_tags( + &Tags::from_pair("cycleway", "opposite"), + &Locale::builder().build(), + Oneway::No, + &mut RoadWarnings::default(), + ); + assert!(scheme.is_err()); + } + #[test] fn warn_shoulder() { let mut warnings = RoadWarnings::default(); diff --git a/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs b/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs index 6ba230ed..56e2ee97 100644 --- a/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs +++ b/osm2lanes/src/transform/tags_to_lanes/modes/non_motorized.rs @@ -1,6 +1,8 @@ +use osm_tags::Access; + use crate::locale::Locale; -use crate::road::{Designated, Direction}; -use crate::tag::{Access, Tags, HIGHWAY}; +use crate::road::{AccessAndDirection, Designated, Direction}; +use crate::tag::{Tags, HIGHWAY}; use crate::transform::tags_to_lanes::{RoadBuilder, TagsToLanesMsg}; use crate::transform::{Infer, RoadWarnings}; @@ -24,8 +26,14 @@ pub(in crate::transform::tags_to_lanes) fn non_motorized( let lane = road.forward_outside_mut().unwrap(); lane.designated.set(Infer::Direct(Designated::Foot))?; lane.direction.set(Infer::Direct(Direction::Both))?; - lane.access.foot.set(Infer::Direct(Access::Designated))?; - lane.access.motor.set(Infer::Direct(Access::No))?; + lane.access.foot.set(Infer::Direct(AccessAndDirection { + access: Access::Designated, + direction: None, + }))?; + lane.access.motor.set(Infer::Direct(AccessAndDirection { + access: Access::No, + direction: None, + }))?; if v == "steps" { warnings.push(TagsToLanesMsg::unimplemented( "steps becomes sidewalk", diff --git a/osm2lanes/src/transform/tags_to_lanes/road.rs b/osm2lanes/src/transform/tags_to_lanes/road.rs index 3df2d218..01041410 100644 --- a/osm2lanes/src/transform/tags_to_lanes/road.rs +++ b/osm2lanes/src/transform/tags_to_lanes/road.rs @@ -16,7 +16,7 @@ use crate::road::{ AccessAndDirection as LaneAccessAndDirection, AccessByType as LaneAccessByType, Designated, Direction, Lane, }; -use crate::tag::{Access as AccessValue, Highway, TagKey, Tags}; +use crate::tag::{Highway, TagKey, Tags}; use crate::transform::error::{RoadError, RoadWarnings}; use crate::transform::tags_to_lanes::counts::{CentreTurnLaneScheme, Counts}; use crate::transform::tags_to_lanes::modes::BusLaneCount; @@ -54,23 +54,15 @@ pub struct Width { #[derive(Clone, Default, Debug)] pub struct Access { - pub foot: Infer, - pub bicycle: Infer, - pub taxi: Infer, - pub bus: Infer, - pub motor: Infer, + pub foot: Infer, + pub bicycle: Infer, + pub taxi: Infer, + pub bus: Infer, + pub motor: Infer, } impl From for Option { fn from(inferred: Access) -> Self { - impl LaneAccessAndDirection { - fn from(v: Infer) -> Option { - v.some().map(|v| Self { - access: v, - direction: None, - }) - } - } if inferred.foot.is_none() && inferred.bicycle.is_none() && inferred.taxi.is_none() @@ -80,11 +72,11 @@ impl From for Option { return None; } Some(LaneAccessByType { - foot: LaneAccessAndDirection::from(inferred.foot), - bicycle: LaneAccessAndDirection::from(inferred.bicycle), - taxi: LaneAccessAndDirection::from(inferred.taxi), - bus: LaneAccessAndDirection::from(inferred.bus), - motor: LaneAccessAndDirection::from(inferred.motor), + foot: inferred.foot.some(), + bicycle: inferred.bicycle.some(), + taxi: inferred.taxi.some(), + bus: inferred.bus.some(), + motor: inferred.motor.some(), }) } } From 11485124106a3b13daf3feea0e8fe6612572b3d8 Mon Sep 17 00:00:00 2001 From: Michael Droogleever Fortuyn Date: Sun, 29 May 2022 23:24:40 +0200 Subject: [PATCH 2/2] Implement a simple solution --- data/spec-lanes.json | 49 +++++++++++++++++-- osm2lanes/src/road/lane.rs | 4 +- osm2lanes/src/transform/lanes_to_tags/mod.rs | 35 +++++++++++-- .../transform/tags_to_lanes/modes/bicycle.rs | 2 +- 4 files changed, 79 insertions(+), 11 deletions(-) diff --git a/data/spec-lanes.json b/data/spec-lanes.json index f83bbc9e..be09edc0 100644 --- a/data/spec-lanes.json +++ b/data/spec-lanes.json @@ -82,9 +82,11 @@ }, "access": { "type": "object", - "description": "Access to the lane. Freeform tags matching OSM way access", - "additionalProperties": { - "type": "string" + "description": "Access by mode.", + "properties": { + "bicycle": { + "$ref": "/schemas/access" + } } }, "markings": { @@ -159,6 +161,47 @@ "const": "osm2lanes" } ] + }, + "access": { + "$id": "/schemas/access", + "$schema": "https://json-schema.org/draft/2020-12/schema", + "type": "object", + "description": "Legal access for a lane. Only populated if different than default", + "required": [ + "access" + ], + "properties": { + "access": { + "type": "string", + "description": "The access value.", + "anyOf": [ + { + "const": "yes" + }, + { + "const": "no" + }, + { + "const": "designated" + } + ] + }, + "direction": { + "type": "string", + "description": "The direction of the access, if applicable.", + "anyOf": [ + { + "const": "forward" + }, + { + "const": "backward" + }, + { + "const": "both" + } + ] + } + } } } } \ No newline at end of file diff --git a/osm2lanes/src/road/lane.rs b/osm2lanes/src/road/lane.rs index 6c211254..1bbb5fd1 100644 --- a/osm2lanes/src/road/lane.rs +++ b/osm2lanes/src/road/lane.rs @@ -175,7 +175,7 @@ impl Printable for Direction { // TODO: how to handle the motor_vehicle vs motorcar discussion in https://wiki.openstreetmap.org/wiki/Key:motorcar#Controversy // TODO: separating weight class by usage? #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +#[serde(rename_all = "snake_case")] pub struct AccessByType { #[serde(skip_serializing_if = "Option::is_none")] pub(crate) foot: Option, @@ -191,7 +191,7 @@ pub struct AccessByType { /// Access for a given user #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] -#[serde(tag = "type", rename_all = "snake_case")] +#[serde(rename_all = "snake_case")] pub struct AccessAndDirection { pub(crate) access: AccessTagValue, /// Direction, if different from designated direction diff --git a/osm2lanes/src/transform/lanes_to_tags/mod.rs b/osm2lanes/src/transform/lanes_to_tags/mod.rs index 1a7475f8..7b097e39 100644 --- a/osm2lanes/src/transform/lanes_to_tags/mod.rs +++ b/osm2lanes/src/transform/lanes_to_tags/mod.rs @@ -1,12 +1,13 @@ #![allow(clippy::module_name_repetitions)] // TODO: fix upstream use celes::Country; +use osm_tags::Access; pub use self::error::LanesToTagsMsg; use super::{tags_to_lanes, TagsToLanesConfig}; -use crate::locale::Locale; +use crate::locale::{DrivingSide, Locale}; use crate::metric::Speed; -use crate::road::{Color, Designated, Direction, Lane, Marking, Road}; +use crate::road::{AccessByType, Color, Designated, Direction, Lane, Marking, Road}; use crate::tag::Tags; #[non_exhaustive] @@ -33,6 +34,13 @@ impl Lane { fn is_shoulder(&self) -> bool { matches!(self, Lane::Shoulder { .. }) } + + fn access(&self) -> Option<&AccessByType> { + match self { + Self::Travel { access, .. } => access.as_ref(), + _ => None, + } + } } mod error { @@ -159,7 +167,7 @@ pub fn lanes_to_tags( set_shoulder(lanes, &mut tags)?; set_pedestrian(lanes, &mut tags)?; set_parking(lanes, &mut tags)?; - set_cycleway(lanes, &mut tags, oneway)?; + set_cycleway(lanes, &mut tags, oneway, locale)?; set_busway(lanes, &mut tags, oneway)?; let max_speed = get_max_speed(lanes, &mut tags)?; @@ -326,7 +334,12 @@ fn set_parking(lanes: &[Lane], tags: &mut Tags) -> Result<(), LanesToTagsMsg> { Ok(()) } -fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), LanesToTagsMsg> { +fn set_cycleway( + lanes: &[Lane], + tags: &mut Tags, + oneway: bool, + locale: &Locale, +) -> Result<(), LanesToTagsMsg> { let left_cycle_lane: Option<&Lane> = lanes .iter() .take_while(|lane| !lane.is_motor()) @@ -396,7 +409,19 @@ fn set_cycleway(lanes: &[Lane], tags: &mut Tags, oneway: bool) -> Result<(), Lan } // Handle shared lanes - if lanes.forward_inside() // TODO: this needs to exist... + //if lanes.forward_inside() // TODO: this needs to exist... + if lanes.len() == 1 { + let lane = match locale.driving_side { + DrivingSide::Right => lanes.last(), + DrivingSide::Left => lanes.first(), + }; + if let Some(bicycle) = lane.and_then(Lane::access).and_then(|a| a.bicycle.as_ref()) { + if oneway && bicycle.access == Access::Yes && bicycle.direction == Some(Direction::Both) + { + tags.checked_insert("cycleway", "opposite")?; + } + } + } Ok(()) } diff --git a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs index 2d541a27..b7881878 100644 --- a/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs +++ b/osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs @@ -461,7 +461,7 @@ pub(in crate::transform::tags_to_lanes) fn bicycle( .bicycle = Infer::Direct(AccessAndDirection { access: Access::Yes, direction: Some(Direction::Both), - }) + }); }, }, Location::Both { forward, backward } => {