From 2b6bdaf727872c835d2027e1dd67de5b1a3ab689 Mon Sep 17 00:00:00 2001 From: karenzshea Date: Thu, 7 Dec 2017 12:49:31 +0100 Subject: [PATCH] add assert for untested sliproad cases, removed redundant empty_nameid checks --- include/engine/guidance/collapse_turns.hpp | 1 - .../guidance/intersection_handler.cpp | 4 +- .../guidance/mergable_road_detector.cpp | 6 - src/extractor/guidance/motorway_handler.cpp | 191 +++++++++--------- src/extractor/guidance/roundabout_handler.cpp | 21 +- src/extractor/guidance/sliproad_handler.cpp | 12 +- src/extractor/guidance/turn_handler.cpp | 3 +- 7 files changed, 107 insertions(+), 131 deletions(-) diff --git a/include/engine/guidance/collapse_turns.hpp b/include/engine/guidance/collapse_turns.hpp index 7a15eb0d2..bcdca07d7 100644 --- a/include/engine/guidance/collapse_turns.hpp +++ b/include/engine/guidance/collapse_turns.hpp @@ -1,6 +1,5 @@ #ifndef OSRM_ENGINE_GUIDANCE_COLLAPSE_HPP -#include "engine/datafacade/datafacade_base.hpp" #include "engine/guidance/route_step.hpp" #include "util/attributes.hpp" diff --git a/src/extractor/guidance/intersection_handler.cpp b/src/extractor/guidance/intersection_handler.cpp index 865bcebab..756ee3217 100644 --- a/src/extractor/guidance/intersection_handler.cpp +++ b/src/extractor/guidance/intersection_handler.cpp @@ -490,8 +490,8 @@ bool IntersectionHandler::isSameName(const EdgeID source_edge_id, const EdgeID t const auto &target_edge_data = node_data_container.GetAnnotation( node_based_graph.GetEdgeData(target_edge_id).annotation_data); - return source_edge_data.name_id != EMPTY_NAMEID && // - target_edge_data.name_id != EMPTY_NAMEID && // + return !name_table.GetNameForID(source_edge_data.name_id).empty() && // + !name_table.GetNameForID(target_edge_data.name_id).empty() && // !util::guidance::requiresNameAnnounced(source_edge_data.name_id, target_edge_data.name_id, name_table, diff --git a/src/extractor/guidance/mergable_road_detector.cpp b/src/extractor/guidance/mergable_road_detector.cpp index b81820d3d..4773d631d 100644 --- a/src/extractor/guidance/mergable_road_detector.cpp +++ b/src/extractor/guidance/mergable_road_detector.cpp @@ -38,8 +38,6 @@ inline auto makeCheckRoadForName(const NameID name_id, node_data_container .GetAnnotation(node_based_graph.GetEdgeData(road.eid).annotation_data) .name_id; - if (name_id == EMPTY_NAMEID || road_name_id == EMPTY_NAMEID) - return true; const auto road_name_empty = name_table.GetNameForID(road_name_id).empty(); const auto in_name_empty = name_table.GetNameForID(name_id).empty(); if (in_name_empty || road_name_empty) @@ -470,16 +468,12 @@ bool MergableRoadDetector::IsTrafficIsland(const NodeID intersection_node, node_data_container .GetAnnotation(node_based_graph.GetEdgeData(range.front()).annotation_data) .name_id; - if (required_name_id == EMPTY_NAMEID) - return false; const auto has_required_name = [this, required_name_id](const auto edge_id) { const auto road_name_id = node_data_container .GetAnnotation(node_based_graph.GetEdgeData(edge_id).annotation_data) .name_id; - if (road_name_id == EMPTY_NAMEID) - return false; const auto &road_name_empty = name_table.GetNameForID(road_name_id).empty(); const auto &required_name_empty = name_table.GetNameForID(required_name_id).empty(); if (required_name_empty && road_name_empty) diff --git a/src/extractor/guidance/motorway_handler.cpp b/src/extractor/guidance/motorway_handler.cpp index 36a41f2ce..4689286f1 100644 --- a/src/extractor/guidance/motorway_handler.cpp +++ b/src/extractor/guidance/motorway_handler.cpp @@ -361,120 +361,111 @@ Intersection MotorwayHandler::fromRamp(const EdgeID via_eid, Intersection inters node_based_graph.GetEdgeData(intersection[2].eid).annotation_data); const auto &first_intersection_data = node_data_container.GetAnnotation( node_based_graph.GetEdgeData(intersection[1].eid).annotation_data); - if (first_intersection_data.name_id == EMPTY_NAMEID || - second_intersection_data.name_id == EMPTY_NAMEID) - { - return fallback(std::move(intersection)); - } - else - { - const auto first_second_same_name = - !util::guidance::requiresNameAnnounced(second_intersection_data.name_id, - first_intersection_data.name_id, - name_table, - street_name_suffix_table); + const auto first_second_same_name = + !util::guidance::requiresNameAnnounced(second_intersection_data.name_id, + first_intersection_data.name_id, + name_table, + street_name_suffix_table); - // merging onto a passing highway / or two ramps merging onto the same highway - if (num_valid_turns == 1) + // merging onto a passing highway / or two ramps merging onto the same highway + if (num_valid_turns == 1) + { + BOOST_ASSERT(!intersection[0].entry_allowed); + // check order of highways + // 4 + // 5 3 + // + // 6 2 + // + // 7 1 + // 0 + const auto &first_intersection_name_empty = + name_table.GetNameForID(first_intersection_data.name_id).empty(); + const auto &second_intersection_name_empty = + name_table.GetNameForID(second_intersection_data.name_id).empty(); + if (intersection[1].entry_allowed) { - BOOST_ASSERT(!intersection[0].entry_allowed); - // check order of highways - // 4 - // 5 3 - // - // 6 2 - // - // 7 1 - // 0 - const auto &first_intersection_name_empty = - name_table.GetNameForID(first_intersection_data.name_id).empty(); - const auto &second_intersection_name_empty = - name_table.GetNameForID(second_intersection_data.name_id).empty(); - if (intersection[1].entry_allowed) + if (isMotorwayClass(intersection[1].eid, node_based_graph) && + !second_intersection_name_empty && !first_intersection_name_empty && + first_second_same_name) { - if (isMotorwayClass(intersection[1].eid, node_based_graph) && - !second_intersection_name_empty && !first_intersection_name_empty && - first_second_same_name) - { - // circular order indicates a merge to the left (0-3 onto 4 - if (angularDeviation(intersection[1].angle, STRAIGHT_ANGLE) < - 2 * NARROW_TURN_ANGLE) - intersection[1].instruction = {TurnType::Merge, - DirectionModifier::SlightLeft}; - else // fallback - intersection[1].instruction = {TurnType::Merge, - getTurnDirection(intersection[1].angle)}; - } - else // passing by the end of a motorway - { - intersection[1].instruction = - getInstructionForObvious(intersection.size(), - via_eid, - isThroughStreet(1, intersection), - intersection[1]); - } + // circular order indicates a merge to the left (0-3 onto 4 + if (angularDeviation(intersection[1].angle, STRAIGHT_ANGLE) < + 2 * NARROW_TURN_ANGLE) + intersection[1].instruction = {TurnType::Merge, + DirectionModifier::SlightLeft}; + else // fallback + intersection[1].instruction = {TurnType::Merge, + getTurnDirection(intersection[1].angle)}; } - else + else // passing by the end of a motorway { - BOOST_ASSERT(intersection[2].entry_allowed); - if (isMotorwayClass(intersection[2].eid, node_based_graph) && - !second_intersection_name_empty && !first_intersection_name_empty && - first_second_same_name) - { - // circular order (5-0) onto 4 - if (angularDeviation(intersection[2].angle, STRAIGHT_ANGLE) < - 2 * NARROW_TURN_ANGLE) - intersection[2].instruction = {TurnType::Merge, - DirectionModifier::SlightRight}; - else // fallback - intersection[2].instruction = {TurnType::Merge, - getTurnDirection(intersection[2].angle)}; - } - else // passing the end of a highway - { - intersection[2].instruction = - getInstructionForObvious(intersection.size(), - via_eid, - isThroughStreet(2, intersection), - intersection[2]); - } + intersection[1].instruction = + getInstructionForObvious(intersection.size(), + via_eid, + isThroughStreet(1, intersection), + intersection[1]); } } else { - BOOST_ASSERT(num_valid_turns == 2); - // UTurn on ramps is not possible - BOOST_ASSERT(!intersection[0].entry_allowed); - BOOST_ASSERT(intersection[1].entry_allowed); BOOST_ASSERT(intersection[2].entry_allowed); - // two motorways starting at end of ramp (fork) - // M M - // \ / - // | - // R - if (isMotorwayClass(intersection[1].eid, node_based_graph) && - isMotorwayClass(intersection[2].eid, node_based_graph)) + if (isMotorwayClass(intersection[2].eid, node_based_graph) && + !second_intersection_name_empty && !first_intersection_name_empty && + first_second_same_name) { - assignFork(via_eid, intersection[2], intersection[1]); + // circular order (5-0) onto 4 + if (angularDeviation(intersection[2].angle, STRAIGHT_ANGLE) < + 2 * NARROW_TURN_ANGLE) + intersection[2].instruction = {TurnType::Merge, + DirectionModifier::SlightRight}; + else // fallback + intersection[2].instruction = {TurnType::Merge, + getTurnDirection(intersection[2].angle)}; + } + else // passing the end of a highway + { + intersection[2].instruction = + getInstructionForObvious(intersection.size(), + via_eid, + isThroughStreet(2, intersection), + intersection[2]); + } + } + } + else + { + BOOST_ASSERT(num_valid_turns == 2); + // UTurn on ramps is not possible + BOOST_ASSERT(!intersection[0].entry_allowed); + BOOST_ASSERT(intersection[1].entry_allowed); + BOOST_ASSERT(intersection[2].entry_allowed); + // two motorways starting at end of ramp (fork) + // M M + // \ / + // | + // R + if (isMotorwayClass(intersection[1].eid, node_based_graph) && + isMotorwayClass(intersection[2].eid, node_based_graph)) + { + assignFork(via_eid, intersection[2], intersection[1]); + } + else + { + // continued ramp passing motorway entry + // M R + // M R + // | / + // R + if (isMotorwayClass(intersection[1].eid, node_based_graph)) + { + intersection[1].instruction = {TurnType::Turn, DirectionModifier::SlightRight}; + intersection[2].instruction = {TurnType::Continue, + DirectionModifier::SlightLeft}; } else { - // continued ramp passing motorway entry - // M R - // M R - // | / - // R - if (isMotorwayClass(intersection[1].eid, node_based_graph)) - { - intersection[1].instruction = {TurnType::Turn, - DirectionModifier::SlightRight}; - intersection[2].instruction = {TurnType::Continue, - DirectionModifier::SlightLeft}; - } - else - { - assignFork(via_eid, intersection[2], intersection[1]); - } + assignFork(via_eid, intersection[2], intersection[1]); } } } diff --git a/src/extractor/guidance/roundabout_handler.cpp b/src/extractor/guidance/roundabout_handler.cpp index 101809baf..4bea638fe 100644 --- a/src/extractor/guidance/roundabout_handler.cpp +++ b/src/extractor/guidance/roundabout_handler.cpp @@ -295,22 +295,17 @@ RoundaboutType RoundaboutHandler::getRoundaboutType(const NodeID nid) const return SPECIAL_EDGEID; } - if (edge_data.name_id != EMPTY_NAMEID) + const auto &edge_name_empty = name_table.GetNameForID(edge_data.name_id).empty(); + if (!edge_name_empty) { - const auto &edge_name_empty = - name_table.GetNameForID(edge_data.name_id).empty(); - if (!edge_name_empty) - { - const auto announce = [&](unsigned id) { - return util::guidance::requiresNameAnnounced( - id, edge_data.name_id, name_table, street_name_suffix_table); - }; + const auto announce = [&](unsigned id) { + return util::guidance::requiresNameAnnounced( + id, edge_data.name_id, name_table, street_name_suffix_table); + }; - if (std::all_of( - begin(roundabout_name_ids), end(roundabout_name_ids), announce)) - roundabout_name_ids.insert(edge_data.name_id); - } + if (std::all_of(begin(roundabout_name_ids), end(roundabout_name_ids), announce)) + roundabout_name_ids.insert(edge_data.name_id); } continue_edge = edge_id; } diff --git a/src/extractor/guidance/sliproad_handler.cpp b/src/extractor/guidance/sliproad_handler.cpp index f85ed62c2..96fa4cfde 100644 --- a/src/extractor/guidance/sliproad_handler.cpp +++ b/src/extractor/guidance/sliproad_handler.cpp @@ -1,5 +1,6 @@ #include "extractor/guidance/sliproad_handler.hpp" #include "extractor/guidance/constants.hpp" +#include "util/assert.hpp" #include "util/bearing.hpp" #include "util/coordinate_calculation.hpp" #include "util/guidance/name_announcements.hpp" @@ -474,8 +475,7 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter // Name mismatch: check roads at `c` and `d` for same name const auto name_mismatch = [&](const NameID road_name_id) { - const auto unnamed = - road_name_id == EMPTY_NAMEID || name_table.GetNameForID(road_name_id).empty(); + const auto unnamed = name_table.GetNameForID(road_name_id).empty(); return unnamed || util::guidance::requiresNameAnnounced(road_name_id, // @@ -500,15 +500,12 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter node_data_container .GetAnnotation(node_based_graph.GetEdgeData(main_road.eid).annotation_data) .name_id; - const auto main_road_name_empty = main_road_name_id == EMPTY_NAMEID || - name_table.GetNameForID(main_road_name_id).empty(); + const auto main_road_name_empty = name_table.GetNameForID(main_road_name_id).empty(); const auto &sliproad_annotation = node_data_container.GetAnnotation(sliproad_edge_data.annotation_data); const auto sliproad_name_empty = - sliproad_annotation.name_id == EMPTY_NAMEID || name_table.GetNameForID(sliproad_annotation.name_id).empty(); const auto candidate_road_name_empty = - candidate_data.name_id == EMPTY_NAMEID || name_table.GetNameForID(candidate_data.name_id).empty(); if (!sliproad_edge_data.flags.road_classification.IsLinkClass() && !sliproad_name_empty && !main_road_name_empty && !candidate_road_name_empty && @@ -583,8 +580,9 @@ operator()(const NodeID /*nid*/, const EdgeID source_edge_id, Intersection inter intersection[*obvious].instruction.direction_modifier = getTurnDirection(intersection[*obvious].angle); } - else if (main_annotation.name_id != EMPTY_NAMEID) + else if (!name_table.GetNameForID(main_annotation.name_id).empty()) { + OSRM_ASSERT(false, coordinates[intersection_node_id]); intersection[*obvious].instruction.type = TurnType::NewName; intersection[*obvious].instruction.direction_modifier = getTurnDirection(intersection[*obvious].angle); diff --git a/src/extractor/guidance/turn_handler.cpp b/src/extractor/guidance/turn_handler.cpp index 7f5b2d644..a4be7ede6 100644 --- a/src/extractor/guidance/turn_handler.cpp +++ b/src/extractor/guidance/turn_handler.cpp @@ -199,8 +199,7 @@ bool TurnHandler::isObviousOfTwo(const EdgeID via_edge, const bool turn_is_perfectly_straight = angularDeviation(road.angle, STRAIGHT_ANGLE) < std::numeric_limits::epsilon(); - const auto &via_name_empty = - via_data.name_id == EMPTY_NAMEID || name_table.GetNameForID(via_data.name_id).empty(); + const auto &via_name_empty = name_table.GetNameForID(via_data.name_id).empty(); if (!via_name_empty) { const auto same_name = !util::guidance::requiresNameAnnounced(