From 8cb081727dda229dc25742cb914c40bca68a1651 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 14 Oct 2016 10:48:00 +0300 Subject: [PATCH 01/12] Creating mixed router instead of osrm router. --- geometry/polyline2d.hpp | 12 + map/framework.cpp | 3 +- routing/base/followed_polyline.hpp | 12 + routing/osrm_router.cpp | 207 +++++++++--------- routing/osrm_router.hpp | 46 ++-- routing/road_graph_router.cpp | 12 + routing/road_graph_router.hpp | 2 + routing/route.cpp | 54 +++++ routing/route.hpp | 4 + .../routing_test_tools.cpp | 6 +- 10 files changed, 238 insertions(+), 120 deletions(-) diff --git a/geometry/polyline2d.hpp b/geometry/polyline2d.hpp index 681a4e811b..701f888192 100644 --- a/geometry/polyline2d.hpp +++ b/geometry/polyline2d.hpp @@ -67,6 +67,18 @@ public: void Clear() { m_points.clear(); } void Add(Point const & pt) { m_points.push_back(pt); } + + void Append(PolylineT const & poly) + { + m_points.insert(m_points.end(), poly.m_points.cbegin(), poly.m_points.cend()); + } + + void PopBack() + { + if (!m_points.empty()) + m_points.pop_back(); + } + void Swap(PolylineT & rhs) { m_points.swap(rhs.m_points); diff --git a/map/framework.cpp b/map/framework.cpp index 3276a0fa46..2ecddda15f 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2398,7 +2398,8 @@ void Framework::SetRouterImpl(RouterType type) return m_model.GetIndex().GetMwmIdByCountryFile(CountryFile(countryFile)).IsAlive(); }; - router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter)); + router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter, + CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); fetcher.reset(new OnlineAbsentCountriesFetcher(countryFileGetter, localFileChecker)); m_routingSession.SetRoutingSettings(routing::GetCarRoutingSettings()); } diff --git a/routing/base/followed_polyline.hpp b/routing/base/followed_polyline.hpp index ebf2a87d78..4ceaa5a4b2 100644 --- a/routing/base/followed_polyline.hpp +++ b/routing/base/followed_polyline.hpp @@ -20,6 +20,18 @@ public: void Swap(FollowedPolyline & rhs); + void Append(FollowedPolyline const & poly) + { + m_poly.Append(poly.m_poly); + Update(); + } + + void PopBack() + { + m_poly.PopBack(); + Update(); + } + bool IsValid() const { return (m_current.IsValid() && m_poly.GetSize() > 1); } m2::PolylineD const & GetPolyline() const { return m_poly; } diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 3ce50d4db3..31c8b87eaf 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -5,6 +5,7 @@ #include "routing/osrm_helpers.hpp" #include "routing/osrm_path_segment_factory.hpp" #include "routing/osrm_router.hpp" +#include "routing/road_graph_router.hpp" #include "routing/turns_generator.hpp" #include "platform/country_file.hpp" @@ -15,8 +16,10 @@ #include "geometry/distance_on_sphere.hpp" #include "geometry/mercator.hpp" +#include "indexer/feature_altitude.hpp" #include "indexer/ftypes_matcher.hpp" #include "indexer/index.hpp" +#include "indexer/mwm_set.hpp" #include "indexer/scales.hpp" #include "coding/reader_wrapper.hpp" @@ -205,8 +208,9 @@ bool OsrmRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD c manager.GetMappingByPoint(finalPoint)->IsValid(); } -OsrmRouter::OsrmRouter(Index * index, TCountryFileFn const & countryFileFn) - : m_pIndex(index), m_indexManager(countryFileFn, *index) +OsrmRouter::OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, + unique_ptr roadGraphRouter) + : m_pIndex(index), m_indexManager(countryFileFn, *index), m_roadGraphRouter(move(roadGraphRouter)) { } @@ -222,14 +226,17 @@ void OsrmRouter::ClearState() m_indexManager.Clear(); } -bool OsrmRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, - TFeatureGraphNodeVec const & target, TDataFacade & facade, - RawRoutingResult & rawRoutingResult) +bool OsrmRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) { + ASSERT(mapping, ()); + + Route emptyRoute(GetName()); + route.Swap(emptyRoute); /// @todo (ldargunov) make more complex nearest edge turnaround for (auto const & targetEdge : target) for (auto const & sourceEdge : source) - if (FindSingleRoute(sourceEdge, targetEdge, facade, rawRoutingResult)) + if (FindSingleRouteDispatcher(sourceEdge, targetEdge, delegate, mapping, route)) return true; return false; } @@ -279,7 +286,7 @@ void FindGraphNodeOffsets(uint32_t const nodeId, m2::PointD const & point, } void CalculatePhantomNodeForCross(TRoutingMappingPtr & mapping, FeatureGraphNode & graphNode, - Index const * pIndex, bool forward) + Index const * pIndex, bool forward) { if (graphNode.segment.IsValid()) return; @@ -301,85 +308,22 @@ OsrmRouter::ResultCode OsrmRouter::MakeRouteFromCrossesPath(TCheckedPath const & RouterDelegate const & delegate, Route & route) { - Route::TTurns turnsDir; - Route::TTimes times; - Route::TStreets streets; - vector points; + Route emptyRoute(GetName()); + route.Swap(emptyRoute); + for (RoutePathCross cross : path) { ASSERT_EQUAL(cross.startNode.mwmId, cross.finalNode.mwmId, ()); - RawRoutingResult routingResult; TRoutingMappingPtr mwmMapping = m_indexManager.GetMappingById(cross.startNode.mwmId); ASSERT(mwmMapping->IsValid(), ()); MappingGuard mwmMappingGuard(mwmMapping); UNUSED_VALUE(mwmMappingGuard); CalculatePhantomNodeForCross(mwmMapping, cross.startNode, m_pIndex, true /* forward */); CalculatePhantomNodeForCross(mwmMapping, cross.finalNode, m_pIndex, false /* forward */); - if (!FindSingleRoute(cross.startNode, cross.finalNode, mwmMapping->m_dataFacade, routingResult)) + if (!FindSingleRouteDispatcher(cross.startNode, cross.finalNode, delegate, mwmMapping, route)) return RouteNotFound; - - if (!points.empty()) - { - // Remove road end point and turn instruction. - points.pop_back(); - turnsDir.pop_back(); - times.pop_back(); - // Streets might not point to the last point of the path. - } - - // Get annotated route. - Route::TTurns mwmTurnsDir; - Route::TTimes mwmTimes; - Route::TStreets mwmStreets; - vector mwmJunctions; - - OSRMRoutingResult resultGraph(*m_pIndex, *mwmMapping, routingResult); - if (MakeTurnAnnotation(resultGraph, delegate, mwmJunctions, mwmTurnsDir, mwmTimes, mwmStreets) != NoError) - { - LOG(LWARNING, ("Can't load road path data from disk for", mwmMapping->GetCountryName())); - return RouteNotFound; - } - - vector mwmPoints; - JunctionsToPoints(mwmJunctions, mwmPoints); - - // Connect annotated route. - auto const pSize = static_cast(points.size()); - for (auto turn : mwmTurnsDir) - { - if (turn.m_index == 0) - continue; - turn.m_index += pSize; - turnsDir.push_back(turn); - } - - if (!mwmStreets.empty() && !streets.empty() && mwmStreets.front().second == streets.back().second) - mwmStreets.erase(mwmStreets.begin()); - for (auto street : mwmStreets) - { - if (street.first == 0) - continue; - street.first += pSize; - streets.push_back(street); - } - - double const estimationTime = times.size() ? times.back().second : 0.0; - for (auto time : mwmTimes) - { - if (time.first == 0) - continue; - time.first += pSize; - time.second += estimationTime; - times.push_back(time); - } - - points.insert(points.end(), mwmPoints.begin(), mwmPoints.end()); } - route.SetGeometry(points.begin(), points.end()); - route.SetTurnInstructions(move(turnsDir)); - route.SetSectionTimes(move(times)); - route.SetStreetNames(move(streets)); return NoError; } @@ -456,8 +400,7 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, delegate.OnProgress(kPointsFoundProgress); // 4. Find route. - RawRoutingResult routingResult; - double crossCost = 0; + double crossCostM = 0; TCheckedPath finalPath; // Manually load facade to avoid unmaping files we routing on. @@ -471,11 +414,10 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, { indexPair.second->FreeCrossContext(); }); - ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCost, + ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCostM, delegate, finalPath); LOG(LINFO, ("Found cross path in", timer.ElapsedNano(), "ns.")); - if (!FindRouteFromCases(startTask, m_cachedTargets, startMapping->m_dataFacade, - routingResult)) + if (!FindRouteFromCases(startTask, m_cachedTargets, delegate, startMapping, route)) { if (crossCode == NoError) { @@ -488,9 +430,9 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, } INTERRUPT_WHEN_CANCELLED(delegate); - if (crossCode == NoError && crossCost < routingResult.shortestPathLength) + if (crossCode == NoError && crossCostM < route.GetTotalDistanceMeters()) { - LOG(LINFO, ("Cross mwm path shorter. Cross cost:", crossCost, "single cost:", routingResult.shortestPathLength)); + LOG(LINFO, ("Cross mwm path shorter. Cross cost:", crossCostM, "single cost:", route.GetTotalDistanceMeters())); auto code = MakeRouteFromCrossesPath(finalPath, delegate, route); LOG(LINFO, ("Made final route in", timer.ElapsedNano(), "ns.")); timer.Reset(); @@ -500,34 +442,12 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, INTERRUPT_WHEN_CANCELLED(delegate); delegate.OnProgress(kPathFoundProgress); - // 5. Restore route. - - Route::TTurns turnsDir; - Route::TTimes times; - Route::TStreets streets; - vector junctions; - - OSRMRoutingResult resultGraph(*m_pIndex, *startMapping, routingResult); - if (MakeTurnAnnotation(resultGraph, delegate, junctions, turnsDir, times, streets) != NoError) - { - LOG(LWARNING, ("Can't load road path data from disk!")); - return RouteNotFound; - } - - vector points; - JunctionsToPoints(junctions, points); - - route.SetGeometry(points.begin(), points.end()); - route.SetTurnInstructions(move(turnsDir)); - route.SetSectionTimes(move(times)); - route.SetStreetNames(move(streets)); - return NoError; } else //4.2 Multiple mwm case { LOG(LINFO, ("Multiple mwm routing case")); - ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCost, + ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCostM, delegate, finalPath); timer.Reset(); INTERRUPT_WHEN_CANCELLED(delegate); @@ -569,4 +489,83 @@ IRouter::ResultCode OsrmRouter::FindPhantomNodes(m2::PointD const & point, getter.MakeResult(res, maxCount); return NoError; } + +bool OsrmRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) +{ + MwmSet::MwmHandle const handle = m_pIndex->GetMwmHandleById(mwmId); + if (!handle.IsAlive()) + { + ASSERT(false, ("m_mwmHandle is not alive.")); + return false; + } + + MwmValue const * value = handle.GetValue(); + ASSERT(value, ()); + if (value->GetHeader().GetFormat() < version::Format::v8) + return false; + + // @TODO Remove this string before merge. + string const EDGE_INDEX_FILE_TAG = "edgeidx"; + if (value->m_cont.IsExist(EDGE_INDEX_FILE_TAG.c_str())) + return true; + return false; +} + +bool OsrmRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, + Route & route) +{ + ASSERT_EQUAL(source.mwmId, target.mwmId, ()); + ASSERT(m_pIndex, ()); + ASSERT(m_roadGraphRouter, ()); + + Route mwmRoute(GetName()); + + // @TODO It's not the best place for checking availability of edge index section in mwm. + // Probably it's better to keep if mwm had edge index section in mwmId. + if (IsEdgeIndexExisting(source.mwmId) && m_roadGraphRouter) + { + LOG(LINFO, ("A* route form", MercatorBounds::ToLatLon(source.segmentPoint), + "to", MercatorBounds::ToLatLon(target.segmentPoint))); + + if (m_roadGraphRouter->CalculateRoute(source.segmentPoint, m2::PointD(0, 0), target.segmentPoint, + delegate, mwmRoute) != IRouter::NoError) + { + return false; + } + + } + else + { + vector mwmRouteGeometry; + Route::TTurns mwmTurns; + Route::TTimes mwmTimes; + Route::TStreets mwmStreets; + + LOG(LINFO, ("OSRM route form", MercatorBounds::ToLatLon(source.segmentPoint), + "to", MercatorBounds::ToLatLon(target.segmentPoint))); + + RawRoutingResult routingResult; + if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) + return false; + + OSRMRoutingResult resultGraph(*m_pIndex, *mapping, routingResult); + if (MakeTurnAnnotation(resultGraph, delegate, mwmRouteGeometry, mwmTurns, mwmTimes, mwmStreets) != NoError) + { + LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName())); + return false; + } + + mwmRoute.SetTurnInstructions(move(mwmTurns)); + mwmRoute.SetSectionTimes(move(mwmTimes)); + mwmRoute.SetStreetNames(move(mwmStreets)); + + vector mwmPoints; + JunctionsToPoints(mwmRouteGeometry, mwmPoints); + mwmRoute.SetGeometry(mwmPoints.cbegin(), mwmPoints.cend()); + } + + route.AppendRoute(mwmRoute); + return true; +} } // namespace routing diff --git a/routing/osrm_router.hpp b/routing/osrm_router.hpp index 6c28e3b361..a4a96c559c 100644 --- a/routing/osrm_router.hpp +++ b/routing/osrm_router.hpp @@ -6,6 +6,9 @@ #include "routing/router.hpp" #include "routing/routing_mapping.hpp" +#include "std/unique_ptr.hpp" +#include "std/vector.hpp" + namespace feature { class TypesHolder; } class Index; @@ -16,6 +19,7 @@ class FeatureType; namespace routing { +class RoadGraphRouter; struct RoutePathCross; using TCheckedPath = vector; @@ -26,7 +30,8 @@ class OsrmRouter : public IRouter public: typedef vector GeomTurnCandidateT; - OsrmRouter(Index * index, TCountryFileFn const & countryFileFn); + OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, + unique_ptr roadGraphRouter); virtual string GetName() const override; @@ -36,22 +41,11 @@ public: virtual void ClearState() override; - /*! Find single shortest path in a single MWM between 2 sets of edges - * \param source: vector of source edges to make path - * \param taget: vector of target edges to make path - * \param facade: OSRM routing data facade to recover graph information - * \param rawRoutingResult: routing result store - * \return true when path exists, false otherwise. - */ - static bool FindRouteFromCases(TFeatureGraphNodeVec const & source, - TFeatureGraphNodeVec const & target, TDataFacade & facade, - RawRoutingResult & rawRoutingResult); - /*! Fast checking ability of route construction * @param startPoint starting road point * @param finalPoint final road point * @param countryFileFn function for getting filename from point - * @oaram index mwmSet index + * @param index mwmSet index * @returns true if we can start routing process with a given data. */ static bool CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, @@ -84,11 +78,37 @@ private: ResultCode MakeRouteFromCrossesPath(TCheckedPath const & path, RouterDelegate const & delegate, Route & route); + bool IsEdgeIndexExisting(Index::MwmId const & mwmId); + + /*! + * \brief Builds a route within one mwm using A* if edge index section is available and osrm otherwise. + * Then reconstructs the route and restore all route attributes. + * \param route The found route is added the the |route| if the method returns true. + * \return true if route is build and false otherwise. + */ + // @TODO. The behavior of the method should be changed. This method should check if osrm section + // available and if so use them. If not, RoadGraphRouter and A* should be used. + bool FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, + Route & route); + + /*! Find single shortest path in a single MWM between 2 sets of edges + * \param source: vector of source edges to make path + * \param taget: vector of target edges to make path + * \param facade: OSRM routing data facade to recover graph information + * \param rawRoutingResult: routing result store + * \return true when path exists, false otherwise. + */ + bool FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); + Index const * m_pIndex; TFeatureGraphNodeVec m_cachedTargets; m2::PointD m_cachedTargetPoint; RoutingIndexManager m_indexManager; + + unique_ptr m_roadGraphRouter; }; } // namespace routing diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 312b466a3c..6204c141b2 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -310,4 +310,16 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr move(vehicleModelFactory), move(algorithm), move(directionsEngine))); return router; } + +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) +{ + // @TODO It's necessary to use car classes instead of bicycle ones. + unique_ptr vehicleModelFactory = make_unique(); + unique_ptr algorithm = make_unique(); + unique_ptr directionsEngine = make_unique(index); + unique_ptr router = make_unique( + "astar-bidirectional-bicycle", index, countryFileFn, IRoadGraph::Mode::ObeyOnewayTag, + move(vehicleModelFactory), move(algorithm), move(directionsEngine)); + return router; +} } // namespace routing diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index 17701bdc7a..019a30a584 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -56,4 +56,6 @@ private: unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); + +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); } // namespace routing diff --git a/routing/route.cpp b/routing/route.cpp index 27ed38b71c..c7f20933f3 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -354,6 +354,60 @@ void Route::Update() m_currentTime = 0.0; } +void Route::AppendRoute(Route const & route) +{ + if (!route.IsValid()) + return; + + if (m_poly.GetPolyline().GetSize() != 0) + { + ASSERT(!m_turns.empty(), ()); + ASSERT(!m_times.empty(), ()); + + // Remove road end point and turn instruction. + m_poly.PopBack(); + m_turns.pop_back(); + m_times.pop_back(); + // Streets might not point to the last point of the path. + } + + size_t const polySize = m_poly.GetPolyline().GetSize(); + + // Appending turns. + for (turns::TurnItem t : route.m_turns) + { + if (t.m_index == 0) + continue; + t.m_index += polySize; + m_turns.push_back(move(t)); + } + + // Appending street names. + for (TStreetItem s : route.m_streets) + { + if (s.first == 0) + continue; + s.first += polySize; + m_streets.push_back(move(s)); + } + + // @TODO Implement altitude appending. + + // Appending times. + double const estimationTime = m_times.empty() ? 0.0 : m_times.back().second; + for (TTimeItem t : route.m_times) + { + if (t.first == 0) + continue; + t.first += polySize; + t.second += estimationTime; + m_times.push_back(move(t)); + } + + m_poly.Append(route.m_poly); + Update(); +} + string DebugPrint(Route const & r) { return DebugPrint(r.m_poly.GetPolyline()); diff --git a/routing/route.hpp b/routing/route.hpp index 634ea55e7a..093306b800 100644 --- a/routing/route.hpp +++ b/routing/route.hpp @@ -59,6 +59,10 @@ public: inline void SetSectionTimes(TTimes && v) { m_times = move(v); } inline void SetStreetNames(TStreets && v) { m_streets = move(v); } inline void SetAltitudes(feature::TAltitudes && v) { m_altitudes = move(v); } + + /// \brief Appends all |route| attributes except for altitude. + void AppendRoute(Route const & route); + uint32_t GetTotalTimeSec() const; uint32_t GetCurrentTimeToEndSec() const; diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index 9447911d3e..c292ef0330 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -77,10 +77,12 @@ namespace integration unique_ptr CreateOsrmRouter(Index & index, storage::CountryInfoGetter const & infoGetter) { - unique_ptr osrmRouter(new OsrmRouter(&index, [&infoGetter](m2::PointD const & pt) + auto const countryFileGetter = [&infoGetter](m2::PointD const & pt) { return infoGetter.GetRegionCountryId(pt); - })); + }; + unique_ptr osrmRouter(new OsrmRouter(&index, countryFileGetter, + CreateCarAStarBidirectionalRouter(index, countryFileGetter))); return osrmRouter; } From f4da573250739505a964a6a500ee4157ebe89fe7 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 14 Oct 2016 14:52:36 +0300 Subject: [PATCH 02/12] Renaming osrm router to car router. --- map/framework.cpp | 10 ++-- routing/{osrm_router.cpp => car_router.cpp} | 57 ++++++++++--------- routing/{osrm_router.hpp => car_router.hpp} | 6 +- routing/cross_mwm_road_graph.hpp | 2 +- routing/routing.pro | 4 +- .../routing_test_tools.cpp | 16 +++--- .../routing_test_tools.hpp | 2 +- routing/routing_tests/osrm_router_test.cpp | 2 +- 8 files changed, 51 insertions(+), 48 deletions(-) rename routing/{osrm_router.cpp => car_router.cpp} (89%) rename routing/{osrm_router.hpp => car_router.hpp} (96%) diff --git a/map/framework.cpp b/map/framework.cpp index 2ecddda15f..fc6d551a0d 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -8,8 +8,8 @@ #include "defines.hpp" #include "private.h" +#include "routing/car_router.hpp" #include "routing/online_absent_fetcher.hpp" -#include "routing/osrm_router.hpp" #include "routing/road_graph_router.hpp" #include "routing/route.hpp" #include "routing/routing_algorithm.hpp" @@ -2398,8 +2398,8 @@ void Framework::SetRouterImpl(RouterType type) return m_model.GetIndex().GetMwmIdByCountryFile(CountryFile(countryFile)).IsAlive(); }; - router.reset(new OsrmRouter(&m_model.GetIndex(), countryFileGetter, - CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); + router.reset(new CarRouter(&m_model.GetIndex(), countryFileGetter, + CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); fetcher.reset(new OnlineAbsentCountriesFetcher(countryFileGetter, localFileChecker)); m_routingSession.SetRoutingSettings(routing::GetCarRoutingSettings()); } @@ -2551,8 +2551,8 @@ RouterType Framework::GetBestRouter(m2::PointD const & startPoint, m2::PointD co { return m_infoGetter->GetRegionCountryId(pt); }; - if (!OsrmRouter::CheckRoutingAbility(startPoint, finalPoint, countryFileGetter, - &m_model.GetIndex())) + if (!CarRouter::CheckRoutingAbility(startPoint, finalPoint, countryFileGetter, + &m_model.GetIndex())) { return RouterType::Pedestrian; } diff --git a/routing/osrm_router.cpp b/routing/car_router.cpp similarity index 89% rename from routing/osrm_router.cpp rename to routing/car_router.cpp index 31c8b87eaf..e773852941 100644 --- a/routing/osrm_router.cpp +++ b/routing/car_router.cpp @@ -1,10 +1,10 @@ +#include "routing/car_router.hpp" #include "routing/cross_mwm_router.hpp" #include "routing/loaded_path_segment.hpp" #include "routing/online_cross_fetcher.hpp" #include "routing/osrm2feature_map.hpp" #include "routing/osrm_helpers.hpp" #include "routing/osrm_path_segment_factory.hpp" -#include "routing/osrm_router.hpp" #include "routing/road_graph_router.hpp" #include "routing/turns_generator.hpp" @@ -200,34 +200,35 @@ private: }; // static -bool OsrmRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, - TCountryFileFn const & countryFileFn, Index * index) +bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, + TCountryFileFn const & countryFileFn, Index * index) { RoutingIndexManager manager(countryFileFn, *index); return manager.GetMappingByPoint(startPoint)->IsValid() && manager.GetMappingByPoint(finalPoint)->IsValid(); } -OsrmRouter::OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, - unique_ptr roadGraphRouter) +CarRouter::CarRouter(Index * index, TCountryFileFn const & countryFileFn, + unique_ptr roadGraphRouter) : m_pIndex(index), m_indexManager(countryFileFn, *index), m_roadGraphRouter(move(roadGraphRouter)) { } -string OsrmRouter::GetName() const +string CarRouter::GetName() const { return "vehicle"; } -void OsrmRouter::ClearState() +void CarRouter::ClearState() { m_cachedTargets.clear(); m_cachedTargetPoint = m2::PointD::Zero(); m_indexManager.Clear(); + m_roadGraphRouter->ClearState(); } -bool OsrmRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) +bool CarRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) { ASSERT(mapping, ()); @@ -304,9 +305,9 @@ void CalculatePhantomNodeForCross(TRoutingMappingPtr & mapping, FeatureGraphNode // TODO (ldragunov) move this function to cross mwm router // TODO (ldragunov) process case when the start and the finish points are placed on the same edge. -OsrmRouter::ResultCode OsrmRouter::MakeRouteFromCrossesPath(TCheckedPath const & path, - RouterDelegate const & delegate, - Route & route) +CarRouter::ResultCode CarRouter::MakeRouteFromCrossesPath(TCheckedPath const & path, + RouterDelegate const & delegate, + Route & route) { Route emptyRoute(GetName()); route.Swap(emptyRoute); @@ -327,10 +328,10 @@ OsrmRouter::ResultCode OsrmRouter::MakeRouteFromCrossesPath(TCheckedPath const & return NoError; } -OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, - m2::PointD const & startDirection, - m2::PointD const & finalPoint, - RouterDelegate const & delegate, Route & route) +CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, + m2::PointD const & startDirection, + m2::PointD const & finalPoint, + RouterDelegate const & delegate, Route & route) { my::HighResTimer timer(true); @@ -466,14 +467,14 @@ OsrmRouter::ResultCode OsrmRouter::CalculateRoute(m2::PointD const & startPoint, timer.Reset(); return code; } - return OsrmRouter::RouteNotFound; + return CarRouter::RouteNotFound; } } -IRouter::ResultCode OsrmRouter::FindPhantomNodes(m2::PointD const & point, - m2::PointD const & direction, - TFeatureGraphNodeVec & res, size_t maxCount, - TRoutingMappingPtr const & mapping) +IRouter::ResultCode CarRouter::FindPhantomNodes(m2::PointD const & point, + m2::PointD const & direction, + TFeatureGraphNodeVec & res, size_t maxCount, + TRoutingMappingPtr const & mapping) { ASSERT(mapping, ()); helpers::Point2PhantomNode getter(*mapping, *m_pIndex, direction); @@ -490,7 +491,7 @@ IRouter::ResultCode OsrmRouter::FindPhantomNodes(m2::PointD const & point, return NoError; } -bool OsrmRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) +bool CarRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) { MwmSet::MwmHandle const handle = m_pIndex->GetMwmHandleById(mwmId); if (!handle.IsAlive()) @@ -504,16 +505,17 @@ bool OsrmRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) if (value->GetHeader().GetFormat() < version::Format::v8) return false; - // @TODO Remove this string before merge. + // @TODO This section name will be defined in another PR. + // This const should be removed. string const EDGE_INDEX_FILE_TAG = "edgeidx"; if (value->m_cont.IsExist(EDGE_INDEX_FILE_TAG.c_str())) return true; return false; } -bool OsrmRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, - Route & route) +bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, + Route & route) { ASSERT_EQUAL(source.mwmId, target.mwmId, ()); ASSERT(m_pIndex, ()); @@ -525,6 +527,7 @@ bool OsrmRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Feat // Probably it's better to keep if mwm had edge index section in mwmId. if (IsEdgeIndexExisting(source.mwmId) && m_roadGraphRouter) { + // A* routing LOG(LINFO, ("A* route form", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); @@ -533,10 +536,10 @@ bool OsrmRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Feat { return false; } - } else { + // OSRM Routing vector mwmRouteGeometry; Route::TTurns mwmTurns; Route::TTimes mwmTimes; diff --git a/routing/osrm_router.hpp b/routing/car_router.hpp similarity index 96% rename from routing/osrm_router.hpp rename to routing/car_router.hpp index a4a96c559c..a9eb4f5956 100644 --- a/routing/osrm_router.hpp +++ b/routing/car_router.hpp @@ -25,13 +25,13 @@ using TCheckedPath = vector; typedef vector TFeatureGraphNodeVec; -class OsrmRouter : public IRouter +class CarRouter : public IRouter { public: typedef vector GeomTurnCandidateT; - OsrmRouter(Index * index, TCountryFileFn const & countryFileFn, - unique_ptr roadGraphRouter); + CarRouter(Index * index, TCountryFileFn const & countryFileFn, + unique_ptr roadGraphRouter); virtual string GetName() const override; diff --git a/routing/cross_mwm_road_graph.hpp b/routing/cross_mwm_road_graph.hpp index e0a59911f7..7687c3246f 100644 --- a/routing/cross_mwm_road_graph.hpp +++ b/routing/cross_mwm_road_graph.hpp @@ -1,7 +1,7 @@ #pragma once +#include "car_router.hpp" #include "osrm_engine.hpp" -#include "osrm_router.hpp" #include "router.hpp" #include "indexer/index.hpp" diff --git a/routing/routing.pro b/routing/routing.pro index 6d676f711c..8eca9f8dd7 100644 --- a/routing/routing.pro +++ b/routing/routing.pro @@ -18,6 +18,7 @@ SOURCES += \ bicycle_directions.cpp \ bicycle_model.cpp \ car_model.cpp \ + car_router.cpp \ cross_mwm_road_graph.cpp \ cross_mwm_router.cpp \ cross_routing_context.cpp \ @@ -30,7 +31,6 @@ SOURCES += \ osrm_engine.cpp \ osrm_helpers.cpp \ osrm_path_segment_factory.cpp \ - osrm_router.cpp \ pedestrian_directions.cpp \ pedestrian_model.cpp \ road_graph.cpp \ @@ -57,6 +57,7 @@ HEADERS += \ bicycle_directions.hpp \ bicycle_model.hpp \ car_model.hpp \ + car_router.hpp \ cross_mwm_road_graph.hpp \ cross_mwm_router.hpp \ cross_routing_context.hpp \ @@ -71,7 +72,6 @@ HEADERS += \ osrm_engine.hpp \ osrm_helpers.hpp \ osrm_path_segment_factory.hpp \ - osrm_router.hpp \ pedestrian_directions.hpp \ pedestrian_model.hpp \ road_graph.hpp \ diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index c292ef0330..aae10bd28b 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -74,16 +74,16 @@ namespace integration return storage::CountryInfoReader::CreateCountryInfoReader(platform); } - unique_ptr CreateOsrmRouter(Index & index, - storage::CountryInfoGetter const & infoGetter) + unique_ptr CreateCarRouter(Index & index, + storage::CountryInfoGetter const & infoGetter) { auto const countryFileGetter = [&infoGetter](m2::PointD const & pt) { return infoGetter.GetRegionCountryId(pt); }; - unique_ptr osrmRouter(new OsrmRouter(&index, countryFileGetter, - CreateCarAStarBidirectionalRouter(index, countryFileGetter))); - return osrmRouter; + unique_ptr carRouter(new CarRouter(&index, countryFileGetter, + CreateCarAStarBidirectionalRouter(index, countryFileGetter))); + return carRouter; } unique_ptr CreateAStarRouter(Index & index, @@ -105,14 +105,14 @@ namespace integration public: OsrmRouterComponents(vector const & localFiles) : IRouterComponents(localFiles) - , m_osrmRouter(CreateOsrmRouter(m_featuresFetcher->GetIndex(), *m_infoGetter)) + , m_carRouter(CreateCarRouter(m_featuresFetcher->GetIndex(), *m_infoGetter)) { } - IRouter * GetRouter() const override { return m_osrmRouter.get(); } + IRouter * GetRouter() const override { return m_carRouter.get(); } private: - unique_ptr m_osrmRouter; + unique_ptr m_carRouter; }; class PedestrianRouterComponents : public IRouterComponents diff --git a/routing/routing_integration_tests/routing_test_tools.hpp b/routing/routing_integration_tests/routing_test_tools.hpp index 4145b18bce..cefa6b314b 100644 --- a/routing/routing_integration_tests/routing_test_tools.hpp +++ b/routing/routing_integration_tests/routing_test_tools.hpp @@ -1,6 +1,6 @@ #pragma once -#include "routing/osrm_router.hpp" +#include "routing/car_router.hpp" #include "storage/country_info_getter.hpp" diff --git a/routing/routing_tests/osrm_router_test.cpp b/routing/routing_tests/osrm_router_test.cpp index c9ac49a9ae..d706cf78a3 100644 --- a/routing/routing_tests/osrm_router_test.cpp +++ b/routing/routing_tests/osrm_router_test.cpp @@ -1,6 +1,6 @@ #include "testing/testing.hpp" -#include "routing/osrm_router.hpp" +#include "routing/car_router.hpp" #include "indexer/features_offsets_table.hpp" #include "geometry/mercator.hpp" From 5daa09c9fd47822086ded3e02ead2c99fd1c2470 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 14 Oct 2016 17:39:47 +0300 Subject: [PATCH 03/12] Adding CarModelFactory and using it in CreateCarAStarBidirectionalRouter --- generator/generator_tests/osm_type_test.cpp | 2 +- routing/car_model.cpp | 16 +++++++++++++++- routing/car_model.hpp | 19 ++++++++++++++++--- routing/osrm_helpers.cpp | 4 ++-- routing/road_graph_router.cpp | 3 ++- routing/routing_helpers.hpp | 2 +- 6 files changed, 37 insertions(+), 9 deletions(-) diff --git a/generator/generator_tests/osm_type_test.cpp b/generator/generator_tests/osm_type_test.cpp index de15b240dc..f1b2992005 100644 --- a/generator/generator_tests/osm_type_test.cpp +++ b/generator/generator_tests/osm_type_test.cpp @@ -593,7 +593,7 @@ UNIT_TEST(OsmType_Surface) UNIT_TEST(OsmType_Ferry) { - routing::CarModel const & carModel = routing::CarModel::Instance(); + routing::CarModel const & carModel = routing::CarModel::AllLimitsInstance(); char const * arr[][2] = { { "motorcar", "yes" }, diff --git a/routing/car_model.cpp b/routing/car_model.cpp index 31a6e4c78b..0d11455b66 100644 --- a/routing/car_model.cpp +++ b/routing/car_model.cpp @@ -75,10 +75,24 @@ CarModel::CarModel() } // static -CarModel const & CarModel::Instance() +CarModel const & CarModel::AllLimitsInstance() { static CarModel const instance; return instance; } +CarModelFactory::CarModelFactory() +{ + m_model = make_shared(); +} + +shared_ptr CarModelFactory::GetVehicleModel() const +{ + return m_model; +} + +shared_ptr CarModelFactory::GetVehicleModelForCountry(string const & /* country */) const +{ + return m_model; +} } // namespace routing diff --git a/routing/car_model.hpp b/routing/car_model.hpp index 8170df3a75..680e20ebb7 100644 --- a/routing/car_model.hpp +++ b/routing/car_model.hpp @@ -1,16 +1,29 @@ #pragma once - #include "vehicle_model.hpp" +#include "std/shared_ptr.hpp" + namespace routing { class CarModel : public VehicleModel { +public: CarModel(); -public: - static CarModel const & Instance(); + static CarModel const & AllLimitsInstance(); }; +class CarModelFactory : public IVehicleModelFactory +{ +public: + CarModelFactory(); + + // IVehicleModelFactory overrides: + shared_ptr GetVehicleModel() const override; + shared_ptr GetVehicleModelForCountry(string const & country) const override; + +private: + shared_ptr m_model; +}; } // namespace routing diff --git a/routing/osrm_helpers.cpp b/routing/osrm_helpers.cpp index 2aed745f84..9a5274a75a 100644 --- a/routing/osrm_helpers.cpp +++ b/routing/osrm_helpers.cpp @@ -38,7 +38,7 @@ void Point2PhantomNode::FindNearestSegment(FeatureType const & ft, m2::PointD co void Point2PhantomNode::operator()(FeatureType const & ft) { - if (!CarModel::Instance().IsRoad(ft)) + if (!CarModel::AllLimitsInstance().IsRoad(ft)) return; Candidate res; @@ -280,7 +280,7 @@ void Point2PhantomNode::CalculateWeights(FeatureGraphNode & node) const void Point2Node::operator()(FeatureType const & ft) { - if (!CarModel::Instance().IsRoad(ft)) + if (!CarModel::AllLimitsInstance().IsRoad(ft)) return; uint32_t const featureId = ft.GetID().m_index; for (auto const & n : m_routingMapping.m_segMapping.GetNodeIdByFid(featureId)) diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 6204c141b2..0afa57a2e7 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -1,5 +1,6 @@ #include "routing/bicycle_directions.hpp" #include "routing/bicycle_model.hpp" +#include "routing/car_model.hpp" #include "routing/features_road_graph.hpp" #include "routing/nearest_edge_finder.hpp" #include "routing/pedestrian_directions.hpp" @@ -314,7 +315,7 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { // @TODO It's necessary to use car classes instead of bicycle ones. - unique_ptr vehicleModelFactory = make_unique(); + unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); unique_ptr directionsEngine = make_unique(index); unique_ptr router = make_unique( diff --git a/routing/routing_helpers.hpp b/routing/routing_helpers.hpp index 98365f39c4..76440a457e 100644 --- a/routing/routing_helpers.hpp +++ b/routing/routing_helpers.hpp @@ -10,7 +10,7 @@ namespace routing template bool IsRoad(TTypes const & types) { - return CarModel::Instance().HasRoadType(types) || + return CarModel::AllLimitsInstance().HasRoadType(types) || PedestrianModel::AllLimitsInstance().HasRoadType(types) || BicycleModel::AllLimitsInstance().HasRoadType(types); } From 57c0d5a6dfc85c3074498669fdbcecc28df5e822 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 17 Oct 2016 17:51:22 +0300 Subject: [PATCH 04/12] Adding edgeidx define and detail some comments. --- defines.hpp | 1 + routing/car_router.cpp | 7 ++----- routing/car_router.hpp | 2 -- routing/road_graph_router.cpp | 3 ++- routing/route.cpp | 2 -- routing/route.hpp | 9 ++++++++- 6 files changed, 13 insertions(+), 11 deletions(-) diff --git a/defines.hpp b/defines.hpp index 612169d512..f0db7c48ec 100644 --- a/defines.hpp +++ b/defines.hpp @@ -27,6 +27,7 @@ #define METADATA_FILE_TAG "meta" #define METADATA_INDEX_FILE_TAG "metaidx" #define ALTITUDES_FILE_TAG "altitudes" +#define EDGE_INDEX_FILE_TAG "edgeidx" #define FEATURE_OFFSETS_FILE_TAG "offs" #define RANKS_FILE_TAG "ranks" #define REGION_INFO_FILE_TAG "rgninfo" diff --git a/routing/car_router.cpp b/routing/car_router.cpp index e773852941..c2408e13ac 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -505,10 +505,7 @@ bool CarRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) if (value->GetHeader().GetFormat() < version::Format::v8) return false; - // @TODO This section name will be defined in another PR. - // This const should be removed. - string const EDGE_INDEX_FILE_TAG = "edgeidx"; - if (value->m_cont.IsExist(EDGE_INDEX_FILE_TAG.c_str())) + if (value->m_cont.IsExist(EDGE_INDEX_FILE_TAG)) return true; return false; } @@ -524,7 +521,7 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu Route mwmRoute(GetName()); // @TODO It's not the best place for checking availability of edge index section in mwm. - // Probably it's better to keep if mwm had edge index section in mwmId. + // Probably it's better to keep if mwm has an edge index section in mwmId. if (IsEdgeIndexExisting(source.mwmId) && m_roadGraphRouter) { // A* routing diff --git a/routing/car_router.hpp b/routing/car_router.hpp index a9eb4f5956..6b2d107f6e 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -86,8 +86,6 @@ private: * \param route The found route is added the the |route| if the method returns true. * \return true if route is build and false otherwise. */ - // @TODO. The behavior of the method should be changed. This method should check if osrm section - // available and if so use them. If not, RoadGraphRouter and A* should be used. bool FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 0afa57a2e7..693a4841ca 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -314,9 +314,10 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { - // @TODO It's necessary to use car classes instead of bicycle ones. unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); + // @TODO Bycycle turn generation engine is used now. It's ok for the time being. + // But later a special car turn generation engine should be implemented. unique_ptr directionsEngine = make_unique(index); unique_ptr router = make_unique( "astar-bidirectional-bicycle", index, countryFileFn, IRoadGraph::Mode::ObeyOnewayTag, diff --git a/routing/route.cpp b/routing/route.cpp index c7f20933f3..452367cd75 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -391,8 +391,6 @@ void Route::AppendRoute(Route const & route) m_streets.push_back(move(s)); } - // @TODO Implement altitude appending. - // Appending times. double const estimationTime = m_times.empty() ? 0.0 : m_times.back().second; for (TTimeItem t : route.m_times) diff --git a/routing/route.hpp b/routing/route.hpp index 093306b800..b3fe071212 100644 --- a/routing/route.hpp +++ b/routing/route.hpp @@ -60,7 +60,14 @@ public: inline void SetStreetNames(TStreets && v) { m_streets = move(v); } inline void SetAltitudes(feature::TAltitudes && v) { m_altitudes = move(v); } - /// \brief Appends all |route| attributes except for altitude. + /// \brief Glues all |route| attributes to |this| except for |m_altitudes|. + // @TODO In the future this method should append |m_altitudes| as well. + // It's not implemented now because it's not easy to do it and it'll not be used in + // the short future. The problem is routes genetated by osrm have empty |m_altitudes|. + // So |m_altitudes| should be filled (with correnct or default values) on osrm route + // reconstruction stage to be added with this method. On the other + // hand it seems this method'll not be not used for bicycle and pedestrian routing + // in short future. And altitude is not used for car routing. void AppendRoute(Route const & route); uint32_t GetTotalTimeSec() const; From d0793c5615f4c1176ed3e0b0d4055d5727cbf29e Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 17 Oct 2016 17:58:40 +0300 Subject: [PATCH 05/12] xcode build fix. --- xcode/routing/routing.xcodeproj/project.pbxproj | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/xcode/routing/routing.xcodeproj/project.pbxproj b/xcode/routing/routing.xcodeproj/project.pbxproj index d8bcd4f13a..49d99e28b4 100644 --- a/xcode/routing/routing.xcodeproj/project.pbxproj +++ b/xcode/routing/routing.xcodeproj/project.pbxproj @@ -21,6 +21,8 @@ 56555E561D897C90009D786D /* libalohalitics.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 6742ACE61C68A23B009CB89E /* libalohalitics.a */; }; 56555E581D897C9D009D786D /* liboauthcpp.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 6742ACFA1C68A2D7009CB89E /* liboauthcpp.a */; }; 56555E591D897D28009D786D /* testingmain.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6742ACDE1C68A13F009CB89E /* testingmain.cpp */; }; + 56826BD01DB51C4E00807C62 /* car_router.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 56826BCE1DB51C4E00807C62 /* car_router.cpp */; }; + 56826BD11DB51C4E00807C62 /* car_router.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 56826BCF1DB51C4E00807C62 /* car_router.hpp */; }; 56EA2FD51D8FD8590083F01A /* routing_helpers.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 56EA2FD41D8FD8590083F01A /* routing_helpers.hpp */; }; 56F0D7341D896A5300045886 /* libmap.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 67BD35DE1C69F198003AA26F /* libmap.a */; }; 56F0D7391D896A5300045886 /* libstorage.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 67BD35D41C69F155003AA26F /* libstorage.a */; }; @@ -133,8 +135,6 @@ 674F9BD61B0A580E00704FFA /* turns_generator.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 674F9BC61B0A580E00704FFA /* turns_generator.cpp */; }; 674F9BD71B0A580E00704FFA /* turns_generator.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 674F9BC71B0A580E00704FFA /* turns_generator.hpp */; }; 675344141A3F644F00A0A8C3 /* osrm_data_facade.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 675344061A3F644F00A0A8C3 /* osrm_data_facade.hpp */; }; - 675344171A3F644F00A0A8C3 /* osrm_router.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 675344091A3F644F00A0A8C3 /* osrm_router.cpp */; }; - 675344181A3F644F00A0A8C3 /* osrm_router.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 6753440A1A3F644F00A0A8C3 /* osrm_router.hpp */; }; 675344191A3F644F00A0A8C3 /* osrm2feature_map.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6753440B1A3F644F00A0A8C3 /* osrm2feature_map.cpp */; }; 6753441A1A3F644F00A0A8C3 /* osrm2feature_map.hpp in Headers */ = {isa = PBXBuildFile; fileRef = 6753440C1A3F644F00A0A8C3 /* osrm2feature_map.hpp */; }; 6753441B1A3F644F00A0A8C3 /* route.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 6753440D1A3F644F00A0A8C3 /* route.cpp */; }; @@ -235,6 +235,8 @@ 56099E321CC9247E00A7772A /* directions_engine.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = directions_engine.cpp; sourceTree = ""; }; 563B91C31CC4F1DC00222BC1 /* bicycle_model.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = bicycle_model.cpp; sourceTree = ""; }; 563B91C41CC4F1DC00222BC1 /* bicycle_model.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = bicycle_model.hpp; sourceTree = ""; }; + 56826BCE1DB51C4E00807C62 /* car_router.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = car_router.cpp; sourceTree = ""; }; + 56826BCF1DB51C4E00807C62 /* car_router.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = car_router.hpp; sourceTree = ""; }; 56EA2FD41D8FD8590083F01A /* routing_helpers.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = routing_helpers.hpp; sourceTree = ""; }; 56F0D75F1D896A5300045886 /* routing_benchmarks.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = routing_benchmarks.app; sourceTree = BUILT_PRODUCTS_DIR; }; 670B84BE1A9381D900CE4492 /* cross_routing_context.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = cross_routing_context.cpp; sourceTree = ""; }; @@ -319,8 +321,6 @@ 674F9BC71B0A580E00704FFA /* turns_generator.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = turns_generator.hpp; sourceTree = ""; }; 675343F81A3F640D00A0A8C3 /* librouting.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = librouting.a; sourceTree = BUILT_PRODUCTS_DIR; }; 675344061A3F644F00A0A8C3 /* osrm_data_facade.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = osrm_data_facade.hpp; sourceTree = ""; }; - 675344091A3F644F00A0A8C3 /* osrm_router.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = osrm_router.cpp; sourceTree = ""; }; - 6753440A1A3F644F00A0A8C3 /* osrm_router.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = osrm_router.hpp; sourceTree = ""; }; 6753440B1A3F644F00A0A8C3 /* osrm2feature_map.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = osrm2feature_map.cpp; sourceTree = ""; }; 6753440C1A3F644F00A0A8C3 /* osrm2feature_map.hpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.h; path = osrm2feature_map.hpp; sourceTree = ""; }; 6753440D1A3F644F00A0A8C3 /* route.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = route.cpp; sourceTree = ""; }; @@ -633,6 +633,8 @@ 56EA2FD41D8FD8590083F01A /* routing_helpers.hpp */, 56099E2C1CC8FBDA00A7772A /* osrm_path_segment_factory.cpp */, 56099E2D1CC8FBDA00A7772A /* osrm_path_segment_factory.hpp */, + 56826BCE1DB51C4E00807C62 /* car_router.cpp */, + 56826BCF1DB51C4E00807C62 /* car_router.hpp */, 56099E301CC9247E00A7772A /* bicycle_directions.cpp */, 56099E311CC9247E00A7772A /* bicycle_directions.hpp */, 56099E321CC9247E00A7772A /* directions_engine.cpp */, @@ -694,8 +696,6 @@ 67C7D4261B4EB48F00FE41AA /* turns_sound_settings.hpp */, 670C62111AC5A15700C38A8C /* routing_mapping.cpp */, 675344061A3F644F00A0A8C3 /* osrm_data_facade.hpp */, - 675344091A3F644F00A0A8C3 /* osrm_router.cpp */, - 6753440A1A3F644F00A0A8C3 /* osrm_router.hpp */, 6753440B1A3F644F00A0A8C3 /* osrm2feature_map.cpp */, 6753440C1A3F644F00A0A8C3 /* osrm2feature_map.hpp */, 6753440D1A3F644F00A0A8C3 /* route.cpp */, @@ -741,7 +741,6 @@ 56099E2A1CC7C97D00A7772A /* routing_result_graph.hpp in Headers */, A120B3481B4A7BE5002F3808 /* cross_mwm_router.hpp in Headers */, 674F9BD31B0A580E00704FFA /* road_graph_router.hpp in Headers */, - 675344181A3F644F00A0A8C3 /* osrm_router.hpp in Headers */, 675344141A3F644F00A0A8C3 /* osrm_data_facade.hpp in Headers */, 6753441F1A3F644F00A0A8C3 /* turns.hpp in Headers */, 6753441A1A3F644F00A0A8C3 /* osrm2feature_map.hpp in Headers */, @@ -770,6 +769,7 @@ 670D049F1B0B4A970013A7AC /* nearest_edge_finder.hpp in Headers */, A120B34F1B4A7C0A002F3808 /* online_absent_fetcher.hpp in Headers */, 674F9BD51B0A580E00704FFA /* road_graph.hpp in Headers */, + 56826BD11DB51C4E00807C62 /* car_router.hpp in Headers */, A120B3511B4A7C0A002F3808 /* routing_algorithm.hpp in Headers */, 675344211A3F644F00A0A8C3 /* vehicle_model.hpp in Headers */, 56099E2B1CC7C97D00A7772A /* turn_candidate.hpp in Headers */, @@ -991,6 +991,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 56826BD01DB51C4E00807C62 /* car_router.cpp in Sources */, 56099E2E1CC8FBDA00A7772A /* osrm_path_segment_factory.cpp in Sources */, 675344201A3F644F00A0A8C3 /* vehicle_model.cpp in Sources */, 56099E351CC9247E00A7772A /* directions_engine.cpp in Sources */, @@ -1004,7 +1005,6 @@ 675344191A3F644F00A0A8C3 /* osrm2feature_map.cpp in Sources */, 670D049E1B0B4A970013A7AC /* nearest_edge_finder.cpp in Sources */, 674F9BD61B0A580E00704FFA /* turns_generator.cpp in Sources */, - 675344171A3F644F00A0A8C3 /* osrm_router.cpp in Sources */, A17B42981BCFBD0E00A1EAE4 /* osrm_helpers.cpp in Sources */, 563B91C51CC4F1DC00222BC1 /* bicycle_model.cpp in Sources */, 674F9BD21B0A580E00704FFA /* road_graph_router.cpp in Sources */, From 9d7d305e4ff2edae80aacce5b0c91b0bf734dbe4 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 17 Oct 2016 18:03:29 +0300 Subject: [PATCH 06/12] Minor fixes. --- routing/car_router.cpp | 2 ++ routing/road_graph_router.cpp | 2 +- routing/road_graph_router.hpp | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/routing/car_router.cpp b/routing/car_router.cpp index c2408e13ac..c1485bad5c 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -537,6 +537,8 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu else { // OSRM Routing + // @TODO This branch is implemented to support old maps with osrm section. When osrm + // section is not supported this branch should be removed. vector mwmRouteGeometry; Route::TTurns mwmTurns; Route::TTimes mwmTimes; diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 693a4841ca..7d37ccb4c4 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -316,7 +316,7 @@ unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCo { unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); - // @TODO Bycycle turn generation engine is used now. It's ok for the time being. + // @TODO Bicycle turn generation engine is used now. It's ok for the time being. // But later a special car turn generation engine should be implemented. unique_ptr directionsEngine = make_unique(index); unique_ptr router = make_unique( diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index 019a30a584..d3fdeb7fed 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -56,6 +56,5 @@ private: unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); - unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); } // namespace routing From ef4f16a36b98c5ea63734b550fdf52ec435da085 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 20 Oct 2016 18:20:24 +0300 Subject: [PATCH 07/12] Review fixes. --- geometry/polyline2d.hpp | 27 +++++++++---------- routing/bicycle_model.hpp | 4 +-- routing/car_model.cpp | 3 +++ routing/car_model.hpp | 4 +-- routing/car_router.cpp | 10 ++++--- routing/car_router.hpp | 12 +++++---- routing/features_road_graph.cpp | 4 +-- routing/features_road_graph.hpp | 6 ++--- routing/osrm_engine.hpp | 2 +- routing/pedestrian_model.hpp | 4 +-- routing/road_graph_router.cpp | 10 +++---- routing/road_graph_router.hpp | 2 +- routing/route.cpp | 2 +- .../bicycle_routing_tests.cpp | 4 +-- routing/routing_benchmarks/helpers.hpp | 6 ++--- .../pedestrian_routing_tests.cpp | 4 +-- routing/vehicle_model.hpp | 4 +-- 17 files changed, 58 insertions(+), 50 deletions(-) diff --git a/geometry/polyline2d.hpp b/geometry/polyline2d.hpp index 701f888192..1c67c258a7 100644 --- a/geometry/polyline2d.hpp +++ b/geometry/polyline2d.hpp @@ -13,21 +13,21 @@ namespace m2 { template -class PolylineT +class Polyline { vector > m_points; public: - PolylineT() {} - PolylineT(initializer_list > points) : m_points(points) + Polyline() {} + Polyline(initializer_list > points) : m_points(points) { ASSERT_GREATER(m_points.size(), 1, ()); } - explicit PolylineT(vector > const & points) : m_points(points) + explicit Polyline(vector > const & points) : m_points(points) { ASSERT_GREATER(m_points.size(), 1, ()); } - template PolylineT(IterT beg, IterT end) : m_points(beg, end) + template Polyline(IterT beg, IterT end) : m_points(beg, end) { ASSERT_GREATER(m_points.size(), 1, ()); } @@ -68,25 +68,25 @@ public: void Clear() { m_points.clear(); } void Add(Point const & pt) { m_points.push_back(pt); } - void Append(PolylineT const & poly) + void Append(Polyline const & poly) { m_points.insert(m_points.end(), poly.m_points.cbegin(), poly.m_points.cend()); } void PopBack() { - if (!m_points.empty()) - m_points.pop_back(); + ASSERT(!m_points.empty(), ()); + m_points.pop_back(); } - void Swap(PolylineT & rhs) + void Swap(Polyline & rhs) { m_points.swap(rhs.m_points); } size_t GetSize() const { return m_points.size(); } - bool operator==(PolylineT const & rhs) const { return m_points == rhs.m_points; } + bool operator==(Polyline const & rhs) const { return m_points == rhs.m_points; } typedef vector > TContainer; typedef typename TContainer::const_iterator TIter; @@ -123,12 +123,11 @@ public: vector > const & GetPoints() const { return m_points; } - friend string DebugPrint(PolylineT const & p) + friend string DebugPrint(Polyline const & p) { return ::DebugPrint(p.m_points); } }; -typedef PolylineT PolylineD; - -} +typedef Polyline PolylineD; +} // namespace m2 diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 9293fcc1cd..9defeda9be 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -33,12 +33,12 @@ private: uint32_t m_bidirBicycleType = 0; }; -class BicycleModelFactory : public IVehicleModelFactory +class BicycleModelFactory : public VehicleModelFactory { public: BicycleModelFactory(); - /// @name Overrides from IVehicleModelFactory. + /// @name Overrides from VehicleModelFactory. //@{ shared_ptr GetVehicleModel() const override; shared_ptr GetVehicleModelForCountry(string const & country) const override; diff --git a/routing/car_model.cpp b/routing/car_model.cpp index 0d11455b66..f5453836cf 100644 --- a/routing/car_model.cpp +++ b/routing/car_model.cpp @@ -93,6 +93,9 @@ shared_ptr CarModelFactory::GetVehicleModel() const shared_ptr CarModelFactory::GetVehicleModelForCountry(string const & /* country */) const { + // @TODO(bykoianko) Different vehicle model for different country should be supported + // according to http://wiki.openstreetmap.org/wiki/OSM_tags_for_routing/Access-Restrictions. + // See pedestrian_model.cpp and bicycle_model.cpp for example. return m_model; } } // namespace routing diff --git a/routing/car_model.hpp b/routing/car_model.hpp index 680e20ebb7..44378202e7 100644 --- a/routing/car_model.hpp +++ b/routing/car_model.hpp @@ -14,12 +14,12 @@ public: static CarModel const & AllLimitsInstance(); }; -class CarModelFactory : public IVehicleModelFactory +class CarModelFactory : public VehicleModelFactory { public: CarModelFactory(); - // IVehicleModelFactory overrides: + // VehicleModelFactory overrides: shared_ptr GetVehicleModel() const override; shared_ptr GetVehicleModelForCountry(string const & country) const override; diff --git a/routing/car_router.cpp b/routing/car_router.cpp index c1485bad5c..1c176b64f2 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -236,9 +236,13 @@ bool CarRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeature route.Swap(emptyRoute); /// @todo (ldargunov) make more complex nearest edge turnaround for (auto const & targetEdge : target) + { for (auto const & sourceEdge : source) + { if (FindSingleRouteDispatcher(sourceEdge, targetEdge, delegate, mapping, route)) return true; + } + } return false; } @@ -496,7 +500,7 @@ bool CarRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) MwmSet::MwmHandle const handle = m_pIndex->GetMwmHandleById(mwmId); if (!handle.IsAlive()) { - ASSERT(false, ("m_mwmHandle is not alive.")); + ASSERT(false, ("Mwm handle is not alive.")); return false; } @@ -525,7 +529,7 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu if (IsEdgeIndexExisting(source.mwmId) && m_roadGraphRouter) { // A* routing - LOG(LINFO, ("A* route form", MercatorBounds::ToLatLon(source.segmentPoint), + LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); if (m_roadGraphRouter->CalculateRoute(source.segmentPoint, m2::PointD(0, 0), target.segmentPoint, @@ -544,7 +548,7 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu Route::TTimes mwmTimes; Route::TStreets mwmStreets; - LOG(LINFO, ("OSRM route form", MercatorBounds::ToLatLon(source.segmentPoint), + LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); RawRoutingResult routingResult; diff --git a/routing/car_router.hpp b/routing/car_router.hpp index 6b2d107f6e..b94d7e85e4 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -78,21 +78,23 @@ private: ResultCode MakeRouteFromCrossesPath(TCheckedPath const & path, RouterDelegate const & delegate, Route & route); + // @TODO(bykoianko) When edgeidx section implementation is merged to master + // this method should be moved to edge index loader. bool IsEdgeIndexExisting(Index::MwmId const & mwmId); /*! * \brief Builds a route within one mwm using A* if edge index section is available and osrm otherwise. - * Then reconstructs the route and restore all route attributes. - * \param route The found route is added the the |route| if the method returns true. - * \return true if route is build and false otherwise. + * Then reconstructs the route and restores all route attributes. + * \param route The found route is added to the |route| if the method returns true. + * \return true if route is built and false otherwise. */ bool FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); - /*! Find single shortest path in a single MWM between 2 sets of edges + /*! Finds single shortest path in a single MWM between 2 sets of edges * \param source: vector of source edges to make path - * \param taget: vector of target edges to make path + * \param target: vector of target edges to make path * \param facade: OSRM routing data facade to recover graph information * \param rawRoutingResult: routing result store * \return true when path exists, false otherwise. diff --git a/routing/features_road_graph.cpp b/routing/features_road_graph.cpp index 23912ba928..40bf8ba932 100644 --- a/routing/features_road_graph.cpp +++ b/routing/features_road_graph.cpp @@ -46,7 +46,7 @@ FeaturesRoadGraph::Value::Value(MwmSet::MwmHandle handle) : m_mwmHandle(move(han m_altitudeLoader = make_unique(*m_mwmHandle.GetValue()); } -FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel(unique_ptr && vehicleModelFactory) +FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel(unique_ptr && vehicleModelFactory) : m_vehicleModelFactory(move(vehicleModelFactory)) , m_maxSpeedKMPH(m_vehicleModelFactory->GetVehicleModel()->GetMaxSpeed()) { @@ -108,7 +108,7 @@ void FeaturesRoadGraph::RoadInfoCache::Clear() } FeaturesRoadGraph::FeaturesRoadGraph(Index const & index, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory) + unique_ptr && vehicleModelFactory) : m_index(index) , m_mode(mode) , m_vehicleModel(move(vehicleModelFactory)) diff --git a/routing/features_road_graph.hpp b/routing/features_road_graph.hpp index 1f99d2f78b..ecde6fb869 100644 --- a/routing/features_road_graph.hpp +++ b/routing/features_road_graph.hpp @@ -26,7 +26,7 @@ private: class CrossCountryVehicleModel : public IVehicleModel { public: - CrossCountryVehicleModel(unique_ptr && vehicleModelFactory); + CrossCountryVehicleModel(unique_ptr && vehicleModelFactory); // IVehicleModel overrides: double GetSpeed(FeatureType const & f) const override; @@ -39,7 +39,7 @@ private: private: IVehicleModel * GetVehicleModel(FeatureID const & featureId) const; - unique_ptr const m_vehicleModelFactory; + unique_ptr const m_vehicleModelFactory; double const m_maxSpeedKMPH; mutable map> m_cache; @@ -59,7 +59,7 @@ private: public: FeaturesRoadGraph(Index const & index, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory); + unique_ptr && vehicleModelFactory); static uint32_t GetStreetReadScale(); diff --git a/routing/osrm_engine.hpp b/routing/osrm_engine.hpp index 2d032dde9c..152f984490 100644 --- a/routing/osrm_engine.hpp +++ b/routing/osrm_engine.hpp @@ -88,7 +88,7 @@ using TRawDataFacade = OsrmRawDataFacade; void FindWeightsMatrix(TRoutingNodes const & sources, TRoutingNodes const & targets, TRawDataFacade & facade, vector & result); -/*! Find single shortest path in a single MWM between 2 OSRM nodes +/*! Finds single shortest path in a single MWM between 2 OSRM nodes * \param source Source OSRM graph node to make path. * \param taget Target OSRM graph node to make path. * \param facade OSRM routing data facade to recover graph information. diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index 93ebd6fdb1..bcff1db241 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -28,12 +28,12 @@ private: uint32_t m_yesFootType = 0; }; -class PedestrianModelFactory : public IVehicleModelFactory +class PedestrianModelFactory : public VehicleModelFactory { public: PedestrianModelFactory(); - /// @name Overrides from IVehicleModelFactory. + /// @name Overrides from VehicleModelFactory. //@{ shared_ptr GetVehicleModel() const override; shared_ptr GetVehicleModelForCountry(string const & country) const override; diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 7d37ccb4c4..71c2cecd39 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -140,7 +140,7 @@ RoadGraphRouter::~RoadGraphRouter() {} RoadGraphRouter::RoadGraphRouter(string const & name, Index const & index, TCountryFileFn const & countryFileFn, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory, + unique_ptr && vehicleModelFactory, unique_ptr && algorithm, unique_ptr && directionsEngine) : m_name(name) @@ -281,7 +281,7 @@ void RoadGraphRouter::ReconstructRoute(vector && path, Route & route, unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn const & countryFileFn) { - unique_ptr vehicleModelFactory(new PedestrianModelFactory()); + unique_ptr vehicleModelFactory(new PedestrianModelFactory()); unique_ptr algorithm(new AStarRoutingAlgorithm()); unique_ptr directionsEngine(new PedestrianDirectionsEngine()); unique_ptr router(new RoadGraphRouter( @@ -292,7 +292,7 @@ unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn co unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { - unique_ptr vehicleModelFactory(new PedestrianModelFactory()); + unique_ptr vehicleModelFactory(new PedestrianModelFactory()); unique_ptr algorithm(new AStarBidirectionalRoutingAlgorithm()); unique_ptr directionsEngine(new PedestrianDirectionsEngine()); unique_ptr router(new RoadGraphRouter( @@ -303,7 +303,7 @@ unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCou unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { - unique_ptr vehicleModelFactory(new BicycleModelFactory()); + unique_ptr vehicleModelFactory(new BicycleModelFactory()); unique_ptr algorithm(new AStarBidirectionalRoutingAlgorithm()); unique_ptr directionsEngine(new BicycleDirectionsEngine(index)); unique_ptr router(new RoadGraphRouter( @@ -314,7 +314,7 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { - unique_ptr vehicleModelFactory = make_unique(); + unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); // @TODO Bicycle turn generation engine is used now. It's ok for the time being. // But later a special car turn generation engine should be implemented. diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index d3fdeb7fed..88f756fc2b 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -25,7 +25,7 @@ class RoadGraphRouter : public IRouter public: RoadGraphRouter(string const & name, Index const & index, TCountryFileFn const & countryFileFn, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory, + unique_ptr && vehicleModelFactory, unique_ptr && algorithm, unique_ptr && directionsEngine); ~RoadGraphRouter() override; diff --git a/routing/route.cpp b/routing/route.cpp index 452367cd75..dfc3ee91e1 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -374,7 +374,7 @@ void Route::AppendRoute(Route const & route) size_t const polySize = m_poly.GetPolyline().GetSize(); // Appending turns. - for (turns::TurnItem t : route.m_turns) + for (auto t : route.m_turns) { if (t.m_index == 0) continue; diff --git a/routing/routing_benchmarks/bicycle_routing_tests.cpp b/routing/routing_benchmarks/bicycle_routing_tests.cpp index 1d66d22327..3853d10e64 100644 --- a/routing/routing_benchmarks/bicycle_routing_tests.cpp +++ b/routing/routing_benchmarks/bicycle_routing_tests.cpp @@ -30,9 +30,9 @@ protected: return engine; } - unique_ptr CreateModelFactory() override + unique_ptr CreateModelFactory() override { - unique_ptr factory( + unique_ptr factory( new SimplifiedModelFactory()); return factory; } diff --git a/routing/routing_benchmarks/helpers.hpp b/routing/routing_benchmarks/helpers.hpp index bf3a385e7a..f4793c0d9d 100644 --- a/routing/routing_benchmarks/helpers.hpp +++ b/routing/routing_benchmarks/helpers.hpp @@ -30,7 +30,7 @@ public: protected: virtual unique_ptr CreateDirectionsEngine() = 0; - virtual unique_ptr CreateModelFactory() = 0; + virtual unique_ptr CreateModelFactory() = 0; template unique_ptr CreateRouter(string const & name) @@ -52,7 +52,7 @@ protected: }; template -class SimplifiedModelFactory : public routing::IVehicleModelFactory +class SimplifiedModelFactory : public routing::VehicleModelFactory { public: // Since for test purposes we compare routes lengths to check @@ -79,7 +79,7 @@ public: SimplifiedModelFactory() : m_model(make_shared()) {} - // IVehicleModelFactory overrides: + // VehicleModelFactory overrides: shared_ptr GetVehicleModel() const override { return m_model; } shared_ptr GetVehicleModelForCountry( string const & /*country*/) const override diff --git a/routing/routing_benchmarks/pedestrian_routing_tests.cpp b/routing/routing_benchmarks/pedestrian_routing_tests.cpp index 144688c857..61cfcefe16 100644 --- a/routing/routing_benchmarks/pedestrian_routing_tests.cpp +++ b/routing/routing_benchmarks/pedestrian_routing_tests.cpp @@ -43,9 +43,9 @@ protected: return engine; } - unique_ptr CreateModelFactory() override + unique_ptr CreateModelFactory() override { - unique_ptr factory( + unique_ptr factory( new SimplifiedModelFactory()); return factory; } diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index bd550cc3a0..5dd93f54f3 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -40,10 +40,10 @@ public: virtual bool IsRoad(FeatureType const & f) const = 0; }; -class IVehicleModelFactory +class VehicleModelFactory { public: - virtual ~IVehicleModelFactory() {} + virtual ~VehicleModelFactory() {} /// @return Default vehicle model which corresponds for all countrines, /// but it may be non optimal for some countries From 856cd29717647aec18bcc08afb3357215495a3c9 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 27 Oct 2016 17:15:37 +0300 Subject: [PATCH 08/12] CarRouter refactoring. --- routing/car_router.cpp | 78 ++++++++++++++++++++++++------------------ routing/car_router.hpp | 4 +-- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/routing/car_router.cpp b/routing/car_router.cpp index 1c176b64f2..54632a71de 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -46,7 +46,6 @@ namespace routing { - namespace { size_t constexpr kMaxNodeCandidatesCount = 10; @@ -55,7 +54,6 @@ double constexpr kMwmLoadedProgress = 10.0f; double constexpr kPointsFoundProgress = 15.0f; double constexpr kCrossPathFoundProgress = 50.0f; double constexpr kPathFoundProgress = 70.0f; -} // namespace using RawRouteData = InternalRouteResult; @@ -199,6 +197,42 @@ private: RoutingMapping & m_routingMapping; }; +bool FindSingleOsrmRoute(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Index const & index, + Route & mwmRoute) +{ + vector mwmRouteGeometry; + Route::TTurns mwmTurns; + Route::TTimes mwmTimes; + Route::TStreets mwmStreets; + + LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), + "to", MercatorBounds::ToLatLon(target.segmentPoint))); + + RawRoutingResult routingResult; + if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) + return false; + + OSRMRoutingResult resultGraph(index, *mapping, routingResult); + if (MakeTurnAnnotation(resultGraph, delegate, mwmRouteGeometry, mwmTurns, mwmTimes, mwmStreets) + != routing::IRouter::NoError) + { + LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName())); + return false; + } + + mwmRoute.SetTurnInstructions(move(mwmTurns)); + mwmRoute.SetSectionTimes(move(mwmTimes)); + mwmRoute.SetStreetNames(move(mwmStreets)); + + vector mwmPoints; + JunctionsToPoints(mwmRouteGeometry, mwmPoints); + mwmRoute.SetGeometry(mwmPoints.cbegin(), mwmPoints.cend()); + + return true; +} +} // namespace + // static bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, TCountryFileFn const & countryFileFn, Index * index) @@ -209,8 +243,8 @@ bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD co } CarRouter::CarRouter(Index * index, TCountryFileFn const & countryFileFn, - unique_ptr roadGraphRouter) - : m_pIndex(index), m_indexManager(countryFileFn, *index), m_roadGraphRouter(move(roadGraphRouter)) + unique_ptr roadGraphRouter) + : m_pIndex(index), m_indexManager(countryFileFn, *index), m_aStarRouter(move(roadGraphRouter)) { } @@ -224,7 +258,7 @@ void CarRouter::ClearState() m_cachedTargets.clear(); m_cachedTargetPoint = m2::PointD::Zero(); m_indexManager.Clear(); - m_roadGraphRouter->ClearState(); + m_aStarRouter->ClearState(); } bool CarRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, @@ -520,20 +554,20 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu { ASSERT_EQUAL(source.mwmId, target.mwmId, ()); ASSERT(m_pIndex, ()); - ASSERT(m_roadGraphRouter, ()); + ASSERT(m_aStarRouter, ()); Route mwmRoute(GetName()); // @TODO It's not the best place for checking availability of edge index section in mwm. // Probably it's better to keep if mwm has an edge index section in mwmId. - if (IsEdgeIndexExisting(source.mwmId) && m_roadGraphRouter) + if (IsEdgeIndexExisting(source.mwmId) && m_aStarRouter) { // A* routing LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); - if (m_roadGraphRouter->CalculateRoute(source.segmentPoint, m2::PointD(0, 0), target.segmentPoint, - delegate, mwmRoute) != IRouter::NoError) + if (m_aStarRouter->CalculateRoute(source.segmentPoint, m2::PointD(0, 0), target.segmentPoint, + delegate, mwmRoute) != IRouter::NoError) { return false; } @@ -543,32 +577,8 @@ bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, Featu // OSRM Routing // @TODO This branch is implemented to support old maps with osrm section. When osrm // section is not supported this branch should be removed. - vector mwmRouteGeometry; - Route::TTurns mwmTurns; - Route::TTimes mwmTimes; - Route::TStreets mwmStreets; - - LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), - "to", MercatorBounds::ToLatLon(target.segmentPoint))); - - RawRoutingResult routingResult; - if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) + if (!FindSingleOsrmRoute(source, target, delegate, mapping, *m_pIndex, mwmRoute)) return false; - - OSRMRoutingResult resultGraph(*m_pIndex, *mapping, routingResult); - if (MakeTurnAnnotation(resultGraph, delegate, mwmRouteGeometry, mwmTurns, mwmTimes, mwmStreets) != NoError) - { - LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName())); - return false; - } - - mwmRoute.SetTurnInstructions(move(mwmTurns)); - mwmRoute.SetSectionTimes(move(mwmTimes)); - mwmRoute.SetStreetNames(move(mwmStreets)); - - vector mwmPoints; - JunctionsToPoints(mwmRouteGeometry, mwmPoints); - mwmRoute.SetGeometry(mwmPoints.cbegin(), mwmPoints.cend()); } route.AppendRoute(mwmRoute); diff --git a/routing/car_router.hpp b/routing/car_router.hpp index b94d7e85e4..7064688f42 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -31,7 +31,7 @@ public: typedef vector GeomTurnCandidateT; CarRouter(Index * index, TCountryFileFn const & countryFileFn, - unique_ptr roadGraphRouter); + unique_ptr roadGraphRouter); virtual string GetName() const override; @@ -109,6 +109,6 @@ private: RoutingIndexManager m_indexManager; - unique_ptr m_roadGraphRouter; + unique_ptr m_aStarRouter; }; } // namespace routing From d6cdf211aeb3e4918704f603935316f26c07cab3 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 2 Nov 2016 15:33:39 +0300 Subject: [PATCH 09/12] Review fixes. --- geometry/polyline2d.hpp | 2 +- map/framework.cpp | 4 +- routing/car_router.cpp | 132 +++++++++--------- routing/car_router.hpp | 24 ++-- routing/features_road_graph.cpp | 4 +- routing/features_road_graph.hpp | 4 +- routing/road_graph_router.cpp | 6 +- routing/road_graph_router.hpp | 2 +- routing/route.cpp | 16 ++- routing/route.hpp | 2 +- .../routing_test_tools.cpp | 5 +- 11 files changed, 104 insertions(+), 97 deletions(-) diff --git a/geometry/polyline2d.hpp b/geometry/polyline2d.hpp index 1c67c258a7..9f34a1ca86 100644 --- a/geometry/polyline2d.hpp +++ b/geometry/polyline2d.hpp @@ -129,5 +129,5 @@ public: } }; -typedef Polyline PolylineD; +using PolylineD = Polyline; } // namespace m2 diff --git a/map/framework.cpp b/map/framework.cpp index fc6d551a0d..71f9ea7a84 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2398,7 +2398,7 @@ void Framework::SetRouterImpl(RouterType type) return m_model.GetIndex().GetMwmIdByCountryFile(CountryFile(countryFile)).IsAlive(); }; - router.reset(new CarRouter(&m_model.GetIndex(), countryFileGetter, + router.reset(new CarRouter(m_model.GetIndex(), countryFileGetter, CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); fetcher.reset(new OnlineAbsentCountriesFetcher(countryFileGetter, localFileChecker)); m_routingSession.SetRoutingSettings(routing::GetCarRoutingSettings()); @@ -2552,7 +2552,7 @@ RouterType Framework::GetBestRouter(m2::PointD const & startPoint, m2::PointD co return m_infoGetter->GetRegionCountryId(pt); }; if (!CarRouter::CheckRoutingAbility(startPoint, finalPoint, countryFileGetter, - &m_model.GetIndex())) + m_model.GetIndex())) { return RouterType::Pedestrian; } diff --git a/routing/car_router.cpp b/routing/car_router.cpp index 54632a71de..8c5ef8b5e2 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -1,4 +1,5 @@ #include "routing/car_router.hpp" + #include "routing/cross_mwm_router.hpp" #include "routing/loaded_path_segment.hpp" #include "routing/online_cross_fetcher.hpp" @@ -197,54 +198,56 @@ private: RoutingMapping & m_routingMapping; }; -bool FindSingleOsrmRoute(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Index const & index, - Route & mwmRoute) +IRouter::ResultCode FindSingleOsrmRoute(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, Index const & index, TRoutingMappingPtr & mapping, + Route & route) { - vector mwmRouteGeometry; - Route::TTurns mwmTurns; - Route::TTimes mwmTimes; - Route::TStreets mwmStreets; + vector geometry; + Route::TTurns turns; + Route::TTimes times; + Route::TStreets streets; LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); RawRoutingResult routingResult; if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) - return false; + return routing::IRouter::RouteNotFound; OSRMRoutingResult resultGraph(index, *mapping, routingResult); - if (MakeTurnAnnotation(resultGraph, delegate, mwmRouteGeometry, mwmTurns, mwmTimes, mwmStreets) - != routing::IRouter::NoError) + routing::IRouter::ResultCode const result = + MakeTurnAnnotation(resultGraph, delegate, geometry, turns, times, streets); + if (result != routing::IRouter::NoError) { - LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName())); - return false; + LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName(), + ". result code =", result)); + return result; } - mwmRoute.SetTurnInstructions(move(mwmTurns)); - mwmRoute.SetSectionTimes(move(mwmTimes)); - mwmRoute.SetStreetNames(move(mwmStreets)); + route.SetTurnInstructions(move(turns)); + route.SetSectionTimes(move(times)); + route.SetStreetNames(move(streets)); vector mwmPoints; - JunctionsToPoints(mwmRouteGeometry, mwmPoints); - mwmRoute.SetGeometry(mwmPoints.cbegin(), mwmPoints.cend()); + JunctionsToPoints(geometry, mwmPoints); + route.SetGeometry(mwmPoints.cbegin(), mwmPoints.cend()); - return true; + return routing::IRouter::NoError; } } // namespace // static bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, - TCountryFileFn const & countryFileFn, Index * index) + TCountryFileFn const & countryFileFn, Index & index) { - RoutingIndexManager manager(countryFileFn, *index); + RoutingIndexManager manager(countryFileFn, index); return manager.GetMappingByPoint(startPoint)->IsValid() && manager.GetMappingByPoint(finalPoint)->IsValid(); } -CarRouter::CarRouter(Index * index, TCountryFileFn const & countryFileFn, - unique_ptr roadGraphRouter) - : m_pIndex(index), m_indexManager(countryFileFn, *index), m_aStarRouter(move(roadGraphRouter)) +CarRouter::CarRouter(Index & index, TCountryFileFn const & countryFileFn, + unique_ptr router) + : m_index(index), m_indexManager(countryFileFn, index), m_router(move(router)) { } @@ -258,22 +261,22 @@ void CarRouter::ClearState() m_cachedTargets.clear(); m_cachedTargetPoint = m2::PointD::Zero(); m_indexManager.Clear(); - m_aStarRouter->ClearState(); + m_router->ClearState(); } -bool CarRouter::FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) +bool CarRouter::FindRouteMsmt(TFeatureGraphNodeVec const & sources, TFeatureGraphNodeVec const & targets, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) { ASSERT(mapping, ()); Route emptyRoute(GetName()); route.Swap(emptyRoute); /// @todo (ldargunov) make more complex nearest edge turnaround - for (auto const & targetEdge : target) + for (auto const & targetEdge : targets) { - for (auto const & sourceEdge : source) + for (auto const & sourceEdge : sources) { - if (FindSingleRouteDispatcher(sourceEdge, targetEdge, delegate, mapping, route)) + if (FindSingleRouteDispatcher(sourceEdge, targetEdge, delegate, mapping, route) == NoError) return true; } } @@ -357,10 +360,12 @@ CarRouter::ResultCode CarRouter::MakeRouteFromCrossesPath(TCheckedPath const & p ASSERT(mwmMapping->IsValid(), ()); MappingGuard mwmMappingGuard(mwmMapping); UNUSED_VALUE(mwmMappingGuard); - CalculatePhantomNodeForCross(mwmMapping, cross.startNode, m_pIndex, true /* forward */); - CalculatePhantomNodeForCross(mwmMapping, cross.finalNode, m_pIndex, false /* forward */); - if (!FindSingleRouteDispatcher(cross.startNode, cross.finalNode, delegate, mwmMapping, route)) - return RouteNotFound; + CalculatePhantomNodeForCross(mwmMapping, cross.startNode, &m_index, true /* forward */); + CalculatePhantomNodeForCross(mwmMapping, cross.finalNode, &m_index, false /* forward */); + IRouter::ResultCode const result = + FindSingleRouteDispatcher(cross.startNode, cross.finalNode, delegate, mwmMapping, route); + if (result != NoError) + return result; } return NoError; @@ -439,7 +444,7 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, delegate.OnProgress(kPointsFoundProgress); // 4. Find route. - double crossCostM = 0; + double crossDistanceM = 0; TCheckedPath finalPath; // Manually load facade to avoid unmaping files we routing on. @@ -453,10 +458,10 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, { indexPair.second->FreeCrossContext(); }); - ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCostM, + ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossDistanceM, delegate, finalPath); LOG(LINFO, ("Found cross path in", timer.ElapsedNano(), "ns.")); - if (!FindRouteFromCases(startTask, m_cachedTargets, delegate, startMapping, route)) + if (!FindRouteMsmt(startTask, m_cachedTargets, delegate, startMapping, route)) { if (crossCode == NoError) { @@ -469,9 +474,9 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, } INTERRUPT_WHEN_CANCELLED(delegate); - if (crossCode == NoError && crossCostM < route.GetTotalDistanceMeters()) + if (crossCode == NoError && crossDistanceM < route.GetTotalDistanceMeters()) { - LOG(LINFO, ("Cross mwm path shorter. Cross cost:", crossCostM, "single cost:", route.GetTotalDistanceMeters())); + LOG(LINFO, ("Cross mwm path shorter. Cross cost:", crossDistanceM, "single cost:", route.GetTotalDistanceMeters())); auto code = MakeRouteFromCrossesPath(finalPath, delegate, route); LOG(LINFO, ("Made final route in", timer.ElapsedNano(), "ns.")); timer.Reset(); @@ -486,7 +491,7 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, else //4.2 Multiple mwm case { LOG(LINFO, ("Multiple mwm routing case")); - ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossCostM, + ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossDistanceM, delegate, finalPath); timer.Reset(); INTERRUPT_WHEN_CANCELLED(delegate); @@ -515,12 +520,11 @@ IRouter::ResultCode CarRouter::FindPhantomNodes(m2::PointD const & point, TRoutingMappingPtr const & mapping) { ASSERT(mapping, ()); - helpers::Point2PhantomNode getter(*mapping, *m_pIndex, direction); + helpers::Point2PhantomNode getter(*mapping, m_index, direction); getter.SetPoint(point); - m_pIndex->ForEachInRectForMWM(getter, MercatorBounds::RectByCenterXYAndSizeInMeters( - point, kFeatureFindingRectSideRadiusMeters), - scales::GetUpperScale(), mapping->GetMwmId()); + m_index.ForEachInRectForMWM(getter, MercatorBounds::RectByCenterXYAndSizeInMeters( + point, kFeatureFindingRectSideRadiusMeters), scales::GetUpperScale(), mapping->GetMwmId()); if (!getter.HasCandidates()) return RouteNotFound; @@ -529,9 +533,9 @@ IRouter::ResultCode CarRouter::FindPhantomNodes(m2::PointD const & point, return NoError; } -bool CarRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) +bool CarRouter::DoesEdgeIndexExist(Index::MwmId const & mwmId) { - MwmSet::MwmHandle const handle = m_pIndex->GetMwmHandleById(mwmId); + MwmSet::MwmHandle const handle = m_index.GetMwmHandleById(mwmId); if (!handle.IsAlive()) { ASSERT(false, ("Mwm handle is not alive.")); @@ -539,49 +543,47 @@ bool CarRouter::IsEdgeIndexExisting(Index::MwmId const & mwmId) } MwmValue const * value = handle.GetValue(); - ASSERT(value, ()); + CHECK(value, ()); if (value->GetHeader().GetFormat() < version::Format::v8) return false; - if (value->m_cont.IsExist(EDGE_INDEX_FILE_TAG)) - return true; - return false; + if (!value->m_cont.IsExist(EDGE_INDEX_FILE_TAG)) + return false; + + return true; } -bool CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, - Route & route) +IRouter::ResultCode CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, + Route & route) { ASSERT_EQUAL(source.mwmId, target.mwmId, ()); - ASSERT(m_pIndex, ()); - ASSERT(m_aStarRouter, ()); + ASSERT(m_router, ()); + IRouter::ResultCode result = IRouter::NoError; Route mwmRoute(GetName()); // @TODO It's not the best place for checking availability of edge index section in mwm. // Probably it's better to keep if mwm has an edge index section in mwmId. - if (IsEdgeIndexExisting(source.mwmId) && m_aStarRouter) + if (DoesEdgeIndexExist(source.mwmId) && m_router) { // A* routing LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); - - if (m_aStarRouter->CalculateRoute(source.segmentPoint, m2::PointD(0, 0), target.segmentPoint, - delegate, mwmRoute) != IRouter::NoError) - { - return false; - } + result = m_router->CalculateRoute(source.segmentPoint, m2::PointD(0, 0) /* direction */, + target.segmentPoint, delegate, mwmRoute); } else { // OSRM Routing // @TODO This branch is implemented to support old maps with osrm section. When osrm // section is not supported this branch should be removed. - if (!FindSingleOsrmRoute(source, target, delegate, mapping, *m_pIndex, mwmRoute)) - return false; + result = FindSingleOsrmRoute(source, target, delegate, m_index, mapping, mwmRoute); } - route.AppendRoute(mwmRoute); - return true; + if (result == IRouter::NoError) + route.AppendRoute(mwmRoute); + + return result; } } // namespace routing diff --git a/routing/car_router.hpp b/routing/car_router.hpp index 7064688f42..1e64f35927 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -30,7 +30,7 @@ class CarRouter : public IRouter public: typedef vector GeomTurnCandidateT; - CarRouter(Index * index, TCountryFileFn const & countryFileFn, + CarRouter(Index & index, TCountryFileFn const & countryFileFn, unique_ptr roadGraphRouter); virtual string GetName() const override; @@ -49,7 +49,7 @@ public: * @returns true if we can start routing process with a given data. */ static bool CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, - TCountryFileFn const & countryFileFn, Index * index); + TCountryFileFn const & countryFileFn, Index & index); protected: /*! @@ -80,35 +80,35 @@ private: // @TODO(bykoianko) When edgeidx section implementation is merged to master // this method should be moved to edge index loader. - bool IsEdgeIndexExisting(Index::MwmId const & mwmId); + bool DoesEdgeIndexExist(Index::MwmId const & mwmId); /*! * \brief Builds a route within one mwm using A* if edge index section is available and osrm otherwise. * Then reconstructs the route and restores all route attributes. * \param route The found route is added to the |route| if the method returns true. - * \return true if route is built and false otherwise. */ - bool FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, - Route & route); + IRouter::ResultCode FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, + Route & route); - /*! Finds single shortest path in a single MWM between 2 sets of edges + /*! Finds single shortest path in a single MWM between 2 sets of edges. + * It's a route from multiple sources to multiple targets. * \param source: vector of source edges to make path * \param target: vector of target edges to make path * \param facade: OSRM routing data facade to recover graph information * \param rawRoutingResult: routing result store * \return true when path exists, false otherwise. */ - bool FindRouteFromCases(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); + bool FindRouteMsmt(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, + RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); - Index const * m_pIndex; + Index & m_index; TFeatureGraphNodeVec m_cachedTargets; m2::PointD m_cachedTargetPoint; RoutingIndexManager m_indexManager; - unique_ptr m_aStarRouter; + unique_ptr m_router; }; } // namespace routing diff --git a/routing/features_road_graph.cpp b/routing/features_road_graph.cpp index 40bf8ba932..25e1cbc0b5 100644 --- a/routing/features_road_graph.cpp +++ b/routing/features_road_graph.cpp @@ -46,7 +46,7 @@ FeaturesRoadGraph::Value::Value(MwmSet::MwmHandle handle) : m_mwmHandle(move(han m_altitudeLoader = make_unique(*m_mwmHandle.GetValue()); } -FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel(unique_ptr && vehicleModelFactory) +FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel(unique_ptr vehicleModelFactory) : m_vehicleModelFactory(move(vehicleModelFactory)) , m_maxSpeedKMPH(m_vehicleModelFactory->GetVehicleModel()->GetMaxSpeed()) { @@ -108,7 +108,7 @@ void FeaturesRoadGraph::RoadInfoCache::Clear() } FeaturesRoadGraph::FeaturesRoadGraph(Index const & index, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory) + unique_ptr vehicleModelFactory) : m_index(index) , m_mode(mode) , m_vehicleModel(move(vehicleModelFactory)) diff --git a/routing/features_road_graph.hpp b/routing/features_road_graph.hpp index ecde6fb869..9e21db0d87 100644 --- a/routing/features_road_graph.hpp +++ b/routing/features_road_graph.hpp @@ -26,7 +26,7 @@ private: class CrossCountryVehicleModel : public IVehicleModel { public: - CrossCountryVehicleModel(unique_ptr && vehicleModelFactory); + CrossCountryVehicleModel(unique_ptr vehicleModelFactory); // IVehicleModel overrides: double GetSpeed(FeatureType const & f) const override; @@ -59,7 +59,7 @@ private: public: FeaturesRoadGraph(Index const & index, IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory); + unique_ptr vehicleModelFactory); static uint32_t GetStreetReadScale(); diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 71c2cecd39..c81713a6d6 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -312,15 +312,15 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr return router; } -unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) { unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); // @TODO Bicycle turn generation engine is used now. It's ok for the time being. // But later a special car turn generation engine should be implemented. unique_ptr directionsEngine = make_unique(index); - unique_ptr router = make_unique( - "astar-bidirectional-bicycle", index, countryFileFn, IRoadGraph::Mode::ObeyOnewayTag, + unique_ptr router = make_unique( + "astar-bidirectional-car", index, countryFileFn, IRoadGraph::Mode::ObeyOnewayTag, move(vehicleModelFactory), move(algorithm), move(directionsEngine)); return router; } diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index 88f756fc2b..6d0a2fc67b 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -56,5 +56,5 @@ private: unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); -unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); } // namespace routing diff --git a/routing/route.cpp b/routing/route.cpp index dfc3ee91e1..a60391cda2 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -365,39 +365,43 @@ void Route::AppendRoute(Route const & route) ASSERT(!m_times.empty(), ()); // Remove road end point and turn instruction. + ASSERT_LESS(MercatorBounds::DistanceOnEarth(m_poly.End().m_pt, route.m_poly.Begin().m_pt), 2 /* meters */, ()); m_poly.PopBack(); + CHECK(!m_turns.empty(), ()); + ASSERT_EQUAL(m_turns.back().m_turn, turns::TurnDirection::ReachedYourDestination, ()); m_turns.pop_back(); + CHECK(!m_times.empty(), ()); m_times.pop_back(); // Streets might not point to the last point of the path. } - size_t const polySize = m_poly.GetPolyline().GetSize(); + size_t const indexOffset = m_poly.GetPolyline().GetSize(); // Appending turns. for (auto t : route.m_turns) { if (t.m_index == 0) continue; - t.m_index += polySize; + t.m_index += indexOffset; m_turns.push_back(move(t)); } // Appending street names. - for (TStreetItem s : route.m_streets) + for (auto s : route.m_streets) { if (s.first == 0) continue; - s.first += polySize; + s.first += indexOffset; m_streets.push_back(move(s)); } // Appending times. double const estimationTime = m_times.empty() ? 0.0 : m_times.back().second; - for (TTimeItem t : route.m_times) + for (auto t : route.m_times) { if (t.first == 0) continue; - t.first += polySize; + t.first += indexOffset; t.second += estimationTime; m_times.push_back(move(t)); } diff --git a/routing/route.hpp b/routing/route.hpp index b3fe071212..3e464eb10e 100644 --- a/routing/route.hpp +++ b/routing/route.hpp @@ -63,7 +63,7 @@ public: /// \brief Glues all |route| attributes to |this| except for |m_altitudes|. // @TODO In the future this method should append |m_altitudes| as well. // It's not implemented now because it's not easy to do it and it'll not be used in - // the short future. The problem is routes genetated by osrm have empty |m_altitudes|. + // the short future. The problem is routes generated by osrm have empty |m_altitudes|. // So |m_altitudes| should be filled (with correnct or default values) on osrm route // reconstruction stage to be added with this method. On the other // hand it seems this method'll not be not used for bicycle and pedestrian routing diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index aae10bd28b..80ad52ad1f 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -81,8 +81,9 @@ namespace integration { return infoGetter.GetRegionCountryId(pt); }; - unique_ptr carRouter(new CarRouter(&index, countryFileGetter, - CreateCarAStarBidirectionalRouter(index, countryFileGetter))); + + auto carRouter = make_unique(index, countryFileGetter, + CreateCarAStarBidirectionalRouter(index, countryFileGetter)); return carRouter; } From 9adbfabb753753226a598b242413e3ffa8fe1198 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 2 Nov 2016 15:38:15 +0300 Subject: [PATCH 10/12] git-clang-format --- geometry/polyline2d.hpp | 23 +--- map/framework.cpp | 5 +- routing/car_model.cpp | 15 +-- routing/car_router.cpp | 111 +++++++++--------- routing/car_router.hpp | 21 ++-- routing/features_road_graph.cpp | 8 +- routing/road_graph_router.cpp | 6 +- routing/road_graph_router.hpp | 6 +- routing/route.cpp | 3 +- routing/route.hpp | 1 - routing/routing_benchmarks/helpers.hpp | 1 - .../routing_test_tools.cpp | 8 +- routing/vehicle_model.hpp | 1 - 13 files changed, 93 insertions(+), 116 deletions(-) diff --git a/geometry/polyline2d.hpp b/geometry/polyline2d.hpp index 9f34a1ca86..78594eea7f 100644 --- a/geometry/polyline2d.hpp +++ b/geometry/polyline2d.hpp @@ -11,7 +11,6 @@ namespace m2 { - template class Polyline { @@ -19,15 +18,16 @@ class Polyline public: Polyline() {} - Polyline(initializer_list > points) : m_points(points) + Polyline(initializer_list> points) : m_points(points) { ASSERT_GREATER(m_points.size(), 1, ()); } - explicit Polyline(vector > const & points) : m_points(points) + explicit Polyline(vector> const & points) : m_points(points) { ASSERT_GREATER(m_points.size(), 1, ()); } - template Polyline(IterT beg, IterT end) : m_points(beg, end) + template + Polyline(IterT beg, IterT end) : m_points(beg, end) { ASSERT_GREATER(m_points.size(), 1, ()); } @@ -67,7 +67,6 @@ public: void Clear() { m_points.clear(); } void Add(Point const & pt) { m_points.push_back(pt); } - void Append(Polyline const & poly) { m_points.insert(m_points.end(), poly.m_points.cbegin(), poly.m_points.cend()); @@ -79,15 +78,9 @@ public: m_points.pop_back(); } - void Swap(Polyline & rhs) - { - m_points.swap(rhs.m_points); - } - + void Swap(Polyline & rhs) { m_points.swap(rhs.m_points); } size_t GetSize() const { return m_points.size(); } - bool operator==(Polyline const & rhs) const { return m_points == rhs.m_points; } - typedef vector > TContainer; typedef typename TContainer::const_iterator TIter; TIter Begin() const { return m_points.begin(); } @@ -122,11 +115,7 @@ public: } vector > const & GetPoints() const { return m_points; } - - friend string DebugPrint(Polyline const & p) - { - return ::DebugPrint(p.m_points); - } + friend string DebugPrint(Polyline const & p) { return ::DebugPrint(p.m_points); } }; using PolylineD = Polyline; diff --git a/map/framework.cpp b/map/framework.cpp index 71f9ea7a84..7a9c551d4c 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -2398,8 +2398,9 @@ void Framework::SetRouterImpl(RouterType type) return m_model.GetIndex().GetMwmIdByCountryFile(CountryFile(countryFile)).IsAlive(); }; - router.reset(new CarRouter(m_model.GetIndex(), countryFileGetter, - CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); + router.reset( + new CarRouter(m_model.GetIndex(), countryFileGetter, + CreateCarAStarBidirectionalRouter(m_model.GetIndex(), countryFileGetter))); fetcher.reset(new OnlineAbsentCountriesFetcher(countryFileGetter, localFileChecker)); m_routingSession.SetRoutingSettings(routing::GetCarRoutingSettings()); } diff --git a/routing/car_model.cpp b/routing/car_model.cpp index f5453836cf..75b78b93c8 100644 --- a/routing/car_model.cpp +++ b/routing/car_model.cpp @@ -81,17 +81,10 @@ CarModel const & CarModel::AllLimitsInstance() return instance; } -CarModelFactory::CarModelFactory() -{ - m_model = make_shared(); -} - -shared_ptr CarModelFactory::GetVehicleModel() const -{ - return m_model; -} - -shared_ptr CarModelFactory::GetVehicleModelForCountry(string const & /* country */) const +CarModelFactory::CarModelFactory() { m_model = make_shared(); } +shared_ptr CarModelFactory::GetVehicleModel() const { return m_model; } +shared_ptr CarModelFactory::GetVehicleModelForCountry( + string const & /* country */) const { // @TODO(bykoianko) Different vehicle model for different country should be supported // according to http://wiki.openstreetmap.org/wiki/OSM_tags_for_routing/Access-Restrictions. diff --git a/routing/car_router.cpp b/routing/car_router.cpp index 8c5ef8b5e2..861ad430d6 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -34,15 +34,15 @@ #include "std/limits.hpp" #include "std/string.hpp" -#include "3party/osrm/osrm-backend/data_structures/query_edge.hpp" #include "3party/osrm/osrm-backend/data_structures/internal_route_result.hpp" +#include "3party/osrm/osrm-backend/data_structures/query_edge.hpp" #include "3party/osrm/osrm-backend/descriptors/description_factory.hpp" #define INTERRUPT_WHEN_CANCELLED(DELEGATE) \ - do \ - { \ - if (DELEGATE.IsCancelled()) \ - return Cancelled; \ + do \ + { \ + if (DELEGATE.IsCancelled()) \ + return Cancelled; \ } while (false) namespace routing @@ -63,7 +63,6 @@ class OSRMRoutingResult : public turns::IRoutingResult public: // turns::IRoutingResult overrides: TUnpackedPathSegments const & GetSegments() const override { return m_loadedSegments; } - void GetPossibleTurns(TNodeId node, m2::PointD const & ingoingPoint, m2::PointD const & junctionPoint, size_t & ingoingCount, turns::TurnCandidates & outgoingTurns) const override @@ -147,14 +146,12 @@ public: } sort(outgoingTurns.candidates.begin(), outgoingTurns.candidates.end(), - [](turns::TurnCandidate const & t1, turns::TurnCandidate const & t2) - { - return t1.angle < t2.angle; - }); + [](turns::TurnCandidate const & t1, turns::TurnCandidate const & t2) { + return t1.angle < t2.angle; + }); } double GetPathLength() const override { return m_rawResult.shortestPathLength; } - Junction GetStartPoint() const override { return Junction(m_rawResult.sourceEdge.segmentPoint, feature::kDefaultAltitudeMeters); @@ -178,14 +175,14 @@ public: bool isEndNode = (segmentIndex == numSegments - 1); if (isStartNode || isEndNode) { - OsrmPathSegmentFactory(m_routingMapping, m_index, - pathSegments[segmentIndex], m_rawResult.sourceEdge, - m_rawResult.targetEdge, isStartNode, isEndNode, m_loadedSegments[segmentIndex]); + OsrmPathSegmentFactory(m_routingMapping, m_index, pathSegments[segmentIndex], + m_rawResult.sourceEdge, m_rawResult.targetEdge, isStartNode, + isEndNode, m_loadedSegments[segmentIndex]); } else { - OsrmPathSegmentFactory(m_routingMapping, m_index, - pathSegments[segmentIndex], m_loadedSegments[segmentIndex]); + OsrmPathSegmentFactory(m_routingMapping, m_index, pathSegments[segmentIndex], + m_loadedSegments[segmentIndex]); } } } @@ -198,17 +195,18 @@ private: RoutingMapping & m_routingMapping; }; -IRouter::ResultCode FindSingleOsrmRoute(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, Index const & index, TRoutingMappingPtr & mapping, - Route & route) +IRouter::ResultCode FindSingleOsrmRoute(FeatureGraphNode const & source, + FeatureGraphNode const & target, + RouterDelegate const & delegate, Index const & index, + TRoutingMappingPtr & mapping, Route & route) { vector geometry; Route::TTurns turns; Route::TTimes times; Route::TStreets streets; - LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), - "to", MercatorBounds::ToLatLon(target.segmentPoint))); + LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", + MercatorBounds::ToLatLon(target.segmentPoint))); RawRoutingResult routingResult; if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) @@ -234,7 +232,7 @@ IRouter::ResultCode FindSingleOsrmRoute(FeatureGraphNode const & source, Feature return routing::IRouter::NoError; } -} // namespace +} // namespace // static bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD const & finalPoint, @@ -247,15 +245,11 @@ bool CarRouter::CheckRoutingAbility(m2::PointD const & startPoint, m2::PointD co CarRouter::CarRouter(Index & index, TCountryFileFn const & countryFileFn, unique_ptr router) - : m_index(index), m_indexManager(countryFileFn, index), m_router(move(router)) + : m_index(index), m_indexManager(countryFileFn, index), m_router(move(router)) { } -string CarRouter::GetName() const -{ - return "vehicle"; -} - +string CarRouter::GetName() const { return "vehicle"; } void CarRouter::ClearState() { m_cachedTargets.clear(); @@ -264,8 +258,9 @@ void CarRouter::ClearState() m_router->ClearState(); } -bool CarRouter::FindRouteMsmt(TFeatureGraphNodeVec const & sources, TFeatureGraphNodeVec const & targets, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) +bool CarRouter::FindRouteMsmt(TFeatureGraphNodeVec const & sources, + TFeatureGraphNodeVec const & targets, RouterDelegate const & delegate, + TRoutingMappingPtr & mapping, Route & route) { ASSERT(mapping, ()); @@ -283,9 +278,8 @@ bool CarRouter::FindRouteMsmt(TFeatureGraphNodeVec const & sources, TFeatureGrap return false; } -void FindGraphNodeOffsets(uint32_t const nodeId, m2::PointD const & point, - Index const * pIndex, TRoutingMappingPtr & mapping, - FeatureGraphNode & graphNode) +void FindGraphNodeOffsets(uint32_t const nodeId, m2::PointD const & point, Index const * pIndex, + TRoutingMappingPtr & mapping, FeatureGraphNode & graphNode) { graphNode.segmentPoint = point; @@ -421,17 +415,16 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, TFeatureGraphNodeVec startTask; { - ResultCode const code = FindPhantomNodes(startPoint, startDirection, - startTask, kMaxNodeCandidatesCount, startMapping); + ResultCode const code = FindPhantomNodes(startPoint, startDirection, startTask, + kMaxNodeCandidatesCount, startMapping); if (code != NoError) return code; } { if (finalPoint != m_cachedTargetPoint) { - ResultCode const code = - FindPhantomNodes(finalPoint, m2::PointD::Zero(), - m_cachedTargets, kMaxNodeCandidatesCount, targetMapping); + ResultCode const code = FindPhantomNodes(finalPoint, m2::PointD::Zero(), m_cachedTargets, + kMaxNodeCandidatesCount, targetMapping); if (code != NoError) return code; m_cachedTargetPoint = finalPoint; @@ -454,12 +447,11 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, if (startMapping->GetMwmId() == targetMapping->GetMwmId()) { LOG(LINFO, ("Single mwm routing case")); - m_indexManager.ForEachMapping([](pair const & indexPair) - { - indexPair.second->FreeCrossContext(); - }); - ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossDistanceM, - delegate, finalPath); + m_indexManager.ForEachMapping([](pair const & indexPair) { + indexPair.second->FreeCrossContext(); + }); + ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, + crossDistanceM, delegate, finalPath); LOG(LINFO, ("Found cross path in", timer.ElapsedNano(), "ns.")); if (!FindRouteMsmt(startTask, m_cachedTargets, delegate, startMapping, route)) { @@ -476,7 +468,8 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, if (crossCode == NoError && crossDistanceM < route.GetTotalDistanceMeters()) { - LOG(LINFO, ("Cross mwm path shorter. Cross cost:", crossDistanceM, "single cost:", route.GetTotalDistanceMeters())); + LOG(LINFO, ("Cross mwm path shorter. Cross distance:", crossDistanceM, "single distance:", + route.GetTotalDistanceMeters())); auto code = MakeRouteFromCrossesPath(finalPath, delegate, route); LOG(LINFO, ("Made final route in", timer.ElapsedNano(), "ns.")); timer.Reset(); @@ -488,11 +481,11 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, return NoError; } - else //4.2 Multiple mwm case + else // 4.2 Multiple mwm case { LOG(LINFO, ("Multiple mwm routing case")); - ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossDistanceM, - delegate, finalPath); + ResultCode code = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, + crossDistanceM, delegate, finalPath); timer.Reset(); INTERRUPT_WHEN_CANCELLED(delegate); delegate.OnProgress(kCrossPathFoundProgress); @@ -502,10 +495,9 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, { auto code = MakeRouteFromCrossesPath(finalPath, delegate, route); // Manually free all cross context allocations before geometry unpacking. - m_indexManager.ForEachMapping([](pair const & indexPair) - { - indexPair.second->FreeCrossContext(); - }); + m_indexManager.ForEachMapping([](pair const & indexPair) { + indexPair.second->FreeCrossContext(); + }); LOG(LINFO, ("Made final route in", timer.ElapsedNano(), "ns.")); timer.Reset(); return code; @@ -524,7 +516,8 @@ IRouter::ResultCode CarRouter::FindPhantomNodes(m2::PointD const & point, getter.SetPoint(point); m_index.ForEachInRectForMWM(getter, MercatorBounds::RectByCenterXYAndSizeInMeters( - point, kFeatureFindingRectSideRadiusMeters), scales::GetUpperScale(), mapping->GetMwmId()); + point, kFeatureFindingRectSideRadiusMeters), + scales::GetUpperScale(), mapping->GetMwmId()); if (!getter.HasCandidates()) return RouteNotFound; @@ -553,9 +546,11 @@ bool CarRouter::DoesEdgeIndexExist(Index::MwmId const & mwmId) return true; } -IRouter::ResultCode CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, - Route & route) +IRouter::ResultCode CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const & source, + FeatureGraphNode const & target, + RouterDelegate const & delegate, + TRoutingMappingPtr & mapping, + Route & route) { ASSERT_EQUAL(source.mwmId, target.mwmId, ()); ASSERT(m_router, ()); @@ -568,8 +563,8 @@ IRouter::ResultCode CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const if (DoesEdgeIndexExist(source.mwmId) && m_router) { // A* routing - LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), - "to", MercatorBounds::ToLatLon(target.segmentPoint))); + LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", + MercatorBounds::ToLatLon(target.segmentPoint))); result = m_router->CalculateRoute(source.segmentPoint, m2::PointD(0, 0) /* direction */, target.segmentPoint, delegate, mwmRoute); } diff --git a/routing/car_router.hpp b/routing/car_router.hpp index 1e64f35927..396e6502ec 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -9,7 +9,10 @@ #include "std/unique_ptr.hpp" #include "std/vector.hpp" -namespace feature { class TypesHolder; } +namespace feature +{ +class TypesHolder; +} class Index; struct RawRouteData; @@ -62,9 +65,9 @@ protected: * \param mapping Reference to routing indexes. * \return NoError if there are any nodes in res. RouteNotFound otherwise. */ - IRouter::ResultCode FindPhantomNodes(m2::PointD const & point, - m2::PointD const & direction, TFeatureGraphNodeVec & res, - size_t maxCount, TRoutingMappingPtr const & mapping); + IRouter::ResultCode FindPhantomNodes(m2::PointD const & point, m2::PointD const & direction, + TFeatureGraphNodeVec & res, size_t maxCount, + TRoutingMappingPtr const & mapping); private: /*! @@ -83,13 +86,15 @@ private: bool DoesEdgeIndexExist(Index::MwmId const & mwmId); /*! - * \brief Builds a route within one mwm using A* if edge index section is available and osrm otherwise. + * \brief Builds a route within one mwm using A* if edge index section is available and osrm + * otherwise. * Then reconstructs the route and restores all route attributes. * \param route The found route is added to the |route| if the method returns true. */ - IRouter::ResultCode FindSingleRouteDispatcher(FeatureGraphNode const & source, FeatureGraphNode const & target, - RouterDelegate const & delegate, TRoutingMappingPtr & mapping, - Route & route); + IRouter::ResultCode FindSingleRouteDispatcher(FeatureGraphNode const & source, + FeatureGraphNode const & target, + RouterDelegate const & delegate, + TRoutingMappingPtr & mapping, Route & route); /*! Finds single shortest path in a single MWM between 2 sets of edges. * It's a route from multiple sources to multiple targets. diff --git a/routing/features_road_graph.cpp b/routing/features_road_graph.cpp index 25e1cbc0b5..79c61a0a58 100644 --- a/routing/features_road_graph.cpp +++ b/routing/features_road_graph.cpp @@ -46,7 +46,8 @@ FeaturesRoadGraph::Value::Value(MwmSet::MwmHandle handle) : m_mwmHandle(move(han m_altitudeLoader = make_unique(*m_mwmHandle.GetValue()); } -FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel(unique_ptr vehicleModelFactory) +FeaturesRoadGraph::CrossCountryVehicleModel::CrossCountryVehicleModel( + unique_ptr vehicleModelFactory) : m_vehicleModelFactory(move(vehicleModelFactory)) , m_maxSpeedKMPH(m_vehicleModelFactory->GetVehicleModel()->GetMaxSpeed()) { @@ -106,12 +107,9 @@ void FeaturesRoadGraph::RoadInfoCache::Clear() { m_cache.clear(); } - FeaturesRoadGraph::FeaturesRoadGraph(Index const & index, IRoadGraph::Mode mode, unique_ptr vehicleModelFactory) - : m_index(index) - , m_mode(mode) - , m_vehicleModel(move(vehicleModelFactory)) + : m_index(index), m_mode(mode), m_vehicleModel(move(vehicleModelFactory)) { } diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index c81713a6d6..df437d399d 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -1,3 +1,4 @@ +#include "routing/road_graph_router.hpp" #include "routing/bicycle_directions.hpp" #include "routing/bicycle_model.hpp" #include "routing/car_model.hpp" @@ -5,7 +6,6 @@ #include "routing/nearest_edge_finder.hpp" #include "routing/pedestrian_directions.hpp" #include "routing/pedestrian_model.hpp" -#include "routing/road_graph_router.hpp" #include "routing/route.hpp" #include "coding/reader_wrapper.hpp" @@ -137,7 +137,6 @@ void FindClosestEdges(IRoadGraph const & graph, m2::PointD const & point, } // namespace RoadGraphRouter::~RoadGraphRouter() {} - RoadGraphRouter::RoadGraphRouter(string const & name, Index const & index, TCountryFileFn const & countryFileFn, IRoadGraph::Mode mode, unique_ptr && vehicleModelFactory, @@ -312,7 +311,8 @@ unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountr return router; } -unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn) +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, + TCountryFileFn const & countryFileFn) { unique_ptr vehicleModelFactory = make_unique(); unique_ptr algorithm = make_unique(); diff --git a/routing/road_graph_router.hpp b/routing/road_graph_router.hpp index 6d0a2fc67b..14405a38d6 100644 --- a/routing/road_graph_router.hpp +++ b/routing/road_graph_router.hpp @@ -24,8 +24,7 @@ class RoadGraphRouter : public IRouter { public: RoadGraphRouter(string const & name, Index const & index, TCountryFileFn const & countryFileFn, - IRoadGraph::Mode mode, - unique_ptr && vehicleModelFactory, + IRoadGraph::Mode mode, unique_ptr && vehicleModelFactory, unique_ptr && algorithm, unique_ptr && directionsEngine); ~RoadGraphRouter() override; @@ -56,5 +55,6 @@ private: unique_ptr CreatePedestrianAStarRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreatePedestrianAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); unique_ptr CreateBicycleAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); -unique_ptr CreateCarAStarBidirectionalRouter(Index & index, TCountryFileFn const & countryFileFn); +unique_ptr CreateCarAStarBidirectionalRouter(Index & index, + TCountryFileFn const & countryFileFn); } // namespace routing diff --git a/routing/route.cpp b/routing/route.cpp index a60391cda2..d0adb8f24e 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -365,7 +365,8 @@ void Route::AppendRoute(Route const & route) ASSERT(!m_times.empty(), ()); // Remove road end point and turn instruction. - ASSERT_LESS(MercatorBounds::DistanceOnEarth(m_poly.End().m_pt, route.m_poly.Begin().m_pt), 2 /* meters */, ()); + ASSERT_LESS(MercatorBounds::DistanceOnEarth(m_poly.End().m_pt, route.m_poly.Begin().m_pt), + 2 /* meters */, ()); m_poly.PopBack(); CHECK(!m_turns.empty(), ()); ASSERT_EQUAL(m_turns.back().m_turn, turns::TurnDirection::ReachedYourDestination, ()); diff --git a/routing/route.hpp b/routing/route.hpp index 3e464eb10e..f364fb747b 100644 --- a/routing/route.hpp +++ b/routing/route.hpp @@ -59,7 +59,6 @@ public: inline void SetSectionTimes(TTimes && v) { m_times = move(v); } inline void SetStreetNames(TStreets && v) { m_streets = move(v); } inline void SetAltitudes(feature::TAltitudes && v) { m_altitudes = move(v); } - /// \brief Glues all |route| attributes to |this| except for |m_altitudes|. // @TODO In the future this method should append |m_altitudes| as well. // It's not implemented now because it's not easy to do it and it'll not be used in diff --git a/routing/routing_benchmarks/helpers.hpp b/routing/routing_benchmarks/helpers.hpp index f4793c0d9d..9d442d97e1 100644 --- a/routing/routing_benchmarks/helpers.hpp +++ b/routing/routing_benchmarks/helpers.hpp @@ -78,7 +78,6 @@ public: }; SimplifiedModelFactory() : m_model(make_shared()) {} - // VehicleModelFactory overrides: shared_ptr GetVehicleModel() const override { return m_model; } shared_ptr GetVehicleModelForCountry( diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index 80ad52ad1f..378d8513ac 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -77,13 +77,12 @@ namespace integration unique_ptr CreateCarRouter(Index & index, storage::CountryInfoGetter const & infoGetter) { - auto const countryFileGetter = [&infoGetter](m2::PointD const & pt) - { + auto const countryFileGetter = [&infoGetter](m2::PointD const & pt) { return infoGetter.GetRegionCountryId(pt); }; - auto carRouter = make_unique(index, countryFileGetter, - CreateCarAStarBidirectionalRouter(index, countryFileGetter)); + auto carRouter = make_unique( + index, countryFileGetter, CreateCarAStarBidirectionalRouter(index, countryFileGetter)); return carRouter; } @@ -111,7 +110,6 @@ namespace integration } IRouter * GetRouter() const override { return m_carRouter.get(); } - private: unique_ptr m_carRouter; }; diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index 5dd93f54f3..f0ff71c298 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -44,7 +44,6 @@ class VehicleModelFactory { public: virtual ~VehicleModelFactory() {} - /// @return Default vehicle model which corresponds for all countrines, /// but it may be non optimal for some countries virtual shared_ptr GetVehicleModel() const = 0; From 12934d1ea9ca94f25f022ed31c10b9db0cfc42ff Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 3 Nov 2016 07:31:43 +0300 Subject: [PATCH 11/12] Review fixes. --- defines.hpp | 2 +- geometry/polyline2d.hpp | 23 ++++++++++--------- map/bookmark.cpp | 2 +- map/track.cpp | 2 +- routing/car_router.cpp | 43 ++++++++++++++++++++--------------- routing/car_router.hpp | 8 +++---- routing/road_graph_router.cpp | 1 + routing/route.cpp | 7 +++--- routing/router.hpp | 3 ++- 9 files changed, 51 insertions(+), 40 deletions(-) diff --git a/defines.hpp b/defines.hpp index f0db7c48ec..9eb0cd88de 100644 --- a/defines.hpp +++ b/defines.hpp @@ -27,7 +27,7 @@ #define METADATA_FILE_TAG "meta" #define METADATA_INDEX_FILE_TAG "metaidx" #define ALTITUDES_FILE_TAG "altitudes" -#define EDGE_INDEX_FILE_TAG "edgeidx" +#define ROUTING_FILE_TAG "routing" #define FEATURE_OFFSETS_FILE_TAG "offs" #define RANKS_FILE_TAG "ranks" #define REGION_INFO_FILE_TAG "rgninfo" diff --git a/geometry/polyline2d.hpp b/geometry/polyline2d.hpp index 78594eea7f..40b4a99da6 100644 --- a/geometry/polyline2d.hpp +++ b/geometry/polyline2d.hpp @@ -17,8 +17,11 @@ class Polyline vector > m_points; public: + using Container = vector>; + using Iter = typename Container::const_iterator; + Polyline() {} - Polyline(initializer_list> points) : m_points(points) + Polyline(initializer_list> const & points) : m_points(points) { ASSERT_GREATER(m_points.size(), 1, ()); } @@ -26,8 +29,8 @@ public: { ASSERT_GREATER(m_points.size(), 1, ()); } - template - Polyline(IterT beg, IterT end) : m_points(beg, end) + template + Polyline(Iter beg, Iter end) : m_points(beg, end) { ASSERT_GREATER(m_points.size(), 1, ()); } @@ -46,8 +49,8 @@ public: double res = numeric_limits::max(); m2::DistanceToLineSquare > d; - TIter i = Begin(); - for (TIter j = i + 1; j != End(); ++i, ++j) + Iter i = Begin(); + for (Iter j = i + 1; j != End(); ++i, ++j) { d.SetBounds(*i, *j); res = min(res, d(point)); @@ -80,11 +83,9 @@ public: void Swap(Polyline & rhs) { m_points.swap(rhs.m_points); } size_t GetSize() const { return m_points.size(); } - bool operator==(Polyline const & rhs) const { return m_points == rhs.m_points; } - typedef vector > TContainer; - typedef typename TContainer::const_iterator TIter; - TIter Begin() const { return m_points.begin(); } - TIter End() const { return m_points.end(); } + bool operator==(Polyline const & rhs) const { return m_points == rhs.m_points; } + Iter Begin() const { return m_points.begin(); } + Iter End() const { return m_points.end(); } Point const & Front() const { return m_points.front(); } Point const & Back() const { return m_points.back(); } @@ -115,7 +116,7 @@ public: } vector > const & GetPoints() const { return m_points; } - friend string DebugPrint(Polyline const & p) { return ::DebugPrint(p.m_points); } + friend string DebugPrint(Polyline const & p) { return ::DebugPrint(p.m_points); } }; using PolylineD = Polyline; diff --git a/map/bookmark.cpp b/map/bookmark.cpp index 58d8b3e4c8..644f91beec 100644 --- a/map/bookmark.cpp +++ b/map/bookmark.cpp @@ -769,7 +769,7 @@ void BookmarkCategory::SaveToKML(ostream & s) s << " "; Track::PolylineD const & poly = track->GetPolyline(); - for (Track::PolylineD::TIter pt = poly.Begin(); pt != poly.End(); ++pt) + for (auto pt = poly.Begin(); pt != poly.End(); ++pt) s << PointToString(*pt) << " "; s << " \n" diff --git a/map/track.cpp b/map/track.cpp index fa9cc3a02e..f521c2ead6 100644 --- a/map/track.cpp +++ b/map/track.cpp @@ -27,7 +27,7 @@ double Track::GetLengthMeters() const { double res = 0.0; - PolylineD::TIter i = m_polyline.Begin(); + auto i = m_polyline.Begin(); double lat1 = MercatorBounds::YToLat(i->y); double lon1 = MercatorBounds::XToLon(i->x); for (++i; i != m_polyline.End(); ++i) diff --git a/routing/car_router.cpp b/routing/car_router.cpp index 861ad430d6..4ddd1260ba 100644 --- a/routing/car_router.cpp +++ b/routing/car_router.cpp @@ -208,17 +208,17 @@ IRouter::ResultCode FindSingleOsrmRoute(FeatureGraphNode const & source, LOG(LINFO, ("OSRM route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); - RawRoutingResult routingResult; - if (!FindSingleRoute(source, target, mapping->m_dataFacade, routingResult)) + RawRoutingResult rawRoutingResult; + if (!FindSingleRoute(source, target, mapping->m_dataFacade, rawRoutingResult)) return routing::IRouter::RouteNotFound; - OSRMRoutingResult resultGraph(index, *mapping, routingResult); + OSRMRoutingResult routingResult(index, *mapping, rawRoutingResult); routing::IRouter::ResultCode const result = - MakeTurnAnnotation(resultGraph, delegate, geometry, turns, times, streets); + MakeTurnAnnotation(routingResult, delegate, geometry, turns, times, streets); if (result != routing::IRouter::NoError) { LOG(LWARNING, ("Can't load road path data from disk for", mapping->GetCountryName(), - ". result code =", result)); + ". Result code =", result)); return result; } @@ -249,16 +249,21 @@ CarRouter::CarRouter(Index & index, TCountryFileFn const & countryFileFn, { } -string CarRouter::GetName() const { return "vehicle"; } +string CarRouter::GetName() const { return "mixed-car"; } + void CarRouter::ClearState() { m_cachedTargets.clear(); m_cachedTargetPoint = m2::PointD::Zero(); m_indexManager.Clear(); - m_router->ClearState(); + + if (m_router) + m_router->ClearState(); + else + LOG(LERROR, ("m_router is not initialized.")); } -bool CarRouter::FindRouteMsmt(TFeatureGraphNodeVec const & sources, +bool CarRouter::FindRouteMSMT(TFeatureGraphNodeVec const & sources, TFeatureGraphNodeVec const & targets, RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route) { @@ -453,7 +458,7 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, ResultCode crossCode = CalculateCrossMwmPath(startTask, m_cachedTargets, m_indexManager, crossDistanceM, delegate, finalPath); LOG(LINFO, ("Found cross path in", timer.ElapsedNano(), "ns.")); - if (!FindRouteMsmt(startTask, m_cachedTargets, delegate, startMapping, route)) + if (!FindRouteMSMT(startTask, m_cachedTargets, delegate, startMapping, route)) { if (crossCode == NoError) { @@ -468,8 +473,8 @@ CarRouter::ResultCode CarRouter::CalculateRoute(m2::PointD const & startPoint, if (crossCode == NoError && crossDistanceM < route.GetTotalDistanceMeters()) { - LOG(LINFO, ("Cross mwm path shorter. Cross distance:", crossDistanceM, "single distance:", - route.GetTotalDistanceMeters())); + LOG(LINFO, ("Cross mwm path is shorter than single mwm path. Cross distance:", + crossDistanceM, "single distance:", route.GetTotalDistanceMeters())); auto code = MakeRouteFromCrossesPath(finalPath, delegate, route); LOG(LINFO, ("Made final route in", timer.ElapsedNano(), "ns.")); timer.Reset(); @@ -540,7 +545,7 @@ bool CarRouter::DoesEdgeIndexExist(Index::MwmId const & mwmId) if (value->GetHeader().GetFormat() < version::Format::v8) return false; - if (!value->m_cont.IsExist(EDGE_INDEX_FILE_TAG)) + if (!value->m_cont.IsExist(ROUTING_FILE_TAG)) return false; return true; @@ -553,17 +558,19 @@ IRouter::ResultCode CarRouter::FindSingleRouteDispatcher(FeatureGraphNode const Route & route) { ASSERT_EQUAL(source.mwmId, target.mwmId, ()); - ASSERT(m_router, ()); - - IRouter::ResultCode result = IRouter::NoError; + IRouter::ResultCode result = IRouter::InternalError; Route mwmRoute(GetName()); // @TODO It's not the best place for checking availability of edge index section in mwm. // Probably it's better to keep if mwm has an edge index section in mwmId. - if (DoesEdgeIndexExist(source.mwmId) && m_router) + if (DoesEdgeIndexExist(source.mwmId)) { - // A* routing - LOG(LINFO, ("A* route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", + if (!m_router) + { + LOG(LERROR, ("m_router is not initialized.")); + return IRouter::InternalError; + } + LOG(LINFO, (m_router->GetName(), "route from", MercatorBounds::ToLatLon(source.segmentPoint), "to", MercatorBounds::ToLatLon(target.segmentPoint))); result = m_router->CalculateRoute(source.segmentPoint, m2::PointD(0, 0) /* direction */, target.segmentPoint, delegate, mwmRoute); diff --git a/routing/car_router.hpp b/routing/car_router.hpp index 396e6502ec..60e4f23946 100644 --- a/routing/car_router.hpp +++ b/routing/car_router.hpp @@ -81,8 +81,8 @@ private: ResultCode MakeRouteFromCrossesPath(TCheckedPath const & path, RouterDelegate const & delegate, Route & route); - // @TODO(bykoianko) When edgeidx section implementation is merged to master - // this method should be moved to edge index loader. + // @TODO(bykoianko) When routing section implementation is merged to master + // this method should be moved to routing loader. bool DoesEdgeIndexExist(Index::MwmId const & mwmId); /*! @@ -97,14 +97,14 @@ private: TRoutingMappingPtr & mapping, Route & route); /*! Finds single shortest path in a single MWM between 2 sets of edges. - * It's a route from multiple sources to multiple targets. + * It's a route from multiple sources to multiple targets (MSMT). * \param source: vector of source edges to make path * \param target: vector of target edges to make path * \param facade: OSRM routing data facade to recover graph information * \param rawRoutingResult: routing result store * \return true when path exists, false otherwise. */ - bool FindRouteMsmt(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, + bool FindRouteMSMT(TFeatureGraphNodeVec const & source, TFeatureGraphNodeVec const & target, RouterDelegate const & delegate, TRoutingMappingPtr & mapping, Route & route); Index & m_index; diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index df437d399d..06409c83ef 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -1,4 +1,5 @@ #include "routing/road_graph_router.hpp" + #include "routing/bicycle_directions.hpp" #include "routing/bicycle_model.hpp" #include "routing/car_model.hpp" diff --git a/routing/route.cpp b/routing/route.cpp index d0adb8f24e..4b9b174ea9 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -359,10 +359,13 @@ void Route::AppendRoute(Route const & route) if (!route.IsValid()) return; + double const estimatedTime = m_times.empty() ? 0.0 : m_times.back().second; if (m_poly.GetPolyline().GetSize() != 0) { ASSERT(!m_turns.empty(), ()); ASSERT(!m_times.empty(), ()); + if (!m_streets.empty()) + ASSERT_LESS(m_streets.back().first, m_poly.GetPolyline().GetSize(), ()); // Remove road end point and turn instruction. ASSERT_LESS(MercatorBounds::DistanceOnEarth(m_poly.End().m_pt, route.m_poly.Begin().m_pt), @@ -373,7 +376,6 @@ void Route::AppendRoute(Route const & route) m_turns.pop_back(); CHECK(!m_times.empty(), ()); m_times.pop_back(); - // Streets might not point to the last point of the path. } size_t const indexOffset = m_poly.GetPolyline().GetSize(); @@ -397,13 +399,12 @@ void Route::AppendRoute(Route const & route) } // Appending times. - double const estimationTime = m_times.empty() ? 0.0 : m_times.back().second; for (auto t : route.m_times) { if (t.first == 0) continue; t.first += indexOffset; - t.second += estimationTime; + t.second += estimatedTime; m_times.push_back(move(t)); } diff --git a/routing/router.hpp b/routing/router.hpp index bd96dd6dad..1e8e0da7d9 100644 --- a/routing/router.hpp +++ b/routing/router.hpp @@ -19,7 +19,8 @@ class Route; /// Routing engine type. enum class RouterType { - Vehicle = 0, /// For OSRM vehicle routing + // @TODO It's necessary to rename Vehicle value to Car. + Vehicle = 0, /// For Car routing (OSRM or AStar) Pedestrian, /// For A star pedestrian routing Bicycle, /// For A star bicycle routing Taxi, /// For taxi route calculation Vehicle routing is used. From cbf1ea10814694c0d4c60f3b3e0895e9fe3e30fb Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 3 Nov 2016 19:26:19 +0300 Subject: [PATCH 12/12] More strict conditions for Route::m_streets. --- routing/route.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/routing/route.cpp b/routing/route.cpp index 4b9b174ea9..7ad678977e 100644 --- a/routing/route.cpp +++ b/routing/route.cpp @@ -365,7 +365,7 @@ void Route::AppendRoute(Route const & route) ASSERT(!m_turns.empty(), ()); ASSERT(!m_times.empty(), ()); if (!m_streets.empty()) - ASSERT_LESS(m_streets.back().first, m_poly.GetPolyline().GetSize(), ()); + ASSERT_LESS(m_streets.back().first + 1, m_poly.GetPolyline().GetSize(), ()); // Remove road end point and turn instruction. ASSERT_LESS(MercatorBounds::DistanceOnEarth(m_poly.End().m_pt, route.m_poly.Begin().m_pt),