Skip to content

Commit

Permalink
Improvements to maneuver override processing (Project-OSRM#6215)
Browse files Browse the repository at this point in the history 
This change unblocks the osrm-extract debug build, which is
currently failing on a maneuver override assertion.

The processing of maneuver overrides currently has three issues
- It assumes the via node(s) can't be compressed (the failing assertion)
- It can't handle via-paths containing incompressible nodes
- It doesn't interop with turn restriction on the same path

Turn restrictions and maneuver overrides both use the same
from-via-to path representation.
Therefore, we can fix these issues by consolidating their
structures and reusing the path representation for
turn restrictions, which already is robust to the above
issues.

This also simplifies some of the codebase by removing maneuver
override specific path processing.

There are ~100 maneuver overrides in the OSM database, so the
impact on processing and routing will be minimal.
  • Loading branch information
@mjjbell @mattwigway
mjjbell authored and mattwigway committed Jul 20, 2023
1 parent 213e3ab commit ed78056
Show file tree
Hide file tree
Showing 24 changed files with 1,222 additions and 1,043 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
- CHANGED: Lazily generate optional route path data [#6045](https://github.com/Project-OSRM/osrm-backend/pull/6045)
- FIXED: Completed support for no_entry and no_exit turn restrictions. [#5988](https://github.com/Project-OSRM/osrm-backend/pull/5988)
- ADDED: Add support for non-round-trips with a single fixed endpoint. [#6050](https://github.com/Project-OSRM/osrm-backend/pull/6050)
- FIXED: Improvements to maneuver override processing [#6125](https://github.com/Project-OSRM/osrm-backend/pull/6125)

# 5.26.0
- Changes from 5.25.0
Expand Down
97 changes: 97 additions & 0 deletions features/guidance/maneuver-tag.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,100 @@ Feature: Maneuver tag support
| waypoints | route | turns |
| z,t | NY Ave,395,395 | depart,on ramp left,arrive |
| z,b | NY Ave,,4th St,4th St | depart,on ramp left,fork slight right,arrive |

Scenario: Gracefully handles maneuvers that are redundant for the profile
Given the node map
"""
a--b---c---d----f
|
|
e
"""
And the ways
| nodes | name | oneway | highway |
| abc | A Street | no | primary |
| ce | B Street | no | construction |
| cdf | A Street | no | primary |

And the relations
| type | way:from | node:via | way:to | maneuver | direction |
| maneuver | abc | c | cdf | turn | slight_left |

When I route I should get
| waypoints | route | turns |
| a,f | A Street,A Street | depart,arrive |

Scenario: Handles uncompressed nodes in maneuver path
Given the node map
"""
a--b---c---f
| |
| |
g d---h
|
|
i-------e-------j
"""
And the ways
| nodes | name | oneway |
| abc | A Street | no |
| cf | B Street | no |
| cde | C Street | no |
| bg | D Street | no |
| dh | E Street | no |
| ei | F Street | no |
| ej | G Street | no |

And the relations
| type | way:from | node:via | way:via | way:to | maneuver | direction |
| maneuver | abc | e | cde | ei | turn | sharp_right |

When I route I should get
| waypoints | route | turns |
| a,i | A Street,C Street,F Street,F Street | depart,turn right,turn sharp right,arrive |


Scenario: Can be used with turn restrictions
Given the node map
"""
a---b---c
|
|
d
|
e---f
|
|
h------g---i
"""
And the ways
| nodes | name | oneway |
| ab | A Street | no |
| bc | B Street | no |
| bde | C Street | no |
| ef | D Street | no |
| eg | E Street | no |
| hg | F Street | no |
| gi | G Street | no |

And the relations
| type | way:from | node:via | way:to | maneuver | direction | # |
| maneuver | ab | b | bde | turn | sharp_right | ending on a turn restriction via way |
| maneuver | bde | e | ef | turn | sharp_left | starting on a turn restriction via way |

And the relations
| type | way:from | node:via | way:via | way:to | maneuver | direction | # |
| maneuver | cb | g | bde,eg | gi | turn | slight_left | turn restricted |
| maneuver | cb | g | bde,eg | hg | turn | slight_right | not turn restricted |

And the relations
| type | way:from | way:via | way:to | restriction |
| restriction | ab | bde,eg | hg | no_right_turn |
| restriction | bc | bde,eg | gi | no_left_turn |

When I route I should get
| waypoints | route | turns |
| a,e | A Street,C Street,C Street | depart,turn sharp right,arrive |
| b,f | C Street,D Street,D Street | depart,turn sharp left,arrive |
| c,h | B Street,E Street,F Street,F Street | depart,turn left,turn slight right,arrive |
| c,i | B Street,A Street,E Street,G Street,G Street | depart,turn uturn,turn right,end of road left,arrive |
2 changes: 1 addition & 1 deletion include/extractor/graph_compressor.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include "extractor/scripting_environment.hpp"
#include "util/typedefs.hpp"

#include "extractor/maneuver_override.hpp"
#include "util/node_based_graph.hpp"

#include <memory>
Expand All @@ -18,6 +17,7 @@ namespace extractor

class CompressedEdgeContainer;
struct TurnRestriction;
struct UnresolvedManeuverOverride;

class GraphCompressor
{
Expand Down
95 changes: 78 additions & 17 deletions include/extractor/maneuver_override.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
#include "util/typedefs.hpp"

#include "storage/shared_memory_ownership.hpp"
#include "turn_path.hpp"
#include "util/integer_range.hpp"
#include "util/log.hpp"
#include "util/vector_view.hpp"
#include <algorithm>
#include <boost/functional/hash.hpp>
#include <mapbox/variant.hpp>

namespace osrm
{
Expand All @@ -17,7 +21,7 @@ namespace extractor
// Data that is loaded from the OSM datafile directly
struct InputManeuverOverride
{
std::vector<OSMWayID> via_ways;
InputTurnPath turn_path;
OSMNodeID via_node;
std::string maneuver;
std::string direction;
Expand All @@ -26,9 +30,7 @@ struct InputManeuverOverride
// Object returned by the datafacade
struct ManeuverOverride
{
// util::ViewOrVector<NodeID, storage::Ownership::View> node_sequence;
std::vector<NodeID> node_sequence;
// before the turn, then later, the edge_based_node_id of the turn
NodeID instruction_node; // node-based node ID
guidance::TurnType::Enum override_type;
guidance::DirectionModifier::Enum direction;
Expand All @@ -40,12 +42,12 @@ struct StorageManeuverOverride
std::uint32_t node_sequence_offset_begin;
std::uint32_t node_sequence_offset_end;
NodeID start_node;
// before the turn, then later, the edge_based_node_id of the turn
NodeID instruction_node; // node-based node ID
guidance::TurnType::Enum override_type;
guidance::DirectionModifier::Enum direction;
};

// Used to identify maneuver turns whilst generating edge-based graph
struct NodeBasedTurn
{
NodeID from;
Expand All @@ -58,29 +60,88 @@ struct NodeBasedTurn
}
};

// Internal representation of maneuvers during graph extraction phase
struct UnresolvedManeuverOverride
{

std::vector<NodeBasedTurn>
turn_sequence; // initially the internal node-based-node ID of the node
// before the turn, then later, the edge_based_node_id of the turn
// The turn sequence that the maneuver override applies to.
TurnPath turn_path;
NodeID instruction_node; // node-based node ID
guidance::TurnType::Enum override_type;
guidance::DirectionModifier::Enum direction;

UnresolvedManeuverOverride()
{
turn_path = {ViaNodePath{SPECIAL_NODEID, SPECIAL_NODEID, SPECIAL_NODEID}};
instruction_node = SPECIAL_NODEID;
override_type = guidance::TurnType::Invalid;
direction = guidance::DirectionModifier::MaxDirectionModifier;
}

// check if all parts of the restriction reference an actual node
bool Valid() const
{
return !turn_sequence.empty() &&
std::none_of(turn_sequence.begin(),
turn_sequence.end(),
[](const auto &n) {
return n.from == SPECIAL_NODEID || n.via == SPECIAL_NODEID ||
n.to == SPECIAL_NODEID;
}) &&
(direction != guidance::DirectionModifier::MaxDirectionModifier ||
override_type != guidance::TurnType::Invalid);
if ((direction == guidance::DirectionModifier::MaxDirectionModifier &&
override_type == guidance::TurnType::Invalid) ||
instruction_node == SPECIAL_NODEID || !turn_path.Valid())
{
return false;
}

if (turn_path.Type() == TurnPathType::VIA_NODE_TURN_PATH)
{
const auto node_path = turn_path.AsViaNodePath();
if (node_path.via != instruction_node)
{
util::Log(logDEBUG) << "Maneuver via-node " << node_path.via
<< " does not match instruction node " << instruction_node;
return false;
}
}
else
{
BOOST_ASSERT(turn_path.Type() == TurnPathType::VIA_WAY_TURN_PATH);
const auto way_path = turn_path.AsViaWayPath();

if (std::find(way_path.via.begin(), way_path.via.end(), instruction_node) ==
way_path.via.end())
{
util::Log(logDEBUG) << "Maneuver via-way path does not contain instruction node "
<< instruction_node;
return false;
}
}
return true;
}

// Generate sequence of node-based-node turns. Used to identify the maneuver's edge-based-node
// turns during graph expansion.
std::vector<NodeBasedTurn> Turns() const
{
if (turn_path.Type() == TurnPathType::VIA_NODE_TURN_PATH)
{
const auto node_maneuver = turn_path.AsViaNodePath();
return {{node_maneuver.from, node_maneuver.via, node_maneuver.to}};
}

BOOST_ASSERT(turn_path.Type() == TurnPathType::VIA_WAY_TURN_PATH);
std::vector<NodeBasedTurn> result;
const auto way_maneuver = turn_path.AsViaWayPath();
BOOST_ASSERT(way_maneuver.via.size() >= 2);
result.push_back({way_maneuver.from, way_maneuver.via[0], way_maneuver.via[1]});

for (auto i : util::irange<size_t>(0, way_maneuver.via.size() - 2))
{
result.push_back(
{way_maneuver.via[i], way_maneuver.via[i + 1], way_maneuver.via[i + 2]});
}
result.push_back({way_maneuver.via[way_maneuver.via.size() - 2],
way_maneuver.via.back(),
way_maneuver.to});

return result;
}

static std::string Name() { return "maneuver override"; };
};
} // namespace extractor
} // namespace osrm
Expand Down
Loading

0 comments on commit ed78056

Please sign in to comment.