From 15febd8423abcc4ae48f0e9e4f60f6a228c84b0d Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Thu, 24 Sep 2015 10:50:32 +0300 Subject: [PATCH 1/6] Fix style and two warnings. --- routing/osrm_router.cpp | 24 ++++++++++++------------ routing/router.hpp | 4 ++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 0d6249d53e..82c462b6d7 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -497,7 +497,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeRouteFromCrossesPath(TCheckedPath const & vector mwmPoints; MakeTurnAnnotation(routingResult, mwmMapping, delegate, mwmPoints, mwmTurnsDir, mwmTimes); // Connect annotated route. - const uint32_t pSize = static_cast(Points.size()); + auto const pSize = static_cast(Points.size()); for (auto turn : mwmTurnsDir) { if (turn.m_index == 0) @@ -705,17 +705,17 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( // Get all the coordinates for the computed route size_t const n = segment.size(); - for (size_t j = 0; j < n; ++j) + for (size_t j = 0; j < n; ++j) // todo(mgsergio) rename! { RawPathData const & path_data = segment[j]; if (j > 0 && !points.empty()) { - turns::TurnItem t; - t.m_index = static_cast(points.size() - 1); + turns::TurnItem turnItem; + turnItem.m_index = static_cast(points.size() - 1); turns::TurnInfo turnInfo(*mapping, segment[j - 1].node, segment[j].node); - turns::GetTurnDirection(*m_pIndex, turnInfo, t); + turns::GetTurnDirection(*m_pIndex, turnInfo, turnItem); // ETA information. // Osrm multiples seconds to 10, so we need to divide it back. @@ -726,19 +726,19 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( for (size_t k = lastIdx + 1; k < points.size(); ++k) distMeters += MercatorBounds::DistanceOnEarth(points[k - 1], points[k]); LOG(LDEBUG, ("Speed:", 3.6 * distMeters / nodeTimeSeconds, "kmph; Dist:", distMeters, "Time:", - nodeTimeSeconds, "s", lastIdx, "e", points.size(), "source:", t.m_sourceName, - "target:", t.m_targetName)); + nodeTimeSeconds, "s", lastIdx, "e", points.size(), "source:", turnItem.m_sourceName, + "target:", turnItem.m_targetName)); lastIdx = points.size(); #endif estimatedTime += nodeTimeSeconds; times.push_back(Route::TTimeItem(points.size(), estimatedTime)); // Lane information. - if (t.m_turn != turns::TurnDirection::NoTurn) + if (turnItem.m_turn != turns::TurnDirection::NoTurn) { - t.m_lanes = turns::GetLanesInfo(segment[j - 1].node, + turnItem.m_lanes = turns::GetLanesInfo(segment[j - 1].node, *mapping, turns::GetLastSegmentPointIndex, *m_pIndex); - turnsDir.push_back(move(t)); + turnsDir.push_back(move(turnItem)); } } @@ -769,7 +769,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( } if (j == n - 1) { - if (!segEnd.IsValid()) + if (!segEnd.IsValid()) continue; endK = FindIntersectingSeg(segEnd) + 1; } @@ -827,7 +827,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( if (routingResult.targetEdge.segment.IsValid()) { turnsDir.push_back( - turns::TurnItem(static_cast(points.size() - 1), turns::TurnDirection::ReachedYourDestination)); + turns::TurnItem(static_cast(points.size()) - 1, turns::TurnDirection::ReachedYourDestination)); } turns::FixupTurns(points, turnsDir); diff --git a/routing/router.hpp b/routing/router.hpp index c5dcf0a467..bd46b60400 100644 --- a/routing/router.hpp +++ b/routing/router.hpp @@ -17,7 +17,7 @@ using TCountryFileFn = function; class Route; /// Routing engine type. -enum RouterType +enum class RouterType { Vehicle = 0, /// For OSRM vehicle routing Pedestrian /// For A star pedestrian routing @@ -31,7 +31,7 @@ public: /// Routing possible statuses enumeration. /// \warning this enum has JNI mirror! /// \see android/src/com/mapswithme/maps/data/RoutingResultCodesProcessor.java - enum ResultCode + enum ResultCode // TODO(mgsergio) enum class { NoError = 0, Cancelled = 1, From 3f76a37b4c4ed73d7f7a017d36b60923d3073f45 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Fri, 25 Sep 2015 12:00:27 +0300 Subject: [PATCH 2/6] Style: using, variable names, intermediate variables. --- indexer/feature.hpp | 2 +- routing/turns_generator.cpp | 18 ++++++++++-------- routing/turns_generator.hpp | 5 ++++- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/indexer/feature.hpp b/indexer/feature.hpp index a96882f344..5407b2390b 100644 --- a/indexer/feature.hpp +++ b/indexer/feature.hpp @@ -29,7 +29,7 @@ class FeatureBase public: - typedef char const * BufferT; + using BufferT = char const *; void Deserialize(feature::LoaderBase * pLoader, BufferT buffer); diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 6e351fda5c..ad63403e19 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -49,7 +49,7 @@ struct TurnCandidate TurnCandidate(double a, NodeID n) : angle(a), node(n) {} }; -typedef vector TTurnCandidates; +using TTurnCandidates = vector; /*! * \brief The Point2Geometry class is responsable for looking for all adjacent to junctionPoint @@ -251,8 +251,10 @@ bool KeepTurnByIngoingEdges(m2::PointD const & junctionPoint, m2::PointD const & outgoingPoint, bool hasMultiTurns, RoutingMapping const & routingMapping, Index const & index) { - bool const isGoStraightOrSlightTurn = IsGoStraightOrSlightTurn(IntermediateDirection( - my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPointOneSegment, outgoingPoint)))); + double const turnAngle = + my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPointOneSegment, outgoingPoint)); + bool const isGoStraightOrSlightTurn = IsGoStraightOrSlightTurn(IntermediateDirection(turnAngle)); + // The code below is resposible for cases when there is only one way to leave the junction. // Such junction has to be kept as a turn when it's not a slight turn and it has ingoing edges // (one or more); @@ -738,8 +740,8 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) { return end > start ? start + i : start - i; }); - double const a = my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPoint, outgoingPoint)); - TurnDirection const intermediateDirection = IntermediateDirection(a); + double const turnAngle = my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPoint, outgoingPoint)); + TurnDirection const intermediateDirection = IntermediateDirection(turnAngle); // Getting all the information about ingoing and outgoing edges. turnInfo.m_isIngoingEdgeRoundabout = ftypes::IsRoundAboutChecker::Instance()(ingoingFeature); @@ -768,7 +770,7 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) turnInfo.m_routeMapping, nodes); size_t const nodesSize = nodes.size(); - bool const hasMultiTurns = (nodesSize >= 2); + bool const hasMultiTurns = nodesSize > 1; if (nodesSize == 0) return; @@ -780,9 +782,9 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) else { if (nodes.front().node == turnInfo.m_outgoingNodeID) - turn.m_turn = LeftmostDirection(a); + turn.m_turn = LeftmostDirection(turnAngle); else if (nodes.back().node == turnInfo.m_outgoingNodeID) - turn.m_turn = RightmostDirection(a); + turn.m_turn = RightmostDirection(turnAngle); else turn.m_turn = intermediateDirection; } diff --git a/routing/turns_generator.hpp b/routing/turns_generator.hpp index c9c250ea26..83461c6506 100644 --- a/routing/turns_generator.hpp +++ b/routing/turns_generator.hpp @@ -26,7 +26,7 @@ namespace turns /*! * \brief Returns a segment index by STL-like range [s, e) of segments indices for the passed node. */ -typedef function)> TGetIndexFunction; +using TGetIndexFunction = function)>; /*! * \brief The TurnInfo struct is an accumulator for all junction information. @@ -116,6 +116,9 @@ bool CheckRoundaboutExit(bool isIngoingEdgeRoundabout, bool isOutgoingEdgeRounda */ TurnDirection GetRoundaboutDirection(bool isIngoingEdgeRoundabout, bool isOutgoingEdgeRoundabout, bool isMultiTurnJunction, bool keepTurnByHighwayClass); + + + /*! * \brief GetTurnDirection makes a primary decision about turns on the route. * \param turnInfo is used for cashing some information while turn calculation. From 6e4d4d6890de7317d362bc632c37de8d904a806b Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 29 Sep 2015 12:59:37 +0300 Subject: [PATCH 3/6] Swich to MercatorBounds::FromLatLon for all tests in osrm_turn_test.cpp. Add One More Test. --- integration_tests/osrm_turn_test.cpp | 103 +++++++++++++++++++-------- 1 file changed, 74 insertions(+), 29 deletions(-) diff --git a/integration_tests/osrm_turn_test.cpp b/integration_tests/osrm_turn_test.cpp index 8a7b692a9c..be41140343 100644 --- a/integration_tests/osrm_turn_test.cpp +++ b/integration_tests/osrm_turn_test.cpp @@ -8,11 +8,12 @@ using namespace routing; using namespace routing::turns; -UNIT_TEST(RussiaMoscowLenigradskiy39UturnTurnTest) +UNIT_TEST(RussiaMoscowLenigradskiy39UturnTurnTest) // FAILED! { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.537544383032568, 67.536216737893028}, {0., 0.}, - {37.538908531885973, 67.54544090660923}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.796936768447288557, 37.537544383032567907), {0., 0.}, + MercatorBounds::FromLatLon(55.802121583039429709, 37.538908531885972764)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -34,8 +35,9 @@ UNIT_TEST(RussiaMoscowLenigradskiy39UturnTurnTest) UNIT_TEST(RussiaMoscowSalameiNerisUturnTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.395332276656617, 67.633925439079519}, {0., 0.}, - {37.392503720352721, 67.61975260731343}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.851822802555439296, 37.39533227665661741), {0., 0.}, + MercatorBounds::FromLatLon(55.843866280669693936, 37.392503720352721075)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -61,11 +63,12 @@ UNIT_TEST(RussiaMoscowSalameiNerisUturnTurnTest) integration::TestRouteLength(route, 1637.); } -UNIT_TEST(RussiaMoscowTrikotagniAndPohodniRoundaboutTurnTest) +UNIT_TEST(RussiaMoscowTrikotagniAndPohodniRoundaboutTurnTest) //Failed { - TRouteResult const routeResult = - integration::CalculateRoute(integration::GetOsrmComponents(), {37.405153751040686, 67.5971698246356}, - {0., 0.}, {37.40521071657038, 67.601903779043795}); + TRouteResult const routeResult = integration::CalculateRoute( + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.831185109716038539, 37.405153751040685961), {0., 0.}, + MercatorBounds::FromLatLon(55.833843764465711956, 37.405210716570380214)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -84,8 +87,10 @@ UNIT_TEST(RussiaMoscowTrikotagniAndPohodniRoundaboutTurnTest) UNIT_TEST(RussiaMoscowPlanetnaiTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.546683164991776, 67.545511147376089}, {0., 0.}, - {37.549153861529007, 67.54467404790482}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.802161062035722239, 37.546683164991776493), {0., 0.}, + MercatorBounds::FromLatLon(55.801690565608659256, 37.549153861529006804)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -99,8 +104,10 @@ UNIT_TEST(RussiaMoscowPlanetnaiTurnTest) UNIT_TEST(RussiaMoscowNoTurnsOnMKADTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.391635636579785, 67.62455792789649}, {0., 0.}, - {37.692547253527685, 67.127684414191762}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.846564134248033895, 37.391635636579785285), {0., 0.}, + MercatorBounds::FromLatLon(55.566611639650901111, 37.692547253527685314)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -114,11 +121,13 @@ UNIT_TEST(RussiaMoscowNoTurnsOnMKADTurnTest) integration::TestRouteLength(route, 43233.7); } -UNIT_TEST(RussiaMoscowTTKKashirskoeShosseOutTurnTest) +UNIT_TEST(RussiaMoscowTTKKashirskoeShosseOutTurnTest) // Failed { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.606320023648998, 67.36682695403141}, {0., 0.}, - {37.621220025471168, 67.352441627022912}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.70160163595921432, 37.606320023648997619), {0., 0.}, + MercatorBounds::FromLatLon(55.693494620969019593, 37.621220025471167503)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -132,8 +141,10 @@ UNIT_TEST(RussiaMoscowTTKKashirskoeShosseOutTurnTest) UNIT_TEST(RussiaMoscowPankratevskiPerBolshaySuharedskazPloschadTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.635563528539393, 67.491460725721268}, {0., 0.}, - {37.637054339197832, 67.491929797067371}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.771770051239521138, 37.635563528539393019), {0., 0.}, + MercatorBounds::FromLatLon(55.772033898671161012, 37.637054339197831609)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -145,8 +156,10 @@ UNIT_TEST(RussiaMoscowPankratevskiPerBolshaySuharedskazPloschadTurnTest) UNIT_TEST(RussiaMoscowMKADPutilkovskeShosseTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.394141645624103, 67.63612831787222}, {0., 0.}, - {37.391050708989461, 67.632454269643091}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.853059336007213176, 37.394141645624102921), {0., 0.}, + MercatorBounds::FromLatLon(55.850996974780500182, 37.391050708989460816)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -159,8 +172,10 @@ UNIT_TEST(RussiaMoscowMKADPutilkovskeShosseTurnTest) UNIT_TEST(RussiaMoscowPetushkovaShodniaReverTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.405917692164508, 67.614731601278493}, {0., 0.}, - {37.408550782937482, 67.61160397953185}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.841047134659639539, 37.405917692164507571), {0., 0.}, + MercatorBounds::FromLatLon(55.839290964451890886, 37.408550782937481927)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -171,8 +186,10 @@ UNIT_TEST(RussiaMoscowPetushkovaShodniaReverTurnTest) UNIT_TEST(RussiaHugeRoundaboutTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.325810690728495, 67.544175542376436}, {0., 0.}, - {37.325360456262153, 67.543013703414516}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.801410375094782523, 37.325810690728495445), {0., 0.}, + MercatorBounds::FromLatLon(55.800757342904596214, 37.325360456262153264)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -191,8 +208,10 @@ UNIT_TEST(RussiaHugeRoundaboutTurnTest) UNIT_TEST(BelarusMiskProspNezavisimostiMKADTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {27.658572046123947, 64.302533720228126}, {0., 0.}, - {27.670461944729382, 64.307480201489582}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(53.936425941857663702, 27.658572046123946819), {0., 0.}, + MercatorBounds::FromLatLon(53.939337747485986085, 27.670461944729382253)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -207,7 +226,10 @@ UNIT_TEST(BelarusMiskProspNezavisimostiMKADTurnTest) UNIT_TEST(RussiaMoscowPetushkovaPetushkovaTest) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {37.40555, 67.60640}, {0., 0.}, {37.40489, 67.60766}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.836368736509733424, 37.405549999999998079), {0., 0.}, + MercatorBounds::FromLatLon(55.837076293494696699, 37.404890000000001749)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -223,6 +245,7 @@ UNIT_TEST(RussiaMoscowMKADLeningradkaTest) TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), MercatorBounds::FromLatLon(55.87992, 37.43940), {0., 0.}, MercatorBounds::FromLatLon(55.87854, 37.44865)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -233,7 +256,10 @@ UNIT_TEST(RussiaMoscowMKADLeningradkaTest) UNIT_TEST(BelarusMKADShosseinai) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {29.43123, 66.68486}, {0., 0.}, {29.42626, 66.68687}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(55.315418609956843454, 29.431229999999999336), {0., 0.}, + MercatorBounds::FromLatLon(55.316562400560414403, 29.426259999999999195)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -248,7 +274,10 @@ UNIT_TEST(BelarusMKADShosseinai) UNIT_TEST(ThailandPhuketNearPrabarameeRoad) { TRouteResult const routeResult = integration::CalculateRoute( - integration::GetOsrmComponents(), {98.36937, 7.94330}, {0., 0.}, {98.36785, 7.93247}); + integration::GetOsrmComponents(), + MercatorBounds::FromLatLon(7.9179763567919980716, 98.369370000000003529), {0., 0.}, + MercatorBounds::FromLatLon(7.9072494672603861332, 98.367850000000004229)); + Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -273,3 +302,19 @@ UNIT_TEST(RussiaMoscowVarshavskoeShosseMKAD) integration::GetNthTurn(route, 0).TestValid().TestOneOfDirections( {TurnDirection::TurnSlightRight, TurnDirection::TurnRight}); } + +// Test case: a rote goes in Moscow from Tverskaya street (towards city center) +// to Mokhovaya street. A turn instraction (turn left) shell be generated. +UNIT_TEST(RussiaMoscowTverskajaOkhotnyRyadTest) +{ + TRouteResult const routeResult = integration::CalculateRoute( + integration::GetOsrmComponents(), MercatorBounds::FromLatLon(55.75765, 37.61355), {0., 0.}, + MercatorBounds::FromLatLon(55.75737, 37.61601)); + + Route const & route = *routeResult.first; + IRouter::ResultCode const result = routeResult.second; + + TEST_EQUAL(result, IRouter::NoError, ()); + integration::TestTurnCount(route, 1); + integration::GetNthTurn(route, 0).TestValid().TestDirection(TurnDirection::TurnLeft); +} From 57e9dffb10e1e60e501c252e47ef296cdcded6f6 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 29 Sep 2015 14:23:55 +0300 Subject: [PATCH 4/6] Fix GetTurnDirection. --- routing/turns_generator.cpp | 56 ++++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index ad63403e19..6ff7e151e9 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -332,20 +332,21 @@ TurnDirection FindDirectionByAngle(vector> const & l * - start is an index of the start point of a feature segment. For example, FtSeg::m_pointStart. * - end is an index of the end point of a feature segment. For example, FtSeg::m_pointEnd. * - shift is a number of points which shall be added to end or start index. After that - * the sum reflects an index of a point of a feature segment which will be used for turn calculation. + * the sum reflects an index of a feature segment point which will be used for a turn calculation. * The sum shall belongs to a range [min(start, end), max(start, end)]. * shift belongs to a range [0, abs(end - start)]. - * \return an ingoing or outgoing point for turn calculation. + * \return an ingoing or outgoing point for a turn calculation. */ m2::PointD GetPointForTurn(OsrmMappingTypes::FtSeg const & segment, FeatureType const & ft, m2::PointD const & junctionPoint, - size_t (*GetPointIndex)(const size_t start, const size_t end, const size_t shift)) + size_t (*GetPointIndex)(const size_t start, const size_t end, const size_t shift), + size_t const maxPointsCount = 7, // An ingoing and outgoing point could be farther + // then kMaxPointsCount points from the junctionPoint + double const minDistMeters = 300. // If ft feature is long enough and consists of short segments + // the point for turn generation is taken as the next point + // along the route after kMinDistMeters. + ) { - // An ingoing and outgoing point could be farther then kMaxPointsCount points from the junctionPoint - size_t const kMaxPointsCount = 7; - // If ft feature is long enough and consist of short segments - // the point for turn generation is taken as the next point the route after kMinDistMeters. - double const kMinDistMeters = 300.; double curDistanceMeters = 0.; m2::PointD point = junctionPoint; m2::PointD nextPoint; @@ -353,16 +354,17 @@ m2::PointD GetPointForTurn(OsrmMappingTypes::FtSeg const & segment, FeatureType size_t const numSegPoints = abs(segment.m_pointEnd - segment.m_pointStart); ASSERT_GREATER(numSegPoints, 0, ()); ASSERT_LESS(numSegPoints, ft.GetPointsCount(), ()); - size_t const usedFtPntNum = min(kMaxPointsCount, numSegPoints); + size_t const usedFtPntNum = min(maxPointsCount, numSegPoints); for (size_t i = 1; i <= usedFtPntNum; ++i) { nextPoint = ft.GetPoint(GetPointIndex(segment.m_pointStart, segment.m_pointEnd, i)); curDistanceMeters += MercatorBounds::DistanceOnEarth(point, nextPoint); - if (curDistanceMeters > kMinDistMeters) + if (curDistanceMeters > minDistMeters) return nextPoint; point = nextPoint; } + return nextPoint; } @@ -727,19 +729,21 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) outgoingFeature.GetPoint(turnInfo.m_outgoingSegment.m_pointStart)), kFeaturesNearTurnMeters, ()); + auto const getIngoingPointIndex = [](const size_t start, const size_t end, const size_t i) + { + return end > start ? end - i : end + i; + }; + auto const getOutgoingPointIndex = [](const size_t start, const size_t end, const size_t i) + { + return end > start ? start + i : start - i; + }; + m2::PointD const junctionPoint = ingoingFeature.GetPoint(turnInfo.m_ingoingSegment.m_pointEnd); - m2::PointD const ingoingPoint = - GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, junctionPoint, - [](const size_t start, const size_t end, const size_t i) - { - return end > start ? end - i : end + i; - }); - m2::PointD const outgoingPoint = - GetPointForTurn(turnInfo.m_outgoingSegment, outgoingFeature, junctionPoint, - [](const size_t start, const size_t end, const size_t i) - { - return end > start ? start + i : start - i; - }); + m2::PointD const ingoingPoint = GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, + junctionPoint, getIngoingPointIndex); + m2::PointD const outgoingPoint = GetPointForTurn(turnInfo.m_outgoingSegment, outgoingFeature, + junctionPoint, getOutgoingPointIndex); + double const turnAngle = my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPoint, outgoingPoint)); TurnDirection const intermediateDirection = IntermediateDirection(turnAngle); @@ -804,7 +808,13 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) return; } - if (!KeepTurnByIngoingEdges(junctionPoint, ingoingPointOneSegment, outgoingPoint, hasMultiTurns, + + size_t constexpr kMaxPointsCount = 3; + double constexpr kMinDistMeters = 30.; + auto const notSoCloseToTheTurnPoint = GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, + junctionPoint, getIngoingPointIndex, + kMaxPointsCount, kMinDistMeters); + if (!KeepTurnByIngoingEdges(junctionPoint, notSoCloseToTheTurnPoint, outgoingPoint, hasMultiTurns, turnInfo.m_routeMapping, index)) { turn.m_turn = TurnDirection::NoTurn; From 2f884e795c0a4f324d896ed734d28c55bacdfae6 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 29 Sep 2015 15:17:16 +0300 Subject: [PATCH 5/6] Add turn tests. --- integration_tests/osrm_turn_test.cpp | 34 ++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/integration_tests/osrm_turn_test.cpp b/integration_tests/osrm_turn_test.cpp index be41140343..971010c659 100644 --- a/integration_tests/osrm_turn_test.cpp +++ b/integration_tests/osrm_turn_test.cpp @@ -303,8 +303,8 @@ UNIT_TEST(RussiaMoscowVarshavskoeShosseMKAD) {TurnDirection::TurnSlightRight, TurnDirection::TurnRight}); } -// Test case: a rote goes in Moscow from Tverskaya street (towards city center) -// to Mokhovaya street. A turn instraction (turn left) shell be generated. +// Test case: a route goes in Moscow from Tverskaya street (towards city center) +// to Mokhovaya street. A turn instruction (turn left) shell be generated. UNIT_TEST(RussiaMoscowTverskajaOkhotnyRyadTest) { TRouteResult const routeResult = integration::CalculateRoute( @@ -318,3 +318,33 @@ UNIT_TEST(RussiaMoscowTverskajaOkhotnyRyadTest) integration::TestTurnCount(route, 1); integration::GetNthTurn(route, 0).TestValid().TestDirection(TurnDirection::TurnLeft); } + +UNIT_TEST(RussiaMoscowBolshoyKislovskiyPerBolshayaNikitinskayaUlTest) +{ + TRouteResult const routeResult = integration::CalculateRoute( + integration::GetOsrmComponents(), MercatorBounds::FromLatLon(55.75574, 37.60702), {0., 0.}, + MercatorBounds::FromLatLon(55.75586, 37.60819)); + + Route const & route = *routeResult.first; + IRouter::ResultCode const result = routeResult.second; + + TEST_EQUAL(result, IRouter::NoError, ()); + integration::TestTurnCount(route, 1); + integration::GetNthTurn(route, 0).TestValid().TestDirection(TurnDirection::TurnRight); +} + +// Test case: a route goes in Moscow along Leningradskiy Prpt (towards city center) +// and makes u-turn. A only one turn instruction (turn left) shell be generated. +UNIT_TEST(RussiaMoscowLeningradskiyPrptToTheCenterUTurnTest) +{ + TRouteResult const routeResult = integration::CalculateRoute( + integration::GetOsrmComponents(), MercatorBounds::FromLatLon(55.79231, 37.54951), {0., 0.}, + MercatorBounds::FromLatLon(55.79280, 37.55028)); + + Route const & route = *routeResult.first; + IRouter::ResultCode const result = routeResult.second; + + TEST_EQUAL(result, IRouter::NoError, ()); + integration::TestTurnCount(route, 2); + integration::GetNthTurn(route, 0).TestValid().TestDirection(TurnDirection::TurnLeft); +} From e510c6564293ee8c01fec815db220064e625b0f1 Mon Sep 17 00:00:00 2001 From: Sergey Magidovich Date: Tue, 29 Sep 2015 17:55:28 +0300 Subject: [PATCH 6/6] Remove stupid comments. Truncate all coordinates to five numbers after decimal point. Get rid of default values. Small Fixes. Rename variables. Rename type. Fix according to notes. --- coding/coding_tests/trie_test.cpp | 6 +-- indexer/feature.cpp | 4 +- indexer/feature.hpp | 6 +-- indexer/feature_loader_base.cpp | 2 +- indexer/feature_loader_base.hpp | 6 +-- indexer/geometry_serialization.cpp | 14 +++--- indexer/geometry_serialization.hpp | 21 ++++---- integration_tests/osrm_turn_test.cpp | 72 ++++++++++++++-------------- routing/osrm_router.cpp | 32 ++++++------- routing/turns_generator.cpp | 54 ++++++++++----------- routing/turns_generator.hpp | 2 - 11 files changed, 109 insertions(+), 110 deletions(-) diff --git a/coding/coding_tests/trie_test.cpp b/coding/coding_tests/trie_test.cpp index b5789f93fe..cc936e0114 100644 --- a/coding/coding_tests/trie_test.cpp +++ b/coding/coding_tests/trie_test.cpp @@ -132,7 +132,7 @@ private: class Uint32ValueList { public: - using BufferT = vector; + using TBuffer = vector; void Append(uint32_t value) { @@ -146,11 +146,11 @@ public: template void Dump(TSink & sink) const { - sink.Write(m_values.data(), m_values.size() * sizeof(BufferT::value_type)); + sink.Write(m_values.data(), m_values.size() * sizeof(TBuffer::value_type)); } private: - BufferT m_values; + TBuffer m_values; }; } // unnamed namespace diff --git a/indexer/feature.cpp b/indexer/feature.cpp index 4d4f9fcfee..bc40766f35 100644 --- a/indexer/feature.cpp +++ b/indexer/feature.cpp @@ -17,7 +17,7 @@ using namespace feature; // FeatureBase implementation /////////////////////////////////////////////////////////////////////////////////////////////////// -void FeatureBase::Deserialize(feature::LoaderBase * pLoader, BufferT buffer) +void FeatureBase::Deserialize(feature::LoaderBase * pLoader, TBuffer buffer) { m_pLoader = pLoader; m_pLoader->Init(buffer); @@ -76,7 +76,7 @@ string FeatureBase::DebugString() const // FeatureType implementation /////////////////////////////////////////////////////////////////////////////////////////////////// -void FeatureType::Deserialize(feature::LoaderBase * pLoader, BufferT buffer) +void FeatureType::Deserialize(feature::LoaderBase * pLoader, TBuffer buffer) { base_type::Deserialize(pLoader, buffer); diff --git a/indexer/feature.hpp b/indexer/feature.hpp index 5407b2390b..bcd8211736 100644 --- a/indexer/feature.hpp +++ b/indexer/feature.hpp @@ -29,9 +29,9 @@ class FeatureBase public: - using BufferT = char const *; + using TBuffer = char const *; - void Deserialize(feature::LoaderBase * pLoader, BufferT buffer); + void Deserialize(feature::LoaderBase * pLoader, TBuffer buffer); /// @name Parse functions. Do simple dispatching to m_pLoader. //@{ @@ -151,7 +151,7 @@ class FeatureType : public FeatureBase FeatureID m_id; public: - void Deserialize(feature::LoaderBase * pLoader, BufferT buffer); + void Deserialize(feature::LoaderBase * pLoader, TBuffer buffer); inline void SetID(FeatureID const & id) { m_id = id; } inline FeatureID GetID() const { return m_id; } diff --git a/indexer/feature_loader_base.cpp b/indexer/feature_loader_base.cpp index b769199502..61639c4fbe 100644 --- a/indexer/feature_loader_base.cpp +++ b/indexer/feature_loader_base.cpp @@ -71,7 +71,7 @@ LoaderBase::LoaderBase(SharedLoadInfo const & info) { } -void LoaderBase::Init(BufferT data) +void LoaderBase::Init(TBuffer data) { m_Data = data; m_pF = 0; diff --git a/indexer/feature_loader_base.hpp b/indexer/feature_loader_base.hpp index 4fa2fc5c60..1ed33b20d0 100644 --- a/indexer/feature_loader_base.hpp +++ b/indexer/feature_loader_base.hpp @@ -58,11 +58,11 @@ namespace feature virtual ~LoaderBase() {} // It seems like no need to store a copy of buffer (see FeaturesVector). - typedef char const * BufferT; + typedef char const * TBuffer; /// @name Initialize functions. //@{ - void Init(BufferT data); + void Init(TBuffer data); inline void InitFeature(FeatureType * p) { m_pF = p; } void ResetGeometry(); @@ -99,7 +99,7 @@ namespace feature SharedLoadInfo const & m_Info; FeatureType * m_pF; - BufferT m_Data; + TBuffer m_Data; static uint32_t const m_TypesOffset = 1; uint32_t m_CommonOffset, m_Header2Offset; diff --git a/indexer/geometry_serialization.cpp b/indexer/geometry_serialization.cpp index 941f60e94a..da84f21c0b 100644 --- a/indexer/geometry_serialization.cpp +++ b/indexer/geometry_serialization.cpp @@ -135,20 +135,20 @@ namespace serial }; } - void TrianglesChainSaver::operator() (PointT arr[3], vector edges) + void TrianglesChainSaver::operator() (TPoint arr[3], vector edges) { - m_buffers.push_back(BufferT()); - MemWriter writer(m_buffers.back()); + m_buffers.push_back(TBuffer()); + MemWriter writer(m_buffers.back()); WriteVarUint(writer, EncodeDelta(arr[0], m_base)); WriteVarUint(writer, EncodeDelta(arr[1], arr[0])); - EdgeT curr = edges.front(); + TEdge curr = edges.front(); curr.m_delta = EncodeDelta(arr[2], arr[1]); sort(edges.begin(), edges.end(), edge_less_p0()); - stack st; + stack st; while (true) { CHECK_EQUAL ( curr.m_delta >> 62, 0, () ); @@ -156,7 +156,7 @@ namespace serial // find next edges int const nextNode = curr.m_p[1]; - vector::iterator i = lower_bound(edges.begin(), edges.end(), nextNode, edge_less_p0()); + auto i = lower_bound(edges.begin(), edges.end(), nextNode, edge_less_p0()); bool const found = (i != edges.end() && i->m_p[0] == nextNode); if (found) { @@ -168,7 +168,7 @@ namespace serial // first child delta |= (one << i->m_side); - vector::iterator j = i+1; + vector::iterator j = i+1; if (j != edges.end() && j->m_p[0] == nextNode) { // second child diff --git a/indexer/geometry_serialization.hpp b/indexer/geometry_serialization.hpp index fea5e02da1..aa0a6b7234 100644 --- a/indexer/geometry_serialization.hpp +++ b/indexer/geometry_serialization.hpp @@ -159,27 +159,28 @@ namespace serial class TrianglesChainSaver { - typedef m2::PointU PointT; - typedef tesselator::Edge EdgeT; - typedef vector BufferT; + using TPoint = m2::PointU; + using TEdge = tesselator::Edge; + using TBuffer = vector; - PointT m_base, m_max; + TPoint m_base; + TPoint m_max; - list m_buffers; + list m_buffers; public: explicit TrianglesChainSaver(CodingParams const & params); - PointT GetBasePoint() const { return m_base; } - PointT GetMaxPoint() const { return m_max; } + TPoint GetBasePoint() const { return m_base; } + TPoint GetMaxPoint() const { return m_max; } - void operator() (PointT arr[3], vector edges); + void operator() (TPoint arr[3], vector edges); size_t GetBufferSize() const { size_t sz = 0; - for (list::const_iterator i = m_buffers.begin(); i != m_buffers.end(); ++i) - sz += i->size(); + for (auto const & i : m_buffers) + sz += i.size(); return sz; } diff --git a/integration_tests/osrm_turn_test.cpp b/integration_tests/osrm_turn_test.cpp index 971010c659..66360f193e 100644 --- a/integration_tests/osrm_turn_test.cpp +++ b/integration_tests/osrm_turn_test.cpp @@ -8,12 +8,12 @@ using namespace routing; using namespace routing::turns; -UNIT_TEST(RussiaMoscowLenigradskiy39UturnTurnTest) // FAILED! +UNIT_TEST(RussiaMoscowLenigradskiy39UturnTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.796936768447288557, 37.537544383032567907), {0., 0.}, - MercatorBounds::FromLatLon(55.802121583039429709, 37.538908531885972764)); + MercatorBounds::FromLatLon(55.79693, 37.53754), {0., 0.}, + MercatorBounds::FromLatLon(55.80212, 37.5389)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -36,8 +36,8 @@ UNIT_TEST(RussiaMoscowSalameiNerisUturnTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.851822802555439296, 37.39533227665661741), {0., 0.}, - MercatorBounds::FromLatLon(55.843866280669693936, 37.392503720352721075)); + MercatorBounds::FromLatLon(55.85182, 37.39533), {0., 0.}, + MercatorBounds::FromLatLon(55.84386, 37.39250)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -45,30 +45,30 @@ UNIT_TEST(RussiaMoscowSalameiNerisUturnTurnTest) integration::TestTurnCount(route, 4); integration::GetNthTurn(route, 0) .TestValid() - .TestPoint({37.388482521388539, 67.633382734905041}, 20.) + .TestPoint({37.38848, 67.63338}, 20.) .TestDirection(TurnDirection::TurnSlightRight); integration::GetNthTurn(route, 1) .TestValid() - .TestPoint({37.387117276989784, 67.633369323859881}, 20.) + .TestPoint({37.38711, 67.63336}, 20.) .TestDirection(TurnDirection::TurnLeft); integration::GetNthTurn(route, 2) .TestValid() - .TestPoint({37.387380133475205, 67.632781920081243}, 20.) + .TestPoint({37.38738, 67.63278}, 20.) .TestDirection(TurnDirection::TurnLeft); integration::GetNthTurn(route, 3) .TestValid() - .TestPoint({37.390526364673121, 67.633106467374461}, 20.) + .TestPoint({37.39052, 67.63310}, 20.) .TestDirection(TurnDirection::TurnRight); integration::TestRouteLength(route, 1637.); } -UNIT_TEST(RussiaMoscowTrikotagniAndPohodniRoundaboutTurnTest) //Failed +UNIT_TEST(RussiaMoscowTrikotagniAndPohodniRoundaboutTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.831185109716038539, 37.405153751040685961), {0., 0.}, - MercatorBounds::FromLatLon(55.833843764465711956, 37.405210716570380214)); + MercatorBounds::FromLatLon(55.83118, 37.40515), {0., 0.}, + MercatorBounds::FromLatLon(55.83384, 37.40521)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -88,8 +88,8 @@ UNIT_TEST(RussiaMoscowPlanetnaiTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.802161062035722239, 37.546683164991776493), {0., 0.}, - MercatorBounds::FromLatLon(55.801690565608659256, 37.549153861529006804)); + MercatorBounds::FromLatLon(55.80216, 37.54668), {0., 0.}, + MercatorBounds::FromLatLon(55.80169, 37.54915)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -105,8 +105,8 @@ UNIT_TEST(RussiaMoscowNoTurnsOnMKADTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.846564134248033895, 37.391635636579785285), {0., 0.}, - MercatorBounds::FromLatLon(55.566611639650901111, 37.692547253527685314)); + MercatorBounds::FromLatLon(55.84656, 37.39163), {0., 0.}, + MercatorBounds::FromLatLon(55.56661, 37.69254)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -115,18 +115,18 @@ UNIT_TEST(RussiaMoscowNoTurnsOnMKADTurnTest) integration::TestTurnCount(route, 1); integration::GetNthTurn(route, 0) .TestValid() - .TestPoint({37.682761085650043, 67.140620702062705}) + .TestPoint({37.68276, 67.14062}) .TestOneOfDirections({TurnDirection::TurnSlightRight, TurnDirection::TurnRight}); integration::TestRouteLength(route, 43233.7); } -UNIT_TEST(RussiaMoscowTTKKashirskoeShosseOutTurnTest) // Failed +UNIT_TEST(RussiaMoscowTTKKashirskoeShosseOutTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.70160163595921432, 37.606320023648997619), {0., 0.}, - MercatorBounds::FromLatLon(55.693494620969019593, 37.621220025471167503)); + MercatorBounds::FromLatLon(55.70160, 37.60632), {0., 0.}, + MercatorBounds::FromLatLon(55.69349, 37.62122)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -142,8 +142,8 @@ UNIT_TEST(RussiaMoscowPankratevskiPerBolshaySuharedskazPloschadTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.771770051239521138, 37.635563528539393019), {0., 0.}, - MercatorBounds::FromLatLon(55.772033898671161012, 37.637054339197831609)); + MercatorBounds::FromLatLon(55.77177, 37.63556), {0., 0.}, + MercatorBounds::FromLatLon(55.77203, 37.63705)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -157,8 +157,8 @@ UNIT_TEST(RussiaMoscowMKADPutilkovskeShosseTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.853059336007213176, 37.394141645624102921), {0., 0.}, - MercatorBounds::FromLatLon(55.850996974780500182, 37.391050708989460816)); + MercatorBounds::FromLatLon(55.85305, 37.39414), {0., 0.}, + MercatorBounds::FromLatLon(55.85099, 37.39105)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -173,8 +173,8 @@ UNIT_TEST(RussiaMoscowPetushkovaShodniaReverTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.841047134659639539, 37.405917692164507571), {0., 0.}, - MercatorBounds::FromLatLon(55.839290964451890886, 37.408550782937481927)); + MercatorBounds::FromLatLon(55.84104, 37.40591), {0., 0.}, + MercatorBounds::FromLatLon(55.83929, 37.40855)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -187,8 +187,8 @@ UNIT_TEST(RussiaHugeRoundaboutTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.801410375094782523, 37.325810690728495445), {0., 0.}, - MercatorBounds::FromLatLon(55.800757342904596214, 37.325360456262153264)); + MercatorBounds::FromLatLon(55.80141, 37.32581), {0., 0.}, + MercatorBounds::FromLatLon(55.80075, 37.32536)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -209,8 +209,8 @@ UNIT_TEST(BelarusMiskProspNezavisimostiMKADTurnTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(53.936425941857663702, 27.658572046123946819), {0., 0.}, - MercatorBounds::FromLatLon(53.939337747485986085, 27.670461944729382253)); + MercatorBounds::FromLatLon(53.93642, 27.65857), {0., 0.}, + MercatorBounds::FromLatLon(53.93933, 27.67046)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -227,8 +227,8 @@ UNIT_TEST(RussiaMoscowPetushkovaPetushkovaTest) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.836368736509733424, 37.405549999999998079), {0., 0.}, - MercatorBounds::FromLatLon(55.837076293494696699, 37.404890000000001749)); + MercatorBounds::FromLatLon(55.83636, 37.40555), {0., 0.}, + MercatorBounds::FromLatLon(55.83707, 37.40489)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -257,8 +257,8 @@ UNIT_TEST(BelarusMKADShosseinai) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(55.315418609956843454, 29.431229999999999336), {0., 0.}, - MercatorBounds::FromLatLon(55.316562400560414403, 29.426259999999999195)); + MercatorBounds::FromLatLon(55.31541, 29.43123), {0., 0.}, + MercatorBounds::FromLatLon(55.31656, 29.42626)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; @@ -275,8 +275,8 @@ UNIT_TEST(ThailandPhuketNearPrabarameeRoad) { TRouteResult const routeResult = integration::CalculateRoute( integration::GetOsrmComponents(), - MercatorBounds::FromLatLon(7.9179763567919980716, 98.369370000000003529), {0., 0.}, - MercatorBounds::FromLatLon(7.9072494672603861332, 98.367850000000004229)); + MercatorBounds::FromLatLon(7.91797, 98.36937), {0., 0.}, + MercatorBounds::FromLatLon(7.90724, 98.36785)); Route const & route = *routeResult.first; IRouter::ResultCode const result = routeResult.second; diff --git a/routing/osrm_router.cpp b/routing/osrm_router.cpp index 82c462b6d7..e95e9a11ba 100644 --- a/routing/osrm_router.cpp +++ b/routing/osrm_router.cpp @@ -699,27 +699,27 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( size_t lastIdx = 0; #endif - for (auto const & segment : routingResult.unpackedPathSegments) + for (auto const & pathSegments : routingResult.unpackedPathSegments) { INTERRUPT_WHEN_CANCELLED(delegate); - // Get all the coordinates for the computed route - size_t const n = segment.size(); - for (size_t j = 0; j < n; ++j) // todo(mgsergio) rename! + // Get all computed route coordinates. + size_t const numSegments = pathSegments.size(); + for (size_t segmentIndex = 0; segmentIndex < numSegments; ++segmentIndex) { - RawPathData const & path_data = segment[j]; + RawPathData const & pathData = pathSegments[segmentIndex]; - if (j > 0 && !points.empty()) + if (segmentIndex > 0 && !points.empty()) { turns::TurnItem turnItem; turnItem.m_index = static_cast(points.size() - 1); - turns::TurnInfo turnInfo(*mapping, segment[j - 1].node, segment[j].node); + turns::TurnInfo turnInfo(*mapping, pathSegments[segmentIndex - 1].node, pathSegments[segmentIndex].node); turns::GetTurnDirection(*m_pIndex, turnInfo, turnItem); // ETA information. // Osrm multiples seconds to 10, so we need to divide it back. - double const nodeTimeSeconds = path_data.segmentWeight / 10.0; + double const nodeTimeSeconds = pathData.segmentWeight / 10.0; #ifdef DEBUG double distMeters = 0.0; @@ -736,14 +736,14 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( // Lane information. if (turnItem.m_turn != turns::TurnDirection::NoTurn) { - turnItem.m_lanes = turns::GetLanesInfo(segment[j - 1].node, + turnItem.m_lanes = turns::GetLanesInfo(pathSegments[segmentIndex - 1].node, *mapping, turns::GetLastSegmentPointIndex, *m_pIndex); turnsDir.push_back(move(turnItem)); } } buffer_vector buffer; - mapping->m_segMapping.ForEachFtSeg(path_data.node, MakeBackInsertFunctor(buffer)); + mapping->m_segMapping.ForEachFtSeg(pathData.node, MakeBackInsertFunctor(buffer)); auto FindIntersectingSeg = [&buffer] (TSeg const & seg) -> size_t { @@ -761,13 +761,13 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( //Do not put out node geometry (we do not have it)! size_t startK = 0, endK = buffer.size(); - if (j == 0) + if (segmentIndex == 0) { if (!segBegin.IsValid()) continue; startK = FindIntersectingSeg(segBegin); } - if (j == n - 1) + if (segmentIndex + 1 == numSegments) { if (!segEnd.IsValid()) continue; @@ -785,11 +785,11 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( auto startIdx = seg.m_pointStart; auto endIdx = seg.m_pointEnd; - bool const needTime = (j == 0) || (j == n - 1); + bool const needTime = (segmentIndex == 0) || (segmentIndex == numSegments - 1); - if (j == 0 && k == startK && segBegin.IsValid()) + if (segmentIndex == 0 && k == startK && segBegin.IsValid()) startIdx = (seg.m_pointEnd > seg.m_pointStart) ? segBegin.m_pointStart : segBegin.m_pointEnd; - if (j == n - 1 && k == endK - 1 && segEnd.IsValid()) + if (segmentIndex == numSegments - 1 && k == endK - 1 && segEnd.IsValid()) endIdx = (seg.m_pointEnd > seg.m_pointStart) ? segEnd.m_pointEnd : segEnd.m_pointStart; if (seg.m_pointEnd > seg.m_pointStart) @@ -826,7 +826,7 @@ OsrmRouter::ResultCode OsrmRouter::MakeTurnAnnotation( times.push_back(Route::TTimeItem(points.size() - 1, estimatedTime)); if (routingResult.targetEdge.segment.IsValid()) { - turnsDir.push_back( + turnsDir.emplace_back( turns::TurnItem(static_cast(points.size()) - 1, turns::TurnDirection::ReachedYourDestination)); } turns::FixupTurns(points, turnsDir); diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 6ff7e151e9..b0cab345e3 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -22,6 +22,10 @@ using namespace routing::turns; namespace { double const kFeaturesNearTurnMeters = 3.0; +size_t constexpr kMaxPointsCount = 7; +double constexpr kMinDistMeters = 300.; +size_t constexpr kNotSoCloseMaxPointsCount = 3; +double constexpr kNotSoCloseMinDistMeters = 30.; typedef vector TGeomTurnCandidate; @@ -326,26 +330,24 @@ TurnDirection FindDirectionByAngle(vector> const & l * \param segment is a ingoing or outgoing feature segment. * \param ft is a ingoing or outgoing feature. * \param junctionPoint is a junction point. + * \param maxPointsCount returned poit could't be more than maxPointsCount poins away from junctionPoint + * \param minDistMeters returned point should be minDistMeters away from junctionPoint if ft is long and consists of short segments * \param GetPointIndex is a function for getting points by index. * It defines a direction of following along a feature. So it differs for ingoing and outgoing cases. * It has following parameters: * - start is an index of the start point of a feature segment. For example, FtSeg::m_pointStart. * - end is an index of the end point of a feature segment. For example, FtSeg::m_pointEnd. * - shift is a number of points which shall be added to end or start index. After that - * the sum reflects an index of a feature segment point which will be used for a turn calculation. + * the sum reflects an index of a feature segment point which will be used for a turn calculation. * The sum shall belongs to a range [min(start, end), max(start, end)]. * shift belongs to a range [0, abs(end - start)]. * \return an ingoing or outgoing point for a turn calculation. */ m2::PointD GetPointForTurn(OsrmMappingTypes::FtSeg const & segment, FeatureType const & ft, m2::PointD const & junctionPoint, - size_t (*GetPointIndex)(const size_t start, const size_t end, const size_t shift), - size_t const maxPointsCount = 7, // An ingoing and outgoing point could be farther - // then kMaxPointsCount points from the junctionPoint - double const minDistMeters = 300. // If ft feature is long enough and consists of short segments - // the point for turn generation is taken as the next point - // along the route after kMinDistMeters. - ) + size_t const maxPointsCount, + double const minDistMeters, + size_t (*GetPointIndex)(const size_t start, const size_t end, const size_t shift)) { double curDistanceMeters = 0.; m2::PointD point = junctionPoint; @@ -442,6 +444,16 @@ void GetPossibleTurns(Index const & index, NodeID node, m2::PointD const & ingoi return t1.angle < t2.angle; }); } + +size_t GetIngoingPointIndex(const size_t start, const size_t end, const size_t i) +{ + return end > start ? end - i : end + i; +} + +size_t GetOutgoingPointIndex(const size_t start, const size_t end, const size_t i) +{ + return end > start ? start + i : start - i; +} } // namespace namespace routing @@ -729,20 +741,11 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) outgoingFeature.GetPoint(turnInfo.m_outgoingSegment.m_pointStart)), kFeaturesNearTurnMeters, ()); - auto const getIngoingPointIndex = [](const size_t start, const size_t end, const size_t i) - { - return end > start ? end - i : end + i; - }; - auto const getOutgoingPointIndex = [](const size_t start, const size_t end, const size_t i) - { - return end > start ? start + i : start - i; - }; - m2::PointD const junctionPoint = ingoingFeature.GetPoint(turnInfo.m_ingoingSegment.m_pointEnd); m2::PointD const ingoingPoint = GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, - junctionPoint, getIngoingPointIndex); + junctionPoint, kMaxPointsCount, kMinDistMeters, GetIngoingPointIndex); m2::PointD const outgoingPoint = GetPointForTurn(turnInfo.m_outgoingSegment, outgoingFeature, - junctionPoint, getOutgoingPointIndex); + junctionPoint, kMaxPointsCount, kMinDistMeters, GetOutgoingPointIndex); double const turnAngle = my::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPoint, outgoingPoint)); TurnDirection const intermediateDirection = IntermediateDirection(turnAngle); @@ -773,10 +776,10 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) GetPossibleTurns(index, turnInfo.m_ingoingNodeID, ingoingPointOneSegment, junctionPoint, turnInfo.m_routeMapping, nodes); - size_t const nodesSize = nodes.size(); - bool const hasMultiTurns = nodesSize > 1; + size_t const numNodes = nodes.size(); + bool const hasMultiTurns = numNodes > 1; - if (nodesSize == 0) + if (numNodes == 0) return; if (!hasMultiTurns) @@ -808,12 +811,9 @@ void GetTurnDirection(Index const & index, TurnInfo & turnInfo, TurnItem & turn) return; } + auto const notSoCloseToTheTurnPoint = GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, junctionPoint, + kNotSoCloseMaxPointsCount, kNotSoCloseMinDistMeters, GetIngoingPointIndex); - size_t constexpr kMaxPointsCount = 3; - double constexpr kMinDistMeters = 30.; - auto const notSoCloseToTheTurnPoint = GetPointForTurn(turnInfo.m_ingoingSegment, ingoingFeature, - junctionPoint, getIngoingPointIndex, - kMaxPointsCount, kMinDistMeters); if (!KeepTurnByIngoingEdges(junctionPoint, notSoCloseToTheTurnPoint, outgoingPoint, hasMultiTurns, turnInfo.m_routeMapping, index)) { diff --git a/routing/turns_generator.hpp b/routing/turns_generator.hpp index 83461c6506..e6f18fc398 100644 --- a/routing/turns_generator.hpp +++ b/routing/turns_generator.hpp @@ -117,8 +117,6 @@ bool CheckRoundaboutExit(bool isIngoingEdgeRoundabout, bool isOutgoingEdgeRounda TurnDirection GetRoundaboutDirection(bool isIngoingEdgeRoundabout, bool isOutgoingEdgeRoundabout, bool isMultiTurnJunction, bool keepTurnByHighwayClass); - - /*! * \brief GetTurnDirection makes a primary decision about turns on the route. * \param turnInfo is used for cashing some information while turn calculation.