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

Split of "#111 Fix lane count calculation" #146

Merged
merged 55 commits into from
Apr 25, 2022
Merged

Split of "#111 Fix lane count calculation" #146

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
a491eef
Add a failing test for the case on the website
BudgieInWA Mar 25, 2022
bf5df9d
Rewrite driving_lane_directions to consider all tags together
BudgieInWA Mar 25, 2022
dc5de27
Add placeholder warn! calls to show how tag validation can be added t…
BudgieInWA Mar 25, 2022
f2c96fc
replace use of `is_some_and`
BudgieInWA Mar 26, 2022
fe479f4
Remove unused `#![feature(is_some_with)]`
BudgieInWA Mar 26, 2022
42ae376
Add lanes:{forward,backward} to lanes_to_tags
BudgieInWA Mar 26, 2022
e3cf57e
Calculate both_ways in driving_lane_directions too
BudgieInWA Mar 27, 2022
b032684
Convert temporary `warn!` calls into `RoadMsg` warnings.
BudgieInWA Mar 27, 2022
4a30b89
Fix misleading panic message when failing to parse data.yaml
BudgieInWA Mar 28, 2022
295e8bc
Represent in the tests that odd lane count warns about guessing
BudgieInWA Mar 28, 2022
f472da3
Avoid guessing lanes= and lanes:forward= in the tests just yet
BudgieInWA Mar 28, 2022
28300bc
guess lanes= and lanes:{forward,backward}= in more situations with bu…
BudgieInWA Mar 28, 2022
a68efd7
fmt
BudgieInWA Mar 28, 2022
c9ca0eb
Start to split parsing into schema understanding, then combining.
BudgieInWA Apr 2, 2022
0496070
wip: Experimental top down rewrite of tag parsing and road building s…
BudgieInWA Apr 2, 2022
392910c
Split into Hunch: "data with certainty" and Theory: "hunch with history"
BudgieInWA Apr 4, 2022
beec80a
Delete experimental Hunch, Theory, and Scheme code from this branch
BudgieInWA Apr 8, 2022
538a676
Fix build and tweak comments
BudgieInWA Apr 8, 2022
96d29e1
Merge remote-tracking branch 'origin/main' into rust_guess_lane_count
BudgieInWA Apr 8, 2022
57fbad0
Move driving_lane_directions to LanesScheme::new
BudgieInWA Apr 8, 2022
012319d
Enable passing test :) add TODO
BudgieInWA Apr 8, 2022
0dd3481
fmt
BudgieInWA Apr 9, 2022
768e441
Add a CentreTurnLaneScheme, add warnings and stuff.
BudgieInWA Apr 9, 2022
fabbb9d
Rename `busway.forward_direction` to `forward_side_direction` to be c…
BudgieInWA Apr 9, 2022
a14ccd7
Remove unused get_and_parse. Not needed now that...
BudgieInWA Apr 9, 2022
7b0b139
fix TODO: does it make sense to have a backward lane on the forward_s…
BudgieInWA Apr 9, 2022
fe1024f
Add comments describing the different Infer variants.
BudgieInWA Apr 9, 2022
1bfb561
remove empty impl
BudgieInWA Apr 9, 2022
ca818c7
Support LHT outputting turn:lanes:both_ways
BudgieInWA Apr 9, 2022
9f07ce1
Add use statement so doctests compile
BudgieInWA Apr 9, 2022
ec7f50d
Use new `road: lanes:` nesting for modified test
BudgieInWA Apr 10, 2022
9c88ebc
Use the new `road: lanes:` nesting for more modified tests.
BudgieInWA Apr 10, 2022
5a63091
Merge branch 'main' into rust_guess_lane_count
droogmic Apr 11, 2022
5212f2e
Fix Clippy
droogmic Apr 11, 2022
08884cb
Merge pull request #1 from droogmic/rust_guess_lane_count
BudgieInWA Apr 18, 2022
2b96964
Fix support for oneway=yes busway:<backward>=opposite_lane
BudgieInWA Apr 19, 2022
eaf0117
Merge branch 'main' into rust_guess_lane_count
BudgieInWA Apr 20, 2022
497b5c7
Fix build after merge, using new TagsToLanesMsg type
BudgieInWA Apr 20, 2022
b890bd2
fix passing 1 byte type as reference (thanks to Clippy)
BudgieInWA Apr 20, 2022
fb56a07
Skip new test in python and kotlin, because they fail
BudgieInWA Apr 20, 2022
551ab2e
add relevant tag to warning
BudgieInWA Apr 20, 2022
9c94da2
warn on calculated inconsistency, as well as direct
BudgieInWA Apr 20, 2022
7303b56
Settle on TurnLanesScheme (with #[allow(unused)])
BudgieInWA Apr 20, 2022
1267f2d
Remove commented code and TODOs
BudgieInWA Apr 20, 2022
2f36852
Remove TurnMarking as struct with bools, do we need a bitset?
BudgieInWA Apr 20, 2022
c8f2774
More succinct centre_turn_lane parsing
BudgieInWA Apr 21, 2022
41694ba
rename bothways to proper both_ways
BudgieInWA Apr 22, 2022
c0df013
rename bothways proper both_ways
BudgieInWA Apr 22, 2022
a264b3c
whitespace
BudgieInWA Apr 22, 2022
c5963f9
Merge branch 'rust_refactor_busway' into rust_guess_lane_count_target
droogmic Apr 24, 2022
5087d2d
Merge branch 'lane_count_test' into rust_guess_lane_count_target
droogmic Apr 24, 2022
31ae8d0
Merge branch 'rust_guess_lane_count' into rust_guess_lane_count_source
droogmic Apr 24, 2022
b18803d
Rework
droogmic Apr 24, 2022
38d6dec
Rework Centre Turn Lanes
droogmic Apr 24, 2022
0cba794
Merge commit '38d6dec77786193001b1cf5408021b12d060ad47' into rust_gue…
droogmic Apr 25, 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
51 changes: 21 additions & 30 deletions rust/osm2lanes/src/transform/tags_to_lanes/lane.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ use crate::transform::{RoadWarnings, TagsToLanesMsg};

const LANES: TagKey = TagKey::from("lanes");
#[derive(Debug)]
pub struct LanesScheme {
pub struct Counts {
pub lanes: Infer<usize>,
pub forward: Infer<usize>,
pub backward: Infer<usize>,
pub both_ways: Infer<usize>,
}

impl LanesScheme {
impl Counts {
/// Parses and validates the `lanes` scheme (which excludes parking lanes, bike lanes, etc.).
/// See <https://wiki.openstreetmap.org/wiki/Key:lanes>.
///
Expand Down Expand Up @@ -108,20 +108,11 @@ impl LanesScheme {
} else {
// Assume 1 lane, but guess 1 normal lane plus bus lanes.
let assumed_forward = 1; // TODO depends on highway tag
if forward_bus_lanes > 0 {
Self {
lanes: Infer::Guessed(assumed_forward + forward_bus_lanes),
forward: Infer::Guessed(assumed_forward + forward_bus_lanes),
backward: Infer::Default(0),
both_ways: bothways,
}
} else {
Self {
lanes: Infer::Default(assumed_forward),
forward: Infer::Default(assumed_forward),
backward: Infer::Default(0),
both_ways: bothways,
}
Self {
lanes: Infer::Default(assumed_forward + forward_bus_lanes),
forward: Infer::Default(assumed_forward + forward_bus_lanes),
backward: Infer::Default(0),
both_ways: bothways,
}
}
} else {
Expand Down Expand Up @@ -154,15 +145,15 @@ impl LanesScheme {
// Without the "lanes" tag, assume one normal lane in each dir, plus bus lanes.
let f = tagged_forward.unwrap_or(1 + forward_bus_lanes);
let b = tagged_backward.unwrap_or(1 + backward_bus_lanes);
let forward = match (tagged_forward.is_some(), forward_bus_lanes) {
(true, _) => Infer::Direct(f),
(false, 0) => Infer::Default(f),
(false, _) => Infer::Guessed(f),
let forward = if tagged_forward.is_some() {
Infer::Direct(f)
} else {
Infer::Default(f)
};
let backward = match (tagged_backward.is_some(), backward_bus_lanes) {
(true, _) => Infer::Direct(b),
(false, 0) => Infer::Default(b),
(false, _) => Infer::Guessed(b),
let backward = if tagged_backward.is_some() {
Infer::Direct(b)
} else {
Infer::Default(b)
};
let lanes = Infer::Default(f + b + bothway_lanes);
// TODO lanes.downgrade(&[forward, backward, bothways]);
Expand Down Expand Up @@ -190,16 +181,16 @@ impl LanesScheme {
lanes: Infer::Direct(1),
forward: Infer::Default(0),
backward: Infer::Default(0),
both_ways: Infer::Guessed(1),
both_ways: Infer::Default(1),
},
(Some(l), None, None) => {
if l % 2 == 0 && centre_turn_lane.present.some().unwrap_or(false) {
// Only tagged with lanes and deprecated center_turn_lane tag.
// Assume the center_turn_lane is in addition to evenly divided lanes.
Self {
lanes: Infer::Calculated(l + 1),
forward: Infer::Guessed(l / 2),
backward: Infer::Guessed(l / 2),
forward: Infer::Default(l / 2),
backward: Infer::Default(l / 2),
both_ways: Infer::Calculated(1),
}
} else {
Expand All @@ -212,8 +203,8 @@ impl LanesScheme {
let half = (remaining_lanes + 1) / 2; // usize division rounded up.
Self {
lanes: Infer::Direct(l),
forward: Infer::Guessed(half + forward_bus_lanes),
backward: Infer::Guessed(
forward: Infer::Default(half + forward_bus_lanes),
backward: Infer::Default(
remaining_lanes - half - bothway_lanes + backward_bus_lanes,
),
both_ways: bothways,
Expand Down Expand Up @@ -249,7 +240,7 @@ impl CentreTurnLaneScheme {
warnings.push(TagsToLanesMsg::unsupported_tags(
tags.subset(&[CENTRE_TURN_LANE]),
));
Infer::Guessed(false)
Infer::Default(false)
},
}
} else {
Expand Down
74 changes: 19 additions & 55 deletions rust/osm2lanes/src/transform/tags_to_lanes/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub use error::TagsToLanesMsg;
mod access_by_lane;

mod lane;
use lane::{CentreTurnLaneScheme, LanesScheme};
use lane::{CentreTurnLaneScheme, Counts};

mod modes;
use modes::BusLanesCount;
Expand Down Expand Up @@ -82,51 +82,17 @@ impl std::convert::From<Oneway> for bool {
/// A value with various levels of inference
#[derive(Copy, Clone, Debug)]
pub enum Infer<T> {
/// We don't know anything about the value.
None,

/// We can only guess what the value should be in this situation. Available tags don't
/// suggest any good default.
/// ```
/// use osm2lanes::transform::tags_to_lanes::Infer;
/// let tagged_backrest = Some(false);
/// let has_backrest = match tagged_backrest {
/// None => Infer::Guessed(true),
/// Some(v) => Infer::Direct(v),
/// };
/// ```
Guessed(T),

/// The value is an understood default for this situation. The absence of available tags implies
/// the value.
/// ```
/// use osm2lanes::transform::tags_to_lanes::Infer;
/// let tagged_oneway = Some(true);
/// let is_oneway = match tagged_oneway {
/// Some(v) => Infer::Direct(v),
/// None => Infer::Default(false),
/// };
/// ```
Default(T),

/// The value has been calculated from other tags.
/// ```
/// use osm2lanes::transform::tags_to_lanes::Infer;
/// let tagged_forward_lanes = 1;
/// let tagged_backward_lanes = 1;
/// let total_lanes = Infer::Calculated(tagged_backward_lanes + tagged_backward_lanes);
/// ```
Calculated(T),

/// The value is tagged as such.
Direct(T),
}

impl<T> Infer<T> {
pub fn some(self) -> Option<T> {
match self {
Self::None => None,
Self::Guessed(v) | Self::Default(v) | Self::Calculated(v) | Self::Direct(v) => Some(v),
Self::Default(v) | Self::Calculated(v) | Self::Direct(v) => Some(v),
}
}
fn direct(some: Option<T>) -> Self {
Expand All @@ -139,7 +105,6 @@ impl<T> Infer<T> {
fn map<U, F: FnOnce(T) -> U>(self, func: F) -> Infer<U> {
match self {
Self::None => Infer::None,
Self::Guessed(v) => Infer::Guessed(func(v)),
Self::Default(v) => Infer::Default(func(v)),
Self::Calculated(v) => Infer::Calculated(func(v)),
Self::Direct(v) => Infer::Direct(func(v)),
Expand Down Expand Up @@ -244,11 +209,24 @@ impl RoadBuilder {
#[allow(clippy::items_after_statements)]
pub fn from(
tags: &Tags,
oneway: Oneway,
lanes: &LanesScheme,
_locale: &Locale,
locale: &Locale,
warnings: &mut RoadWarnings,
) -> Result<Self, RoadError> {
// Parse lane count schemas first.
let oneway = Oneway::from(
tags.is_any("oneway", &["yes", "-1"]) || tags.is("junction", "roundabout"),
);
let bus_lane_counts = BusLanesCount::from_tags(tags, locale, oneway, warnings)?;
let centre_turn_lanes = CentreTurnLaneScheme::new(tags, oneway, locale, warnings);
let lanes = Counts::new(
tags,
oneway,
&centre_turn_lanes,
&bus_lane_counts,
locale,
warnings,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love the idea that the road builder is responsible for all of this. It's pretty clearly the right spot.

Combined with "builder" in the name, it makes me think of something like this. The call graph becomes the things that describes the dependencies, which is interesting.

struct RoadBuilder {
    // ...
    pub oneway: Oneway,
    pub bus_lane_counts: BusLanesCount,
    pub centre_turn_lanes: CentreTurnLaneScheme,
    pub lane_counts: Counts,

    pub locale: Locale,
    warnings: RoadWarnings,
}

impl  RoadBuilder {
	pub fn from_tags(tags: &Tags, locale: &Locale) -> Result {
	      let road = Self::local_default(locale);
	      road.oneway = Oneway::from(tags.is_any("oneway", &["yes", "-1"]) || tags.is("junction", "roundabout"));
	      road.bus_lane_counts = BusLanesCount::from_tags(tags, &self)?;
	      road.centre_turn_lanes = CentreTurnLaneScheme::from_tags(tags, &self)?;
	      road.lane_counts = Counts::from_tags(tags, &self);
	}
	pub fn add_warning(...){...}
}

// ::default() for everything...


let designated = if tags.is("access", "no")
&& (tags.is("bus", "yes") || tags.is("psv", "yes")) // West Seattle
|| tags
Expand Down Expand Up @@ -653,22 +631,8 @@ pub fn tags_to_lanes(
// Early return if we find unimplemented tags.
unsupported(tags, locale, &mut warnings)?;

// Parse lane count schemas first.
let oneway =
Oneway::from(tags.is_any("oneway", &["yes", "-1"]) || tags.is("junction", "roundabout"));
let bus_lane_counts = BusLanesCount::from_tags(tags, locale, oneway, &mut warnings)?;
let centre_turn_lanes = CentreTurnLaneScheme::new(tags, oneway, locale, &mut warnings);
let lanes = LanesScheme::new(
tags,
oneway,
&centre_turn_lanes,
&bus_lane_counts,
locale,
&mut warnings,
);

// Create the road builder and start giving it schemes.
let mut road: RoadBuilder = RoadBuilder::from(tags, oneway, &lanes, locale, &mut warnings)?;
let mut road: RoadBuilder = RoadBuilder::from(tags, locale, &mut warnings)?;

// Early return for non-motorized ways (pedestrian paths, cycle paths, etc.)
if let Some(spec) = modes::non_motorized(tags, locale, &road)? {
Expand Down
5 changes: 2 additions & 3 deletions rust/osm2lanes/src/transform/tags_to_lanes/modes/bus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ fn lanes_bus(
Ok(())
}

//noinspection ALL
fn bus_lanes(
tags: &Tags,
locale: &Locale,
Expand Down Expand Up @@ -348,7 +347,7 @@ fn bus_lanes(
| (None, (None, None), None, (forward, backward)) => {
if let Some(forward) = forward {
let forward_access = Access::split(forward).map_err(|a| {
RoadError::Msg(TagsToLanesMsg::unsupported(
RoadError::from(TagsToLanesMsg::unsupported(
&format!("lanes access {}", a),
tags.subset(&["bus:lanes:backward", "psv:lanes:backward"]),
))
Expand All @@ -361,7 +360,7 @@ fn bus_lanes(
}
if let Some(backward) = backward {
let backward_access = Access::split(backward).map_err(|a| {
RoadError::Msg(TagsToLanesMsg::unsupported(
RoadError::from(TagsToLanesMsg::unsupported(
&format!("lanes access {}", a),
tags.subset(&["bus:lanes:backward", "psv:lanes:backward"]),
))
Expand Down