Generating turn from first route segment. Fixing unique segment invariant violation.

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
@@ -65,6 +65,13 @@ Edge Edge::MakeReal(FeatureID const & featureId, bool forward, uint32_t segId,
 }
 
 // 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)
 {
   return {Type::FakeWithoutRealPart, FeatureID(), true /* forward */, 0 /* segmentId */,
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<VehicleType::Bicycle>(),
-                                  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