From 7d47da4c7e35fabcefca0531b572cba3a7dd793e Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Sun, 23 Apr 2017 18:06:54 +0300 Subject: [PATCH 1/5] [routing]Graph shortcut optimization for long routes. --- routing/cross_mwm_graph.hpp | 12 ++++- routing/cross_mwm_index_graph.hpp | 17 +++++++ routing/cross_mwm_osrm_graph.hpp | 15 ++++++ routing/edge_estimator.cpp | 9 ++++ routing/edge_estimator.hpp | 4 ++ routing/index_graph_starter.cpp | 54 ++++++++++++++++++--- routing/index_graph_starter.hpp | 34 +++++++------ routing/index_router.cpp | 36 +++++++++++--- routing/index_router.hpp | 2 + routing/routing_tests/index_graph_tools.cpp | 6 +++ routing/routing_tests/index_graph_tools.hpp | 1 + routing/world_graph.cpp | 17 +++++-- routing/world_graph.hpp | 20 ++++++-- 13 files changed, 190 insertions(+), 37 deletions(-) diff --git a/routing/cross_mwm_graph.hpp b/routing/cross_mwm_graph.hpp index 065740499f..16559c332a 100644 --- a/routing/cross_mwm_graph.hpp +++ b/routing/cross_mwm_graph.hpp @@ -16,6 +16,7 @@ #include #include #include +#include #include namespace routing @@ -89,6 +90,15 @@ public: void Clear(); + template + void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) + { + if (CrossMwmSectionExists(numMwmId)) + m_crossMwmIndexGraph.ForEachTransition(numMwmId, isEnter, std::forward(fn)); + else + m_crossMwmOsrmGraph.ForEachTransition(numMwmId, isEnter, std::forward(fn)); + } + private: struct ClosestSegment { @@ -126,7 +136,7 @@ private: /// added to |twins| anyway. If there's no such segment in mwm it tries find the closet one and adds it /// to |minDistSegs|. void FindBestTwins(NumMwmId sMwmId, bool isOutgoing, FeatureType const & ft, m2::PointD const & point, - map & minDistSegs, vector & twins); + std::map & minDistSegs, std::vector & twins); /// \brief Fills |neighbors| with number mwm id of all loaded neighbors of |numMwmId| and /// sets |allNeighborsHaveCrossMwmSection| to true if all loaded neighbors have cross mwm section diff --git a/routing/cross_mwm_index_graph.hpp b/routing/cross_mwm_index_graph.hpp index 51e2c7aa89..e81ad46466 100644 --- a/routing/cross_mwm_index_graph.hpp +++ b/routing/cross_mwm_index_graph.hpp @@ -43,6 +43,23 @@ public: bool InCache(NumMwmId numMwmId) const { return m_connectors.count(numMwmId) != 0; } CrossMwmConnector const & GetCrossMwmConnectorWithTransitions(NumMwmId numMwmId); + template + void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) + { + if (isEnter) + { + std::vector const & enters = GetCrossMwmConnectorWithTransitions(numMwmId).GetEnters(); + for (Segment const & enter : enters) + fn(enter); + } + else + { + std::vector const & exits = GetCrossMwmConnectorWithTransitions(numMwmId).GetExits(); + for (Segment const & exit : exits) + fn(exit); + } + } + private: CrossMwmConnector const & GetCrossMwmConnectorWithWeights(NumMwmId numMwmId); diff --git a/routing/cross_mwm_osrm_graph.hpp b/routing/cross_mwm_osrm_graph.hpp index f446eb77ab..444681147c 100644 --- a/routing/cross_mwm_osrm_graph.hpp +++ b/routing/cross_mwm_osrm_graph.hpp @@ -31,6 +31,21 @@ public: void Clear(); TransitionPoints GetTransitionPoints(Segment const & s, bool isOutgoing); + template + void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) + { + if (isEnter) + { + for (auto const & kv : GetSegmentMaps(numMwmId).m_ingoing) + fn(kv.first); + } + else + { + for (auto const & kv : GetSegmentMaps(numMwmId).m_outgoing) + fn(kv.first); + } + } + private: struct TransitionSegments { diff --git a/routing/edge_estimator.cpp b/routing/edge_estimator.cpp index 15efa0ad50..45a629d336 100644 --- a/routing/edge_estimator.cpp +++ b/routing/edge_estimator.cpp @@ -46,6 +46,7 @@ public: // EdgeEstimator overrides: double CalcSegmentWeight(Segment const & segment, RoadGeometry const & road) const override; double CalcHeuristic(m2::PointD const & from, m2::PointD const & to) const override; + double CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const override; double GetUTurnPenalty() const override; bool LeapIsAllowed(NumMwmId mwmId) const override; @@ -100,6 +101,14 @@ double CarEdgeEstimator::CalcHeuristic(m2::PointD const & from, m2::PointD const return TimeBetweenSec(from, to, m_maxSpeedMPS); } +double CarEdgeEstimator::CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const +{ + // Let us assume for the time being that + // leap edges will be added with a half of max speed. + // @TODO(bykoianko) It's necessary to gather statistics to calculate better factor(s) instead of one below. + return TimeBetweenSec(from, to, m_maxSpeedMPS / 2.0); +} + double CarEdgeEstimator::GetUTurnPenalty() const { // Adds 2 minutes penalty for U-turn. The value is quite arbitrary diff --git a/routing/edge_estimator.hpp b/routing/edge_estimator.hpp index b9c954180f..a13fd0490e 100644 --- a/routing/edge_estimator.hpp +++ b/routing/edge_estimator.hpp @@ -24,6 +24,10 @@ public: virtual double CalcSegmentWeight(Segment const & segment, RoadGeometry const & road) const = 0; virtual double CalcHeuristic(m2::PointD const & from, m2::PointD const & to) const = 0; + // Returns time in seconds is taken for going from point |from| to point |to| along a leap (fake) edge |from|-|to|. + // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge (|form|, |to|) in road graph. + // Note 2. The result of the method should be less or equal to CalcHeuristic(|form|, |to|). + virtual double CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const = 0; virtual double GetUTurnPenalty() const = 0; // The leap is the shortcut edge from mwm border enter to exit. // Router can't use leaps on some mwms: e.g. mwm with loaded traffic data. diff --git a/routing/index_graph_starter.cpp b/routing/index_graph_starter.cpp index 064d96bb5b..b4f4d1c991 100644 --- a/routing/index_graph_starter.cpp +++ b/routing/index_graph_starter.cpp @@ -54,13 +54,13 @@ void IndexGraphStarter::GetEdgesList(Segment const & segment, bool isOutgoing, if (segment == kStartFakeSegment) { - GetFakeToNormalEdges(m_start, edges); + GetFakeToNormalEdges(m_start, isOutgoing, edges); return; } if (segment == kFinishFakeSegment) { - GetFakeToNormalEdges(m_finish, edges); + GetFakeToNormalEdges(m_finish, isOutgoing, edges); return; } @@ -70,8 +70,15 @@ void IndexGraphStarter::GetEdgesList(Segment const & segment, bool isOutgoing, } void IndexGraphStarter::GetFakeToNormalEdges(FakeVertex const & fakeVertex, + bool isOutgoing, vector & edges) { + if (m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) + { + ConnectLeapToTransitions(fakeVertex, isOutgoing, edges); + return; + } + GetFakeToNormalEdge(fakeVertex, true /* forward */, edges); if (!m_graph.GetRoadGeometry(fakeVertex.GetMwmId(), fakeVertex.GetFeatureId()).IsOneWay()) @@ -84,7 +91,7 @@ void IndexGraphStarter::GetFakeToNormalEdge(FakeVertex const & fakeVertex, bool Segment const segment(fakeVertex.GetMwmId(), fakeVertex.GetFeatureId(), fakeVertex.GetSegmentIdx(), forward); m2::PointD const & pointTo = GetPoint(segment, true /* front */); - double const weight = m_graph.GetEstimator().CalcHeuristic(fakeVertex.GetPoint(), pointTo); + double const weight = m_graph.GetEstimator().CalcLeapEdgeTime(fakeVertex.GetPoint(), pointTo); edges.emplace_back(segment, weight); } @@ -92,11 +99,46 @@ void IndexGraphStarter::GetNormalToFakeEdge(Segment const & segment, FakeVertex Segment const & fakeSegment, bool isOutgoing, vector & edges) { + m2::PointD const & pointFrom = GetPoint(segment, isOutgoing); + if (segment.GetMwmId() == fakeVertex.GetMwmId() && m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) + { + // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == GetEstimator().CalcLeapEdgeTime(p2, p1). + if (m_graph.IsTransition(segment, isOutgoing)) + edges.emplace_back(fakeSegment, m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); + return; + } + if (!fakeVertex.Fits(segment)) return; - m2::PointD const & pointFrom = GetPoint(segment, isOutgoing); - double const weight = m_graph.GetEstimator().CalcHeuristic(pointFrom, fakeVertex.GetPoint()); - edges.emplace_back(fakeSegment, weight); + edges.emplace_back(fakeSegment, m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); +} + +void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, bool isOutgoing, + vector & edges) +{ + edges.clear(); + m2::PointD const & segmentPoint = fakeVertex.GetPoint(); + + // Note. If |isOutgoing| == true it's necessary to add edges which connect the start with all + // exits of its mwm. + // So |isEnter| below should be set to false. If |isOutgoing| == false all enters of the + // finish mwm should be connected with the finish point. So |isEnter| below should be set to true. + m_graph.ForEachTransition( + fakeVertex.GetMwmId(), !isOutgoing /* isEnter */, [&](Segment const & transition) { + // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == + // GetEstimator().CalcLeapEdgeTime(p2, p1). + edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapEdgeTime( + segmentPoint, GetPoint(transition, isOutgoing))); + }); +} + +Segment const & IndexGraphStarter::ConvertSegment(Segment const & segment) +{ + if (segment == kStartFakeSegment) + return m_start.GetSegment(); + if (segment == kFinishFakeSegment) + return m_finish.GetSegment(); + return segment; } } // namespace routing diff --git a/routing/index_graph_starter.hpp b/routing/index_graph_starter.hpp index 54787c512c..2b76c1c906 100644 --- a/routing/index_graph_starter.hpp +++ b/routing/index_graph_starter.hpp @@ -24,33 +24,28 @@ public: { public: FakeVertex(NumMwmId mwmId, uint32_t featureId, uint32_t segmentIdx, m2::PointD const & point) - : m_featureId(featureId), m_segmentIdx(segmentIdx), m_mwmId(mwmId), m_point(point) + : m_segment(mwmId, featureId, segmentIdx, true /* forward */), m_point(point) { } FakeVertex(Segment const & segment, m2::PointD const & point) - : m_featureId(segment.GetFeatureId()) - , m_segmentIdx(segment.GetSegmentIdx()) - , m_mwmId(segment.GetMwmId()) - , m_point(point) + : m_segment(segment), m_point(point) { } - NumMwmId GetMwmId() const { return m_mwmId; } - uint32_t GetFeatureId() const { return m_featureId; } - uint32_t GetSegmentIdx() const { return m_segmentIdx; } + NumMwmId GetMwmId() const { return m_segment.GetMwmId(); } + uint32_t GetFeatureId() const { return m_segment.GetFeatureId(); } + uint32_t GetSegmentIdx() const { return m_segment.GetSegmentIdx(); } + Segment const & GetSegment() const { return m_segment; } m2::PointD const & GetPoint() const { return m_point; } - bool Fits(Segment const & segment) const { - return segment.GetFeatureId() == m_featureId && segment.GetSegmentIdx() == m_segmentIdx && - segment.GetMwmId() == m_mwmId; + return segment.GetMwmId() == GetMwmId() && segment.GetFeatureId() == GetFeatureId() && + segment.GetSegmentIdx() == GetSegmentIdx(); } private: - uint32_t const m_featureId; - uint32_t const m_segmentIdx; - NumMwmId const m_mwmId; + Segment m_segment; m2::PointD const m_point; }; @@ -99,6 +94,8 @@ public: return segment.GetFeatureId() == kFakeFeatureId; } + Segment const & ConvertSegment(Segment const & segment); + private: static uint32_t constexpr kFakeFeatureId = numeric_limits::max(); static uint32_t constexpr kFakeSegmentIdx = numeric_limits::max(); @@ -107,12 +104,19 @@ private: static Segment constexpr kFinishFakeSegment = Segment(kFakeNumMwmId, kFakeFeatureId, kFakeSegmentIdx, true); - void GetFakeToNormalEdges(FakeVertex const & fakeVertex, vector & edges); + void GetFakeToNormalEdges(FakeVertex const & fakeVertex, bool isOutgoing, + vector & edges); void GetFakeToNormalEdge(FakeVertex const & fakeVertex, bool forward, vector & edges); void GetNormalToFakeEdge(Segment const & segment, FakeVertex const & fakeVertex, Segment const & fakeSegment, bool isOutgoing, vector & edges); + /// \brief If |toExits| == true fills |edges| with SegmentEdge(s) which connects + /// |segment| with all exits of mwm. + /// \brief If |toExits| == false fills |edges| with SegmentEdge(s) which connects + /// all enters to mwm with |segment|. + void ConnectLeapToTransitions(FakeVertex const & fakeVertex, bool isOutgoing, + vector & edges); WorldGraph & m_graph; FakeVertex const m_start; diff --git a/routing/index_router.cpp b/routing/index_router.cpp index a9ef90a35f..616276a83b 100644 --- a/routing/index_router.cpp +++ b/routing/index_router.cpp @@ -138,7 +138,13 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(string const & startCountry, m_index, m_indexManager), IndexGraphLoader::Create(m_numMwmIds, m_vehicleModelFactory, m_estimator, m_index), m_estimator); - graph.SetMode(forSingleMwm ? WorldGraph::Mode::SingleMwm : WorldGraph::Mode::WorldWithLeaps); + + auto const getRoutingMode = [&](){ + return AreMwmsNear(start.GetMwmId(), finish.GetMwmId()) ? WorldGraph::Mode::LeapsIfPossible + : WorldGraph::Mode::LeapsOnly; + }; + graph.SetMode(forSingleMwm ? WorldGraph::Mode::SingleMwm : getRoutingMode()); + LOG(LINFO, ("Routing in mode:", graph.GetMode())); IndexGraphStarter starter(start, finish, graph); @@ -230,27 +236,30 @@ IRouter::ResultCode IndexRouter::ProcessLeaps(vector const & input, output.reserve(input.size()); WorldGraph & worldGraph = starter.GetGraph(); + WorldGraph::Mode const worldRouteMode = worldGraph.GetMode(); worldGraph.SetMode(WorldGraph::Mode::SingleMwm); for (size_t i = 0; i < input.size(); ++i) { Segment const & current = input[i]; - if (!starter.IsLeap(current.GetMwmId())) + if (worldRouteMode == WorldGraph::Mode::LeapsIfPossible && !starter.IsLeap(current.GetMwmId())) { output.push_back(current); continue; } + Segment const & convertedCurrent = starter.ConvertSegment(input[i]); ++i; CHECK_LESS(i, input.size(), ()); - Segment const & next = input[i]; - + Segment const & convertedNext = starter.ConvertSegment(input[i]); CHECK_EQUAL( - current.GetMwmId(), next.GetMwmId(), + convertedCurrent.GetMwmId(), convertedNext.GetMwmId(), ("Different mwm ids for leap enter and exit, i:", i, "size of input:", input.size())); - IndexGraphStarter::FakeVertex const start(current, starter.GetPoint(current, true /* front */)); - IndexGraphStarter::FakeVertex const finish(next, starter.GetPoint(next, true /* front */)); + IndexGraphStarter::FakeVertex const start(convertedCurrent, + starter.GetPoint(convertedCurrent, true /* front */)); + IndexGraphStarter::FakeVertex const finish(convertedNext, + starter.GetPoint(convertedNext, true /* front */)); IndexGraphStarter leapStarter(start, finish, starter.GetGraph()); // Clear previous loaded graphs. @@ -293,7 +302,7 @@ bool IndexRouter::RedressRoute(vector const & segments, RouterDelegate IndexRoadGraph roadGraph(m_numMwmIds, starter, segments, junctions, m_index); if (!forSingleMwm) - starter.GetGraph().SetMode(WorldGraph::Mode::WorldWithoutLeaps); + starter.GetGraph().SetMode(WorldGraph::Mode::NoLeaps); CHECK(m_directionsEngine, ()); ReconstructRoute(*m_directionsEngine, roadGraph, m_trafficStash, delegate, junctions, route); @@ -319,6 +328,17 @@ bool IndexRouter::RedressRoute(vector const & segments, RouterDelegate return true; } +bool IndexRouter::AreMwmsNear(NumMwmId startId, NumMwmId finishId) const +{ + m2::RectD const startMwmRect = m_countryRectFn(m_numMwmIds->GetFile(startId).GetName()); + bool areMwmsNear = false; + m_numMwmTree->ForEachInRect(startMwmRect, [&](NumMwmId id) { + if (id == finishId) + areMwmsNear = true; + }); + return areMwmsNear; +} + // static unique_ptr IndexRouter::CreateCarRouter(TCountryFileFn const & countryFileFn, CourntryRectFn const & coutryRectFn, diff --git a/routing/index_router.hpp b/routing/index_router.hpp index 4aee8ace8d..5bc3049939 100644 --- a/routing/index_router.hpp +++ b/routing/index_router.hpp @@ -77,6 +77,8 @@ private: bool RedressRoute(vector const & segments, RouterDelegate const & delegate, bool forSingleMwm, IndexGraphStarter & starter, Route & route) const; + bool AreMwmsNear(NumMwmId startId, NumMwmId finishId) const; + string const m_name; Index & m_index; TCountryFileFn const m_countryFileFn; diff --git a/routing/routing_tests/index_graph_tools.cpp b/routing/routing_tests/index_graph_tools.cpp index c2c8fd4565..8d765035d2 100644 --- a/routing/routing_tests/index_graph_tools.cpp +++ b/routing/routing_tests/index_graph_tools.cpp @@ -68,6 +68,12 @@ double WeightedEdgeEstimator::CalcHeuristic(m2::PointD const & /* from */, return 0.0; } +double WeightedEdgeEstimator::CalcLeapEdgeTime(m2::PointD const & /* from */, + m2::PointD const & /* to */) const +{ + return 0.0; +} + double WeightedEdgeEstimator::GetUTurnPenalty() const { return 0.0; } bool WeightedEdgeEstimator::LeapIsAllowed(NumMwmId /* mwmId */) const { return false; } diff --git a/routing/routing_tests/index_graph_tools.hpp b/routing/routing_tests/index_graph_tools.hpp index f904672ecc..fbb5a6d772 100644 --- a/routing/routing_tests/index_graph_tools.hpp +++ b/routing/routing_tests/index_graph_tools.hpp @@ -101,6 +101,7 @@ public: // EdgeEstimator overrides: double CalcSegmentWeight(Segment const & segment, RoadGeometry const & /* road */) const override; double CalcHeuristic(m2::PointD const & /* from */, m2::PointD const & /* to */) const override; + double CalcLeapEdgeTime(m2::PointD const & /* from */, m2::PointD const & /* to */) const override; double GetUTurnPenalty() const override; bool LeapIsAllowed(NumMwmId /* mwmId */) const override; diff --git a/routing/world_graph.cpp b/routing/world_graph.cpp index 1f7e4c1249..814fbf5e7e 100644 --- a/routing/world_graph.cpp +++ b/routing/world_graph.cpp @@ -12,10 +12,9 @@ WorldGraph::WorldGraph(unique_ptr crossMwmGraph, unique_ptr & edges) +void WorldGraph::GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, std::vector & edges) { - if (isLeap && m_mode == Mode::WorldWithLeaps) + if (m_mode != Mode::NoLeaps && (isLeap || m_mode == Mode::LeapsOnly)) { CHECK(m_crossMwmGraph, ()); if (m_crossMwmGraph->IsTransition(segment, isOutgoing)) @@ -59,4 +58,16 @@ void WorldGraph::GetTwins(Segment const & segment, bool isOutgoing, vector #include +#include #include namespace routing @@ -19,15 +20,15 @@ public: enum class Mode { SingleMwm, - WorldWithLeaps, - WorldWithoutLeaps, + LeapsOnly, + LeapsIfPossible, + NoLeaps, }; WorldGraph(std::unique_ptr crossMwmGraph, std::unique_ptr loader, std::shared_ptr estimator); - void GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, - std::vector & edges); + void GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, std::vector & edges); IndexGraph & GetIndexGraph(NumMwmId numMwmId) { return m_loader->GetIndexGraph(numMwmId); } EdgeEstimator const & GetEstimator() const { return *m_estimator; } @@ -38,6 +39,15 @@ public: // Clear memory used by loaded index graphs. void ClearIndexGraphs() { m_loader->Clear(); } void SetMode(Mode mode) { m_mode = mode; } + Mode GetMode() const { return m_mode; } + + template + void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) + { + m_crossMwmGraph->ForEachTransition(numMwmId, isEnter, std::forward(fn)); + } + + bool IsTransition(Segment const & s, bool isOutgoing) { return m_crossMwmGraph->IsTransition(s, isOutgoing);} private: void GetTwins(Segment const & s, bool isOutgoing, std::vector & edges); @@ -48,4 +58,6 @@ private: std::vector m_twins; Mode m_mode = Mode::SingleMwm; }; + +std::string DebugPrint(WorldGraph::Mode mode); } // namespace routing From 82c7adf6b52fa7e6cdee39671bfa7215c4e09fdd Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 25 Apr 2017 16:52:06 +0300 Subject: [PATCH 2/5] git-clang-format --- routing/cross_mwm_graph.hpp | 5 +++-- routing/cross_mwm_index_graph.hpp | 3 ++- routing/edge_estimator.cpp | 3 ++- routing/edge_estimator.hpp | 6 ++++-- routing/index_graph_starter.cpp | 17 ++++++++++------- routing/index_router.cpp | 2 +- routing/routing_tests/index_graph_tools.hpp | 3 ++- routing/world_graph.cpp | 11 ++++++----- routing/world_graph.hpp | 9 ++++++--- 9 files changed, 36 insertions(+), 23 deletions(-) diff --git a/routing/cross_mwm_graph.hpp b/routing/cross_mwm_graph.hpp index 16559c332a..cb7deea232 100644 --- a/routing/cross_mwm_graph.hpp +++ b/routing/cross_mwm_graph.hpp @@ -135,8 +135,9 @@ private: /// \note If the method finds twin segment with a point which is very close to |point| the twin segment is /// added to |twins| anyway. If there's no such segment in mwm it tries find the closet one and adds it /// to |minDistSegs|. - void FindBestTwins(NumMwmId sMwmId, bool isOutgoing, FeatureType const & ft, m2::PointD const & point, - std::map & minDistSegs, std::vector & twins); + void FindBestTwins(NumMwmId sMwmId, bool isOutgoing, FeatureType const & ft, + m2::PointD const & point, std::map & minDistSegs, + std::vector & twins); /// \brief Fills |neighbors| with number mwm id of all loaded neighbors of |numMwmId| and /// sets |allNeighborsHaveCrossMwmSection| to true if all loaded neighbors have cross mwm section diff --git a/routing/cross_mwm_index_graph.hpp b/routing/cross_mwm_index_graph.hpp index e81ad46466..693509cb66 100644 --- a/routing/cross_mwm_index_graph.hpp +++ b/routing/cross_mwm_index_graph.hpp @@ -48,7 +48,8 @@ public: { if (isEnter) { - std::vector const & enters = GetCrossMwmConnectorWithTransitions(numMwmId).GetEnters(); + std::vector const & enters = + GetCrossMwmConnectorWithTransitions(numMwmId).GetEnters(); for (Segment const & enter : enters) fn(enter); } diff --git a/routing/edge_estimator.cpp b/routing/edge_estimator.cpp index 45a629d336..d3d4665d87 100644 --- a/routing/edge_estimator.cpp +++ b/routing/edge_estimator.cpp @@ -105,7 +105,8 @@ double CarEdgeEstimator::CalcLeapEdgeTime(m2::PointD const & from, m2::PointD co { // Let us assume for the time being that // leap edges will be added with a half of max speed. - // @TODO(bykoianko) It's necessary to gather statistics to calculate better factor(s) instead of one below. + // @TODO(bykoianko) It's necessary to gather statistics to calculate better factor(s) instead of + // one below. return TimeBetweenSec(from, to, m_maxSpeedMPS / 2.0); } diff --git a/routing/edge_estimator.hpp b/routing/edge_estimator.hpp index a13fd0490e..e55ece5440 100644 --- a/routing/edge_estimator.hpp +++ b/routing/edge_estimator.hpp @@ -24,8 +24,10 @@ public: virtual double CalcSegmentWeight(Segment const & segment, RoadGeometry const & road) const = 0; virtual double CalcHeuristic(m2::PointD const & from, m2::PointD const & to) const = 0; - // Returns time in seconds is taken for going from point |from| to point |to| along a leap (fake) edge |from|-|to|. - // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge (|form|, |to|) in road graph. + // Returns time in seconds is taken for going from point |from| to point |to| along a leap (fake) + // edge |from|-|to|. + // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge + // (|form|, |to|) in road graph. // Note 2. The result of the method should be less or equal to CalcHeuristic(|form|, |to|). virtual double CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const = 0; virtual double GetUTurnPenalty() const = 0; diff --git a/routing/index_graph_starter.cpp b/routing/index_graph_starter.cpp index b4f4d1c991..e4c60085bc 100644 --- a/routing/index_graph_starter.cpp +++ b/routing/index_graph_starter.cpp @@ -69,8 +69,7 @@ void IndexGraphStarter::GetEdgesList(Segment const & segment, bool isOutgoing, GetNormalToFakeEdge(segment, m_finish, kFinishFakeSegment, isOutgoing, edges); } -void IndexGraphStarter::GetFakeToNormalEdges(FakeVertex const & fakeVertex, - bool isOutgoing, +void IndexGraphStarter::GetFakeToNormalEdges(FakeVertex const & fakeVertex, bool isOutgoing, vector & edges) { if (m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) @@ -100,18 +99,22 @@ void IndexGraphStarter::GetNormalToFakeEdge(Segment const & segment, FakeVertex vector & edges) { m2::PointD const & pointFrom = GetPoint(segment, isOutgoing); - if (segment.GetMwmId() == fakeVertex.GetMwmId() && m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) + if (segment.GetMwmId() == fakeVertex.GetMwmId() && + m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) { - // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == GetEstimator().CalcLeapEdgeTime(p2, p1). + // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == + // GetEstimator().CalcLeapEdgeTime(p2, p1). if (m_graph.IsTransition(segment, isOutgoing)) - edges.emplace_back(fakeSegment, m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); + edges.emplace_back(fakeSegment, + m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); return; } if (!fakeVertex.Fits(segment)) return; - edges.emplace_back(fakeSegment, m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); + edges.emplace_back(fakeSegment, + m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); } void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, bool isOutgoing, @@ -129,7 +132,7 @@ void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == // GetEstimator().CalcLeapEdgeTime(p2, p1). edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapEdgeTime( - segmentPoint, GetPoint(transition, isOutgoing))); + segmentPoint, GetPoint(transition, isOutgoing))); }); } diff --git a/routing/index_router.cpp b/routing/index_router.cpp index 616276a83b..0229ac28b4 100644 --- a/routing/index_router.cpp +++ b/routing/index_router.cpp @@ -139,7 +139,7 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(string const & startCountry, IndexGraphLoader::Create(m_numMwmIds, m_vehicleModelFactory, m_estimator, m_index), m_estimator); - auto const getRoutingMode = [&](){ + auto const getRoutingMode = [&]() { return AreMwmsNear(start.GetMwmId(), finish.GetMwmId()) ? WorldGraph::Mode::LeapsIfPossible : WorldGraph::Mode::LeapsOnly; }; diff --git a/routing/routing_tests/index_graph_tools.hpp b/routing/routing_tests/index_graph_tools.hpp index fbb5a6d772..cc9adb1f5b 100644 --- a/routing/routing_tests/index_graph_tools.hpp +++ b/routing/routing_tests/index_graph_tools.hpp @@ -101,7 +101,8 @@ public: // EdgeEstimator overrides: double CalcSegmentWeight(Segment const & segment, RoadGeometry const & /* road */) const override; double CalcHeuristic(m2::PointD const & /* from */, m2::PointD const & /* to */) const override; - double CalcLeapEdgeTime(m2::PointD const & /* from */, m2::PointD const & /* to */) const override; + double CalcLeapEdgeTime(m2::PointD const & /* from */, + m2::PointD const & /* to */) const override; double GetUTurnPenalty() const override; bool LeapIsAllowed(NumMwmId /* mwmId */) const override; diff --git a/routing/world_graph.cpp b/routing/world_graph.cpp index 814fbf5e7e..1db7e2a160 100644 --- a/routing/world_graph.cpp +++ b/routing/world_graph.cpp @@ -12,7 +12,8 @@ WorldGraph::WorldGraph(unique_ptr crossMwmGraph, unique_ptr & edges) +void WorldGraph::GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, + std::vector & edges) { if (m_mode != Mode::NoLeaps && (isLeap || m_mode == Mode::LeapsOnly)) { @@ -63,10 +64,10 @@ string DebugPrint(WorldGraph::Mode mode) { switch (mode) { - case WorldGraph::Mode::SingleMwm: return "SingleMwm"; - case WorldGraph::Mode::LeapsOnly: return "LeapsOnly"; - case WorldGraph::Mode::LeapsIfPossible: return "LeapsIfPossible"; - case WorldGraph::Mode::NoLeaps: return "NoLeaps"; + case WorldGraph::Mode::SingleMwm: return "SingleMwm"; + case WorldGraph::Mode::LeapsOnly: return "LeapsOnly"; + case WorldGraph::Mode::LeapsIfPossible: return "LeapsIfPossible"; + case WorldGraph::Mode::NoLeaps: return "NoLeaps"; } return "Unknown mode"; } diff --git a/routing/world_graph.hpp b/routing/world_graph.hpp index 47407331ea..283092f19b 100644 --- a/routing/world_graph.hpp +++ b/routing/world_graph.hpp @@ -28,7 +28,8 @@ public: WorldGraph(std::unique_ptr crossMwmGraph, std::unique_ptr loader, std::shared_ptr estimator); - void GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, std::vector & edges); + void GetEdgeList(Segment const & segment, bool isOutgoing, bool isLeap, + std::vector & edges); IndexGraph & GetIndexGraph(NumMwmId numMwmId) { return m_loader->GetIndexGraph(numMwmId); } EdgeEstimator const & GetEstimator() const { return *m_estimator; } @@ -40,14 +41,16 @@ public: void ClearIndexGraphs() { m_loader->Clear(); } void SetMode(Mode mode) { m_mode = mode; } Mode GetMode() const { return m_mode; } - template void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) { m_crossMwmGraph->ForEachTransition(numMwmId, isEnter, std::forward(fn)); } - bool IsTransition(Segment const & s, bool isOutgoing) { return m_crossMwmGraph->IsTransition(s, isOutgoing);} + bool IsTransition(Segment const & s, bool isOutgoing) + { + return m_crossMwmGraph->IsTransition(s, isOutgoing); + } private: void GetTwins(Segment const & s, bool isOutgoing, std::vector & edges); From 14057a90343bc302f104b241541329513d02ebb5 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 26 Apr 2017 14:08:37 +0300 Subject: [PATCH 3/5] Review fixes. --- routing/cross_mwm_index_graph.hpp | 4 +- routing/edge_estimator.cpp | 4 +- routing/edge_estimator.hpp | 2 +- routing/index_graph_starter.cpp | 29 +++++------- routing/index_graph_starter.hpp | 18 ++++---- routing/index_router.cpp | 51 +++++++++++++-------- routing/routing_tests/index_graph_tools.cpp | 4 +- routing/routing_tests/index_graph_tools.hpp | 4 +- routing/world_graph.hpp | 12 +++-- 9 files changed, 70 insertions(+), 58 deletions(-) diff --git a/routing/cross_mwm_index_graph.hpp b/routing/cross_mwm_index_graph.hpp index 693509cb66..d3456849f7 100644 --- a/routing/cross_mwm_index_graph.hpp +++ b/routing/cross_mwm_index_graph.hpp @@ -48,9 +48,9 @@ public: { if (isEnter) { - std::vector const & enters = + auto const & enters = GetCrossMwmConnectorWithTransitions(numMwmId).GetEnters(); - for (Segment const & enter : enters) + for (auto const & enter : enters) fn(enter); } else diff --git a/routing/edge_estimator.cpp b/routing/edge_estimator.cpp index d3d4665d87..90b9f781a9 100644 --- a/routing/edge_estimator.cpp +++ b/routing/edge_estimator.cpp @@ -46,7 +46,7 @@ public: // EdgeEstimator overrides: double CalcSegmentWeight(Segment const & segment, RoadGeometry const & road) const override; double CalcHeuristic(m2::PointD const & from, m2::PointD const & to) const override; - double CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const override; + double CalcLeapWeight(m2::PointD const & from, m2::PointD const & to) const override; double GetUTurnPenalty() const override; bool LeapIsAllowed(NumMwmId mwmId) const override; @@ -101,7 +101,7 @@ double CarEdgeEstimator::CalcHeuristic(m2::PointD const & from, m2::PointD const return TimeBetweenSec(from, to, m_maxSpeedMPS); } -double CarEdgeEstimator::CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const +double CarEdgeEstimator::CalcLeapWeight(m2::PointD const & from, m2::PointD const & to) const { // Let us assume for the time being that // leap edges will be added with a half of max speed. diff --git a/routing/edge_estimator.hpp b/routing/edge_estimator.hpp index e55ece5440..af8883f694 100644 --- a/routing/edge_estimator.hpp +++ b/routing/edge_estimator.hpp @@ -29,7 +29,7 @@ public: // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge // (|form|, |to|) in road graph. // Note 2. The result of the method should be less or equal to CalcHeuristic(|form|, |to|). - virtual double CalcLeapEdgeTime(m2::PointD const & from, m2::PointD const & to) const = 0; + virtual double CalcLeapWeight(m2::PointD const & from, m2::PointD const & to) const = 0; virtual double GetUTurnPenalty() const = 0; // The leap is the shortcut edge from mwm border enter to exit. // Router can't use leaps on some mwms: e.g. mwm with loaded traffic data. diff --git a/routing/index_graph_starter.cpp b/routing/index_graph_starter.cpp index e4c60085bc..35b6eb5a3a 100644 --- a/routing/index_graph_starter.cpp +++ b/routing/index_graph_starter.cpp @@ -90,7 +90,7 @@ void IndexGraphStarter::GetFakeToNormalEdge(FakeVertex const & fakeVertex, bool Segment const segment(fakeVertex.GetMwmId(), fakeVertex.GetFeatureId(), fakeVertex.GetSegmentIdx(), forward); m2::PointD const & pointTo = GetPoint(segment, true /* front */); - double const weight = m_graph.GetEstimator().CalcLeapEdgeTime(fakeVertex.GetPoint(), pointTo); + double const weight = m_graph.GetEstimator().CalcLeapWeight(fakeVertex.GetPoint(), pointTo); edges.emplace_back(segment, weight); } @@ -102,11 +102,13 @@ void IndexGraphStarter::GetNormalToFakeEdge(Segment const & segment, FakeVertex if (segment.GetMwmId() == fakeVertex.GetMwmId() && m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) { - // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == - // GetEstimator().CalcLeapEdgeTime(p2, p1). + // It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == + // GetEstimator().CalcLeapWeight(p2, p1). if (m_graph.IsTransition(segment, isOutgoing)) + { edges.emplace_back(fakeSegment, - m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); + m_graph.GetEstimator().CalcLeapWeight(pointFrom, fakeVertex.GetPoint())); + } return; } @@ -114,7 +116,7 @@ void IndexGraphStarter::GetNormalToFakeEdge(Segment const & segment, FakeVertex return; edges.emplace_back(fakeSegment, - m_graph.GetEstimator().CalcLeapEdgeTime(pointFrom, fakeVertex.GetPoint())); + m_graph.GetEstimator().CalcLeapWeight(pointFrom, fakeVertex.GetPoint())); } void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, bool isOutgoing, @@ -129,19 +131,10 @@ void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, // finish mwm should be connected with the finish point. So |isEnter| below should be set to true. m_graph.ForEachTransition( fakeVertex.GetMwmId(), !isOutgoing /* isEnter */, [&](Segment const & transition) { - // It's assumed here that GetEstimator().CalcLeapEdgeTime(p1, p2) == - // GetEstimator().CalcLeapEdgeTime(p2, p1). - edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapEdgeTime( - segmentPoint, GetPoint(transition, isOutgoing))); + // It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == + // GetEstimator().CalcLeapWeight(p2, p1). + edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapWeight( + segmentPoint, GetPoint(transition, isOutgoing))); }); } - -Segment const & IndexGraphStarter::ConvertSegment(Segment const & segment) -{ - if (segment == kStartFakeSegment) - return m_start.GetSegment(); - if (segment == kFinishFakeSegment) - return m_finish.GetSegment(); - return segment; -} } // namespace routing diff --git a/routing/index_graph_starter.hpp b/routing/index_graph_starter.hpp index 2b76c1c906..68e002e395 100644 --- a/routing/index_graph_starter.hpp +++ b/routing/index_graph_starter.hpp @@ -49,11 +49,20 @@ public: m2::PointD const m_point; }; + static uint32_t constexpr kFakeFeatureId = numeric_limits::max(); + static uint32_t constexpr kFakeSegmentIdx = numeric_limits::max(); + static Segment constexpr kStartFakeSegment = + Segment(kFakeNumMwmId, kFakeFeatureId, kFakeSegmentIdx, false); + static Segment constexpr kFinishFakeSegment = + Segment(kFakeNumMwmId, kFakeFeatureId, kFakeSegmentIdx, true); + IndexGraphStarter(FakeVertex const & start, FakeVertex const & finish, WorldGraph & graph); WorldGraph & GetGraph() { return m_graph; } Segment const & GetStart() const { return kStartFakeSegment; } Segment const & GetFinish() const { return kFinishFakeSegment; } + FakeVertex const & GetStartVertex() const { return m_start; } + FakeVertex const & GetFinishVertex() const { return m_finish; } m2::PointD const & GetPoint(Segment const & segment, bool front); static size_t GetRouteNumPoints(vector const & route); @@ -94,16 +103,7 @@ public: return segment.GetFeatureId() == kFakeFeatureId; } - Segment const & ConvertSegment(Segment const & segment); - private: - static uint32_t constexpr kFakeFeatureId = numeric_limits::max(); - static uint32_t constexpr kFakeSegmentIdx = numeric_limits::max(); - static Segment constexpr kStartFakeSegment = - Segment(kFakeNumMwmId, kFakeFeatureId, kFakeSegmentIdx, false); - static Segment constexpr kFinishFakeSegment = - Segment(kFakeNumMwmId, kFakeFeatureId, kFakeSegmentIdx, true); - void GetFakeToNormalEdges(FakeVertex const & fakeVertex, bool isOutgoing, vector & edges); void GetFakeToNormalEdge(FakeVertex const & fakeVertex, bool forward, diff --git a/routing/index_router.cpp b/routing/index_router.cpp index 0229ac28b4..73dbc7f262 100644 --- a/routing/index_router.cpp +++ b/routing/index_router.cpp @@ -134,16 +134,20 @@ IRouter::ResultCode IndexRouter::DoCalculateRoute(string const & startCountry, TrafficStash::Guard guard(*m_trafficStash); WorldGraph graph( - make_unique(m_numMwmIds, m_numMwmTree, m_vehicleModelFactory, m_countryRectFn, - m_index, m_indexManager), - IndexGraphLoader::Create(m_numMwmIds, m_vehicleModelFactory, m_estimator, m_index), - m_estimator); + make_unique(m_numMwmIds, m_numMwmTree, m_vehicleModelFactory, m_countryRectFn, + m_index, m_indexManager), + IndexGraphLoader::Create(m_numMwmIds, m_vehicleModelFactory, m_estimator, m_index), + m_estimator); + + WorldGraph::Mode mode = WorldGraph::Mode::SingleMwm; + if (forSingleMwm) + mode = WorldGraph::Mode::SingleMwm; + else if (AreMwmsNear(start.GetMwmId(), finish.GetMwmId())) + mode = WorldGraph::Mode::LeapsIfPossible; + else + mode = WorldGraph::Mode::LeapsOnly; + graph.SetMode(mode); - auto const getRoutingMode = [&]() { - return AreMwmsNear(start.GetMwmId(), finish.GetMwmId()) ? WorldGraph::Mode::LeapsIfPossible - : WorldGraph::Mode::LeapsOnly; - }; - graph.SetMode(forSingleMwm ? WorldGraph::Mode::SingleMwm : getRoutingMode()); LOG(LINFO, ("Routing in mode:", graph.GetMode())); IndexGraphStarter starter(start, finish, graph); @@ -248,18 +252,29 @@ IRouter::ResultCode IndexRouter::ProcessLeaps(vector const & input, continue; } - Segment const & convertedCurrent = starter.ConvertSegment(input[i]); ++i; CHECK_LESS(i, input.size(), ()); - Segment const & convertedNext = starter.ConvertSegment(input[i]); - CHECK_EQUAL( - convertedCurrent.GetMwmId(), convertedNext.GetMwmId(), - ("Different mwm ids for leap enter and exit, i:", i, "size of input:", input.size())); + Segment const & next = input[i]; + + CHECK_NOT_EQUAL(current, IndexGraphStarter::kFinishFakeSegment, ()); + CHECK_NOT_EQUAL(next, IndexGraphStarter::kStartFakeSegment, ()); + if (current != IndexGraphStarter::kStartFakeSegment && + next != IndexGraphStarter::kFinishFakeSegment) + { + CHECK_EQUAL( + current.GetMwmId(), next.GetMwmId(), + ("Different mwm ids for leap enter and exit, i:", i, "size of input:", input.size())); + } + + IndexGraphStarter::FakeVertex const start = + (current == IndexGraphStarter::kStartFakeSegment) + ? starter.GetStartVertex() + : IndexGraphStarter::FakeVertex(current, starter.GetPoint(current, true /* front */)); + IndexGraphStarter::FakeVertex const finish = + (next == IndexGraphStarter::kFinishFakeSegment) + ? starter.GetFinishVertex() + : IndexGraphStarter::FakeVertex(next, starter.GetPoint(next, true /* front */)); - IndexGraphStarter::FakeVertex const start(convertedCurrent, - starter.GetPoint(convertedCurrent, true /* front */)); - IndexGraphStarter::FakeVertex const finish(convertedNext, - starter.GetPoint(convertedNext, true /* front */)); IndexGraphStarter leapStarter(start, finish, starter.GetGraph()); // Clear previous loaded graphs. diff --git a/routing/routing_tests/index_graph_tools.cpp b/routing/routing_tests/index_graph_tools.cpp index 8d765035d2..b64221a261 100644 --- a/routing/routing_tests/index_graph_tools.cpp +++ b/routing/routing_tests/index_graph_tools.cpp @@ -68,8 +68,8 @@ double WeightedEdgeEstimator::CalcHeuristic(m2::PointD const & /* from */, return 0.0; } -double WeightedEdgeEstimator::CalcLeapEdgeTime(m2::PointD const & /* from */, - m2::PointD const & /* to */) const +double WeightedEdgeEstimator::CalcLeapWeight(m2::PointD const & /* from */, + m2::PointD const & /* to */) const { return 0.0; } diff --git a/routing/routing_tests/index_graph_tools.hpp b/routing/routing_tests/index_graph_tools.hpp index cc9adb1f5b..476651a0d2 100644 --- a/routing/routing_tests/index_graph_tools.hpp +++ b/routing/routing_tests/index_graph_tools.hpp @@ -101,8 +101,8 @@ public: // EdgeEstimator overrides: double CalcSegmentWeight(Segment const & segment, RoadGeometry const & /* road */) const override; double CalcHeuristic(m2::PointD const & /* from */, m2::PointD const & /* to */) const override; - double CalcLeapEdgeTime(m2::PointD const & /* from */, - m2::PointD const & /* to */) const override; + double CalcLeapWeight(m2::PointD const & /* from */, + m2::PointD const & /* to */) const override; double GetUTurnPenalty() const override; bool LeapIsAllowed(NumMwmId /* mwmId */) const override; diff --git a/routing/world_graph.hpp b/routing/world_graph.hpp index 283092f19b..e0c7c067ba 100644 --- a/routing/world_graph.hpp +++ b/routing/world_graph.hpp @@ -19,10 +19,14 @@ class WorldGraph final public: enum class Mode { - SingleMwm, - LeapsOnly, - LeapsIfPossible, - NoLeaps, + SingleMwm, // Mode for building a route within single mwm. + LeapsOnly, // Mode for building a cross mwm route contains of only leaps. In case of start and + // finish they (start and finish) will be connected with all transition segments of + // their mwm with leap (fake) edges. + LeapsIfPossible, // Mode for build cross mwm and single mwm routes. In case of cross mwm route + // if they are neighboring mwms the route will be made without leaps. + // If not the route is made with leaps for intermediate mwms. + NoLeaps, // Mode for building route and getting outgoing/ingoing edges without leaps anyway. }; WorldGraph(std::unique_ptr crossMwmGraph, std::unique_ptr loader, From 2836b6ea911a98c3db34afbd18d4497b6c79cf6f Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 26 Apr 2017 19:10:31 +0300 Subject: [PATCH 4/5] Review fixes. --- routing/edge_estimator.hpp | 6 ++++-- routing/index_graph_starter.cpp | 4 ---- routing/world_graph.cpp | 1 + routing/world_graph.hpp | 5 +++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/routing/edge_estimator.hpp b/routing/edge_estimator.hpp index af8883f694..def69fa598 100644 --- a/routing/edge_estimator.hpp +++ b/routing/edge_estimator.hpp @@ -24,11 +24,13 @@ public: virtual double CalcSegmentWeight(Segment const & segment, RoadGeometry const & road) const = 0; virtual double CalcHeuristic(m2::PointD const & from, m2::PointD const & to) const = 0; - // Returns time in seconds is taken for going from point |from| to point |to| along a leap (fake) + // Returns time in seconds it takes to go from point |from| to point |to| along a leap (fake) // edge |from|-|to|. // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge - // (|form|, |to|) in road graph. + // (|from|, |to|) in road graph. // Note 2. The result of the method should be less or equal to CalcHeuristic(|form|, |to|). + // Note 3. It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == + // GetEstimator().CalcLeapWeight(p2, p1). virtual double CalcLeapWeight(m2::PointD const & from, m2::PointD const & to) const = 0; virtual double GetUTurnPenalty() const = 0; // The leap is the shortcut edge from mwm border enter to exit. diff --git a/routing/index_graph_starter.cpp b/routing/index_graph_starter.cpp index 35b6eb5a3a..778a4fb93c 100644 --- a/routing/index_graph_starter.cpp +++ b/routing/index_graph_starter.cpp @@ -102,8 +102,6 @@ void IndexGraphStarter::GetNormalToFakeEdge(Segment const & segment, FakeVertex if (segment.GetMwmId() == fakeVertex.GetMwmId() && m_graph.GetMode() == WorldGraph::Mode::LeapsOnly) { - // It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == - // GetEstimator().CalcLeapWeight(p2, p1). if (m_graph.IsTransition(segment, isOutgoing)) { edges.emplace_back(fakeSegment, @@ -131,8 +129,6 @@ void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, // finish mwm should be connected with the finish point. So |isEnter| below should be set to true. m_graph.ForEachTransition( fakeVertex.GetMwmId(), !isOutgoing /* isEnter */, [&](Segment const & transition) { - // It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == - // GetEstimator().CalcLeapWeight(p2, p1). edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapWeight( segmentPoint, GetPoint(transition, isOutgoing))); }); diff --git a/routing/world_graph.cpp b/routing/world_graph.cpp index 1db7e2a160..248ddafec1 100644 --- a/routing/world_graph.cpp +++ b/routing/world_graph.cpp @@ -69,6 +69,7 @@ string DebugPrint(WorldGraph::Mode mode) case WorldGraph::Mode::LeapsIfPossible: return "LeapsIfPossible"; case WorldGraph::Mode::NoLeaps: return "NoLeaps"; } + ASSERT(false, ("Unknown mode:", static_cast(mode))); return "Unknown mode"; } } // namespace routing diff --git a/routing/world_graph.hpp b/routing/world_graph.hpp index e0c7c067ba..76160c25c8 100644 --- a/routing/world_graph.hpp +++ b/routing/world_graph.hpp @@ -20,10 +20,10 @@ public: enum class Mode { SingleMwm, // Mode for building a route within single mwm. - LeapsOnly, // Mode for building a cross mwm route contains of only leaps. In case of start and + LeapsOnly, // Mode for building a cross mwm route containing only leaps. In case of start and // finish they (start and finish) will be connected with all transition segments of // their mwm with leap (fake) edges. - LeapsIfPossible, // Mode for build cross mwm and single mwm routes. In case of cross mwm route + LeapsIfPossible, // Mode for building cross mwm and single mwm routes. In case of cross mwm route // if they are neighboring mwms the route will be made without leaps. // If not the route is made with leaps for intermediate mwms. NoLeaps, // Mode for building route and getting outgoing/ingoing edges without leaps anyway. @@ -45,6 +45,7 @@ public: void ClearIndexGraphs() { m_loader->Clear(); } void SetMode(Mode mode) { m_mode = mode; } Mode GetMode() const { return m_mode; } + template void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) { From 29c7af5c6ba2241550e3c86e0c9a6b97dc6a1263 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 27 Apr 2017 14:10:30 +0300 Subject: [PATCH 5/5] Review fixes. --- routing/cross_mwm_index_graph.hpp | 16 +++------------- routing/cross_mwm_osrm_graph.hpp | 13 +++---------- routing/edge_estimator.hpp | 5 ++--- routing/index_graph_starter.cpp | 6 +++--- routing/index_graph_starter.hpp | 8 ++++---- routing/routing_tests/index_graph_test.cpp | 14 +++++++------- routing/world_graph.hpp | 2 +- 7 files changed, 23 insertions(+), 41 deletions(-) diff --git a/routing/cross_mwm_index_graph.hpp b/routing/cross_mwm_index_graph.hpp index d3456849f7..f58fb5c857 100644 --- a/routing/cross_mwm_index_graph.hpp +++ b/routing/cross_mwm_index_graph.hpp @@ -46,19 +46,9 @@ public: template void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) { - if (isEnter) - { - auto const & enters = - GetCrossMwmConnectorWithTransitions(numMwmId).GetEnters(); - for (auto const & enter : enters) - fn(enter); - } - else - { - std::vector const & exits = GetCrossMwmConnectorWithTransitions(numMwmId).GetExits(); - for (Segment const & exit : exits) - fn(exit); - } + auto const & connectors = GetCrossMwmConnectorWithTransitions(numMwmId); + for (Segment const & t : (isEnter ? connectors.GetEnters() : connectors.GetExits())) + fn(t); } private: diff --git a/routing/cross_mwm_osrm_graph.hpp b/routing/cross_mwm_osrm_graph.hpp index 444681147c..279aef1924 100644 --- a/routing/cross_mwm_osrm_graph.hpp +++ b/routing/cross_mwm_osrm_graph.hpp @@ -34,16 +34,9 @@ public: template void ForEachTransition(NumMwmId numMwmId, bool isEnter, Fn && fn) { - if (isEnter) - { - for (auto const & kv : GetSegmentMaps(numMwmId).m_ingoing) - fn(kv.first); - } - else - { - for (auto const & kv : GetSegmentMaps(numMwmId).m_outgoing) - fn(kv.first); - } + auto const & ts = GetSegmentMaps(numMwmId); + for (auto const & kv : isEnter ? ts.m_ingoing : ts.m_outgoing) + fn(kv.first); } private: diff --git a/routing/edge_estimator.hpp b/routing/edge_estimator.hpp index def69fa598..a866c3e581 100644 --- a/routing/edge_estimator.hpp +++ b/routing/edge_estimator.hpp @@ -28,9 +28,8 @@ public: // edge |from|-|to|. // Note 1. The result of the method should be used if it's necessary to add a leap (fake) edge // (|from|, |to|) in road graph. - // Note 2. The result of the method should be less or equal to CalcHeuristic(|form|, |to|). - // Note 3. It's assumed here that GetEstimator().CalcLeapWeight(p1, p2) == - // GetEstimator().CalcLeapWeight(p2, p1). + // Note 2. The result of the method should be less or equal to CalcHeuristic(|from|, |to|). + // Note 3. It's assumed here that CalcLeapWeight(p1, p2) == CalcLeapWeight(p2, p1). virtual double CalcLeapWeight(m2::PointD const & from, m2::PointD const & to) const = 0; virtual double GetUTurnPenalty() const = 0; // The leap is the shortcut edge from mwm border enter to exit. diff --git a/routing/index_graph_starter.cpp b/routing/index_graph_starter.cpp index 778a4fb93c..a7585487d9 100644 --- a/routing/index_graph_starter.cpp +++ b/routing/index_graph_starter.cpp @@ -124,9 +124,9 @@ void IndexGraphStarter::ConnectLeapToTransitions(FakeVertex const & fakeVertex, m2::PointD const & segmentPoint = fakeVertex.GetPoint(); // Note. If |isOutgoing| == true it's necessary to add edges which connect the start with all - // exits of its mwm. - // So |isEnter| below should be set to false. If |isOutgoing| == false all enters of the - // finish mwm should be connected with the finish point. So |isEnter| below should be set to true. + // exits of its mwm. So |isEnter| below should be set to false. + // If |isOutgoing| == false all enters of the finish mwm should be connected with the finish point. + // So |isEnter| below should be set to true. m_graph.ForEachTransition( fakeVertex.GetMwmId(), !isOutgoing /* isEnter */, [&](Segment const & transition) { edges.emplace_back(transition, m_graph.GetEstimator().CalcLeapWeight( diff --git a/routing/index_graph_starter.hpp b/routing/index_graph_starter.hpp index 68e002e395..3403fba00f 100644 --- a/routing/index_graph_starter.hpp +++ b/routing/index_graph_starter.hpp @@ -111,10 +111,10 @@ private: void GetNormalToFakeEdge(Segment const & segment, FakeVertex const & fakeVertex, Segment const & fakeSegment, bool isOutgoing, vector & edges); - /// \brief If |toExits| == true fills |edges| with SegmentEdge(s) which connects - /// |segment| with all exits of mwm. - /// \brief If |toExits| == false fills |edges| with SegmentEdge(s) which connects - /// all enters to mwm with |segment|. + /// \brief If |isOutgoing| == true fills |edges| with SegmentEdge(s) which connects + /// |fakeVertex| with all exits of mwm. + /// \brief If |isOutgoing| == false fills |edges| with SegmentEdge(s) which connects + /// all enters to mwm with |fakeVertex|. void ConnectLeapToTransitions(FakeVertex const & fakeVertex, bool isOutgoing, vector & edges); diff --git a/routing/routing_tests/index_graph_test.cpp b/routing/routing_tests/index_graph_test.cpp index e8bce04a1e..ef92644d38 100644 --- a/routing/routing_tests/index_graph_test.cpp +++ b/routing/routing_tests/index_graph_test.cpp @@ -462,20 +462,20 @@ UNIT_TEST(SerializeSimpleGraph) // F2 // | // | -// 0.0003 *---------* +// 0.0003 6*---------*5 // | | // | | // | | // | | // | | -// 0.0002 * * +// 0.0002 7* *4 // | | // | | -// 0.00015 * F0 * +// 0.00015 8* F0 *3 // \ / -// \ / -// 0.0001 *---F0-*----* -// ^ +// \ / 1 0 +// 0.0001 9*---F0-*----* +// 2 ^ // | // F1 // | @@ -524,7 +524,7 @@ UNIT_CLASS_TEST(RestrictionTest, LoopGraph) routing::IndexGraphStarter::FakeVertex(kTestNumMwmId, 2, 0 /* seg id */, m2::PointD(0.00005, 0.0004)) /* finish */); - double constexpr kExpectedRouteTimeSec = 3.48; + double constexpr kExpectedRouteTimeSec = 3.92527; TestRouteTime(*m_starter, AStarAlgorithm::Result::OK, kExpectedRouteTimeSec); } diff --git a/routing/world_graph.hpp b/routing/world_graph.hpp index 76160c25c8..984cafe1f5 100644 --- a/routing/world_graph.hpp +++ b/routing/world_graph.hpp @@ -26,7 +26,7 @@ public: LeapsIfPossible, // Mode for building cross mwm and single mwm routes. In case of cross mwm route // if they are neighboring mwms the route will be made without leaps. // If not the route is made with leaps for intermediate mwms. - NoLeaps, // Mode for building route and getting outgoing/ingoing edges without leaps anyway. + NoLeaps, // Mode for building route and getting outgoing/ingoing edges without leaps at all. }; WorldGraph(std::unique_ptr crossMwmGraph, std::unique_ptr loader,