From 79f5dadc82f329413a74ddc4e02af11830dea614 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 5 Apr 2017 11:23:07 +0300 Subject: [PATCH] Review fixes. --- routing/cross_mwm_connector.cpp | 2 +- routing/cross_mwm_graph.cpp | 151 ++++++++++++++---------------- routing/cross_mwm_graph.hpp | 34 +++++-- routing/cross_mwm_index_graph.cpp | 4 +- routing/cross_mwm_index_graph.hpp | 30 ++++-- routing/cross_mwm_osrm_graph.cpp | 36 ++++--- routing/cross_mwm_osrm_graph.hpp | 7 +- 7 files changed, 149 insertions(+), 115 deletions(-) diff --git a/routing/cross_mwm_connector.cpp b/routing/cross_mwm_connector.cpp index c3c6e35a9d..b52773b563 100644 --- a/routing/cross_mwm_connector.cpp +++ b/routing/cross_mwm_connector.cpp @@ -57,7 +57,7 @@ bool CrossMwmConnector::IsTransition(Segment const & segment, bool isOutgoing) c // Note. If |isOutgoing| == true |segment| should be an exit transition segment // (|isEnter| == false) to a transition segment. // Otherwise |segment| should be an enter transition segment (|isEnter| == true) - // to be a trasition segment. If not, |segment| is not a transition segment. + // to be a transition segment. If not, |segment| is not a transition segment. // Please see documentation on CrossMwmGraph::IsTransition() method for details. bool const isEnter = (segment.IsForward() == transition.m_forwardIsEnter); return isEnter != isOutgoing; diff --git a/routing/cross_mwm_graph.cpp b/routing/cross_mwm_graph.cpp index 3e8c39bf62..94a085996a 100644 --- a/routing/cross_mwm_graph.cpp +++ b/routing/cross_mwm_graph.cpp @@ -1,9 +1,11 @@ #include "routing/cross_mwm_graph.hpp" +#include "routing/routing_exceptions.hpp" #include "indexer/scales.hpp" #include "geometry/mercator.hpp" +#include "base/math.hpp" #include "base/stl_helpers.hpp" #include "defines.hpp" @@ -19,23 +21,28 @@ namespace { double constexpr kTransitionEqualityDistM = 20.0; double constexpr kInvalidDistance = numeric_limits::max(); - -struct ClosestSegment -{ - ClosestSegment() = default; - ClosestSegment(double minDistM, Segment const & bestSeg, bool exactMatchFound) - : m_bestDistM(minDistM), m_bestSeg(bestSeg), m_exactMatchFound(exactMatchFound) - { - } - - double m_bestDistM = kInvalidDistance; - Segment m_bestSeg; - bool m_exactMatchFound = false; -}; } // namespace namespace routing { +// ClosestSegment --------------------------------------------------------------------------------- +CrossMwmGraph::ClosestSegment::ClosestSegment() : m_bestDistM(kInvalidDistance), m_exactMatchFound(false) {} + +CrossMwmGraph::ClosestSegment::ClosestSegment(double minDistM, Segment const & bestSeg, bool exactMatchFound) + : m_bestDistM(minDistM), m_bestSeg(bestSeg), m_exactMatchFound(exactMatchFound) +{ +} + +void CrossMwmGraph::ClosestSegment::Update(double distM, Segment const & bestSeg) +{ + if (!m_exactMatchFound && distM <= kTransitionEqualityDistM && distM < m_bestDistM) + { + m_bestDistM = distM; + m_bestSeg = bestSeg; + } +} + +// CrossMwmGraph ---------------------------------------------------------------------------------- CrossMwmGraph::CrossMwmGraph(Index & index, shared_ptr numMwmIds, shared_ptr vehicleModelFactory, RoutingIndexManager & indexManager) @@ -51,10 +58,47 @@ CrossMwmGraph::CrossMwmGraph(Index & index, shared_ptr numMwmIds, bool CrossMwmGraph::IsTransition(Segment const & s, bool isOutgoing) { - // Index graph based cross-mwm information. - if (CrossMwmSectionExists(s.GetMwmId())) - return m_crossMwmIndexGraph.IsTransition(s, isOutgoing); - return m_crossMwmOsrmGraph.IsTransition(s, isOutgoing); + return CrossMwmSectionExists(s.GetMwmId()) ? m_crossMwmIndexGraph.IsTransition(s, isOutgoing) + : m_crossMwmOsrmGraph.IsTransition(s, isOutgoing); +} + +void CrossMwmGraph::FindBestTwins(NumMwmId sMwmId, bool isOutgoing, FeatureType const & ft, m2::PointD const & point, + map & minDistSegs, vector & twins) +{ + if (!ft.GetID().IsValid()) + return; + + if (ft.GetID().m_mwmId.GetInfo()->GetType() != MwmInfo::COUNTRY) + return; + + NumMwmId const numMwmId = + m_numMwmIds->GetId(ft.GetID().m_mwmId.GetInfo()->GetLocalFile().GetCountryFile()); + if (numMwmId == sMwmId) + return; + + if (!m_vehicleModelFactory->GetVehicleModelForCountry(ft.GetID().GetMwmName())->IsRoad(ft)) + return; + + ft.ParseGeometry(FeatureType::BEST_GEOMETRY); + vector twinCandidates; + GetTwinCandidates(ft, !isOutgoing, twinCandidates); + for (Segment const & tc : twinCandidates) + { + TransitionPoints const twinPoints = GetTransitionPoints(tc, !isOutgoing); + for (m2::PointD const & tp : twinPoints) + { + double const distM = MercatorBounds::DistanceOnEarth(point, tp); + ClosestSegment & closestSegment = minDistSegs[numMwmId]; + double constexpr kEpsMeters = 2.0; + if (my::AlmostEqualAbs(distM, 0.0, kEpsMeters)) + { + twins.push_back(tc); + closestSegment.m_exactMatchFound = true; + continue; + } + closestSegment.Update(distM, tc); + } + } } void CrossMwmGraph::GetTwins(Segment const & s, bool isOutgoing, vector & twins) @@ -80,43 +124,8 @@ void CrossMwmGraph::GetTwins(Segment const & s, bool isOutgoing, vector // Node. The map below is necessary because twin segments could belong to several mwm. // It happens when a segment crosses more than one feature. map minDistSegs; - auto const findBestTwins = [&](FeatureType & ft) { - if (ft.GetID().m_mwmId.GetInfo()->GetType() != MwmInfo::COUNTRY) - return; - - if (!ft.GetID().IsValid()) - return; - - NumMwmId const numMwmId = - m_numMwmIds->GetId(ft.GetID().m_mwmId.GetInfo()->GetLocalFile().GetCountryFile()); - if (numMwmId == s.GetMwmId()) - return; - - if (!m_vehicleModelFactory->GetVehicleModelForCountry(ft.GetID().GetMwmName())->IsRoad(ft)) - return; - - ft.ParseGeometry(FeatureType::BEST_GEOMETRY); - vector twinCandidates; - GetTwinCandidates(ft, !isOutgoing, twinCandidates); - for (Segment const & tc : twinCandidates) - { - TransitionPoints const twinPoints = GetTransitionPoints(tc, !isOutgoing); - for (m2::PointD const & tp : twinPoints) - { - double const distM = MercatorBounds::DistanceOnEarth(p, tp); - if (distM == 0.0) - { - twins.push_back(tc); - minDistSegs[numMwmId].m_exactMatchFound = true; - } - if (!minDistSegs[numMwmId].m_exactMatchFound && distM <= kTransitionEqualityDistM && - distM < minDistSegs[numMwmId].m_bestDistM) - { - minDistSegs[numMwmId].m_bestDistM = distM; - minDistSegs[numMwmId].m_bestSeg = tc; - } - } - } + auto const findBestTwins = [&](FeatureType const & ft) { + FindBestTwins(s.GetMwmId(), isOutgoing, ft, p, minDistSegs, twins); }; m_index.ForEachInRect( @@ -145,27 +154,8 @@ void CrossMwmGraph::GetEdgeList(Segment const & s, bool isOutgoing, vectorGetFile(numMwmId)); - CHECK(handle.IsAlive(), ()); + if (!handle.IsAlive()) + MYTHROW(RoutingException, ("Mwm", m_numMwmIds->GetFile(numMwmId), "cannot be loaded.")); + MwmValue * value = handle.GetValue(); CHECK(value != nullptr, ("Country file:", m_numMwmIds->GetFile(numMwmId))); return value->m_cont.IsExist(CROSS_MWM_FILE_TAG); diff --git a/routing/cross_mwm_graph.hpp b/routing/cross_mwm_graph.hpp index e83b68cfa0..350a267a71 100644 --- a/routing/cross_mwm_graph.hpp +++ b/routing/cross_mwm_graph.hpp @@ -27,7 +27,7 @@ public: RoutingIndexManager & indexManager); /// \brief Transition segment is a segment which is crossed by mwm border. That means - /// start and finsh of such segment have to lie in different mwms. If a segment is + /// start and finish of such segment have to lie in different mwms. If a segment is /// crossed by mwm border but its start and finish lie in the same mwm it's not /// a transition segment. /// For most cases there is only one transition segment for a transition feature. @@ -56,7 +56,7 @@ public: /// \brief Fills |twins| with duplicates of |s| transition segment in neighbouring mwm. /// For most cases there is only one twin for |s|. /// If |isOutgoing| == true |s| should be an exit transition segment and - /// the mehtod fills |twins| with appropriate enter transition segments. + /// the method fills |twins| with appropriate enter transition segments. /// If |isOutgoing| == false |s| should be an enter transition segment and /// the method fills |twins| with appropriate exit transition segments. /// \note GetTwins(s, isOutgoing, ...) shall be called only if IsTransition(s, isOutgoing) returns @@ -80,13 +80,22 @@ public: void Clear(); private: - /// \returns points of |s|. |s| should be a transition segment of mwm with an OSRM cross-mwm - /// sections or + struct ClosestSegment + { + ClosestSegment(); + ClosestSegment(double minDistM, Segment const & bestSeg, bool exactMatchFound); + void Update(double distM, Segment const & bestSeg); + + double m_bestDistM; + Segment m_bestSeg; + bool m_exactMatchFound; + }; + + /// \returns points of |s|. |s| should be a transition segment of mwm with an OSRM cross-mwm sections or /// with an index graph cross-mwm section. /// \param s is a transition segment of type |isOutgoing|. /// \note the result of the method is returned by value because the size of the vector is usually - /// one - /// or very small in rare cases in OSRM. + /// one or very small in rare cases in OSRM. TransitionPoints GetTransitionPoints(Segment const & s, bool isOutgoing); bool CrossMwmSectionExists(NumMwmId numMwmId); @@ -95,6 +104,19 @@ private: void GetTwinCandidates(FeatureType const & ft, bool isOutgoing, std::vector & twinCandidates); + /// \brief Fills structure |twins| or for feature |ft| if |ft| contains transition segment(s). + /// \param sMwmId mwm id of a segment which twins are looked for + /// \param ft feature which could contain twin segments + /// \param point point of a segment which twins are looked for + /// \param minDistSegs is used to keep the best twin candidate + /// \param twins is filled with twins if there're twins (one or more) that have a point which is + /// very near or equal to |point|. + /// \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, + map & minDistSegs, vector & twins); + Index & m_index; std::shared_ptr m_numMwmIds; std::shared_ptr m_vehicleModelFactory; diff --git a/routing/cross_mwm_index_graph.cpp b/routing/cross_mwm_index_graph.cpp index 342b042bc8..9f92e01e59 100644 --- a/routing/cross_mwm_index_graph.cpp +++ b/routing/cross_mwm_index_graph.cpp @@ -26,7 +26,7 @@ CrossMwmConnector const & CrossMwmIndexGraph::GetCrossMwmConnectorWithTransition return Deserialize( numMwmId, - CrossMwmConnectorSerializer::DeserializeTransitions>); + CrossMwmConnectorSerializer::DeserializeTransitions); } CrossMwmConnector const & CrossMwmIndexGraph::GetCrossMwmConnectorWithWeights(NumMwmId numMwmId) @@ -37,7 +37,7 @@ CrossMwmConnector const & CrossMwmIndexGraph::GetCrossMwmConnectorWithWeights(Nu return Deserialize( numMwmId, - CrossMwmConnectorSerializer::DeserializeWeights>); + CrossMwmConnectorSerializer::DeserializeWeights); } TransitionPoints CrossMwmIndexGraph::GetTransitionPoints(Segment const & s, bool isOutgoing) diff --git a/routing/cross_mwm_index_graph.hpp b/routing/cross_mwm_index_graph.hpp index 9e6b839e82..279dadc257 100644 --- a/routing/cross_mwm_index_graph.hpp +++ b/routing/cross_mwm_index_graph.hpp @@ -3,6 +3,7 @@ #include "routing/cross_mwm_connector.hpp" #include "routing/num_mwm_id.hpp" #include "routing/segment.hpp" +#include "routing/routing_exceptions.hpp" #include "routing/transition_points.hpp" #include "routing/vehicle_mask.hpp" @@ -22,6 +23,8 @@ namespace routing class CrossMwmIndexGraph final { public: + using ReaderSourceFile = ReaderSource; + CrossMwmIndexGraph(Index & index, std::shared_ptr numMwmIds) : m_index(index), m_numMwmIds(numMwmIds) { @@ -31,25 +34,36 @@ public: void GetEdgeList(Segment const & s, bool isOutgoing, std::vector & edges); void Clear() { m_connectors.clear(); } TransitionPoints GetTransitionPoints(Segment const & s, bool isOutgoing); - bool HasCache(NumMwmId numMwmId) const { return m_connectors.count(numMwmId) != 0; } + bool InCache(NumMwmId numMwmId) const { return m_connectors.count(numMwmId) != 0; } + private: CrossMwmConnector const & GetCrossMwmConnectorWithTransitions(NumMwmId numMwmId); CrossMwmConnector const & GetCrossMwmConnectorWithWeights(NumMwmId numMwmId); + /// \brief Deserializes connectors for an mwm with |numMwmId|. + /// \param fn is a function implementing deserialization. + /// \note Each CrossMwmConnector contained in |m_connectors| may be deserizalize in two stages. + /// The first one is transition deserialization and the second is weight deserialization. + /// Transition deserialization is much faster and used more often. template CrossMwmConnector const & Deserialize(NumMwmId numMwmId, Fn && fn) { MwmSet::MwmHandle handle = m_index.GetMwmHandleByCountryFile(m_numMwmIds->GetFile(numMwmId)); - CHECK(handle.IsAlive(), ()); + if (!handle.IsAlive()) + MYTHROW(RoutingException, ("Mwm", m_numMwmIds->GetFile(numMwmId), "cannot be loaded.")); + MwmValue * value = handle.GetValue(); CHECK(value != nullptr, ("Country file:", m_numMwmIds->GetFile(numMwmId))); - auto const reader = - make_unique(value->m_cont.GetReader(CROSS_MWM_FILE_TAG)); - ReaderSource src(*reader); - auto const p = m_connectors.emplace(numMwmId, CrossMwmConnector(numMwmId)); - fn(VehicleType::Car, p.first->second, src); - return p.first->second; + FilesContainerR::TReader const reader = + FilesContainerR::TReader(value->m_cont.GetReader(CROSS_MWM_FILE_TAG)); + ReaderSourceFile src(reader); + auto it = m_connectors.find(numMwmId); + if (it == m_connectors.end()) + it = m_connectors.emplace(numMwmId, CrossMwmConnector(numMwmId)).first; + + fn(VehicleType::Car, it->second, src); + return it->second; } Index & m_index; diff --git a/routing/cross_mwm_osrm_graph.cpp b/routing/cross_mwm_osrm_graph.cpp index ef73ef0b99..3a96525054 100644 --- a/routing/cross_mwm_osrm_graph.cpp +++ b/routing/cross_mwm_osrm_graph.cpp @@ -52,12 +52,12 @@ bool GetFirstValidSegment(OsrmFtSegMapping const & segMapping, NumMwmId numMwmId // GetSegmentsRange(). segMapping.GetSegmentByIndex(segmentIndex, seg); - if (!seg.IsValid()) - continue; - - CHECK_NOT_EQUAL(seg.m_pointStart, seg.m_pointEnd, ()); - segment = Segment(numMwmId, seg.m_fid, min(seg.m_pointStart, seg.m_pointEnd), seg.IsForward()); - return true; + if (seg.IsValid()) + { + CHECK_NOT_EQUAL(seg.m_pointStart, seg.m_pointEnd, ()); + segment = Segment(numMwmId, seg.m_fid, min(seg.m_pointStart, seg.m_pointEnd), seg.IsForward()); + return true; + } } LOG(LDEBUG, ("No valid segments in the range returned by OsrmFtSegMapping::GetSegmentsRange(", nodeId, "). Num mwm id:", numMwmId)); @@ -117,6 +117,7 @@ CrossMwmOsrmGraph::CrossMwmOsrmGraph(shared_ptr numMwmIds, } CrossMwmOsrmGraph::~CrossMwmOsrmGraph() {} + bool CrossMwmOsrmGraph::IsTransition(Segment const & s, bool isOutgoing) { TransitionSegments const & t = GetSegmentMaps(s.GetMwmId()); @@ -128,6 +129,20 @@ bool CrossMwmOsrmGraph::IsTransition(Segment const & s, bool isOutgoing) void CrossMwmOsrmGraph::GetEdgeList(Segment const & s, bool isOutgoing, vector & edges) { + // Note. According to cross-mwm OSRM sections there is a node id that could be ingoing and + // outgoing + // at the same time. For example in Berlin mwm on Nordlicher Berliner Ring (A10) near crossing + // with A11 there's such node id. It's an extremely rare case. There're probably several such node + // id + // for the whole Europe. Such cases are not processed in WorldGraph::GetEdgeList() for the time + // being. + // To prevent filling |edges| with twins instead of leap edges and vice versa in + // WorldGraph::GetEdgeList() + // CrossMwmGraph::GetEdgeList() does not fill |edges| if |s| is a transition segment which + // corresponces node id described above. + if (IsTransition(s, isOutgoing)) + return; + auto const fillEdgeList = [&](TRoutingMappingPtr const & mapping) { vector borderCrosses; GetBorderCross(mapping, s, isOutgoing, borderCrosses); @@ -144,10 +159,8 @@ void CrossMwmOsrmGraph::GetEdgeList(Segment const & s, bool isOutgoing, vectorm_segMapping, edge, isOutgoing, s.GetMwmId(), edges); } }; - bool const isLoaded = LoadWith(s.GetMwmId(), fillEdgeList); - UNUSED_VALUE(isLoaded); - CHECK(isLoaded, ("Mmw", m_numMwmIds->GetFile(s.GetMwmId()), "was not loaded.")); + LoadWith(s.GetMwmId(), fillEdgeList); my::SortUnique(edges); } @@ -192,8 +205,7 @@ CrossMwmOsrmGraph::TransitionSegments const & CrossMwmOsrmGraph::LoadSegmentMaps it = p.first; }; - if (!LoadWith(numMwmId, fillAllTransitionSegments)) - it = m_transitionCache.emplace(numMwmId, TransitionSegments()).first; + LoadWith(numMwmId, fillAllTransitionSegments); return it->second; } @@ -230,8 +242,6 @@ CrossMwmOsrmGraph::TransitionSegments const & CrossMwmOsrmGraph::GetSegmentMaps( auto it = m_transitionCache.find(numMwmId); if (it == m_transitionCache.cend()) return LoadSegmentMaps(numMwmId); - - CHECK(it != m_transitionCache.cend(), ("Mwm ", numMwmId, "has not been downloaded.")); return it->second; } diff --git a/routing/cross_mwm_osrm_graph.hpp b/routing/cross_mwm_osrm_graph.hpp index 5e6e5cdfa4..f446eb77ab 100644 --- a/routing/cross_mwm_osrm_graph.hpp +++ b/routing/cross_mwm_osrm_graph.hpp @@ -56,14 +56,12 @@ private: std::vector const & GetOutgoingTransitionPoints(Segment const & s); template - bool LoadWith(NumMwmId numMwmId, Fn && fn) + void LoadWith(NumMwmId numMwmId, Fn && fn) { platform::CountryFile const & countryFile = m_numMwmIds->GetFile(numMwmId); TRoutingMappingPtr mapping = m_indexManager.GetMappingByName(countryFile.GetName()); CHECK(mapping, ("No routing mapping file for countryFile:", countryFile)); - - if (!mapping->IsValid()) - return false; // mwm was not loaded. + CHECK(mapping->IsValid(), ("Mwm:", countryFile, "was not loaded.")); auto const it = m_mappingGuards.find(numMwmId); if (it == m_mappingGuards.cend()) @@ -73,7 +71,6 @@ private: } fn(mapping); - return true; } std::shared_ptr m_numMwmIds;