diff --git a/plugins/distance_table.hpp b/plugins/distance_table.hpp index 18dff21dc..c023a5da1 100644 --- a/plugins/distance_table.hpp +++ b/plugins/distance_table.hpp @@ -181,11 +181,11 @@ template class DistanceTablePlugin final : public BasePlugin BOOST_ASSERT((phantom_node_target_out_iter - phantom_node_target_vector.begin()) == number_of_destination); - snapPhantomNodes(phantom_node_source_vector); - snapPhantomNodes(phantom_node_target_vector); + // FIXME we should clear phantom_node_source_vector and phantom_node_target_vector after this + auto snapped_source_phantoms = snapPhantomNodes(phantom_node_source_vector); + auto snapped_target_phantoms = snapPhantomNodes(phantom_node_target_vector); - std::shared_ptr> result_table = search_engine_ptr->distance_table( - phantom_node_source_vector, phantom_node_target_vector); + auto result_table = search_engine_ptr->distance_table(snapped_source_phantoms, snapped_target_phantoms); if (!result_table) { @@ -203,23 +203,22 @@ template class DistanceTablePlugin final : public BasePlugin matrix_json_array.values.push_back(json_row); } json_result.values["distance_table"] = matrix_json_array; + osrm::json::Array target_coord_json_array; - for (const auto &pair : phantom_node_target_vector) + for (const auto &phantom : snapped_target_phantoms) { osrm::json::Array json_coord; - FixedPointCoordinate coord = pair.first.location; - json_coord.values.push_back(coord.lat / COORDINATE_PRECISION); - json_coord.values.push_back(coord.lon / COORDINATE_PRECISION); + json_coord.values.push_back(phantom.location.lat / COORDINATE_PRECISION); + json_coord.values.push_back(phantom.location.lon / COORDINATE_PRECISION); target_coord_json_array.values.push_back(json_coord); } json_result.values["destination_coordinates"] = target_coord_json_array; osrm::json::Array source_coord_json_array; - for (const auto &pair : phantom_node_source_vector) + for (const auto &phantom : snapped_source_phantoms) { osrm::json::Array json_coord; - FixedPointCoordinate coord = pair.first.location; - json_coord.values.push_back(coord.lat / COORDINATE_PRECISION); - json_coord.values.push_back(coord.lon / COORDINATE_PRECISION); + json_coord.values.push_back(phantom.location.lat / COORDINATE_PRECISION); + json_coord.values.push_back(phantom.location.lon / COORDINATE_PRECISION); source_coord_json_array.values.push_back(json_coord); } json_result.values["source_coordinates"] = source_coord_json_array; diff --git a/plugins/plugin_base.hpp b/plugins/plugin_base.hpp index fe8d2fba4..babd4d07c 100644 --- a/plugins/plugin_base.hpp +++ b/plugins/plugin_base.hpp @@ -46,14 +46,14 @@ class BasePlugin virtual ~BasePlugin() {} virtual const std::string GetDescriptor() const = 0; virtual int HandleRequest(const RouteParameters &, osrm::json::Object &) = 0; - virtual bool - check_all_coordinates(const std::vector &coordinates, const unsigned min = 2) const final + virtual bool check_all_coordinates(const std::vector &coordinates, + const unsigned min = 2) const final { if (min > coordinates.size() || std::any_of(std::begin(coordinates), std::end(coordinates), - [](const FixedPointCoordinate &coordinate) - { - return !coordinate.is_valid(); - })) + [](const FixedPointCoordinate &coordinate) + { + return !coordinate.is_valid(); + })) { return false; } @@ -62,16 +62,18 @@ class BasePlugin // Decides whether to use the phantom node from a big or small component if both are found. // Returns true if all phantom nodes are in the same component after snapping. - bool snapPhantomNodes(std::vector> &phantom_node_pair_list) const + std::vector snapPhantomNodes( + const std::vector> &phantom_node_pair_list) const { - const auto check_component_id_is_tiny = [](const std::pair &phantom_pair) + const auto check_component_id_is_tiny = + [](const std::pair &phantom_pair) { return phantom_pair.first.component.is_tiny; }; - // are all phantoms from a tiny cc? - const auto check_all_in_same_component = [](const std::vector> &nodes) + const auto check_all_in_same_component = + [](const std::vector> &nodes) { const auto component_id = nodes.front().first.component.id; @@ -82,13 +84,20 @@ class BasePlugin }); }; - const auto swap_phantom_from_big_cc_into_front = [](std::pair &phantom_pair) + const auto fallback_to_big_component = + [](const std::pair &phantom_pair) { - if (phantom_pair.first.component.is_tiny && phantom_pair.second.is_valid() && !phantom_pair.second.component.is_tiny) + if (phantom_pair.first.component.is_tiny && phantom_pair.second.is_valid() && + !phantom_pair.second.component.is_tiny) { - using std::swap; - swap(phantom_pair.first, phantom_pair.second); + return phantom_pair.second; } + return phantom_pair.first; + }; + + const auto use_closed_phantom = [](const std::pair &phantom_pair) + { + return phantom_pair.first; }; const bool every_phantom_is_in_tiny_cc = @@ -96,17 +105,23 @@ class BasePlugin check_component_id_is_tiny); auto all_in_same_component = check_all_in_same_component(phantom_node_pair_list); - // The only case we don't snap to the big component if all phantoms are in the same small component - if (!every_phantom_is_in_tiny_cc || !all_in_same_component) - { - std::for_each(std::begin(phantom_node_pair_list), std::end(phantom_node_pair_list), - swap_phantom_from_big_cc_into_front); + std::vector snapped_phantoms; + snapped_phantoms.reserve(phantom_node_pair_list.size()); - // update check with new component ids - all_in_same_component = check_all_in_same_component(phantom_node_pair_list); + // The only case we don't snap to the big component if all phantoms are in the same small + // component + if (every_phantom_is_in_tiny_cc && all_in_same_component) + { + std::transform(phantom_node_pair_list.begin(), phantom_node_pair_list.end(), + std::back_inserter(snapped_phantoms), use_closed_phantom); + } + else + { + std::transform(phantom_node_pair_list.begin(), phantom_node_pair_list.end(), + std::back_inserter(snapped_phantoms), fallback_to_big_component); } - return all_in_same_component; + return snapped_phantoms; } }; diff --git a/plugins/trip.hpp b/plugins/trip.hpp index de9c7f682..12ec631bd 100644 --- a/plugins/trip.hpp +++ b/plugins/trip.hpp @@ -72,12 +72,14 @@ template class RoundTripPlugin final : public BasePlugin const std::string GetDescriptor() const override final { return descriptor_string; } - void GetPhantomNodes(const RouteParameters &route_parameters, - std::vector &phantom_node_pair_list) + std::vector GetPhantomNodes(const RouteParameters &route_parameters) { const bool checksum_OK = (route_parameters.check_sum == facade->GetCheckSum()); const auto &input_bearings = route_parameters.bearings; + std::vector phantom_node_list; + phantom_node_list.reserve(route_parameters.coordinates.size()); + // find phantom nodes for all input coords for (const auto i : osrm::irange(0, route_parameters.coordinates.size())) { @@ -89,17 +91,17 @@ template class RoundTripPlugin final : public BasePlugin ObjectEncoder::DecodeFromBase64(route_parameters.hints[i], current_phantom_node); if (current_phantom_node.is_valid(facade->GetNumberOfNodes())) { - phantom_node_pair_list[i] = std::make_pair(current_phantom_node, current_phantom_node); + phantom_node_list.push_back(std::move(current_phantom_node)); continue; } } const int bearing = input_bearings.size() > 0 ? input_bearings[i].first : 0; const int range = input_bearings.size() > 0 ? (input_bearings[i].second?*input_bearings[i].second:10) : 180; - auto phantom_nodes = facade->NearestPhantomNodes(route_parameters.coordinates[i], 1, bearing, range); - // FIXME we only use the pair because that is what DistanceTable expects - phantom_node_pair_list[i] = std::make_pair(phantom_nodes.front().phantom_node, phantom_nodes.front().phantom_node); - BOOST_ASSERT(phantom_node_pair_list[i].first.is_valid(facade->GetNumberOfNodes())); + phantom_node_list.push_back(facade->NearestPhantomNodes(route_parameters.coordinates[i], 1, bearing, range).front().phantom_node); + BOOST_ASSERT(phantom_node_list.back().is_valid(facade->GetNumberOfNodes())); } + + return phantom_node_list; } // Object to hold all strongly connected components (scc) of a graph @@ -203,7 +205,7 @@ template class RoundTripPlugin final : public BasePlugin json_result.values["permutation"] = json_permutation; } - InternalRouteResult ComputeRoute(const std::vector &phantom_node_pair_list, + InternalRouteResult ComputeRoute(const std::vector &phantom_node_list, const RouteParameters &route_parameters, const std::vector &trip) { @@ -220,7 +222,7 @@ template class RoundTripPlugin final : public BasePlugin const auto to_node = std::next(it) != end ? *std::next(it) : *start; viapoint = - PhantomNodes{phantom_node_pair_list[from_node].first, phantom_node_pair_list[to_node].first}; + PhantomNodes{phantom_node_list[from_node], phantom_node_list[to_node]}; min_route.segment_end_coordinates.emplace_back(viapoint); } BOOST_ASSERT(min_route.segment_end_coordinates.size() == trip.size()); @@ -243,24 +245,24 @@ template class RoundTripPlugin final : public BasePlugin // check if all inputs are coordinates if (!check_all_coordinates(route_parameters.coordinates)) { + json_result.values["status_message"] = "Invalid coordinates."; return 400; } const auto &input_bearings = route_parameters.bearings; if (input_bearings.size() > 0 && route_parameters.coordinates.size() != input_bearings.size()) { - json_result.values["status"] = "Number of bearings does not match number of coordinates ."; + json_result.values["status_message"] = "Number of bearings does not match number of coordinates."; return 400; } // get phantom nodes - std::vector phantom_node_pair_list(route_parameters.coordinates.size()); - GetPhantomNodes(route_parameters, phantom_node_pair_list); - const auto number_of_locations = phantom_node_pair_list.size(); + auto phantom_node_list = GetPhantomNodes(route_parameters); + const auto number_of_locations = phantom_node_list.size(); // compute the distance table of all phantom nodes const auto result_table = DistTableWrapper( - *search_engine_ptr->distance_table(phantom_node_pair_list, phantom_node_pair_list), number_of_locations); + *search_engine_ptr->distance_table(phantom_node_list, phantom_node_list), number_of_locations); if (result_table.size() == 0) { @@ -328,7 +330,7 @@ template class RoundTripPlugin final : public BasePlugin comp_route.reserve(route_result.size()); for (auto &elem : route_result) { - comp_route.push_back(ComputeRoute(phantom_node_pair_list, route_parameters, elem)); + comp_route.push_back(ComputeRoute(phantom_node_list, route_parameters, elem)); } TIMER_STOP(TRIP_TIMER); diff --git a/plugins/viaroute.hpp b/plugins/viaroute.hpp index 4ef80d916..b9f8a4436 100644 --- a/plugins/viaroute.hpp +++ b/plugins/viaroute.hpp @@ -77,13 +77,16 @@ template class ViaRoutePlugin final : public BasePlugin { if (!check_all_coordinates(route_parameters.coordinates)) { + json_result.values["status_message"] = "Invalid coordinates."; return 400; } const auto &input_bearings = route_parameters.bearings; - if (input_bearings.size() > 0 && route_parameters.coordinates.size() != input_bearings.size()) + if (input_bearings.size() > 0 && + route_parameters.coordinates.size() != input_bearings.size()) { - json_result.values["status"] = "Number of bearings does not match number of coordinates ."; + json_result.values["status_message"] = + "Number of bearings does not match number of coordinates."; return 400; } @@ -103,28 +106,32 @@ template class ViaRoutePlugin final : public BasePlugin } } const int bearing = input_bearings.size() > 0 ? input_bearings[i].first : 0; - const int range = input_bearings.size() > 0 ? (input_bearings[i].second?*input_bearings[i].second:10) : 180; - phantom_node_pair_list[i] = facade->NearestPhantomNodeWithAlternativeFromBigComponent(route_parameters.coordinates[i], bearing, range); + const int range = input_bearings.size() > 0 + ? (input_bearings[i].second ? *input_bearings[i].second : 10) + : 180; + phantom_node_pair_list[i] = facade->NearestPhantomNodeWithAlternativeFromBigComponent( + route_parameters.coordinates[i], bearing, range); // we didn't found a fitting node, return error if (!phantom_node_pair_list[i].first.is_valid(facade->GetNumberOfNodes())) { - json_result.values["status_message"] = std::string("Could not find a matching segment for coordinate ") + std::to_string(i); + json_result.values["status_message"] = + std::string("Could not find a matching segment for coordinate ") + + std::to_string(i); return 400; } BOOST_ASSERT(phantom_node_pair_list[i].first.is_valid(facade->GetNumberOfNodes())); BOOST_ASSERT(phantom_node_pair_list[i].second.is_valid(facade->GetNumberOfNodes())); } - auto all_in_same_component = snapPhantomNodes(phantom_node_pair_list); + auto snapped_phantoms = snapPhantomNodes(phantom_node_pair_list); InternalRouteResult raw_route; - auto build_phantom_pairs = - [&raw_route](const PhantomNodePair &first_pair, const PhantomNodePair &second_pair) + auto build_phantom_pairs = [&raw_route](const PhantomNode &first_node, + const PhantomNode &second_node) { - raw_route.segment_end_coordinates.emplace_back( - PhantomNodes{first_pair.first, second_pair.first}); + raw_route.segment_end_coordinates.push_back(PhantomNodes{first_node, second_node}); }; - osrm::for_each_pair(phantom_node_pair_list, build_phantom_pairs); + osrm::for_each_pair(snapped_phantoms, build_phantom_pairs); if (1 == raw_route.segment_end_coordinates.size()) { @@ -171,10 +178,20 @@ template class ViaRoutePlugin final : public BasePlugin // we can only know this after the fact, different SCC ids still // allow for connection in one direction. - if (!all_in_same_component && no_route) + if (no_route) { - json_result.values["status_message"] = "Impossible route between points."; - return 400; + auto first_component_id = snapped_phantoms.front().component.id; + auto not_in_same_component = + std::any_of(snapped_phantoms.begin(), snapped_phantoms.end(), + [first_component_id](const PhantomNode &node) + { + return node.component.id != first_component_id; + }); + if (not_in_same_component) + { + json_result.values["status_message"] = "Impossible route between points."; + return 400; + } } return 200; diff --git a/routing_algorithms/many_to_many.hpp b/routing_algorithms/many_to_many.hpp index 192e140e1..c5dfb7ca5 100644 --- a/routing_algorithms/many_to_many.hpp +++ b/routing_algorithms/many_to_many.hpp @@ -67,7 +67,7 @@ class ManyToManyRouting final ~ManyToManyRouting() {} std::shared_ptr> - operator()(const std::vector &phantom_sources_array, const std::vector &phantom_targets_array) const + operator()(const std::vector &phantom_sources_array, const std::vector &phantom_targets_array) const { const auto number_of_sources = phantom_sources_array.size(); const auto number_of_targets = phantom_targets_array.size(); @@ -83,35 +83,22 @@ class ManyToManyRouting final SearchSpaceWithBuckets search_space_with_buckets; unsigned target_id = 0; - for (const auto &pair : phantom_targets_array) + for (const auto &phantom : phantom_targets_array) { query_heap.Clear(); // insert target(s) at distance 0 - if (SPECIAL_NODEID != pair.first.forward_node_id) + if (SPECIAL_NODEID != phantom.forward_node_id) { - query_heap.Insert(pair.first.forward_node_id, - pair.first.GetForwardWeightPlusOffset(), - pair.first.forward_node_id); + query_heap.Insert(phantom.forward_node_id, + phantom.GetForwardWeightPlusOffset(), + phantom.forward_node_id); } - if (SPECIAL_NODEID != pair.first.reverse_node_id) + if (SPECIAL_NODEID != phantom.reverse_node_id) { - query_heap.Insert(pair.first.reverse_node_id, - pair.first.GetReverseWeightPlusOffset(), - pair.first.reverse_node_id); - } - - if (SPECIAL_NODEID != pair.second.forward_node_id) - { - query_heap.Insert(pair.second.forward_node_id, - pair.second.GetForwardWeightPlusOffset(), - pair.second.forward_node_id); - } - if (SPECIAL_NODEID != pair.second.reverse_node_id) - { - query_heap.Insert(pair.second.reverse_node_id, - pair.second.GetReverseWeightPlusOffset(), - pair.second.reverse_node_id); + query_heap.Insert(phantom.reverse_node_id, + phantom.GetReverseWeightPlusOffset(), + phantom.reverse_node_id); } // explore search space @@ -124,35 +111,22 @@ class ManyToManyRouting final // for each source do forward search unsigned source_id = 0; - for (const auto &pair : phantom_sources_array) + for (const auto &phantom : phantom_sources_array) { query_heap.Clear(); // insert target(s) at distance 0 - if (SPECIAL_NODEID != pair.first.forward_node_id) + if (SPECIAL_NODEID != phantom.forward_node_id) { - query_heap.Insert(pair.first.forward_node_id, - -pair.first.GetForwardWeightPlusOffset(), - pair.first.forward_node_id); + query_heap.Insert(phantom.forward_node_id, + -phantom.GetForwardWeightPlusOffset(), + phantom.forward_node_id); } - if (SPECIAL_NODEID != pair.first.reverse_node_id) + if (SPECIAL_NODEID != phantom.reverse_node_id) { - query_heap.Insert(pair.first.reverse_node_id, - -pair.first.GetReverseWeightPlusOffset(), - pair.first.reverse_node_id); - } - - if (SPECIAL_NODEID != pair.second.forward_node_id) - { - query_heap.Insert(pair.second.forward_node_id, - -pair.second.GetForwardWeightPlusOffset(), - pair.second.forward_node_id); - } - if (SPECIAL_NODEID != pair.second.reverse_node_id) - { - query_heap.Insert(pair.second.reverse_node_id, - -pair.second.GetReverseWeightPlusOffset(), - pair.second.reverse_node_id); + query_heap.Insert(phantom.reverse_node_id, + -phantom.GetReverseWeightPlusOffset(), + phantom.reverse_node_id); } // explore search space