Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added post process logic to collapse segregated turn instructions #4925

Merged
merged 36 commits into from
Mar 30, 2018
Merged
Show file tree
Hide file tree
Changes from 35 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7b74910
Added post process logic to collapse segregated turn instructions
dgearhart Feb 28, 2018
1d3eac5
format updates
dgearhart Feb 28, 2018
eea551d
Fixed coordinates to reflect reality
dgearhart Feb 28, 2018
b56d669
fixed coordinates to fix test
dgearhart Feb 28, 2018
ea47f79
Skip last step when processing segregated steps
dgearhart Mar 1, 2018
9a11432
updated segregated turn test
dgearhart Mar 1, 2018
20f47e6
Updated segregated test
dgearhart Mar 1, 2018
482529e
Updated test: Segregated Intersection, Cross Belonging to Correct Str…
dgearhart Mar 1, 2018
e7c7f3e
Fixed all but one for features/guidance/collapse.feature:124
dgearhart Mar 13, 2018
9c81b73
Fixed Scenario: Partly Segregated Intersection, Two Segregated Roads,…
dgearhart Mar 13, 2018
b72b9de
Fixed 7 of th 9 failures for Scenario: Partly Segregated Intersection…
dgearhart Mar 13, 2018
0bf22f8
Fixed 7 of the 9 failures for Scenario: Segregated Intersection, Cros…
dgearhart Mar 13, 2018
e9c6116
Fixed Scenario: Segregated Intersection into Slight Turn - features/g…
dgearhart Mar 13, 2018
7d5e912
Updated Scenario: U-turn after a traffic light - features/guidance/tu…
dgearhart Mar 15, 2018
f956ddd
Updated how we combine segregated steps
dgearhart Mar 15, 2018
4690c93
Added test to Verify end of road left turn across divided roads
dgearhart Mar 15, 2018
8385f6a
Fixed divided highwat tests
dgearhart Mar 17, 2018
c6b1d57
Fixed test failure
dgearhart Mar 17, 2018
07a2a2d
fixed Scenario: Partitioned turn, Slight Curve - maxspeed - features/…
dgearhart Mar 18, 2018
06d11a0
Fixed Scenario: Partitioned turn, Slight Curve - features/guidance/tu…
dgearhart Mar 18, 2018
832b52d
Added strategies to combine segrgated intersections
dgearhart Mar 20, 2018
99275c6
Added setModifier alias for readability
dgearhart Mar 20, 2018
3e0084a
Added strategies to combine segrgated intersections
dgearhart Mar 20, 2018
6ad4d27
Format updates
dgearhart Mar 20, 2018
492e2fa
Fixes segregated indentification to not mark `circular` edge as segre…
dgearhart Mar 21, 2018
6578825
Merge branch 'master' of https://github.com/Project-OSRM/osrm-backend…
dgearhart Mar 26, 2018
6ed640c
Added intersection prior to turn so we still call out end of road
dgearhart Mar 26, 2018
7027cb1
updated expectation to be turn instead of continue
dgearhart Mar 26, 2018
acad9f1
Confirmed with @oxidase that the lane information if correct - update…
dgearhart Mar 26, 2018
468bbec
Added logic to handle wider straights
dgearhart Mar 28, 2018
6749f8b
Update CHANGELOG.md
dgearhart Mar 28, 2018
fbbba61
Merge branch 'master' into gdg_segregated_post_process
dgearhart Mar 28, 2018
498ee5e
Merge branch 'master' of https://github.com/Project-OSRM/osrm-backend…
dgearhart Mar 28, 2018
18e2ad3
Merge branch 'gdg_segregated_post_process' of https://github.com/Proj…
dgearhart Mar 28, 2018
37f74ca
Removed TODO
dgearhart Mar 28, 2018
2f341b1
Process straight step prior to wider straight step
dgearhart Mar 30, 2018
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@
- Guidance:
- CHANGED #4929: Don't use obviousness for links bifurcations [#4929](https://github.com/Project-OSRM/osrm-backend/pull/4929)
- FIXED #4929: Adjust Straight direction modifiers of side roads in driveway handler [#4929](https://github.com/Project-OSRM/osrm-backend/pull/4929)
- CHANGED #4925: Added post process logic to collapse segregated turn instructions [#4925](https://github.com/Project-OSRM/osrm-backend/pull/4925)
- Tools:
- `osrm-routed` accepts a new property `--memory_file` to store memory in a file on disk.
- NodeJS:
- `OSRM` object accepts a new option `memory_file` that stores the memory in a file on disk.
- Internals
- CHANGED #4845 #4968: Updated segregated intersection identification
- CHANGED #4845 #4968: Updated segregated intersection identification [#4845](https://github.com/Project-OSRM/osrm-backend/pull/4845) [#4968](https://github.com/Project-OSRM/osrm-backend/pull/4968)
- REMOVED: Remove `.timestamp` file since it was unused.
- Documentation:
- ADDED: Add documentation about OSM node ids in nearest service response [#4436](https://github.com/Project-OSRM/osrm-backend/pull/4436)
Expand Down
36 changes: 18 additions & 18 deletions features/car/conditional_restrictions.feature
Original file line number Diff line number Diff line change
Expand Up @@ -691,14 +691,14 @@ Feature: Car - Turn restrictions
# """
Given the node locations
| node | lat | lon |
| a | 38.9113 | -77.0091 |
Copy link
Contributor

Choose a reason for hiding this comment

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

The change of locations does not seem necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated to match current map - it was producing odd results

| b | 38.9108 | -77.0091 |
| c | 38.9104 | -77.0091 |
| d | 38.9110 | -77.0096 |
| e | 38.9106 | -77.0086 |
| f | 38.9105 | -77.0090 |
| g | 38.9108 | -77.0090 |
| h | 38.9113 | -77.0090 |
| a | 38.91124 | -77.00909 |
| b | 38.91080 | -77.00909 |
| c | 38.91038 | -77.00909 |
| d | 38.91105 | -77.00967 |
| e | 38.91037 | -77.00807 |
| f | 38.91036 | -77.00899 |
| g | 38.91076 | -77.00901 |
| h | 38.91124 | -77.00900 |

And the ways
| nodes | oneway | name |
Expand All @@ -719,7 +719,7 @@ Feature: Car - Turn restrictions
When I route I should get
| from | to | route | turns |
| a | e | cap south,florida nw,florida nw,florida ne | depart,turn right,continue uturn,arrive |
| f | d | cap north,florida,florida nw | depart,turn left,arrive |
| f | d | cap north,florida nw,florida nw | depart,turn left,arrive |
| e | c | florida ne,florida nw,cap south,cap south | depart,continue uturn,turn right,arrive |

@no_turning @conditionals
Expand All @@ -738,14 +738,14 @@ Feature: Car - Turn restrictions
# """
Given the node locations
| node | lat | lon |
| a | 38.9113 | -77.0091 |
| b | 38.9108 | -77.0091 |
| c | 38.9104 | -77.0091 |
| d | 38.9110 | -77.0096 |
| e | 38.9106 | -77.0086 |
| f | 38.9105 | -77.0090 |
| g | 38.9108 | -77.0090 |
| h | 38.9113 | -77.0090 |
| a | 38.91124 | -77.00909 |
| b | 38.91080 | -77.00909 |
| c | 38.91038 | -77.00909 |
| d | 38.91105 | -77.00967 |
| e | 38.91037 | -77.00807 |
| f | 38.91036 | -77.00899 |
| g | 38.91076 | -77.00901 |
| h | 38.91124 | -77.00900 |

And the ways
| nodes | oneway | name |
Expand All @@ -765,7 +765,7 @@ Feature: Car - Turn restrictions

When I route I should get
| from | to | route | turns |
| a | e | cap south,florida,florida ne | depart,turn left,arrive |
| a | e | cap south,florida ne,florida ne | depart,turn left,arrive |
| f | d | cap north,florida ne,florida ne,florida nw | depart,turn sharp right,continue uturn,arrive |
| e | c | florida ne,cap south,cap south | depart,turn left,arrive |

Expand Down
106 changes: 54 additions & 52 deletions features/guidance/collapse.feature

Large diffs are not rendered by default.

63 changes: 50 additions & 13 deletions features/guidance/divided-highways.feature
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,23 @@ Feature: Divided road entry
d-------e-----f
|
|
g
i---g---j
Copy link
Contributor

Choose a reason for hiding this comment

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

The test modification changes result of previous_step.intersections.size() < MIN_END_OF_ROAD_INTERSECTIONS condition to false. But the PR modifies intersections clustering and respectively the value of previous_step.intersections.size() at node g now is 1 instead of 2, so it is ok to adjust expectation to turn instruction.

If end of road instructions are still required then the commit or PR #2587 must be revisited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above - intersection before the turn was added so the end of road would still be marked properly

|
|
h
"""

And the ways
| nodes | name | highway | oneway |
| abc | main st | residential | -1 |
| def | main st | residential | yes |
| be | main st | residential | |
| eg | side st | residential | |
| nodes | name | highway | oneway |
| abc | main st | residential | -1 |
| def | main st | residential | yes |
| be | main st | residential | |
| egh | side st | residential | |
| igj | maple st | residential | |

When I route I should get
| waypoints | route | turns |
| g,a | side st,main st,main st| depart,end of road left,arrive |
| h,a | side st,main st,main st| depart,end of road left,arrive |


# Similar to previous one, but the joining way is tagged with the side-street name
Expand All @@ -37,18 +41,22 @@ Feature: Divided road entry
d-------e-----f
|
|
g
i---g---j
|
|
h
"""

And the ways
| nodes | name | highway | oneway |
| abc | main st | residential | -1 |
| def | main st | residential | yes |
| beg | side st | residential | |
| nodes | name | highway | oneway |
| abc | main st | residential | -1 |
| def | main st | residential | yes |
| begh | side st | residential | |
| igj | maple st | residential | |

When I route I should get
| waypoints | route | turns |
| g,a | side st,main st,main st| depart,end of road left,arrive |
| h,a | side st,main st,main st| depart,end of road left,arrive |


# Center join named after crossroad
Expand Down Expand Up @@ -100,3 +108,32 @@ Feature: Divided road entry
When I route I should get
| waypoints | route | turns |
| g,a | side st,main st,main st| depart,turn left,arrive |

# Verify end of road left turn across divided roads
Scenario: Join on a divided road, named after the side street
Given the node map
"""
a-----h--b-----c
| |
d-----i--e-----f
| |
| |
m---j--g---n
| |
| |
k l
"""

And the ways
| nodes | name | highway | oneway |
| ahbc | main st | residential | -1 |
| dief | main st | residential | yes |
| begl | side st | residential | -1 |
| hijk | side st | residential | yes |
| mjgn | maple st| residential | no |

When I route I should get
| waypoints | route | turns |
| l,a | side st,main st,main st| depart,end of road left,arrive |


2 changes: 1 addition & 1 deletion features/guidance/obvious-turn-discovery.feature
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@ Feature: Simple Turns
When I route I should get
| from | to | route | turns |
| g | c | woll,brei,brei | depart,turn slight right,arrive |
| g | f | woll,scho,scho | depart,continue sharp left,arrive |
| g | f | woll,scho,scho | depart,turn sharp left,arrive |
| a | c | scho,brei | depart,arrive |
| d | f | brei,scho | depart,arrive |

Expand Down
42 changes: 23 additions & 19 deletions features/guidance/turn-lanes.feature
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ Feature: Turn Lane Guidance
| e,l | road,cross,cross | depart,turn right,arrive | ,none:false straight:false straight;right:true, |
| i,h | cross,road,road | depart,turn right,arrive | ,, |
| i,j | cross,cross | depart,arrive | ;;left:false straight:true, |
| i,l | cross,cross,cross | depart,continue uturn,arrive | ;,left:true straight:false;left:true straight:false;left:false straight:true, |
| i,l | cross,cross,cross | depart,continue uturn,arrive | ,left:true straight:false;left:true straight:false;left:true straight:false;left:false straight:true, |

@partition-lanes
Scenario: Turn Lanes at Segregated Road
Expand Down Expand Up @@ -937,48 +937,52 @@ Feature: Turn Lane Guidance
Scenario: Partitioned turn, Slight Curve - maxspeed
Given the node map
"""
f e
| |
| |
| c
a - b ' |
g d
f e
Copy link
Contributor

Choose a reason for hiding this comment

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

The PR moves maneuvers locations to node b, i think it is ok to keep the original mini-map and update test expectations to

| ,left:false right:true,                        | a,b,g     |
| ,left:true right:false;left:true right:false,  | a,b,e     |

for this test and tests below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mini map was updated so there would be an intersection prior to the turn so end of road would be marked. I think before the end of road was being marked on a false intersection - since the turn is at the intersection

| |
i | |
| | c
h - a - b ' |
j g d
"""

And the ways
| nodes | name | highway | oneway | turn:lanes:forward | maxspeed |
| ha | road | primary | yes | | 1 |
| ab | road | primary | yes | left\|right | 1 |
| bc | cross | primary | yes | | 1 |
| fbg | cross | primary | yes | | 1 |
| dce | cross | primary | yes | | 1 |
| iaj | kross | primary | no | | 1 |

When I route I should get
| waypoints | route | turns | lanes | locations |
| a,g | road,cross,cross | depart,turn right,arrive | ,left:false right:true, | a,b,g |
| a,e | road,cross,cross | depart,end of road left,arrive | ;left:true right:false,left:true right:false, | a,c,e |
| h,g | road,cross,cross | depart,turn right,arrive | ;,left:false right:true, | h,b,g |
| h,e | road,cross,cross | depart,end of road left,arrive | ;,left:true right:false;left:true right:false, | h,b,e |

Scenario: Partitioned turn, Slight Curve
Given the node map
"""
f e
| |
| |
| c
a - b ' |
g d
f e
| |
i | |
| | c
h - a - b ' |
j g d
"""

And the ways
| nodes | name | highway | oneway | turn:lanes:forward |
| ha | road | primary | yes | |
| ab | road | primary | yes | left\|right |
| bc | cross | primary | yes | |
| fbg | cross | primary | yes | |
| dce | cross | primary | yes | |
| iaj | kross | primary | no | |

When I route I should get
| waypoints | route | turns | lanes | locations |
| a,g | road,cross,cross | depart,turn right,arrive | ,left:false right:true, | a,b,g |
| a,e | road,cross,cross | depart,end of road left,arrive | ;left:true right:false,left:true right:false, | a,c,e |
| waypoints | route | turns | lanes | locations |
| h,g | road,cross,cross | depart,turn right,arrive | ;,left:false right:true, | h,b,g |
| h,e | road,cross,cross | depart,end of road left,arrive | ;,left:true right:false;left:true right:false, | h,b,e |

Scenario: Lane Parsing Issue #2694
Given the node map
Expand Down Expand Up @@ -1244,4 +1248,4 @@ Feature: Turn Lane Guidance

When I route I should get
| waypoints | route | turns | lanes | locations |
| a,f | road1,road1,road1 | depart,continue uturn,arrive | ;left:false straight:true straight;right:false,left:true straight:false straight;right:false;;, | a,d,f |
| a,f | road1,road1,road1 | depart,continue uturn,arrive | ,;left:true straight:false straight;right:false;;, | a,c,f |
3 changes: 3 additions & 0 deletions include/engine/api/route_api.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ class RouteAPI : public BaseAPI
// processing is performed
guidance::applyOverrides(BaseAPI::facade, steps, leg_geometry);

// Collapse segregated steps before others
steps = guidance::collapseSegregatedTurnInstructions(std::move(steps));

/* Perform step-based post-processing.
*
* Using post-processing on basis of route-steps for a single leg at a time
Expand Down
26 changes: 24 additions & 2 deletions include/engine/guidance/collapse_turns.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@ namespace engine
namespace guidance
{

// Multiple possible reasons can result in unnecessary/confusing instructions
// Collapsing such turns into a single turn instruction, we give a clearer
// set of instructions that is not cluttered by unnecessary turns/name changes.
OSRM_ATTR_WARN_UNUSED
std::vector<RouteStep> collapseTurnInstructions(std::vector<RouteStep> steps);

// Multiple possible reasons can result in unnecessary/confusing instructions
// A prime example would be a segregated intersection. Turning around at this
// intersection would result in two instructions to turn left.
// Collapsing such turns into a single turn instruction, we give a clearer
// set of instructionst that is not cluttered by unnecessary turns/name changes.
// set of instructions that is not cluttered by unnecessary turns/name changes.
OSRM_ATTR_WARN_UNUSED
std::vector<RouteStep> collapseTurnInstructions(std::vector<RouteStep> steps);
std::vector<RouteStep> collapseSegregatedTurnInstructions(std::vector<RouteStep> steps);

// A combined turn is a set of two instructions that actually form a single turn, as far as we
// perceive it. A u-turn consisting of two left turns is one such example. But there are also lots
Expand Down Expand Up @@ -93,6 +99,22 @@ struct StaggeredTurnStrategy : CombineStrategy
const RouteStep &step_prior_to_intersection;
};

// Handling of consecutive segregated steps
struct CombineSegregatedStepsStrategy : CombineStrategy
{
void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;
};

// Handling of segregated intersections
struct SegregatedTurnStrategy : CombineStrategy
{
SegregatedTurnStrategy(const RouteStep &step_prior_to_intersection);

void operator()(RouteStep &step_at_turn_location, const RouteStep &transfer_from_step) const;

const RouteStep &step_prior_to_intersection;
};

// Signage Strategies

// Transfer the signage from the next step onto this step
Expand Down
Loading