From a8cdf86dbd46774ef13abe6eff1ed9f90f11e517 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 2 Aug 2017 15:40:34 +0300 Subject: [PATCH] Review fixes. --- map/routing_manager.cpp | 5 +- openlr/router.cpp | 2 +- routing/index_router.cpp | 175 ++++++------------ routing/index_router.hpp | 61 ++---- routing/nearest_edge_finder.cpp | 29 +-- routing/nearest_edge_finder.hpp | 22 +-- routing/road_graph.cpp | 18 +- routing/road_graph.hpp | 16 +- routing/road_graph_router.cpp | 2 +- .../routing_test_tools.cpp | 7 +- routing/routing_session.cpp | 13 +- routing/routing_session.hpp | 3 +- routing/routing_tests/index_graph_test.cpp | 77 ++++---- .../nearest_edge_finder_tests.cpp | 8 +- 14 files changed, 198 insertions(+), 240 deletions(-) diff --git a/map/routing_manager.cpp b/map/routing_manager.cpp index c06807dc4a..1bc11104ed 100644 --- a/map/routing_manager.cpp +++ b/map/routing_manager.cpp @@ -389,9 +389,8 @@ void RoutingManager::SetRouterImpl(RouterType type) }; auto fetcher = make_unique(countryFileGetter, localFileChecker); - auto router = IndexRouter::Create(vehicleType, countryFileGetter, getMwmRectByName, numMwmIds, - MakeNumMwmTree(*numMwmIds, m_callbacks.m_countryInfoGetter()), - m_routingSession, index); + auto router = make_unique(vehicleType, countryFileGetter, getMwmRectByName, numMwmIds, + MakeNumMwmTree(*numMwmIds, m_callbacks.m_countryInfoGetter()), m_routingSession, index); m_routingSession.SetRoutingSettings(GetRoutingSettings(vehicleType)); m_routingSession.SetRouter(std::move(router), std::move(fetcher)); diff --git a/openlr/router.cpp b/openlr/router.cpp index d1a7ac9ec0..a5ce4c442e 100644 --- a/openlr/router.cpp +++ b/openlr/router.cpp @@ -22,7 +22,7 @@ namespace openlr { namespace { -size_t const kMaxRoadCandidates = 10; +size_t const kMaxRoadCandidates = 20; double const kDistanceAccuracyM = 1000; double const kEps = 1e-9; double const kBearingDist = 25; diff --git a/routing/index_router.cpp b/routing/index_router.cpp index d7f76a8f18..ebbb7dfb53 100644 --- a/routing/index_router.cpp +++ b/routing/index_router.cpp @@ -40,7 +40,7 @@ using namespace std; namespace { -size_t constexpr kMaxRoadCandidates = 6; +size_t constexpr kMaxRoadCandidates = 12; float constexpr kProgressInterval = 2; uint32_t constexpr kVisitPeriod = 40; @@ -93,6 +93,15 @@ unique_ptr CreateDirectionsEngine(VehicleType vehicleType, } } +shared_ptr CreateTrafficStash(VehicleType vehicleType, shared_ptr numMwmIds, + traffic::TrafficCache const & trafficCache) +{ + if (vehicleType != VehicleType::Car) + return nullptr; + + return make_shared(trafficCache, numMwmIds); +} + template IRouter::ResultCode ConvertResult(typename AStarAlgorithm::Result result) { @@ -230,60 +239,40 @@ void PushPassedSubroutes(Checkpoints const & checkpoints, vector= kMinCosAngForAlmostParallelVectors) - return true; - - if (cosAng <= -kMinCosAngForAlmostParallelVectors) - { - forward = false; - return true; - } - return false; + double constexpr kMinCosAngForAlmostCodirectionalVectors = 0.97; + return cosAng >= kMinCosAngForAlmostCodirectionalVectors; } double IndexRouter::BestEdgeComparator::GetSquaredDist(Edge const & edge) const @@ -294,27 +283,23 @@ double IndexRouter::BestEdgeComparator::GetSquaredDist(Edge const & edge) const } // IndexRouter ------------------------------------------------------------------------------------ -IndexRouter::IndexRouter(string const & name, TCountryFileFn const & countryFileFn, +IndexRouter::IndexRouter(VehicleType vehicleType, TCountryFileFn const & countryFileFn, CourntryRectFn const & countryRectFn, shared_ptr numMwmIds, - unique_ptr> numMwmTree, - shared_ptr trafficStash, - VehicleType vehicleType, - shared_ptr vehicleModelFactory, - shared_ptr estimator, - unique_ptr directionsEngine, Index & index) - : m_name(name) + unique_ptr> numMwmTree, traffic::TrafficCache const & trafficCache, + Index & index) + : m_vehicleType(vehicleType) + , m_name("astar-bidirectional-" + ToString(m_vehicleType)) , m_index(index) + , m_vehicleModelFactory(CreateVehicleModelFactory(m_vehicleType)) , m_countryFileFn(countryFileFn) , m_countryRectFn(countryRectFn) , m_numMwmIds(move(numMwmIds)) , m_numMwmTree(move(numMwmTree)) - , m_trafficStash(move(trafficStash)) + , m_trafficStash(CreateTrafficStash(m_vehicleType, m_numMwmIds, trafficCache)) , m_indexManager(countryFileFn, m_index) - , m_roadGraph(index, IRoadGraph::Mode::ObeyOnewayTag, vehicleModelFactory) - , m_vehicleType(vehicleType) - , m_vehicleModelFactory(vehicleModelFactory) - , m_estimator(move(estimator)) - , m_directionsEngine(move(directionsEngine)) + , m_roadGraph(m_index, IRoadGraph::Mode::ObeyOnewayTag, m_vehicleModelFactory) + , m_estimator(EdgeEstimator::Create(m_vehicleType, CalcMaxSpeed(*m_numMwmIds, *m_vehicleModelFactory), m_trafficStash)) + , m_directionsEngine(CreateDirectionsEngine(m_vehicleType, m_numMwmIds, m_index)) { CHECK(!m_name.empty(), ()); CHECK(m_numMwmIds, ()); @@ -416,9 +401,9 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(Checkpoints const & checkpoint segments.push_back(IndexGraphStarter::kStartFakeSegment); Segment startSegment; - bool startSegmentIsAlmostParallelDirection = false; + bool startSegmentIsAlmostCodirectionalDirection = false; if (!FindBestSegment(checkpoints.GetPointFrom(), startDirection, true /* isOutgoing */, graph, - startSegment, startSegmentIsAlmostParallelDirection)) + startSegment, startSegmentIsAlmostCodirectionalDirection)) { return IRouter::StartPointNotFound; } @@ -432,7 +417,7 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(Checkpoints const & checkpoint vector subroute; Junction startJunction; auto const result = - CalculateSubroute(checkpoints, i, startSegment, startSegmentIsAlmostParallelDirection, + CalculateSubroute(checkpoints, i, startSegment, startSegmentIsAlmostCodirectionalDirection, delegate, graph, subroute, startJunction); if (result != IRouter::NoError) @@ -489,7 +474,7 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(Checkpoints const & checkpoint IRouter::ResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoints, size_t subrouteIdx, Segment const & startSegment, - bool startSegmentIsAlmostParallelDirection, + bool startSegmentIsAlmostCodirectional, RouterDelegate const & delegate, WorldGraph & graph, vector & subroute, Junction & startJunction) @@ -501,8 +486,8 @@ IRouter::ResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoin Segment finishSegment; bool dummy = false; - if (!FindBestSegment(finishCheckpoint, m2::PointD::Zero(), false /* isOutgoing */, graph, - finishSegment, dummy)) + if (!FindBestSegment(finishCheckpoint, m2::PointD::Zero() /* direction */, false /* isOutgoing */, graph, + finishSegment, dummy /* bestSegmentIsAlmostCodirectional */)) { bool const isLastSubroute = subrouteIdx == checkpoints.GetNumSubroutes() - 1; return isLastSubroute ? IRouter::EndPointNotFound : IRouter::IntermediatePointNotFound; @@ -514,7 +499,7 @@ IRouter::ResultCode IndexRouter::CalculateSubroute(Checkpoints const & checkpoin LOG(LINFO, ("Routing in mode:", graph.GetMode())); bool const isStartSegmentStrictForward = - subrouteIdx == checkpoints.GetPassedIdx() ? startSegmentIsAlmostParallelDirection : true; + subrouteIdx == checkpoints.GetPassedIdx() ? startSegmentIsAlmostCodirectional : true; IndexGraphStarter starter( MakeFakeVertex(startSegment, startCheckpoint, isStartSegmentStrictForward, graph), MakeFakeVertex(finishSegment, finishCheckpoint, false /* strictForward */, graph), graph); @@ -570,9 +555,9 @@ IRouter::ResultCode IndexRouter::AdjustRoute(Checkpoints const & checkpoints, Segment startSegment; m2::PointD const & pointFrom = checkpoints.GetPointFrom(); - bool bestSegmentIsAlmostConformDirection = false; - if (!FindBestSegment(pointFrom, startDirection, true, graph, startSegment, - bestSegmentIsAlmostConformDirection)) + bool bestSegmentIsAlmostCodirectional = false; + if (!FindBestSegment(pointFrom, startDirection, true /* isOutgoing */, graph, startSegment, + bestSegmentIsAlmostCodirectional)) return IRouter::StartPointNotFound; auto const & lastSubroutes = m_lastRoute->GetSubroutes(); @@ -583,7 +568,7 @@ IRouter::ResultCode IndexRouter::AdjustRoute(Checkpoints const & checkpoints, CHECK(!steps.empty(), ()); IndexGraphStarter starter( - MakeFakeVertex(startSegment, pointFrom, bestSegmentIsAlmostConformDirection, graph), + MakeFakeVertex(startSegment, pointFrom, bestSegmentIsAlmostCodirectional, graph), MakeFakeVertex(steps[lastSubroute.GetEndSegmentIdx() - 1].GetSegment(), checkpoints.GetPointTo(), false /* strictForward */, graph), graph); @@ -674,7 +659,7 @@ WorldGraph IndexRouter::MakeWorldGraph() bool IndexRouter::FindBestSegment(m2::PointD const & point, m2::PointD const & direction, bool isOutgoing, WorldGraph & worldGraph, Segment & bestSegment, - bool & bestSegmentIsAlmostConformDirection) const + bool & bestSegmentIsAlmostCodirectional) const { auto const file = platform::CountryFile(m_countryFileFn(point)); MwmSet::MwmHandle handle = m_index.GetMwmHandleByCountryFile(file); @@ -687,39 +672,31 @@ bool IndexRouter::FindBestSegment(m2::PointD const & point, m2::PointD const & d vector> candidates; m_roadGraph.FindClosestEdges(point, kMaxRoadCandidates, candidates); - auto const getSegmentByEdge = [&numMwmId](Edge const & edge, bool isForward) { - return Segment(numMwmId, edge.GetFeatureId().m_index, edge.GetSegId(), isForward); + auto const getSegmentByEdge = [&numMwmId](Edge const & edge) { + return Segment(numMwmId, edge.GetFeatureId().m_index, edge.GetSegId(), edge.IsForward()); }; // Getting rid of knowingly bad candidates. my::EraseIf(candidates, [&](pair const & p){ Edge const & edge = p.first; - return edge.GetFeatureId().m_mwmId != mwmId || - IsDeadEnd(getSegmentByEdge(edge, edge.IsForward()), isOutgoing, worldGraph); + return edge.GetFeatureId().m_mwmId != mwmId || IsDeadEnd(getSegmentByEdge(edge), isOutgoing, worldGraph); }); if (candidates.empty()) return false; - BestEdgeComparator bestEdgeComparator(point, direction, [this](FeatureID const & featureId) { - return this->IsOneWayFeature(featureId); - }); + BestEdgeComparator bestEdgeComparator(point, direction); Edge bestEdge = candidates[0].first; for (size_t i = 1; i < candidates.size(); ++i) { Edge const & edge = candidates[i].first; - if (bestEdgeComparator.Compare(edge, bestEdge)) + if (bestEdgeComparator.Compare(edge, bestEdge) < 0) bestEdge = edge; } - bestSegmentIsAlmostConformDirection = - bestEdgeComparator.IsDirectionValid() && bestEdgeComparator.IsAlmostConformed(bestEdge); - - bool bestSegmentForward = true; - if (bestEdgeComparator.IsDirectionValid()) - bestEdgeComparator.IsAlmostCollinear(bestEdge, bestSegmentForward); - - bestSegment = getSegmentByEdge(bestEdge, bestSegmentForward); + bestSegmentIsAlmostCodirectional = + bestEdgeComparator.IsDirectionValid() && bestEdgeComparator.IsAlmostCodirectional(bestEdge); + bestSegment = getSegmentByEdge(bestEdge); return true; } @@ -834,42 +811,4 @@ bool IndexRouter::AreMwmsNear(NumMwmId startId, NumMwmId finishId) const }); return areMwmsNear; } - -bool IndexRouter::IsOneWayFeature(FeatureID const & featureId) const -{ - Index::FeaturesLoaderGuard const guard(m_index, featureId.m_mwmId); - FeatureType ft; - if (!guard.GetFeatureByIndex(featureId.m_index, ft)) - { - LOG(LERROR, ("Feature can't be loaded:", featureId)); - return false; - } - shared_ptr model = - m_vehicleModelFactory->GetVehicleModelForCountry(featureId.GetMwmName()); - CHECK(model, ()); - return model->IsOneWay(ft); -} - -// static -unique_ptr IndexRouter::Create(VehicleType vehicleType,TCountryFileFn const & countryFileFn, - CourntryRectFn const & coutryRectFn, - shared_ptr numMwmIds, - unique_ptr> numMwmTree, - traffic::TrafficCache const & trafficCache, - Index & index) -{ - CHECK(numMwmIds, ()); - CHECK(numMwmTree, ()); - auto vehicleModelFactory = CreateVehicleModelFactory(vehicleType); - auto directionsEngine = CreateDirectionsEngine(vehicleType, numMwmIds, index); - double const maxSpeed = CalcMaxSpeed(*numMwmIds, *vehicleModelFactory); - auto trafficStash = vehicleType == VehicleType::Car ? - make_shared(trafficCache, numMwmIds) : - nullptr; - auto estimator = EdgeEstimator::Create(vehicleType, maxSpeed, trafficStash); - auto router = make_unique( - "astar-bidirectional-" + ToString(vehicleType), countryFileFn, coutryRectFn, numMwmIds, move(numMwmTree), - trafficStash, vehicleType, vehicleModelFactory, estimator, move(directionsEngine), index); - return router; -} } // namespace routing diff --git a/routing/index_router.hpp b/routing/index_router.hpp index d2f0599777..471955aa80 100644 --- a/routing/index_router.hpp +++ b/routing/index_router.hpp @@ -34,27 +34,18 @@ public: class BestEdgeComparator final { public: - using IsOnewayFn = std::function; + BestEdgeComparator(m2::PointD const & point, m2::PointD const & direction); - BestEdgeComparator(m2::PointD const & point, m2::PointD const & direction, - IsOnewayFn const & isOnewayFn); - - /// \returns true if |edge1| is closer to |m_point| and |m_direction| than |edge2|. - bool Compare(Edge const & edge1, Edge const & edge2) const; - - /// \returns true if |edge| has a oneway feature and if |edge| is almost parallel to vector - /// |m_direction|. - /// returns true if |edge| has a twoway feature and if |edge| is almost collinear to vector - /// |m_direction|. - /// returns false otherwise. - bool IsAlmostConformed(Edge const & edge) const; + /// \returns -1 if |edge1| is closer to |m_point| and |m_direction| than |edge2|. + /// returns 0 if |edge1| and |edge2| have almost the same direction and are equidistant from |m_point|. + /// returns 1 if |edge1| is further from |m_point| and |m_direction| than |edge2|. + int Compare(Edge const & edge1, Edge const & edge2) const; bool IsDirectionValid() const { return !m_direction.IsAlmostZero(); } + /// \brief According to current implementation vectors |edge| and |m_direction| - /// are almost collinear if angle between line of the vectors less than 14 degrees. - /// If also the angle between the vectors is less than 14 degrees |forward| will be set to true. - /// If the vectors have almost opposite direction |forward| will be set to false. - bool IsAlmostCollinear(Edge const & edge, bool & forward) const; + /// are almost collinear and co-directional if the angle between them is less than 14 degrees. + bool IsAlmostCodirectional(Edge const & edge) const; private: /// \returns the square of shortest distance from |m_point| to |edge| in mercator. @@ -62,16 +53,10 @@ public: m2::PointD const m_point; m2::PointD const m_direction; - IsOnewayFn const m_isOnewayFn; }; - - IndexRouter(std::string const & name, TCountryFileFn const & countryFileFn, - CourntryRectFn const & countryRectFn, std::shared_ptr numMwmIds, - std::unique_ptr> numMwmTree, std::shared_ptr trafficStash, - VehicleType vehicleType, std::shared_ptr vehicleModelFactory, - std::shared_ptr estimator, std::unique_ptr directionsEngine, - Index & index); + IndexRouter(VehicleType vehicleType, TCountryFileFn const & countryFileFn, CourntryRectFn const & countryRectFn, + shared_ptr numMwmIds, unique_ptr> numMwmTree, traffic::TrafficCache const & trafficCache, Index & index); // IRouter overrides: std::string GetName() const override { return m_name; } @@ -79,20 +64,13 @@ public: bool adjustToPrevRoute, RouterDelegate const & delegate, Route & route) override; - static std::unique_ptr Create(VehicleType vehicleType, - TCountryFileFn const & countryFileFn, - CourntryRectFn const & coutryRectFn, - std::shared_ptr numMwmIds, - std::unique_ptr> numMwmTree, - traffic::TrafficCache const & trafficCache, Index & index); - private: IRouter::ResultCode DoCalculateRoute(Checkpoints const & checkpoints, m2::PointD const & startDirection, RouterDelegate const & delegate, Route & route); IRouter::ResultCode CalculateSubroute(Checkpoints const & checkpoints, size_t subrouteIdx, Segment const & startSegment, - bool startSegmentIsAlmostParallelDirection, + bool startSegmentIsAlmostCodirectional, RouterDelegate const & delegate, WorldGraph & graph, std::vector & subroute, Junction & startJunction); @@ -103,17 +81,16 @@ private: WorldGraph MakeWorldGraph(); /// \brief Finds the best segment (edge) which may be considered as the start of the finish of the route. - /// According to current implementation if a segment is near |point| and is almost parallel - /// to |direction| vector, the segment will be better than others. If there's no an an almost parallel + /// According to current implementation if a segment is near |point| and is almost codirectional + /// to |direction| vector, the segment will be better than others. If there's no an an almost codirectional /// segment in neighbourhoods the closest segment to |point| will be chosen. /// \param isOutgoing == true is |point| is considered as the start of the route. /// isOutgoing == false is |point| is considered as the finish of the route. - /// \param bestSegmentIsAlmostConformDirection is filled with true if |bestSegment| is chosen - /// because - /// vector |direction| and vector of |bestSegment| are almost equal and with false otherwise. + /// \param bestSegmentIsAlmostCodirectional is filled with true if |bestSegment| is chosen + /// because vector |direction| and vector of |bestSegment| are almost equal and with false otherwise. bool FindBestSegment(m2::PointD const & point, m2::PointD const & direction, bool isOutgoing, WorldGraph & worldGraph, Segment & bestSegment, - bool & bestSegmentIsAlmostConformDirection) const; + bool & bestSegmentIsAlmostCodirectional) const; // Input route may contains 'leaps': shortcut edges from mwm border enter to exit. // ProcessLeaps replaces each leap with calculated route through mwm. IRouter::ResultCode ProcessLeaps(std::vector const & input, @@ -124,10 +101,12 @@ private: Route & route) const; bool AreMwmsNear(NumMwmId startId, NumMwmId finishId) const; - bool IsOneWayFeature(FeatureID const & featureId) const; + VehicleType m_vehicleType; std::string const m_name; Index & m_index; + std::shared_ptr m_vehicleModelFactory; + TCountryFileFn const m_countryFileFn; CourntryRectFn const m_countryRectFn; std::shared_ptr m_numMwmIds; @@ -136,8 +115,6 @@ private: RoutingIndexManager m_indexManager; FeaturesRoadGraph m_roadGraph; - VehicleType m_vehicleType; - std::shared_ptr m_vehicleModelFactory; std::shared_ptr m_estimator; std::unique_ptr m_directionsEngine; std::unique_ptr m_lastRoute; diff --git a/routing/nearest_edge_finder.cpp b/routing/nearest_edge_finder.cpp index fc70555492..5771a0df5d 100644 --- a/routing/nearest_edge_finder.cpp +++ b/routing/nearest_edge_finder.cpp @@ -6,11 +6,10 @@ #include "base/assert.hpp" -#include "std/limits.hpp" - namespace routing { -NearestEdgeFinder::Candidate::Candidate() : m_dist(numeric_limits::max()), m_segId(0) {} +using namespace std; + NearestEdgeFinder::NearestEdgeFinder(m2::PointD const & point) : m_point(point) { @@ -56,6 +55,7 @@ void NearestEdgeFinder::AddInformationSource(FeatureID const & featureId, IRoadG res.m_segId = static_cast(i - 1); res.m_segStart = segStart; res.m_segEnd = segEnd; + res.m_bidirectional = roadInfo.m_bidirectional; ASSERT_NOT_EQUAL(res.m_segStart.GetAltitude() , feature::kInvalidAltitude, ()); ASSERT_NOT_EQUAL(res.m_segEnd.GetAltitude(), feature::kInvalidAltitude, ()); @@ -68,8 +68,7 @@ void NearestEdgeFinder::AddInformationSource(FeatureID const & featureId, IRoadG m_candidates.push_back(res); } -void NearestEdgeFinder::MakeResult(vector> & res, - size_t const maxCountFeatures) +void NearestEdgeFinder::MakeResult(vector> & res, size_t const maxCountFeatures) { sort(m_candidates.begin(), m_candidates.end(), [](Candidate const & r1, Candidate const & r2) { @@ -78,17 +77,21 @@ void NearestEdgeFinder::MakeResult(vector> & res, res.clear(); res.reserve(maxCountFeatures); - - size_t i = 0; + for (Candidate const & candidate : m_candidates) { res.emplace_back(Edge(candidate.m_fid, true /* forward */, candidate.m_segId, - candidate.m_segStart, candidate.m_segEnd), - candidate.m_projPoint); - ++i; - if (i == maxCountFeatures) - break; + candidate.m_segStart, candidate.m_segEnd), candidate.m_projPoint); + if (res.size() >= maxCountFeatures) + return; + + if (candidate.m_bidirectional) + { + res.emplace_back(Edge(candidate.m_fid, false /* forward */, candidate.m_segId, + candidate.m_segEnd, candidate.m_segStart), candidate.m_projPoint); + if (res.size() >= maxCountFeatures) + return; + } } } - } // namespace routing diff --git a/routing/nearest_edge_finder.hpp b/routing/nearest_edge_finder.hpp index 7edc2e5c61..e9257337e3 100644 --- a/routing/nearest_edge_finder.hpp +++ b/routing/nearest_edge_finder.hpp @@ -11,9 +11,11 @@ #include "indexer/index.hpp" #include "indexer/mwm_set.hpp" -#include "std/unique_ptr.hpp" -#include "std/utility.hpp" -#include "std/vector.hpp" +#include +#include +#include +#include +#include namespace routing { @@ -24,27 +26,25 @@ class NearestEdgeFinder { struct Candidate { - double m_dist; - uint32_t m_segId; + double m_dist = std::numeric_limits::max(); + uint32_t m_segId = 0; Junction m_segStart; Junction m_segEnd; Junction m_projPoint; FeatureID m_fid; - - Candidate(); + bool m_bidirectional = true; }; m2::PointD const m_point; - vector m_candidates; + std::vector m_candidates; public: - NearestEdgeFinder(m2::PointD const & point); + explicit NearestEdgeFinder(m2::PointD const & point); inline bool HasCandidates() const { return !m_candidates.empty(); } void AddInformationSource(FeatureID const & featureId, IRoadGraph::RoadInfo const & roadInfo); - void MakeResult(vector> & res, size_t const maxCountFeatures); + void MakeResult(std::vector> & res, size_t const maxCountFeatures); }; - } // namespace routing diff --git a/routing/road_graph.cpp b/routing/road_graph.cpp index 2b4f59d510..204b41e950 100644 --- a/routing/road_graph.cpp +++ b/routing/road_graph.cpp @@ -52,10 +52,24 @@ string DebugPrint(Junction const & r) } // Edge ------------------------------------------------------------------------ - +// static Edge Edge::MakeFake(Junction const & startJunction, Junction const & endJunction, bool partOfReal) { - Edge edge(FeatureID(), true /* forward */, 0 /* segId */, startJunction, endJunction); + return MakeFakeWithForward(startJunction, endJunction, partOfReal, true /* forward */); +} + +// static +Edge Edge::MakeFakeForTesting(Junction const & startJunction, Junction const & endJunction, + bool partOfReal, bool forward) +{ + return MakeFakeWithForward(startJunction, endJunction, partOfReal, forward); +} + +// static +Edge Edge::MakeFakeWithForward(Junction const & startJunction, Junction const & endJunction, + bool partOfReal, bool forward) +{ + Edge edge(FeatureID(), forward, 0 /* segId */, startJunction, endJunction); edge.m_partOfReal = partOfReal; return edge; } diff --git a/routing/road_graph.hpp b/routing/road_graph.hpp index b84db0e3ac..4274ada5ba 100644 --- a/routing/road_graph.hpp +++ b/routing/road_graph.hpp @@ -53,15 +53,16 @@ inline bool AlmostEqualAbs(Junction const & lhs, Junction const & rhs) class Edge { public: - static Edge MakeFake(Junction const & startJunction, Junction const & endJunction, - bool partOfReal); - Edge(); Edge(FeatureID const & featureId, bool forward, uint32_t segId, Junction const & startJunction, Junction const & endJunction); Edge(Edge const &) = default; Edge & operator=(Edge const &) = default; + static Edge MakeFake(Junction const & startJunction, Junction const & endJunction, bool partOfReal); + static Edge MakeFakeForTesting(Junction const & startJunction, Junction const & endJunction, + bool partOfReal, bool forward); + inline FeatureID GetFeatureId() const { return m_featureId; } inline bool IsForward() const { return m_forward; } inline uint32_t GetSegId() const { return m_segId; } @@ -69,6 +70,7 @@ public: inline Junction const & GetEndJunction() const { return m_endJunction; } inline bool IsFake() const { return !m_featureId.IsValid(); } inline bool IsPartOfReal() const { return m_partOfReal; } + inline m2::PointD GetDirection() const { return GetEndJunction().GetPoint() - GetStartJunction().GetPoint(); } Edge GetReverseEdge() const; @@ -81,6 +83,9 @@ public: private: friend string DebugPrint(Edge const & r); + static Edge MakeFakeWithForward(Junction const & startJunction, Junction const & endJunction, + bool partOfReal, bool forward); + // Feature for which edge is defined. FeatureID m_featureId; @@ -98,6 +103,11 @@ private: // End junction of the segment on the road. Junction m_endJunction; + + // Note. If |m_forward| == true index of |m_startJunction| point at the feature |m_featureId| + // is less than index |m_endJunction|. + // If |m_forward| == false index of |m_startJunction| point at the feature |m_featureId| + // is more than index |m_endJunction|. }; class RoadGraphBase diff --git a/routing/road_graph_router.cpp b/routing/road_graph_router.cpp index 7615aac1d1..f72dd3507a 100644 --- a/routing/road_graph_router.cpp +++ b/routing/road_graph_router.cpp @@ -40,7 +40,7 @@ namespace routing namespace { -size_t constexpr kMaxRoadCandidates = 6; +size_t constexpr kMaxRoadCandidates = 12; size_t constexpr kTestConnectivityVisitJunctionsLimit = 25; diff --git a/routing/routing_integration_tests/routing_test_tools.cpp b/routing/routing_integration_tests/routing_test_tools.cpp index 27f6291ab9..c535533457 100644 --- a/routing/routing_integration_tests/routing_test_tools.cpp +++ b/routing/routing_integration_tests/routing_test_tools.cpp @@ -100,10 +100,11 @@ namespace integration if (mwmId.GetInfo()->GetType() == MwmInfo::COUNTRY && countryFile.GetName() != "minsk-pass") numMwmIds->RegisterFile(countryFile); } + + auto vehicleType = VehicleType::Car; + auto indexRouter = make_unique(vehicleType, countryFileGetter, getMwmRectByName, numMwmIds, + MakeNumMwmTree(*numMwmIds, infoGetter), trafficCache, index); - auto indexRouter = - IndexRouter::Create(VehicleType::Car, countryFileGetter, getMwmRectByName, numMwmIds, - MakeNumMwmTree(*numMwmIds, infoGetter), trafficCache, index); return indexRouter; } diff --git a/routing/routing_session.cpp b/routing/routing_session.cpp index 1bfd55a975..b06db98977 100644 --- a/routing/routing_session.cpp +++ b/routing/routing_session.cpp @@ -234,7 +234,8 @@ RoutingSession::State RoutingSession::OnLocationPositionChanged(GpsInfo const & } } - m_lastGoodPosition = m_userCurrentPosition; + if (m_userCurrentPositionValid) + m_lastGoodPosition = m_userCurrentPosition; } else { @@ -267,7 +268,7 @@ RoutingSession::State RoutingSession::OnLocationPositionChanged(GpsInfo const & } } - if (m_userFormerPosition != m2::PointD::Zero() && m_userCurrentPosition != m2::PointD::Zero()) + if (m_userCurrentPositionValid && m_userFormerPositionValid) m_currentDirection = m_userCurrentPosition - m_userFormerPosition; return m_state; @@ -517,12 +518,10 @@ void RoutingSession::SetCheckpointCallback(CheckpointCallback const & checkpoint void RoutingSession::SetUserCurrentPosition(m2::PointD const & position) { m_userFormerPosition = m_userCurrentPosition; - m_userCurrentPosition = position; -} + m_userFormerPositionValid = m_userCurrentPositionValid; -m2::PointD const & RoutingSession::GetUserCurrentPosition() const -{ - return m_userCurrentPosition; + m_userCurrentPosition = position; + m_userCurrentPositionValid = true; } void RoutingSession::EnableTurnNotifications(bool enable) diff --git a/routing/routing_session.hpp b/routing/routing_session.hpp index 2caf64bda5..d9e4103725 100644 --- a/routing/routing_session.hpp +++ b/routing/routing_session.hpp @@ -132,7 +132,6 @@ public: traffic::SpeedGroup MatchTraffic(location::RouteMatchingInfo const & routeMatchingInfo) const; void SetUserCurrentPosition(m2::PointD const & position); - m2::PointD const & GetUserCurrentPosition() const; void ActivateAdditionalFeatures() {} @@ -226,6 +225,8 @@ private: m2::PointD m_currentDirection; m2::PointD m_userCurrentPosition; m2::PointD m_userFormerPosition; + bool m_userCurrentPositionValid = false; + bool m_userFormerPositionValid = false; // Sound turn notification parameters. turns::sound::NotificationManager m_turnNotificationsMgr; diff --git a/routing/routing_tests/index_graph_test.cpp b/routing/routing_tests/index_graph_test.cpp index 4d3a71f0e6..7679539b7e 100644 --- a/routing/routing_tests/index_graph_test.cpp +++ b/routing/routing_tests/index_graph_test.cpp @@ -589,18 +589,16 @@ UNIT_TEST(IndexGraph_OnlyTopology_3) // *-------*-------> edge2 (-0.002, 0) - (0.002, 0) // point (0, 0) // -// Test that a parallel edge is always better than others. -UNIT_TEST(BestEdgeComparator_OneParallelEdge) +// Test that a codirectional edge is always better than others. +UNIT_TEST(BestEdgeComparator_OneCodirectionalEdge) { - Edge edge1 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), - MakeJunctionForTesting({-0.002, 0.002}), true /* partOfRead */); - Edge edge2 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), - MakeJunctionForTesting({0.002, 0.0}), true /* partOfRead */); - IndexRouter::BestEdgeComparator bestEdgeComparator( - m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001), - [](FeatureID const & /* featureId */) { return true; }); + Edge const edge1 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), + MakeJunctionForTesting({-0.002, 0.002}), true /* partOfReal */); + Edge const edge2 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), + MakeJunctionForTesting({0.002, 0.0}), true /* partOfReal */); + IndexRouter::BestEdgeComparator bestEdgeComparator(m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001)); - TEST(bestEdgeComparator.Compare(edge1, edge2), ()); + TEST_EQUAL(bestEdgeComparator.Compare(edge1, edge2), -1, ()); } // @@ -615,38 +613,55 @@ UNIT_TEST(BestEdgeComparator_OneParallelEdge) // *-------* // point (0, 0) // -// Test that if there are two parallel edges the closet one to |point| is better. -UNIT_TEST(BestEdgeComparator_TwoParallelEdges) +// Test that if there are two codirectional edges the closest one to |point| is better. +UNIT_TEST(BestEdgeComparator_TwoCodirectionalEdges) { - Edge edge1 = - Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), MakeJunctionForTesting({-0.002, 0.004}), true /* partOfRead */); - Edge edge2 = - Edge::MakeFake(MakeJunctionForTesting({0.0, 0.0}), MakeJunctionForTesting({0.0, 0.002}), true /* partOfRead */); - IndexRouter::BestEdgeComparator bestEdgeComparator( - m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001), - [](FeatureID const & /* featureId */) { return true; }); + Edge const edge1 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), + MakeJunctionForTesting({-0.002, 0.004}), true /* partOfReal */); + Edge const edge2 = Edge::MakeFake(MakeJunctionForTesting({0.0, 0.0}), + MakeJunctionForTesting({0.0, 0.002}), true /* partOfReal */); + IndexRouter::BestEdgeComparator bestEdgeComparator(m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001)); - TEST(!bestEdgeComparator.Compare(edge1, edge2), ()); + TEST_EQUAL(bestEdgeComparator.Compare(edge1, edge2), 1, ()); } -// *---------------> edge1 (-0.002, 0.002) - (-0.002, 0.002) +// *---------------> edge1 (-0.002, 0.002) - (0.002, 0.002) // // ^ direction vector (0, 0.001) // | // *-------*-------> edge2 (-0.002, 0) - (0.002, 0) // point (0, 0) // -// Test that if two edges are not parallel the closet one to |point| is better. -UNIT_TEST(BestEdgeComparator_TwoNotParallelEdges) +// Test that if two edges are not codirectional the closet one to |point| is better. +UNIT_TEST(BestEdgeComparator_TwoNotCodirectionalEdges) { - Edge edge1 = - Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.002}), MakeJunctionForTesting({-0.002, 0.002}), true /* partOfRead */); - Edge edge2 = - Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), MakeJunctionForTesting({0.002, 0.0}), true /* partOfRead */); - IndexRouter::BestEdgeComparator bestEdgeComparator( - m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001), - [](FeatureID const & /* featureId */) { return true; }); + Edge const edge1 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.002}), + MakeJunctionForTesting({0.002, 0.002}), true /* partOfReal */); + Edge const edge2 = Edge::MakeFake(MakeJunctionForTesting({-0.002, 0.0}), + MakeJunctionForTesting({0.002, 0.0}), true) /* partOfReal */; + IndexRouter::BestEdgeComparator bestEdgeComparator(m2::PointD(0.0, 0.0), m2::PointD(0.0, 0.001)); - TEST(!bestEdgeComparator.Compare(edge1, edge2), ()); + TEST_EQUAL(bestEdgeComparator.Compare(edge1, edge2), 1, ()); +} + +// point(0, 0.001) +// *--> direction vector (0.001, 0) +// +// edge1 (-0.002, 0) - (0.002, 0) <-------------> edge2 (0.002, 0) - (-0.002, 0) +// (0, 0) +// +// Test that if two edges are made by one bidirectional feature the one which have almost the same direction is better. +UNIT_TEST(BestEdgeComparator_TwoEdgesOfOneFeature) +{ + // Please see a note in class Edge definition about start and end point of Edge. + Edge const edge1 = Edge::MakeFakeForTesting(MakeJunctionForTesting({-0.002, 0.0}), + MakeJunctionForTesting({0.002, 0.0}), + true /* partOfReal */, true /* forward */); + Edge const edge2 = edge1.GetReverseEdge(); + + IndexRouter::BestEdgeComparator bestEdgeComparator(m2::PointD(0.0, 0.001), m2::PointD(0.001, 0.0)); + + TEST_EQUAL(bestEdgeComparator.Compare(edge1, edge2), -1, ()); + TEST_EQUAL(bestEdgeComparator.Compare(edge2, edge1), 1, ()); } } // namespace routing_test diff --git a/routing/routing_tests/nearest_edge_finder_tests.cpp b/routing/routing_tests/nearest_edge_finder_tests.cpp index 549c5cdeb7..4b41c959f6 100644 --- a/routing/routing_tests/nearest_edge_finder_tests.cpp +++ b/routing/routing_tests/nearest_edge_finder_tests.cpp @@ -54,9 +54,9 @@ UNIT_TEST(MiddleSegmentTest_Mock1Graph) MakeJunctionForTesting(m2::PointD(10, 0)), MakeJunctionForTesting(m2::PointD(15, 0))), MakeJunctionForTesting(m2::PointD(12.5, 0))), - make_pair(Edge(MakeTestFeatureID(1), true /* forward */, 2, - MakeJunctionForTesting(m2::PointD(10, 0)), - MakeJunctionForTesting(m2::PointD(10, 5))), - MakeJunctionForTesting(m2::PointD(10, 2.5)))}; + make_pair(Edge(MakeTestFeatureID(0), false /* forward */, 2, + MakeJunctionForTesting(m2::PointD(15, 0)), + MakeJunctionForTesting(m2::PointD(10, 0))), + MakeJunctionForTesting(m2::PointD(12.5, 0)))}; TestNearestOnMock1(m2::PointD(12.5, 2.5), 2, expected); }