From dddde4ddabb5dc9f2f67b5b6319aca0c20690460 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Sat, 27 Jun 2015 15:27:08 +0200 Subject: [PATCH 1/4] Fix backwards speed on oneway=-1 streets --- extractor/extractor_callbacks.cpp | 104 ++++++++++++++---------------- 1 file changed, 48 insertions(+), 56 deletions(-) diff --git a/extractor/extractor_callbacks.cpp b/extractor/extractor_callbacks.cpp index bab309d1d..ed9087adb 100644 --- a/extractor/extractor_callbacks.cpp +++ b/extractor/extractor_callbacks.cpp @@ -167,76 +167,68 @@ void ExtractorCallbacks::ProcessWay(const osmium::Way &input_way, const Extracti ((parsed_way.forward_speed != parsed_way.backward_speed) || (parsed_way.forward_travel_mode != parsed_way.backward_travel_mode)); - // lambda to add edge to container - auto pair_wise_segment_split_forward = - [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) - { - const bool forward_only = split_edge || TRAVEL_MODE_INACCESSIBLE == parsed_way.backward_travel_mode; - external_memory.all_edges_list.push_back(InternalExtractorEdge( - first_node.ref(), last_node.ref(), name_id, forward_weight_data, - true, !forward_only, parsed_way.roundabout, parsed_way.ignore_in_grid, - parsed_way.is_access_restricted, parsed_way.forward_travel_mode, split_edge)); - external_memory.used_node_id_list.push_back(first_node.ref()); - }; + std::transform(input_way.nodes().begin(), input_way.nodes().end(), + std::back_inserter(external_memory.used_node_id_list), + [](const osmium::NodeRef& ref) { return ref.ref(); }); const bool is_opposite_way = TRAVEL_MODE_INACCESSIBLE == parsed_way.forward_travel_mode; + // traverse way in reverse in this case if (is_opposite_way) { - // why don't we have to swap the parsed_way.forward/backward speed here as well - const_cast(parsed_way).forward_travel_mode = - parsed_way.backward_travel_mode; - const_cast(parsed_way).backward_travel_mode = TRAVEL_MODE_INACCESSIBLE; + BOOST_ASSERT(split_edge == false); + BOOST_ASSERT(parsed_way.backward_travel_mode != TRAVEL_MODE_INACCESSIBLE); osrm::for_each_pair(input_way.nodes().crbegin(), input_way.nodes().crend(), - pair_wise_segment_split_forward); - external_memory.used_node_id_list.push_back(input_way.nodes().front().ref()); + [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) + { + external_memory.all_edges_list.push_back(InternalExtractorEdge( + first_node.ref(), last_node.ref(), name_id, backward_weight_data, + true, false, parsed_way.roundabout, parsed_way.ignore_in_grid, + parsed_way.is_access_restricted, parsed_way.backward_travel_mode, false)); + }); + + external_memory.way_start_end_id_list.push_back( + { + static_cast(input_way.id()), + static_cast(input_way.nodes().back().ref()), + static_cast(input_way.nodes()[input_way.nodes().size() - 2].ref()), + static_cast(input_way.nodes()[1].ref()), + static_cast(input_way.nodes()[0].ref()) + } + ); } else { + const bool forward_only = split_edge || TRAVEL_MODE_INACCESSIBLE == parsed_way.backward_travel_mode; osrm::for_each_pair(input_way.nodes().cbegin(), input_way.nodes().cend(), - pair_wise_segment_split_forward); - external_memory.used_node_id_list.push_back(input_way.nodes().back().ref()); - } - - // The following information is needed to identify start and end segments of restrictions - external_memory.way_start_end_id_list.push_back( - {(EdgeID)input_way.id(), - (NodeID)input_way.nodes()[0].ref(), - (NodeID)input_way.nodes()[1].ref(), - (NodeID)input_way.nodes()[input_way.nodes().size() - 2].ref(), - (NodeID)input_way.nodes().back().ref()}); - - if (split_edge) - { // Only true if the way should be split - BOOST_ASSERT(parsed_way.backward_travel_mode != TRAVEL_MODE_INACCESSIBLE); - auto pair_wise_segment_split_2 = - [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) - { - external_memory.all_edges_list.push_back(InternalExtractorEdge( - last_node.ref(), first_node.ref(), name_id, backward_weight_data, - true, false, parsed_way.roundabout, parsed_way.ignore_in_grid, - parsed_way.is_access_restricted, parsed_way.backward_travel_mode, split_edge)); - external_memory.used_node_id_list.push_back(last_node.ref()); - }; - - if (is_opposite_way) - { - osrm::for_each_pair(input_way.nodes().crbegin(), input_way.nodes().crend(), - pair_wise_segment_split_2); - external_memory.used_node_id_list.push_back(input_way.nodes().front().ref()); - } - else + [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) + { + external_memory.all_edges_list.push_back(InternalExtractorEdge( + first_node.ref(), last_node.ref(), name_id, forward_weight_data, + true, !forward_only, parsed_way.roundabout, parsed_way.ignore_in_grid, + parsed_way.is_access_restricted, parsed_way.forward_travel_mode, split_edge)); + }); + if (split_edge) { + BOOST_ASSERT(parsed_way.backward_travel_mode != TRAVEL_MODE_INACCESSIBLE); osrm::for_each_pair(input_way.nodes().cbegin(), input_way.nodes().cend(), - pair_wise_segment_split_2); - external_memory.used_node_id_list.push_back(input_way.nodes().front().ref()); + [&](const osmium::NodeRef &first_node, const osmium::NodeRef &last_node) + { + external_memory.all_edges_list.push_back(InternalExtractorEdge( + first_node.ref(), last_node.ref(), name_id, backward_weight_data, + false, true, parsed_way.roundabout, parsed_way.ignore_in_grid, + parsed_way.is_access_restricted, parsed_way.backward_travel_mode, true)); + }); } external_memory.way_start_end_id_list.push_back( - {(EdgeID)input_way.id(), - (NodeID)input_way.nodes()[1].ref(), - (NodeID)input_way.nodes()[0].ref(), - (NodeID)input_way.nodes().back().ref(), - (NodeID)input_way.nodes()[input_way.nodes().size() - 2].ref()}); + { + static_cast(input_way.id()), + static_cast(input_way.nodes().back().ref()), + static_cast(input_way.nodes()[input_way.nodes().size() - 2].ref()), + static_cast(input_way.nodes()[1].ref()), + static_cast(input_way.nodes()[0].ref()) + } + ); } } From 8a2652f53db1237085030fd4c517fe94b244d001 Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 26 Jun 2015 14:34:49 +0200 Subject: [PATCH 2/4] Only penaltize bidirectional ways if they have 1 lane --- features/car/maxspeed.feature | 8 ++++++++ profiles/car.lua | 6 ++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/features/car/maxspeed.feature b/features/car/maxspeed.feature index 84cfa4ff8..764de728a 100644 --- a/features/car/maxspeed.feature +++ b/features/car/maxspeed.feature @@ -112,3 +112,11 @@ OSRM will use 4/5 of the projected free-flow speed. | primary | 15 | | 30 | 60 | 34 km/h | 59 km/h | | primary | 15 | 1 | 30 | 60 | 15 km/h | 30 km/h | + Scenario: Car - Single lane streets only incure a penalty for two-way streets + Then routability should be + | highway | maxspeed | lanes | oneway | forw | backw | + | primary | 30 | 1 | yes | 34 km/h | | + | primary | 30 | 1 | -1 | | 34 km/h | + | primary | 30 | 1 | | 15 km/h | 15 km/h | + | primary | 30 | 2 | | 34 km/h | 34 km/h | + diff --git a/profiles/car.lua b/profiles/car.lua index 8c68bc994..6c2b5b7dd 100644 --- a/profiles/car.lua +++ b/profiles/car.lua @@ -412,11 +412,13 @@ function way_function (way, result) end end + local is_bidirectional = result.forward_mode ~= 0 and result.backward_mode ~= 0 + -- scale speeds to get better avg driving times if result.forward_speed > 0 then local scaled_speed = result.forward_speed*speed_reduction + 11; local penalized_speed = math.huge - if width <= 3 or lanes <= 1 then + if width <= 3 or (lanes <= 1 and is_bidirectional) then penalized_speed = result.forward_speed / 2; end result.forward_speed = math.min(penalized_speed, scaled_speed) @@ -425,7 +427,7 @@ function way_function (way, result) if result.backward_speed > 0 then local scaled_speed = result.backward_speed*speed_reduction + 11; local penalized_speed = math.huge - if width <= 3 or lanes <= 1 then + if width <= 3 or (lanes <= 1 and is_bidirectional) then penalized_speed = result.backward_speed / 2; end result.backward_speed = math.min(penalized_speed, scaled_speed) From f19c57200da0f9c79cdda5c2bab274df09be79ac Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Tue, 30 Jun 2015 00:22:40 +0200 Subject: [PATCH 3/4] Fix endless loop --- extractor/extraction_containers.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/extractor/extraction_containers.cpp b/extractor/extraction_containers.cpp index caca05ccb..c661ba2c8 100644 --- a/extractor/extraction_containers.cpp +++ b/extractor/extraction_containers.cpp @@ -320,6 +320,7 @@ void ExtractionContainers::PrepareEdges() // skip invalid edges if (all_edges_list[i].result.target == SPECIAL_NODEID) { + ++i; continue; } From 9958937fd1c1f9dd60126a56e1c4f25ceefaf70e Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Mon, 29 Jun 2015 22:01:06 +0200 Subject: [PATCH 4/4] At least check 4*LEAF_SIZE edges before returning none. --- data_structures/static_rtree.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/data_structures/static_rtree.hpp b/data_structures/static_rtree.hpp index fa5607198..8c05cf521 100644 --- a/data_structures/static_rtree.hpp +++ b/data_structures/static_rtree.hpp @@ -646,7 +646,8 @@ class StaticRTree const FixedPointCoordinate &input_coordinate, std::vector &result_phantom_node_vector, const unsigned max_number_of_phantom_nodes, - const float max_distance = 1100) + const float max_distance = 1100, + const unsigned max_checked_elements = 4 * LEAF_NODE_SIZE) { unsigned inspected_elements = 0; unsigned number_of_elements_from_big_cc = 0; @@ -663,7 +664,7 @@ class StaticRTree while (!traversal_queue.empty()) { const IncrementalQueryCandidate current_query_node = traversal_queue.top(); - if (current_query_node.min_dist > max_distance) + if (current_query_node.min_dist > max_distance && inspected_elements > max_checked_elements) { break; }