Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

Refactor Bicycle Scheme #167

Merged
merged 2 commits into from
Apr 30, 2022
Merged

Refactor Bicycle Scheme #167

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5fa2700
Refactor Bicycle Scheme
droogmic Apr 30, 2022
f2fc647
Update osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs
droogmic Apr 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
329 changes: 226 additions & 103 deletions osm2lanes/src/transform/tags_to_lanes/modes/bicycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,219 @@ use crate::locale::Locale;
use crate::road::{Designated, Direction};
use crate::tag::Tags;
use crate::transform::tags::CYCLEWAY;
use crate::transform::tags_to_lanes::oneway::Oneway;
use crate::transform::tags_to_lanes::road::LaneType;
use crate::transform::tags_to_lanes::{Infer, LaneBuilder, RoadBuilder, TagsToLanesMsg};
use crate::transform::{RoadError, RoadWarnings, WaySide};
use crate::transform::{RoadWarnings, WaySide};

struct UnknownVariant;

impl Tags {
fn is_cycleway(&self, side: Option<WaySide>) -> bool {
fn get_variant<T: AsRef<str>>(&self, k: T) -> Result<Option<Variant>, UnknownVariant> {
match self.get(k) {
Some("lane") => Ok(Some(Variant::Lane)),
Some("track") => Ok(Some(Variant::Track)),
Some(_) => Err(UnknownVariant),
None => Ok(None),
}
}
fn cycleway_variant(&self, side: Option<WaySide>) -> Result<Option<Variant>, UnknownVariant> {
if let Some(side) = side {
self.is_any(CYCLEWAY + side.as_str(), &["lane", "track"])
self.get_variant(CYCLEWAY + side.as_str())
} else {
self.is_any(CYCLEWAY, &["lane", "track"])
self.get_variant(CYCLEWAY)
}
}
}

#[derive(Debug, PartialEq, Clone, Copy)]
pub(in crate::transform::tags_to_lanes) enum Variant {
Lane,
Track,
}

#[derive(Debug, PartialEq)]
pub(in crate::transform::tags_to_lanes) struct Way {
variant: Variant,
direction: Direction,
}

#[derive(Debug, PartialEq)]
pub(in crate::transform::tags_to_lanes) enum Location {
None,
_No,
Forward(Way),
Backward(Way),
Both { forward: Way, backward: Way },
}

/// Inferred busway scheme for forward lane and backward lane existing
#[derive(Debug)]
pub(in crate::transform::tags_to_lanes) struct Scheme(Location);

impl Scheme {
#[allow(clippy::unnecessary_wraps, clippy::too_many_lines)]
pub(in crate::transform::tags_to_lanes) fn from_tags(
tags: &Tags,
locale: &Locale,
road_oneway: Oneway,
warnings: &mut RoadWarnings,
) -> Result<Self, TagsToLanesMsg> {
if let Ok(Some(variant)) = tags.cycleway_variant(None) {
if tags
.cycleway_variant(Some(WaySide::Both))
.ok()
.flatten()
.is_some()
|| tags
.cycleway_variant(Some(WaySide::Left))
.ok()
.flatten()
.is_some()
|| tags
.cycleway_variant(Some(WaySide::Right))
.ok()
.flatten()
.is_some()
{
return Err(TagsToLanesMsg::unsupported_str(
"cycleway=* with any cycleway:* values",
));
}
if road_oneway.into() {
Ok(Self(Location::Forward(Way {
variant,
direction: Direction::Forward,
})))
} else {
Ok(Self(Location::Both {
forward: Way {
variant,
direction: Direction::Forward,
},
backward: Way {
variant,
direction: Direction::Backward,
},
}))
}
} else if let Ok(Some(variant)) = tags.cycleway_variant(Some(WaySide::Both)) {
Ok(Self(Location::Both {
forward: Way {
variant,
direction: Direction::Forward,
},
backward: Way {
variant,
direction: Direction::Backward,
},
}))
} else {
// cycleway=opposite_lane
if tags.is(CYCLEWAY, "opposite_lane") {
warnings.push(TagsToLanesMsg::deprecated_tags(
tags.subset(&["cycleway", "oneway"]),
));
return Ok(Self(Location::Backward(Way {
variant: Variant::Lane,
direction: Direction::Backward,
})));
}
// cycleway=opposite oneway=yes oneway:bicycle=no
if tags.is(CYCLEWAY, "opposite") {
if !(road_oneway.into() && tags.is("oneway:bicycle", "no")) {
return Err(TagsToLanesMsg::unsupported_str(
"cycleway=opposite without oneway=yes oneway:bicycle=no",
));
}
return Ok(Self(Location::Backward(Way {
variant: Variant::Lane,
direction: Direction::Backward,
})));
}
// cycleway:FORWARD=*
if let Ok(Some(variant)) = tags.cycleway_variant(Some(locale.driving_side.into())) {
if tags.is(CYCLEWAY + locale.driving_side.tag() + "oneway", "no")
|| tags.is("oneway:bicycle", "no")
{
return Ok(Self(Location::Forward(Way {
variant,
direction: Direction::Both,
})));
}
return Ok(Self(Location::Forward(Way {
variant,
direction: Direction::Forward,
})));
}
// cycleway:FORWARD=opposite_lane
if tags.is_any(
CYCLEWAY + locale.driving_side.tag(),
&["opposite_lane", "opposite_track"],
) {
warnings.push(TagsToLanesMsg::deprecated_tags(
tags.subset(&[CYCLEWAY + locale.driving_side.tag()]),
));
return Ok(Self(Location::Forward(Way {
variant: Variant::Lane, // TODO distinguish oposite_ values
direction: Direction::Backward,
})));
}
// cycleway:BACKWARD=*
if let Ok(Some(variant)) =
tags.cycleway_variant(Some(locale.driving_side.opposite().into()))
{
return Ok(Self(
if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"yes",
) {
Location::Backward(Way {
variant,
direction: Direction::Forward,
})
} else if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"-1",
) {
Location::Backward(Way {
variant,
direction: Direction::Backward,
})
} else if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"no",
) || tags.is("oneway:bicycle", "no")
{
Location::Backward(Way {
variant,
direction: Direction::Both,
})
} else if road_oneway.into() {
// A oneway road with a cycleway on the wrong side
Location::Backward(Way {
variant,
direction: Direction::Forward,
})
} else {
// A contraflow bicycle lane
Location::Backward(Way {
variant,
direction: Direction::Backward,
})
},
));
}
// cycleway:BACKWARD=opposite_lane
if tags.is_any(
CYCLEWAY + locale.driving_side.opposite().tag(),
&["opposite_lane", "opposite_track"],
) {
return Err(TagsToLanesMsg::unsupported_tags(
tags.subset(&[CYCLEWAY + locale.driving_side.opposite().tag()]),
));
}
Ok(Self(Location::None))
}
}
}
Expand Down Expand Up @@ -48,105 +251,25 @@ pub(in crate::transform::tags_to_lanes) fn bicycle(
locale: &Locale,
road: &mut RoadBuilder,
warnings: &mut RoadWarnings,
) -> Result<(), RoadError> {
if tags.is_cycleway(None) {
if tags.is_cycleway(Some(WaySide::Both))
|| tags.is_cycleway(Some(WaySide::Right))
|| tags.is_cycleway(Some(WaySide::Left))
{
return Err(
TagsToLanesMsg::unsupported_str("cycleway=* with any cycleway:* values").into(),
);
}
road.push_forward_outside(LaneBuilder::cycle_forward(locale));
if road.oneway.into() {
if road.backward_outside().is_some() {
// TODO validity of this safety check
warnings.push(TagsToLanesMsg::unimplemented(
"oneway has backwards lanes when adding cycleways",
tags.subset(&["oneway", "cycleway"]),
));
}
} else {
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
}
} else if tags.is_cycleway(Some(WaySide::Both)) {
road.push_forward_outside(LaneBuilder::cycle_forward(locale));
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
} else {
// cycleway=opposite_lane
if tags.is(CYCLEWAY, "opposite_lane") {
warnings.push(TagsToLanesMsg::deprecated_tags(
tags.subset(&["cycleway", "oneway"]),
));
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
}
// cycleway=opposite oneway=yes oneway:bicycle=no
if tags.is(CYCLEWAY, "opposite") {
if !(road.oneway.into() && tags.is("oneway:bicycle", "no")) {
return Err(TagsToLanesMsg::unsupported_str(
"cycleway=opposite without oneway=yes oneway:bicycle=no",
)
.into());
}
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
}
// cycleway:FORWARD=*
if tags.is_cycleway(Some(locale.driving_side.into())) {
if tags.is(CYCLEWAY + locale.driving_side.tag() + "oneway", "no")
|| tags.is("oneway:bicycle", "no")
{
road.push_forward_outside(LaneBuilder::cycle_both(locale));
} else {
road.push_forward_outside(LaneBuilder::cycle_forward(locale));
}
}
// cycleway:FORWARD=opposite_lane
if tags.is_any(
CYCLEWAY + locale.driving_side.tag(),
&["opposite_lane", "opposite_track"],
) {
warnings.push(TagsToLanesMsg::deprecated_tags(
tags.subset(&[CYCLEWAY + locale.driving_side.tag()]),
));
road.push_forward_outside(LaneBuilder::cycle_backward(locale));
}
// cycleway:BACKWARD=*
if tags.is_cycleway(Some(locale.driving_side.opposite().into())) {
if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"yes",
) {
road.push_forward_inside(LaneBuilder::cycle_forward(locale));
} else if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"-1",
) {
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
} else if tags.is(
CYCLEWAY + locale.driving_side.opposite().tag() + "oneway",
"no",
) || tags.is("oneway:bicycle", "no")
{
road.push_backward_outside(LaneBuilder::cycle_both(locale));
} else if road.oneway.into() {
// A oneway road with a cycleway on the wrong side
road.push_forward_inside(LaneBuilder::cycle_forward(locale));
} else {
// A contraflow bicycle lane
road.push_backward_outside(LaneBuilder::cycle_backward(locale));
}
}
// cycleway:BACKWARD=opposite_lane
if tags.is_any(
CYCLEWAY + locale.driving_side.opposite().tag(),
&["opposite_lane", "opposite_track"],
) {
return Err(TagsToLanesMsg::unsupported_tags(
tags.subset(&[CYCLEWAY + locale.driving_side.opposite().tag()]),
)
.into());
}
) -> Result<(), TagsToLanesMsg> {
let scheme = Scheme::from_tags(tags, locale, road.oneway, warnings)?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separating tag parsing from modifying still looks like a good approach to me -- nice. A good test of the code maintainability will come when we start parsing tagged cycle lane widths or separator info.

let lane = |way: Way| match way.direction {
Direction::Forward => LaneBuilder::cycle_forward(locale),
Direction::Backward => LaneBuilder::cycle_backward(locale),
Direction::Both => LaneBuilder::cycle_both(locale),
};
match scheme.0 {
Location::None | Location::_No => {},
Location::Forward(way) => {
road.push_forward_outside(lane(way));
},
Location::Backward(way) => {
road.push_backward_outside(lane(way));
},
Location::Both { forward, backward } => {
road.push_forward_outside(lane(forward));
road.push_backward_outside(lane(backward));
},
}
Ok(())
}
2 changes: 1 addition & 1 deletion osm2lanes/src/transform/tags_to_lanes/road.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl RoadBuilder {
self.backward_lanes.back_mut()
}
/// Push new inner-most forward lane
pub fn push_forward_inside(&mut self, lane: LaneBuilder) {
pub fn _push_forward_inside(&mut self, lane: LaneBuilder) {
self.forward_lanes.push_front(lane);
}
/// Push new outer-most forward lane
Expand Down