From ea451326ed5a17d583c0404ff10e089d3a9d0e18 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 22 Dec 2017 12:52:56 +0300 Subject: [PATCH] Review fixes. --- routing/bicycle_directions.cpp | 31 +++++++++---------- routing/bicycle_directions.hpp | 12 +++---- routing/loaded_path_segment.hpp | 24 +++----------- routing/road_graph.cpp | 11 +++++-- routing/routing_result_graph.hpp | 4 +-- .../routing_tests/turns_generator_test.cpp | 8 ++--- routing/turn_candidate.hpp | 11 +++---- routing/turns.cpp | 16 +++++----- routing/turns.hpp | 26 +++++++--------- routing/turns_generator.cpp | 10 +++--- 10 files changed, 69 insertions(+), 84 deletions(-) diff --git a/routing/bicycle_directions.cpp b/routing/bicycle_directions.cpp index 00293415a6..69e13b5bbc 100644 --- a/routing/bicycle_directions.cpp +++ b/routing/bicycle_directions.cpp @@ -50,14 +50,14 @@ public: // turns::IRoutingResult overrides: TUnpackedPathSegments const & GetSegments() const override { return m_pathSegments; } - void GetPossibleTurns(UniNodeId const & node, m2::PointD const & /* ingoingPoint */, + void GetPossibleTurns(SegmentRange const & segmentRange, m2::PointD const & /* ingoingPoint */, m2::PointD const & /* junctionPoint */, size_t & ingoingCount, TurnCandidates & outgoingTurns) const override { ingoingCount = 0; outgoingTurns.candidates.clear(); - auto const adjacentEdges = m_adjacentEdges.find(node); + auto const adjacentEdges = m_adjacentEdges.find(segmentRange); if (adjacentEdges == m_adjacentEdges.cend()) { ASSERT(false, ()); @@ -225,17 +225,14 @@ void BicycleDirectionsEngine::LoadPathAttributes(FeatureID const & featureId, Lo pathSegment.m_onRoundabout = ftypes::IsRoundAboutChecker::Instance()(ft); } -void BicycleDirectionsEngine::GetUniNodeIdAndAdjacentEdges(IRoadGraph::TEdgeVector const & outgoingEdges, - Edge const & inEdge, - uint32_t startSegId, - uint32_t endSegId, - UniNodeId & uniNodeId, - TurnCandidates & outgoingTurns) +void BicycleDirectionsEngine::GetSegmentRangeAndAdjacentEdges( + IRoadGraph::TEdgeVector const & outgoingEdges, Edge const & inEdge, uint32_t startSegId, + uint32_t endSegId, SegmentRange & segmentRange, TurnCandidates & outgoingTurns) { outgoingTurns.isCandidatesAngleValid = true; outgoingTurns.candidates.reserve(outgoingEdges.size()); - uniNodeId = UniNodeId(inEdge.GetFeatureId(), startSegId, endSegId, inEdge.IsForward()); - CHECK(uniNodeId.IsCorrect(), ()); + segmentRange = SegmentRange(inEdge.GetFeatureId(), startSegId, endSegId, inEdge.IsForward()); + CHECK(segmentRange.IsCorrect(), ()); m2::PointD const & ingoingPoint = inEdge.GetStartJunction().GetPoint(); m2::PointD const & junctionPoint = inEdge.GetEndJunction().GetPoint(); @@ -271,7 +268,7 @@ void BicycleDirectionsEngine::GetUniNodeIdAndAdjacentEdges(IRoadGraph::TEdgeVect // should not be used for turn generation. outgoingTurns.isCandidatesAngleValid = false; } - outgoingTurns.candidates.emplace_back(angle, uniNodeId, highwayClass); + outgoingTurns.candidates.emplace_back(angle, segmentRange, highwayClass); } } @@ -335,14 +332,14 @@ void BicycleDirectionsEngine::FillPathSegmentsAndAdjacentEdgesMap( prevJunctions.push_back(currJunction); AdjacentEdges adjacentEdges(ingoingEdges.size()); - UniNodeId uniNodeId; - GetUniNodeIdAndAdjacentEdges(outgoingEdges, inEdge, startSegId, inSegId, uniNodeId, - adjacentEdges.m_outgoingTurns); + SegmentRange segmentRange; + GetSegmentRangeAndAdjacentEdges(outgoingEdges, inEdge, startSegId, inSegId, segmentRange, + adjacentEdges.m_outgoingTurns); size_t const prevJunctionSize = prevJunctions.size(); LoadedPathSegment pathSegment; - LoadPathAttributes(uniNodeId.GetFeature(), pathSegment); - pathSegment.m_nodeId = uniNodeId; + LoadPathAttributes(segmentRange.GetFeature(), pathSegment); + pathSegment.m_segmentRange = segmentRange; pathSegment.m_path = move(prevJunctions); // @TODO(bykoianko) |pathSegment.m_weight| should be filled here. @@ -351,7 +348,7 @@ void BicycleDirectionsEngine::FillPathSegmentsAndAdjacentEdgesMap( CHECK_EQUAL(prevSegments.size() + 1, prevJunctionSize, ()); pathSegment.m_segments = move(prevSegments); - auto const it = m_adjacentEdges.insert(make_pair(uniNodeId, move(adjacentEdges))); + auto const it = m_adjacentEdges.insert(make_pair(segmentRange, move(adjacentEdges))); ASSERT(it.second, ()); UNUSED_VALUE(it); m_pathSegments.push_back(move(pathSegment)); diff --git a/routing/bicycle_directions.hpp b/routing/bicycle_directions.hpp index b94e72ff36..142eb3504b 100644 --- a/routing/bicycle_directions.hpp +++ b/routing/bicycle_directions.hpp @@ -24,7 +24,7 @@ public: size_t m_ingoingTurnsCount; }; - using AdjacentEdgesMap = std::map; + using AdjacentEdgesMap = std::map; BicycleDirectionsEngine(Index const & index, std::shared_ptr numMwmIds); @@ -37,12 +37,10 @@ public: private: Index::FeaturesLoaderGuard & GetLoader(MwmSet::MwmId const & id); void LoadPathAttributes(FeatureID const & featureId, LoadedPathSegment & pathSegment); - void GetUniNodeIdAndAdjacentEdges(IRoadGraph::TEdgeVector const & outgoingEdges, - Edge const & inEdge, - uint32_t startSegId, - uint32_t endSegId, - UniNodeId & uniNodeId, - turns::TurnCandidates & outgoingTurns); + void GetSegmentRangeAndAdjacentEdges(IRoadGraph::TEdgeVector const & outgoingEdges, + Edge const & inEdge, uint32_t startSegId, uint32_t endSegId, + SegmentRange & segmentRange, + turns::TurnCandidates & outgoingTurns); /// \brief The method gathers sequence of segments according to IsJoint() method /// and fills |m_adjacentEdges| and |m_pathSegments|. void FillPathSegmentsAndAdjacentEdgesMap(RoadGraphBase const & graph, diff --git a/routing/loaded_path_segment.hpp b/routing/loaded_path_segment.hpp index 2c25803056..8d53e7655d 100644 --- a/routing/loaded_path_segment.hpp +++ b/routing/loaded_path_segment.hpp @@ -28,26 +28,12 @@ struct LoadedPathSegment vector m_path; vector m_lanes; string m_name; - TEdgeWeight m_weight; /*!< Time in seconds to pass the segment. */ - UniNodeId m_nodeId; /*!< Node id for A*. */ + double m_weight = 0.0; /*!< Time in seconds to pass the segment. */ + SegmentRange m_segmentRange; vector m_segments; /*!< Traffic segments for |m_path|. */ - ftypes::HighwayClass m_highwayClass; - bool m_onRoundabout; - bool m_isLink; - - LoadedPathSegment() { Clear(); } - void Clear() - { - m_path.clear(); - m_lanes.clear(); - m_name.clear(); - m_weight = 0; - m_nodeId.Clear(); - m_segments.clear(); - m_highwayClass = ftypes::HighwayClass::Undefined; - m_onRoundabout = false; - m_isLink = false; - } + ftypes::HighwayClass m_highwayClass = ftypes::HighwayClass::Undefined; + bool m_onRoundabout = false; + bool m_isLink = false; bool IsValid() const { return !m_path.empty(); } }; diff --git a/routing/road_graph.cpp b/routing/road_graph.cpp index 6790f74ce5..3fa096a51e 100644 --- a/routing/road_graph.cpp +++ b/routing/road_graph.cpp @@ -8,6 +8,7 @@ #include "geometry/segment2d.hpp" #include "base/assert.hpp" +#include "base/checked_cast.hpp" #include "base/math.hpp" #include "base/stl_helpers.hpp" @@ -167,7 +168,10 @@ void IRoadGraph::CrossOutgoingLoader::LoadEdges(FeatureID const & featureId, Roa ForEachEdge(roadInfo, [&featureId, &roadInfo, this](size_t segId, Junction const & endJunction, bool forward) { if (forward || roadInfo.m_bidirectional || m_mode == IRoadGraph::Mode::IgnoreOnewayTag) - m_edges.push_back(Edge::MakeReal(featureId, forward, static_cast(segId), m_cross, endJunction)); + { + m_edges.push_back(Edge::MakeReal(featureId, forward, base::asserted_cast(segId), + m_cross, endJunction)); + } }); } @@ -177,7 +181,10 @@ void IRoadGraph::CrossIngoingLoader::LoadEdges(FeatureID const & featureId, Road ForEachEdge(roadInfo, [&featureId, &roadInfo, this](size_t segId, Junction const & endJunction, bool forward) { if (!forward || roadInfo.m_bidirectional || m_mode == IRoadGraph::Mode::IgnoreOnewayTag) - m_edges.push_back(Edge::MakeReal(featureId, !forward, static_cast(segId), endJunction, m_cross)); + { + m_edges.push_back(Edge::MakeReal(featureId, !forward, base::asserted_cast(segId), + endJunction, m_cross)); + } }); } diff --git a/routing/routing_result_graph.hpp b/routing/routing_result_graph.hpp index 8736a942dc..bddbf3b12e 100644 --- a/routing/routing_result_graph.hpp +++ b/routing/routing_result_graph.hpp @@ -19,9 +19,9 @@ class IRoutingResult public: /// \returns information about all route segments. virtual TUnpackedPathSegments const & GetSegments() const = 0; - /// \brief For a |node|, |junctionPoint| and |ingoingPoint| (point before the |node|) + /// \brief For a |segmentRange|, |junctionPoint| and |ingoingPoint| (point before the |node|) /// this method computes number of ingoing ways to |junctionPoint| and fills |outgoingTurns|. - virtual void GetPossibleTurns(UniNodeId const & node, m2::PointD const & ingoingPoint, + virtual void GetPossibleTurns(SegmentRange const & segmentRange, m2::PointD const & ingoingPoint, m2::PointD const & junctionPoint, size_t & ingoingCount, TurnCandidates & outgoingTurns) const = 0; virtual double GetPathLength() const = 0; diff --git a/routing/routing_tests/turns_generator_test.cpp b/routing/routing_tests/turns_generator_test.cpp index 8f57018982..5649fe4031 100644 --- a/routing/routing_tests/turns_generator_test.cpp +++ b/routing/routing_tests/turns_generator_test.cpp @@ -330,7 +330,7 @@ UNIT_TEST(TestCheckUTurnOnRoute) TUnpackedPathSegments pathSegments(4, LoadedPathSegment()); pathSegments[0].m_name = "A road"; pathSegments[0].m_weight = 1; - pathSegments[0].m_nodeId = UniNodeId(FeatureID(), 0 /* start seg id */, 1 /* end seg id */, + pathSegments[0].m_segmentRange = SegmentRange(FeatureID(), 0 /* start seg id */, 1 /* end seg id */, true /* forward */); pathSegments[0].m_highwayClass = ftypes::HighwayClass::Trunk; pathSegments[0].m_onRoundabout = false; @@ -338,17 +338,17 @@ UNIT_TEST(TestCheckUTurnOnRoute) pathSegments[0].m_path = {{{0, 0}, 0}, {{0, 1}, 0}}; pathSegments[1] = pathSegments[0]; - pathSegments[1].m_nodeId = UniNodeId(FeatureID(), 1 /* start seg id */, 2 /* end seg id */, + pathSegments[1].m_segmentRange = SegmentRange(FeatureID(), 1 /* start seg id */, 2 /* end seg id */, true /* forward */); pathSegments[1].m_path = {{{0, 1}, 0}, {{0, 0}, 0}}; pathSegments[2] = pathSegments[0]; - pathSegments[2].m_nodeId = UniNodeId(FeatureID(), 2 /* start seg id */, 3 /* end seg id */, + pathSegments[2].m_segmentRange = SegmentRange(FeatureID(), 2 /* start seg id */, 3 /* end seg id */, true /* forward */); pathSegments[2].m_path = {{{0, 0}, 0}, {{0, 1}, 0}}; pathSegments[3] = pathSegments[0]; - pathSegments[3].m_nodeId = UniNodeId(FeatureID(), 3 /* start seg id */, 4 /* end seg id */, + pathSegments[3].m_segmentRange = SegmentRange(FeatureID(), 3 /* start seg id */, 4 /* end seg id */, true /* forward */); pathSegments[3].m_path.clear(); diff --git a/routing/turn_candidate.hpp b/routing/turn_candidate.hpp index d4fb7f4f04..f5ba69b39a 100644 --- a/routing/turn_candidate.hpp +++ b/routing/turn_candidate.hpp @@ -26,19 +26,18 @@ struct TurnCandidate */ double angle; /*! - * |m_nodeId| is a possible node (a possible way) from the juction. - * |m_nodeId| contain either only unique NodeID for OSRM case or mwm id, feature id, segment id - * and direction in case of A*. + * |m_segmentRange| is a possible way from the junction. + * |m_segmentRange| contains mwm id, feature id, segment id and direction in case of A*. */ - UniNodeId m_nodeId; + SegmentRange m_segmentRange; /*! * \brief highwayClass field for the road class caching. Because feature reading is a long * function. */ ftypes::HighwayClass highwayClass; - TurnCandidate(double a, UniNodeId const & n, ftypes::HighwayClass c) - : angle(a), m_nodeId(n), highwayClass(c) + TurnCandidate(double a, SegmentRange const & segmentRange, ftypes::HighwayClass c) + : angle(a), m_segmentRange(segmentRange), highwayClass(c) { } }; diff --git a/routing/turns.cpp b/routing/turns.cpp index a03ed490cf..15069973b5 100644 --- a/routing/turns.cpp +++ b/routing/turns.cpp @@ -51,8 +51,8 @@ static_assert(g_turnNames.size() == static_cast(CarDirection::Count), namespace routing { -// UniNodeId ------------------------------------------------------------------- -UniNodeId::UniNodeId(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, +// SegmentRange ----------------------------------------------------------------------------------- +SegmentRange::SegmentRange(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, bool forward) : m_featureId(featureId) , m_startSegId(startSegId) @@ -61,13 +61,13 @@ UniNodeId::UniNodeId(FeatureID const & featureId, uint32_t startSegId, uint32_t { } -bool UniNodeId::operator==(UniNodeId const & rhs) const +bool SegmentRange::operator==(SegmentRange const & rhs) const { return m_featureId == rhs.m_featureId && m_startSegId == rhs.m_startSegId && m_endSegId == rhs.m_endSegId && m_forward == rhs.m_forward; } -bool UniNodeId::operator<(UniNodeId const & rhs) const +bool SegmentRange::operator<(SegmentRange const & rhs) const { if (m_featureId != rhs.m_featureId) return m_featureId < rhs.m_featureId; @@ -81,7 +81,7 @@ bool UniNodeId::operator<(UniNodeId const & rhs) const return m_forward < rhs.m_forward; } -void UniNodeId::Clear() +void SegmentRange::Clear() { m_featureId = FeatureID(); m_startSegId = 0; @@ -89,19 +89,19 @@ void UniNodeId::Clear() m_forward = true; } -FeatureID const & UniNodeId::GetFeature() const +FeatureID const & SegmentRange::GetFeature() const { return m_featureId; } -bool UniNodeId::IsCorrect() const +bool SegmentRange::IsCorrect() const { return (m_forward && m_startSegId <= m_endSegId) || (!m_forward && m_endSegId <= m_startSegId); } namespace turns { -// SingleLaneInfo -------------------------------------------------------------- +// SingleLaneInfo --------------------------------------------------------------------------------- bool SingleLaneInfo::operator==(SingleLaneInfo const & other) const { return m_lane == other.m_lane && m_isRecommended == other.m_isRecommended; diff --git a/routing/turns.hpp b/routing/turns.hpp index e3af50d81a..adbe32d711 100644 --- a/routing/turns.hpp +++ b/routing/turns.hpp @@ -11,29 +11,27 @@ namespace routing { -using TNodeId = uint32_t; -using TEdgeWeight = double; - -/// \brief Unique identification for a road edge between two junctions (joints). -/// It's mwm id, feature id, range of segment ids [|m_startSegId|, |m_endSegId|) and direction. -struct UniNodeId +/// \brief Unique identification for a road edge between two junctions (joints). The identifier +/// is represented by an mwm id, a feature id, a range of segment ids [|m_startSegId|, |m_endSegId|) +/// and a direction. +struct SegmentRange { - UniNodeId() = default; - UniNodeId(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, bool forward); - bool operator==(UniNodeId const & rh) const; - bool operator<(UniNodeId const & rh) const; + SegmentRange() = default; + SegmentRange(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, bool forward); + bool operator==(SegmentRange const & rh) const; + bool operator<(SegmentRange const & rh) const; void Clear(); FeatureID const & GetFeature() const; - /// \returns true if the instance of UniNodeId is correct. + /// \returns true if the instance of SegmentRange is correct. bool IsCorrect() const; private: FeatureID m_featureId; - // Note. In mwm case if UniNodeId represents two directional feature |m_endSegId| is greater + // Note. If SegmentRange represents two directional feature |m_endSegId| is greater // than |m_startSegId| if |m_forward| == true. - uint32_t m_startSegId = 0; // The first segment index of UniNodeId. - uint32_t m_endSegId = 0; // The last segment index UniNodeId. + uint32_t m_startSegId = 0; // The first segment index of SegmentRange. + uint32_t m_endSegId = 0; // The last segment index of SegmentRange. bool m_forward = true; // Segment direction in |m_featureId|. }; diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index e43b461e4f..a24027f978 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -45,7 +45,7 @@ bool KeepTurnByHighwayClass(CarDirection turn, TurnCandidates const & possibleTu ftypes::HighwayClass maxClassForPossibleTurns = ftypes::HighwayClass::Error; for (auto const & t : possibleTurns.candidates) { - if (t.m_nodeId == turnInfo.m_outgoing.m_nodeId) + if (t.m_segmentRange == turnInfo.m_outgoing.m_segmentRange) continue; ftypes::HighwayClass const highwayClass = t.highwayClass; if (static_cast(highwayClass) > static_cast(maxClassForPossibleTurns)) @@ -84,7 +84,7 @@ bool KeepRoundaboutTurnByHighwayClass(CarDirection turn, TurnCandidates const & { for (auto const & t : possibleTurns.candidates) { - if (t.m_nodeId == turnInfo.m_outgoing.m_nodeId) + if (t.m_segmentRange == turnInfo.m_outgoing.m_segmentRange) continue; if (static_cast(t.highwayClass) != static_cast(ftypes::HighwayClass::Service)) return true; @@ -572,7 +572,7 @@ void GetTurnDirection(IRoutingResult const & result, TurnInfo & turnInfo, TurnIt m2::PointD const ingoingPointOneSegment = turnInfo.m_ingoing.m_path[turnInfo.m_ingoing.m_path.size() - 2].GetPoint(); TurnCandidates nodes; size_t ingoingCount; - result.GetPossibleTurns(turnInfo.m_ingoing.m_nodeId, ingoingPointOneSegment, junctionPoint, + result.GetPossibleTurns(turnInfo.m_ingoing.m_segmentRange, ingoingPointOneSegment, junctionPoint, ingoingCount, nodes); size_t const numNodes = nodes.candidates.size(); @@ -587,9 +587,9 @@ void GetTurnDirection(IRoutingResult const & result, TurnInfo & turnInfo, TurnIt } else { - if (nodes.candidates.front().m_nodeId == turnInfo.m_outgoing.m_nodeId) + if (nodes.candidates.front().m_segmentRange == turnInfo.m_outgoing.m_segmentRange) turn.m_turn = LeftmostDirection(turnAngle); - else if (nodes.candidates.back().m_nodeId == turnInfo.m_outgoing.m_nodeId) + else if (nodes.candidates.back().m_segmentRange == turnInfo.m_outgoing.m_segmentRange) turn.m_turn = RightmostDirection(turnAngle); else turn.m_turn = intermediateDirection;