From 20d8de0d7ca40acc18ab895916039a4e572fb20a Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 7 Feb 2018 16:39:16 +0300 Subject: [PATCH] Generating turn from first route segment. Fixing unique segment invariant violation. --- routing/bicycle_directions.cpp | 7 ++--- routing/bicycle_directions.hpp | 1 + routing/index_road_graph.cpp | 14 +++++++++- routing/road_graph.cpp | 7 +++++ routing/road_graph.hpp | 2 ++ routing/routing_helpers.cpp | 1 - .../bicycle_turn_test.cpp | 15 +++++------ .../routing_integration_tests/turn_test.cpp | 7 ++--- routing/turns.cpp | 26 +++++++++++++------ routing/turns.hpp | 26 ++++++++++++++----- 10 files changed, 74 insertions(+), 32 deletions(-) diff --git a/routing/bicycle_directions.cpp b/routing/bicycle_directions.cpp index 3075b77aee..b6a583b683 100644 --- a/routing/bicycle_directions.cpp +++ b/routing/bicycle_directions.cpp @@ -243,7 +243,8 @@ void BicycleDirectionsEngine::GetSegmentRangeAndAdjacentEdges( { outgoingTurns.isCandidatesAngleValid = true; outgoingTurns.candidates.reserve(outgoingEdges.size()); - segmentRange = SegmentRange(inEdge.GetFeatureId(), startSegId, endSegId, inEdge.IsForward()); + segmentRange = SegmentRange(inEdge.GetFeatureId(), startSegId, endSegId, inEdge.IsForward(), + inEdge.GetStartPoint(), inEdge.GetEndPoint()); CHECK(segmentRange.IsCorrect(), ()); m2::PointD const & ingoingPoint = inEdge.GetStartJunction().GetPoint(); m2::PointD const & junctionPoint = inEdge.GetEndJunction().GetPoint(); @@ -366,9 +367,9 @@ void BicycleDirectionsEngine::FillPathSegmentsAndAdjacentEdgesMap( if (!segmentRange.IsEmpty()) { auto const it = m_adjacentEdges.find(segmentRange); - // A route may be build through intermediate points. So it may contain the same |segmentRange| + // A route may be built through intermediate points. So it may contain the same |segmentRange| // several times. But in that case |adjacentEdges| corresponds to |segmentRange| - // should be the same as well. + // should be the same as well except for one case. ASSERT(it == m_adjacentEdges.cend() || it->second.IsAlmostEqual(adjacentEdges), ("segmentRange:", segmentRange, "corresponds to adjacent edges which aren't equal.")); diff --git a/routing/bicycle_directions.hpp b/routing/bicycle_directions.hpp index 8f223d8323..7dd85e91f9 100644 --- a/routing/bicycle_directions.hpp +++ b/routing/bicycle_directions.hpp @@ -2,6 +2,7 @@ #include "routing/directions_engine.hpp" #include "routing/loaded_path_segment.hpp" +#include "routing/segment.hpp" #include "routing/turn_candidate.hpp" #include "routing_common/num_mwm_id.hpp" diff --git a/routing/index_road_graph.cpp b/routing/index_road_graph.cpp index 937dbbf3f8..107f499e26 100644 --- a/routing/index_road_graph.cpp +++ b/routing/index_road_graph.cpp @@ -86,7 +86,19 @@ void IndexRoadGraph::GetRouteEdges(TEdgeVector & edges) const auto const & junctionTo = m_starter.GetJunction(segment, true /* front */); if (IndexGraphStarter::IsFakeSegment(segment) || TransitGraph::IsTransitSegment(segment)) { - edges.push_back(Edge::MakeFake(junctionFrom, junctionTo)); + Segment real = segment; + if (m_starter.ConvertToReal(real)) + { + platform::CountryFile const & file = m_numMwmIds->GetFile(real.GetMwmId()); + MwmSet::MwmId const mwmId = m_index.GetMwmIdByCountryFile(file); + edges.push_back(Edge::MakeFakeWithRealPart(FeatureID(mwmId, real.GetFeatureId()), + real.IsForward(), real.GetSegmentIdx(), + junctionFrom, junctionTo)); + } + else + { + edges.push_back(Edge::MakeFake(junctionFrom, junctionTo)); + } } else { diff --git a/routing/road_graph.cpp b/routing/road_graph.cpp index adabf38ac1..f152dbfd9e 100644 --- a/routing/road_graph.cpp +++ b/routing/road_graph.cpp @@ -64,6 +64,13 @@ Edge Edge::MakeReal(FeatureID const & featureId, bool forward, uint32_t segId, return {Type::Real, featureId, forward, segId, startJunction, endJunction}; } +// static +Edge Edge::MakeFakeWithRealPart(FeatureID const & featureId, bool forward, uint32_t segId, + Junction const & startJunction, Junction const & endJunction) +{ + return {Type::FakeWithRealPart, featureId, forward, segId, startJunction, endJunction}; +} + // static Edge Edge::MakeFake(Junction const & startJunction, Junction const & endJunction) { diff --git a/routing/road_graph.hpp b/routing/road_graph.hpp index 9b6d635d74..b86b8c1dcf 100644 --- a/routing/road_graph.hpp +++ b/routing/road_graph.hpp @@ -68,6 +68,8 @@ public: static Edge MakeReal(FeatureID const & featureId, bool forward, uint32_t segId, Junction const & startJunction, Junction const & endJunction); + static Edge MakeFakeWithRealPart(FeatureID const & featureId, bool forward, uint32_t segId, + Junction const & startJunction, Junction const & endJunctio); static Edge MakeFake(Junction const & startJunction, Junction const & endJunction); static Edge MakeFake(Junction const & startJunction, Junction const & endJunction, Edge const & prototype); diff --git a/routing/routing_helpers.cpp b/routing/routing_helpers.cpp index 295a7ceb69..c0b1101675 100644 --- a/routing/routing_helpers.cpp +++ b/routing/routing_helpers.cpp @@ -152,7 +152,6 @@ Segment ConvertEdgeToSegment(NumMwmIds const & numMwmIds, Edge const & edge) if (edge.IsFake()) return Segment(); - NumMwmId const numMwmId = numMwmIds.GetId(edge.GetFeatureId().m_mwmId.GetInfo()->GetLocalFile().GetCountryFile()); return Segment(numMwmId, edge.GetFeatureId().m_index, edge.GetSegId(), edge.IsForward()); diff --git a/routing/routing_integration_tests/bicycle_turn_test.cpp b/routing/routing_integration_tests/bicycle_turn_test.cpp index a858a61925..f986bca74e 100644 --- a/routing/routing_integration_tests/bicycle_turn_test.cpp +++ b/routing/routing_integration_tests/bicycle_turn_test.cpp @@ -46,24 +46,22 @@ UNIT_TEST(RussiaMoscowGerPanfilovtsev22BicycleWayTurnTest) integration::GetNthTurn(route, 1).TestValid().TestDirection(CarDirection::TurnLeft); } -// This test fails. The reason is DiscardTurnByIngoingAndOutgoingEdges() method which -// discard turn candidates at very beginning stage by marks which are possible to get -// effectively. After moving to IndexGraph all marks are possible to get efficiently. -// So DiscardTurnByIngoingAndOutgoingEdges() method should be removed. UNIT_TEST(RussiaMoscowSalameiNerisPossibleTurnCorrectionBicycleWayTurnTest) { TRouteResult const routeResult = integration::CalculateRoute(integration::GetVehicleComponents(), - MercatorBounds::FromLatLon(55.85833, 37.36783), {0.0, 0.0}, + MercatorBounds::FromLatLon(55.85836, 37.36773), {0.0, 0.0}, MercatorBounds::FromLatLon(55.85364, 37.37318)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; TEST_EQUAL(result, IRouter::NoError, ()); - integration::TestTurnCount(route, 2 /* expectedTurnCount */); - integration::GetNthTurn(route, 0).TestValid().TestDirection(CarDirection::TurnSlightLeft); + integration::TestTurnCount(route, 4 /* expectedTurnCount */); + integration::GetNthTurn(route, 0).TestValid().TestDirection(CarDirection::TurnSlightRight); integration::GetNthTurn(route, 1).TestValid().TestDirection(CarDirection::TurnSlightLeft); + integration::GetNthTurn(route, 2).TestValid().TestDirection(CarDirection::TurnSlightLeft); + integration::GetNthTurn(route, 3).TestValid().TestDirection(CarDirection::GoStraight); } // Test that there's no uturn notification in case of zero point edges on two way edges. @@ -78,7 +76,8 @@ UNIT_TEST(RussiaMoscowSalameiNerisNoUTurnBicycleWayTurnTest) IRouter::ResultCode const result = routeResult.second; TEST_EQUAL(result, IRouter::NoError, ()); - integration::TestTurnCount(route, 0 /* expectedTurnCount */); + integration::TestTurnCount(route, 1 /* expectedTurnCount */); + integration::GetNthTurn(route, 0).TestValid().TestDirection(CarDirection::TurnRight); } // Fails to build one-point route. diff --git a/routing/routing_integration_tests/turn_test.cpp b/routing/routing_integration_tests/turn_test.cpp index 93018d0ee3..c60886b022 100644 --- a/routing/routing_integration_tests/turn_test.cpp +++ b/routing/routing_integration_tests/turn_test.cpp @@ -472,11 +472,8 @@ UNIT_TEST(RussiaMoscowLeningradskiyPrptToTheCenterUTurnTest) IRouter::ResultCode const result = routeResult.second; TEST_EQUAL(result, IRouter::NoError, ()); - integration::TestTurnCount(route, 2 /* expectedTurnCount */); - integration::GetNthTurn(route, 0).TestValid().TestOneOfDirections( - {CarDirection::TurnLeft, CarDirection::TurnSlightLeft}); - integration::GetNthTurn(route, 1).TestValid().TestOneOfDirections( - {CarDirection::TurnLeft, CarDirection::TurnSlightLeft}); + integration::TestTurnCount(route, 1 /* expectedTurnCount */); + integration::GetNthTurn(route, 0).TestValid().TestDirection(CarDirection::UTurnLeft); } // Fails: generates unnecessary turn. diff --git a/routing/turns.cpp b/routing/turns.cpp index 1ce83e6d9e..fe31c039ff 100644 --- a/routing/turns.cpp +++ b/routing/turns.cpp @@ -61,18 +61,17 @@ namespace routing { // SegmentRange ----------------------------------------------------------------------------------- SegmentRange::SegmentRange(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, - bool forward) - : m_featureId(featureId) - , m_startSegId(startSegId) - , m_endSegId(endSegId) - , m_forward(forward) + bool forward, m2::PointD const & start, m2::PointD const & end) + : m_featureId(featureId), m_startSegId(startSegId), m_endSegId(endSegId), m_forward(forward), + m_start(start), m_end(end) { } 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; + m_endSegId == rhs.m_endSegId && m_forward == rhs.m_forward && m_start == rhs.m_start && + m_end == rhs.m_end; } bool SegmentRange::operator<(SegmentRange const & rhs) const @@ -86,7 +85,13 @@ bool SegmentRange::operator<(SegmentRange const & rhs) const if (m_endSegId != rhs.m_endSegId) return m_endSegId < rhs.m_endSegId; - return m_forward < rhs.m_forward; + if (m_forward != rhs.m_forward) + return m_forward < rhs.m_forward; + + if (m_start != rhs.m_start) + return m_start < rhs.m_start; + + return m_end < rhs.m_end; } void SegmentRange::Clear() @@ -95,11 +100,14 @@ void SegmentRange::Clear() m_startSegId = 0; m_endSegId = 0; m_forward = true; + m_start = m2::PointD::Zero(); + m_end = m2::PointD::Zero(); } bool SegmentRange::IsEmpty() const { - return !m_featureId.IsValid() && m_startSegId == 0 && m_endSegId == 0 && m_forward; + return !m_featureId.IsValid() && m_startSegId == 0 && m_endSegId == 0 && m_forward && + m_start == m2::PointD::Zero() && m_end == m2::PointD::Zero(); } FeatureID const & SegmentRange::GetFeature() const @@ -128,6 +136,8 @@ string DebugPrint(SegmentRange const & segmentRange) << ", m_startSegId = " << segmentRange.m_startSegId << ", m_endSegId = " << segmentRange.m_endSegId << ", m_forward = " << segmentRange.m_forward + << ", m_start = " << DebugPrint(segmentRange.m_start) + << ", m_end = " << DebugPrint(segmentRange.m_end) << "]" << endl; return out.str(); } diff --git a/routing/turns.hpp b/routing/turns.hpp index 3c33c502fe..a61b3e14dc 100644 --- a/routing/turns.hpp +++ b/routing/turns.hpp @@ -16,14 +16,15 @@ namespace routing { /// \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. +/// is represented by an mwm id, a feature id, a range of segment ids [|m_startSegId|, |m_endSegId|), +/// a direction and type. struct SegmentRange { friend std::string DebugPrint(SegmentRange const & segmentRange); SegmentRange() = default; - SegmentRange(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, bool forward); + SegmentRange(FeatureID const & featureId, uint32_t startSegId, uint32_t endSegId, bool forward, + m2::PointD const & start, m2::PointD const & end); bool operator==(SegmentRange const & rh) const; bool operator<(SegmentRange const & rh) const; void Clear(); @@ -38,9 +39,22 @@ private: FeatureID m_featureId; // 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 SegmentRange. - uint32_t m_endSegId = 0; // The last segment index of SegmentRange. - bool m_forward = true; // Segment direction in |m_featureId|. + 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|. + // Note. According to current implementation SegmentRange is filled based on instances of + // Edge class in IDirectionsEngine::GetSegmentRangeAndAdjacentEdges() method. In Edge class + // to identify fake edges (part of real and completely fake) is used coordinates of beginning + // and ending of the edge. To keep SegmentRange instances unique for unique edges + // in case of fake edges it's necessary to have |m_start| and |m_end| fields below. + // @TODO(bykoianko) It's necessary to get rid of |m_start| and |m_end|. + // Fake edges in IndexGraph is identified by instances of Segment + // with Segment::m_mwmId == kFakeNumMwmId. So instead of |m_featureId| field in this class + // number mwm id field and feature id (uint32_t) should be used and |m_start| and |m_end| + // should be removed. To do that classes IndexRoadGraph, BicycleDirectionsEngine, + // PedestrianDirectionsEngine and other should be significant refactored. + m2::PointD m_start; // Coordinates of start of last Edge in SegmentRange. + m2::PointD m_end; // Coordinates of end of SegmentRange. }; namespace turns