From 179bbc170a7be72122e26e18e34abcc06b6e2999 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 3 Mar 2017 20:45:36 +0300 Subject: [PATCH] Review fixes. --- routing/cross_mwm_road_graph.cpp | 196 +++++++++--------- routing/cross_mwm_road_graph.hpp | 7 - routing/cross_routing_context.cpp | 4 +- .../cross_section_tests.cpp | 51 +++-- 4 files changed, 139 insertions(+), 119 deletions(-) diff --git a/routing/cross_mwm_road_graph.cpp b/routing/cross_mwm_road_graph.cpp index ee3022ff8f..c1874c172b 100644 --- a/routing/cross_mwm_road_graph.cpp +++ b/routing/cross_mwm_road_graph.cpp @@ -10,11 +10,11 @@ namespace using namespace routing; inline bool IsValidEdgeWeight(EdgeWeight const & w) { return w != INVALID_EDGE_WEIGHT; } - -template -struct FindingNode +template +class ClosestNodeFinder { - FindingNode(CrossNode const & node, double & minDistance, Node & resultingCrossNode) +public: + ClosestNodeFinder(CrossNode const & node, double & minDistance, Node & resultingCrossNode) : m_node(node), m_minDistance(minDistance), m_resultingCrossNode(resultingCrossNode) { } @@ -32,6 +32,7 @@ struct FindingNode } } +private: CrossNode const & m_node; double & m_minDistance; Node & m_resultingCrossNode; @@ -51,13 +52,18 @@ double GetAdjacencyCost(CrossRoutingContextReader const & currentContext, return GetAdjacencyCost(currentContext, ingoingCrossNode, outgoingCrossNode); } -template -struct FillingEdges +template +class EdgesFiller { - FillingEdges(TRoutingMappingPtr const & currentMapping, CrossRoutingContextReader const & currentContext, - SourceNode const & startingNode, CrossMwmGraph const & crossMwmGraph, vector & adj) - : m_currentMapping(currentMapping), m_currentContext(currentContext), m_startingNode(startingNode), - m_crossMwmGraph(crossMwmGraph), m_adj(adj) +public: + EdgesFiller(TRoutingMappingPtr const & currentMapping, + CrossRoutingContextReader const & currentContext, SourceNode const & startingNode, + CrossMwmGraph const & crossMwmGraph, vector & adj) + : m_currentMapping(currentMapping) + , m_currentContext(currentContext) + , m_startingNode(startingNode) + , m_crossMwmGraph(crossMwmGraph) + , m_adj(adj) { } @@ -66,7 +72,8 @@ struct FillingEdges TWrittenEdgeWeight const outWeight = GetAdjacencyCost(m_currentContext, m_startingNode, node); if (outWeight != kInvalidContextEdgeWeight && outWeight != 0) { - vector const & targets = m_crossMwmGraph.ConstructBorderCross(m_currentMapping, node); + vector const & targets = + m_crossMwmGraph.ConstructBorderCross(m_currentMapping, node); for (auto const & target : targets) { if (target.toNode.IsValid()) @@ -75,6 +82,7 @@ struct FillingEdges } } +private: TRoutingMappingPtr const & m_currentMapping; CrossRoutingContextReader const & m_currentContext; SourceNode const & m_startingNode; @@ -82,42 +90,44 @@ struct FillingEdges vector & m_adj; }; -void FindIngoingCrossNode(CrossRoutingContextReader const & currentContext, CrossNode const & toNode, - IngoingCrossNode & ingoingNode) +bool ForEachNodeNearPoint(CrossRoutingContextReader const & currentContext, + CrossNode const & crossNode, + ClosestNodeFinder const & findingNode) { - double minDistance = std::numeric_limits::max(); - FindingNode findingNode(toNode, minDistance, ingoingNode); - - CHECK(currentContext.ForEachIngoingNodeNearPoint(toNode.point, findingNode), ("toNode.point:", toNode.point)); - CHECK_NOT_EQUAL(minDistance, std::numeric_limits::max(), ("toNode.point:", toNode.point)); + return currentContext.ForEachIngoingNodeNearPoint(crossNode.point, findingNode); } -void FindOutgoingCrossNode(CrossRoutingContextReader const & currentContext, CrossNode const & fromNode, - OutgoingCrossNode & outgoingNode) +bool ForEachNodeNearPoint(CrossRoutingContextReader const & currentContext, + CrossNode const & crossNode, + ClosestNodeFinder const & findingNode) { - double minDistance = std::numeric_limits::max(); - FindingNode findingNode(fromNode, minDistance, outgoingNode); - CHECK(currentContext.ForEachOutgoingNodeNearPoint(fromNode.point, findingNode), ("fromNode.point:", fromNode.point)); - CHECK_NOT_EQUAL(minDistance, std::numeric_limits::max(), ("fromNode.point:", fromNode.point)); + return currentContext.ForEachOutgoingNodeNearPoint(crossNode.point, findingNode); } -vector & FindBorderCross(TWrittenNodeId nodeId, - TRoutingMappingPtr const & currentMapping, - unordered_map, - CrossMwmGraph::Hash> & cachedNextNodes, - bool & result) +template +void FindCrossNode(CrossRoutingContextReader const & currentContext, CrossNode const & crossNode, + Node & node) +{ + double minDistance = std::numeric_limits::max(); + ClosestNodeFinder findingNode(crossNode, minDistance, node); + CHECK(ForEachNodeNearPoint(currentContext, crossNode, findingNode), ()); + CHECK_NOT_EQUAL(minDistance, std::numeric_limits::max(), + ("crossNode.point:", crossNode.point)); +} + +template +vector const & ConstructBorderCrossImpl( + TWrittenNodeId nodeId, TRoutingMappingPtr const & currentMapping, + unordered_map, CrossMwmGraph::Hash> const & + cachedNextNodes, + Fn && borderCrossConstructor /*bool & result*/) { auto const key = make_pair(nodeId, currentMapping->GetMwmId()); auto const it = cachedNextNodes.find(key); - result = (it == cachedNextNodes.end()) ? false : true; if (it != cachedNextNodes.end()) - { - result = true; return it->second; - } - - result = false; - return cachedNextNodes[key]; + borderCrossConstructor(key); + return cachedNextNodes.find(key)->second; } } // namespace @@ -167,7 +177,8 @@ IRouter::ResultCode CrossMwmGraph::SetStartNode(CrossNode const & startNode) { if (IsValidEdgeWeight(weights[i])) { - vector const & nextCrosses = ConstructBorderCrossByOutgoingNode(outgoingNodes[i], startMapping); + vector const & nextCrosses = + ConstructBorderCross(startMapping, outgoingNodes[i]); for (auto const & nextCross : nextCrosses) { if (nextCross.toNode.IsValid()) @@ -242,30 +253,6 @@ IRouter::ResultCode CrossMwmGraph::SetFinalNode(CrossNode const & finalNode) return IRouter::NoError; } -vector const & CrossMwmGraph::ConstructBorderCrossByOutgoingNode(OutgoingCrossNode const & startNode, - TRoutingMappingPtr const & currentMapping) const -{ - bool result = false; - vector & crosses = FindBorderCross(startNode.m_nodeId, currentMapping, m_cachedNextNodesByOutgoing, result); - if (result) - return crosses; - - ConstructBorderCrossByOutgoingImpl(startNode, currentMapping, crosses); - return crosses; -} - -vector const & CrossMwmGraph::ConstructBorderCrossByIngoingNode(IngoingCrossNode const & startNode, - TRoutingMappingPtr const & currentMapping) const -{ - bool result = false; - vector & crosses = FindBorderCross(startNode.m_nodeId, currentMapping, m_cachedNextNodesByIngoing, result); - if (result) - return crosses; - - ConstructBorderCrossByIngoingImpl(startNode, currentMapping, crosses); - return crosses; -} - bool CrossMwmGraph::ConstructBorderCrossByOutgoingImpl(OutgoingCrossNode const & startNode, TRoutingMappingPtr const & currentMapping, vector & crosses) const @@ -278,16 +265,18 @@ bool CrossMwmGraph::ConstructBorderCrossByOutgoingImpl(OutgoingCrossNode const & return false; ASSERT(crosses.empty(), ()); nextMapping->LoadCrossContext(); - nextMapping->m_crossContext.ForEachIngoingNodeNearPoint(startNode.m_point, [&](IngoingCrossNode const & node) - { - if (node.m_nodeId == INVALID_NODE_ID) - return; + nextMapping->m_crossContext.ForEachIngoingNodeNearPoint( + startNode.m_point, [&](IngoingCrossNode const & node) { + if (node.m_nodeId == INVALID_NODE_ID) + return; - if (!node.m_point.EqualDxDy(startNode.m_point, kMwmCrossingNodeEqualityMeters * MercatorBounds::degreeInMetres)) - return; + if (!node.m_point.EqualDxDy( + startNode.m_point, kMwmCrossingNodeEqualityMeters * MercatorBounds::degreeInMetres)) + return; - crosses.emplace_back(fromCross, CrossNode(node.m_nodeId, nextMapping->GetMwmId(), node.m_point)); - }); + crosses.emplace_back(fromCross, + CrossNode(node.m_nodeId, nextMapping->GetMwmId(), node.m_point)); + }); return !crosses.empty(); } @@ -304,45 +293,62 @@ bool CrossMwmGraph::ConstructBorderCrossByIngoingImpl(IngoingCrossNode const & s // to check all neighboring mwms. for (string const & prevMwm : neighboringMwms) { - TRoutingMappingPtr nextMapping = m_indexManager.GetMappingByName(prevMwm); - if (!nextMapping->IsValid()) + TRoutingMappingPtr prevMapping = m_indexManager.GetMappingByName(prevMwm); + if (!prevMapping->IsValid()) continue; - nextMapping->LoadCrossContext(); - nextMapping->m_crossContext.ForEachOutgoingNodeNearPoint(startNode.m_point, [&](OutgoingCrossNode const & node){ - if (node.m_nodeId == INVALID_NODE_ID) - return; + prevMapping->LoadCrossContext(); + prevMapping->m_crossContext.ForEachOutgoingNodeNearPoint( + startNode.m_point, [&](OutgoingCrossNode const & node) { + if (node.m_nodeId == INVALID_NODE_ID) + return; - if (nextMapping->m_crossContext.GetOutgoingMwmName(node) != currentMwm) - return; + if (prevMapping->m_crossContext.GetOutgoingMwmName(node) != currentMwm) + return; - if (!node.m_point.EqualDxDy(startNode.m_point, kMwmCrossingNodeEqualityMeters * MercatorBounds::degreeInMetres)) - return; + if (!node.m_point.EqualDxDy(startNode.m_point, kMwmCrossingNodeEqualityMeters * + MercatorBounds::degreeInMetres)) + return; - crosses.emplace_back(CrossNode(node.m_nodeId, nextMapping->GetMwmId(), node.m_point), toCross); - }); + crosses.emplace_back(CrossNode(node.m_nodeId, prevMapping->GetMwmId(), node.m_point), + toCross); + }); } return !crosses.empty(); } -vector const & CrossMwmGraph::ConstructBorderCross(TRoutingMappingPtr const & currentMapping, - OutgoingCrossNode const & node) const +vector const & CrossMwmGraph::ConstructBorderCross( + TRoutingMappingPtr const & currentMapping, OutgoingCrossNode const & node) const { - return ConstructBorderCrossByOutgoingNode(node, currentMapping); + return ConstructBorderCrossImpl(node.m_nodeId, currentMapping, m_cachedNextNodesByOutgoing, + [&](std::pair const & key) { + vector crosses; + ConstructBorderCrossByOutgoingImpl(node, currentMapping, + crosses); + m_cachedNextNodesByOutgoing[key] = move(crosses); + }); } -vector const & CrossMwmGraph::ConstructBorderCross(TRoutingMappingPtr const & currentMapping, - IngoingCrossNode const & node) const +vector const & CrossMwmGraph::ConstructBorderCross( + TRoutingMappingPtr const & currentMapping, IngoingCrossNode const & node) const { - return ConstructBorderCrossByIngoingNode(node, currentMapping); + return ConstructBorderCrossImpl(node.m_nodeId, currentMapping, m_cachedNextNodesByIngoing, + [&](std::pair const & key) { + vector crosses; + ConstructBorderCrossByIngoingImpl(node, currentMapping, + crosses); + m_cachedNextNodesByIngoing[key] = move(crosses); + }); } -void CrossMwmGraph::GetEdgesList(BorderCross const & v, bool isOutgoing, vector & adj) const +void CrossMwmGraph::GetEdgesList(BorderCross const & v, bool isOutgoing, + vector & adj) const { // Check for virtual edges. adj.clear(); - // Note. Code below processes virtual edges. This code does not work properly if isOutgoing == false. + // Note. Code below processes virtual edges. This code does not work properly if isOutgoing == + // false. // At the same time when this method is called with isOutgoing == false |m_virtualEdges| is empty. if (!isOutgoing && !m_virtualEdges.empty()) { @@ -362,8 +368,8 @@ void CrossMwmGraph::GetEdgesList(BorderCross const & v, bool isOutgoing, vector< } // Loading cross routing section. - TRoutingMappingPtr currentMapping = m_indexManager.GetMappingById(isOutgoing ? v.toNode.mwmId - : v.fromNode.mwmId); + TRoutingMappingPtr currentMapping = + m_indexManager.GetMappingById(isOutgoing ? v.toNode.mwmId : v.fromNode.mwmId); ASSERT(currentMapping->IsValid(), ()); currentMapping->LoadCrossContext(); currentMapping->FreeFileIfPossible(); @@ -373,15 +379,15 @@ void CrossMwmGraph::GetEdgesList(BorderCross const & v, bool isOutgoing, vector< if (isOutgoing) { IngoingCrossNode ingoingNode; - FindIngoingCrossNode(currentContext, v.toNode, ingoingNode); - currentContext.ForEachOutgoingNode(FillingEdges( + FindCrossNode(currentContext, v.toNode, ingoingNode); + currentContext.ForEachOutgoingNode(EdgesFiller( currentMapping, currentContext, ingoingNode, *this, adj)); } else { OutgoingCrossNode outgoingNode; - FindOutgoingCrossNode(currentContext, v.fromNode, outgoingNode); - currentContext.ForEachIngoingNode(FillingEdges( + FindCrossNode(currentContext, v.fromNode, outgoingNode); + currentContext.ForEachIngoingNode(EdgesFiller( currentMapping, currentContext, outgoingNode, *this, adj)); } } diff --git a/routing/cross_mwm_road_graph.hpp b/routing/cross_mwm_road_graph.hpp index 86cade770e..b138f81a17 100644 --- a/routing/cross_mwm_road_graph.hpp +++ b/routing/cross_mwm_road_graph.hpp @@ -115,7 +115,6 @@ public: }; explicit CrossMwmGraph(RoutingIndexManager & indexManager) : m_indexManager(indexManager) {} - void GetOutgoingEdgesList(BorderCross const & v, vector & adj) const { GetEdgesList(v, true /* isOutgoing */, adj); @@ -131,12 +130,6 @@ public: IRouter::ResultCode SetStartNode(CrossNode const & startNode); IRouter::ResultCode SetFinalNode(CrossNode const & finalNode); - // Cashing wrapper for the ConstructBorderCrossByOutgoingImpl function. - vector const & ConstructBorderCrossByOutgoingNode(OutgoingCrossNode const & startNode, - TRoutingMappingPtr const & currentMapping) const; - vector const & ConstructBorderCrossByIngoingNode(IngoingCrossNode const & startNode, - TRoutingMappingPtr const & currentMapping) const; - vector const & ConstructBorderCross(TRoutingMappingPtr const & currentMapping, OutgoingCrossNode const & node) const; vector const & ConstructBorderCross(TRoutingMappingPtr const & currentMapping, diff --git a/routing/cross_routing_context.cpp b/routing/cross_routing_context.cpp index e3115ed37c..555f5e8b01 100644 --- a/routing/cross_routing_context.cpp +++ b/routing/cross_routing_context.cpp @@ -105,8 +105,8 @@ const string & CrossRoutingContextReader::GetOutgoingMwmName( return m_neighborMwmList[outgoingNode.m_outgoingIndex]; } -TWrittenEdgeWeight CrossRoutingContextReader::GetAdjacencyCost(IngoingCrossNode const & ingoing, - OutgoingCrossNode const & outgoing) const +TWrittenEdgeWeight CrossRoutingContextReader::GetAdjacencyCost( + IngoingCrossNode const & ingoing, OutgoingCrossNode const & outgoing) const { if (ingoing.m_adjacencyIndex == kInvalidAdjacencyIndex || outgoing.m_adjacencyIndex == kInvalidAdjacencyIndex) diff --git a/routing/routing_integration_tests/cross_section_tests.cpp b/routing/routing_integration_tests/cross_section_tests.cpp index b29238c77f..8b48249476 100644 --- a/routing/routing_integration_tests/cross_section_tests.cpp +++ b/routing/routing_integration_tests/cross_section_tests.cpp @@ -21,6 +21,8 @@ #include "std/limits.hpp" +#include + using namespace routing; namespace @@ -149,17 +151,29 @@ UNIT_TEST(CrossMwmGraphTest) index.RegisterMap(file); } - Platform p; - unique_ptr infoGetter = storage::CountryInfoReader::CreateCountryInfoReader(p); - auto countryFileGetter = [&infoGetter](m2::PointD const & pt) + for (auto it = localFiles.begin(); it != localFiles.end();) { + string const & countryName = it->GetCountryName(); + MwmSet::MwmId const mwmId = index.GetMwmIdByCountryFile(it->GetCountryFile()); + if (countryName == "minsk-pass" || mwmId.GetInfo()->GetType() != MwmInfo::COUNTRY) + it = localFiles.erase(it); + else + ++it; + } + + Platform p; + unique_ptr infoGetter = + storage::CountryInfoReader::CreateCountryInfoReader(p); + auto countryFileGetter = [&infoGetter](m2::PointD const & pt) { return infoGetter->GetRegionCountryId(pt); }; RoutingIndexManager manager(countryFileGetter, index); CrossMwmGraph crossMwmGraph(manager); - std::minstd_rand rng; + auto const seed = std::chrono::system_clock::now().time_since_epoch().count(); + LOG(LINFO, ("Seed for RandomSample:", seed)); + std::minstd_rand rng(static_cast(seed)); std::vector subset = base::RandomSample(localFiles.size(), 10 /* mwm number */, rng); std::vector subsetCountryFiles; for (size_t i : subset) @@ -168,23 +182,23 @@ UNIT_TEST(CrossMwmGraphTest) for (platform::LocalCountryFile const & file : subsetCountryFiles) { string const & countryName = file.GetCountryName(); + LOG(LINFO, ("Processing", countryName)); MwmSet::MwmId const mwmId = index.GetMwmIdByCountryFile(file.GetCountryFile()); - if (countryName == "minsk-pass" || mwmId.GetInfo()->GetType() != MwmInfo::COUNTRY) - continue; - TEST(mwmId.IsAlive(), ("Mwm name:", countryName, "Subset:", subsetCountryFiles)); TRoutingMappingPtr currentMapping = manager.GetMappingById(mwmId); if (!currentMapping->IsValid()) - continue; // No routing sections in the mwm. + continue; // No routing sections in the mwm. currentMapping->LoadCrossContext(); currentMapping->FreeFileIfPossible(); CrossRoutingContextReader const & currentContext = currentMapping->m_crossContext; - currentContext.ForEachIngoingNode([&](IngoingCrossNode const & node) - { - vector const & targets = crossMwmGraph.ConstructBorderCrossByIngoingNode(node, currentMapping); + size_t ingoingCounter = 0; + currentContext.ForEachIngoingNode([&](IngoingCrossNode const & node) { + ++ingoingCounter; + vector const & targets = + crossMwmGraph.ConstructBorderCross(currentMapping, node); for (BorderCross const & t : targets) { vector outAdjs; @@ -193,15 +207,22 @@ UNIT_TEST(CrossMwmGraphTest) { vector inAdjs; crossMwmGraph.GetIngoingEdgesList(out.GetTarget(), inAdjs); - TEST(find_if(inAdjs.cbegin(), inAdjs.cend(), [&](CrossWeightedEdge const & e){ + TEST(find_if(inAdjs.cbegin(), inAdjs.cend(), + [&](CrossWeightedEdge const & e) { return e.GetTarget() == t && out.GetWeight() == e.GetWeight(); }) != inAdjs.cend(), - ("ForEachOutgoingNodeNearPoint() and ForEachIngoingNodeNearPoint() arn't correlated. Mwm:", - countryName, "Subset:", subsetCountryFiles)); + ("ForEachOutgoingNodeNearPoint() and ForEachIngoingNodeNearPoint() arn't " + "correlated. Mwm:", + countryName, "Subset:", subsetCountryFiles)); } } }); - LOG(LINFO, ("Processed", countryName)); + + size_t outgoingCounter = 0; + currentContext.ForEachOutgoingNode( + [&](OutgoingCrossNode const & /* node */) { ++outgoingCounter; }); + + LOG(LINFO, ("Processed:", countryName, "Exits:", outgoingCounter, "Enters:", ingoingCounter)); } } } // namespace