From adfa27a6e83b9894035a92bdc66e5311d6af6c17 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Wed, 20 Apr 2022 16:09:40 +0300 Subject: [PATCH 1/9] [Routing] Fixes: #2422 + refactoring Currently GetTurnDirection() is overcomplicated. This is an attempt to make is more straightforward. Also some bugs were found and fixed. nodes.candidates was used in many cases where uturn had to be excluded. The issue #2422 was expected to be handled even twice: - Removing a slight turn if ingoing and outgoing edges are not links and all other possible ways out are links. - Usage of RightmostDirection() if the route goes along the rightmost candidate. Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 135 +++++++++++++++++------------------- 1 file changed, 65 insertions(+), 70 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 7257108b2f..bc5fda7978 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -201,10 +201,10 @@ bool DiscardTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo co /// * - or any alternative turn to a similar road if the turn's angle is less than kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn) /// * - or any alternative turn to a smaller road if it's GoStraight or SlightTurn. /// * Returns true otherwise. -/// \param possibleTurns is all possible ways out from a junction. +/// \param candidates_without_uturn is all possible ways out from a junction except uturn. /// \param turnInfo is information about ingoing and outgoing segments of the route. /// it is supposed that current turn is GoStraight or SlightTurn -bool DiscardTurnByNoAlignedAlternatives(TurnCandidates const & possibleTurns, TurnInfo const & turnInfo, +bool DiscardTurnByNoAlignedAlternatives(std::vector const & candidates_without_uturn, TurnInfo const & turnInfo, NumMwmIds const & numMwmIds) { double constexpr kMaxAbsAngleSameRoadClass = 70.0; @@ -217,17 +217,11 @@ bool DiscardTurnByNoAlignedAlternatives(TurnCandidates const & possibleTurns, Tu if (!turnInfo.m_outgoing.m_segmentRange.GetFirstSegment(numMwmIds, firstOutgoingSegment)) return false; - Segment inversedLastIngoingSegment; - if (!turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, inversedLastIngoingSegment)) - return false; - inversedLastIngoingSegment.Inverse(); - - for (auto const & t : possibleTurns.candidates) + for (auto const & t : candidates_without_uturn) { // Let's consider all outgoing segments except for - // (1) route outgoing segment - // (2) a U-turn segment (inversedLastIngoingSegment) - if (t.m_segment == firstOutgoingSegment || t.m_segment == inversedLastIngoingSegment) + // route outgoing segment + if (t.m_segment == firstOutgoingSegment) continue; ftypes::HighwayClass const highwayClass = t.m_highwayClass; // @note The bigger is the road, the lesser is HighwayClass value. @@ -256,7 +250,7 @@ bool DiscardTurnByNoAlignedAlternatives(TurnCandidates const & possibleTurns, Tu /*! * \brief Returns false when other possible turns leads to service roads; */ -bool KeepRoundaboutTurnByHighwayClass(CarDirection turn, TurnCandidates const & possibleTurns, +bool KeepRoundaboutTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo const & turnInfo, NumMwmIds const & numMwmIds) { Segment firstOutgoingSegment; @@ -312,11 +306,10 @@ bool DiscardTurnByIngoingAndOutgoingEdges(CarDirection intermediateDirection, bo (hasMultiTurns && intermediateDirection == CarDirection::GoStraight)); } -// turnEdgesCount calculates both ingoing ond outgoing edges without user's edge. bool KeepTurnByIngoingEdges(m2::PointD const & junctionPoint, m2::PointD const & ingoingPointOneSegment, m2::PointD const & outgoingPoint, bool hasMultiTurns, - size_t const turnEdgesCount) + size_t const ingoingEdgesCount) { double const turnAngle = base::RadToDeg(PiMinusTwoVectorsAngle(junctionPoint, ingoingPointOneSegment, outgoingPoint)); @@ -325,7 +318,7 @@ bool KeepTurnByIngoingEdges(m2::PointD const & junctionPoint, // The code below is responsible 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); - return hasMultiTurns || (!isGoStraightOrSlightTurn && turnEdgesCount > 1); + return hasMultiTurns || (!isGoStraightOrSlightTurn && ingoingEdgesCount > 0); } bool FixupLaneSet(CarDirection turn, vector & lanes, @@ -574,7 +567,8 @@ bool GetPrevInSegmentRoutePoint(RoutePointIndex const & index, RoutePointIndex & void GoStraightCorrection(TurnCandidate const & notRouteCandidate, CarDirection turnToSet, TurnItem & turn) { - CHECK_EQUAL(turn.m_turn, CarDirection::GoStraight, ()); + if (turn.m_turn != CarDirection::GoStraight) + return; if (!IsGoStraightOrSlightTurn(IntermediateDirection(notRouteCandidate.m_angle))) return; @@ -981,11 +975,11 @@ CarDirection RightmostDirection(const double angle) static vector> const kLowerBounds = { {157., CarDirection::TurnSharpRight}, {50., CarDirection::TurnRight}, - {2., CarDirection::TurnSlightRight}, - {-10., CarDirection::GoStraight}, - {-50., CarDirection::TurnSlightLeft}, - {-157., CarDirection::TurnLeft}, - {-180., CarDirection::TurnSharpLeft}}; + {10., CarDirection::TurnSlightRight}, + // For sure it's incorrect to give directions TurnLeft or TurnSlighLeft if we need the rightmost turn. + // So GoStraight direction is given even for needed sharp left turn when other turns on the left and are more sharp. + // The reason: the rightmost turn is the most straight one here. + {-180., CarDirection::GoStraight}}; return FindDirectionByAngle(kLowerBounds, angle); } @@ -1153,11 +1147,22 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex bool const hasMultiTurns = HasMultiTurns(numMwmIds, nodes, turnInfo); + // Check for enter or exit to/from roundabout + if (turnInfo.m_ingoing.m_onRoundabout || turnInfo.m_outgoing.m_onRoundabout) + { + bool const keepTurnByHighwayClass = + KeepRoundaboutTurnByHighwayClass(nodes, turnInfo, numMwmIds); + turn.m_turn = GetRoundaboutDirection(turnInfo.m_ingoing.m_onRoundabout, + turnInfo.m_outgoing.m_onRoundabout, hasMultiTurns, + keepTurnByHighwayClass); + return; + } + // Checking for exits from highways. Segment firstOutgoingSeg; bool const isFirstOutgoingSegValid = turnInfo.m_outgoing.m_segmentRange.GetFirstSegment(numMwmIds, firstOutgoingSeg); - if (!turnInfo.m_ingoing.m_onRoundabout && isFirstOutgoingSegValid && + if (isFirstOutgoingSegValid && IsExit(nodes, turnInfo, firstOutgoingSeg, intermediateDirection, turn.m_turn)) { return; @@ -1166,46 +1171,29 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex if (DiscardTurnByIngoingAndOutgoingEdges(intermediateDirection, hasMultiTurns, turnInfo, turn, nodes)) return; - if (!hasMultiTurns || !nodes.isCandidatesAngleValid) + // Collect in candidates_without_uturn all turn candidates except uturn (if any). + auto candidates_without_uturn = nodes.candidates; + Segment lastIngoingSegment; + if (nodes.candidates.size() > 0 && turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, lastIngoingSegment)) { - turn.m_turn = intermediateDirection; - } - else - { - if (isFirstOutgoingSegValid) - { - if (nodes.candidates.front().m_segment == firstOutgoingSeg) - turn.m_turn = LeftmostDirection(turnAngle); - else if (nodes.candidates.back().m_segment == firstOutgoingSeg) - turn.m_turn = RightmostDirection(turnAngle); - else - turn.m_turn = intermediateDirection; - } - else - { - turn.m_turn = intermediateDirection; - } + if (candidates_without_uturn.front().m_segment.IsInverse(lastIngoingSegment)) + candidates_without_uturn.assign(candidates_without_uturn.cbegin() + 1, candidates_without_uturn.cend()); + else if (candidates_without_uturn.back().m_segment.IsInverse(lastIngoingSegment)) + candidates_without_uturn.assign(candidates_without_uturn.cbegin(), candidates_without_uturn.cend() - 1); } + ASSERT(hasMultiTurns == (candidates_without_uturn.size() >= 2), ()); - if (turnInfo.m_ingoing.m_onRoundabout || turnInfo.m_outgoing.m_onRoundabout) - { - bool const keepTurnByHighwayClass = - KeepRoundaboutTurnByHighwayClass(turn.m_turn, nodes, turnInfo, numMwmIds); - turn.m_turn = GetRoundaboutDirection(turnInfo.m_ingoing.m_onRoundabout, - turnInfo.m_outgoing.m_onRoundabout, hasMultiTurns, - keepTurnByHighwayClass); - return; - } + turn.m_turn = intermediateDirection; // Note 1. If the road significantly changes its direction this turn shall be kept here. // Note 2. If there's only one exit from this junction (nodes.candidates.size() == 1) - // this turn should be kept (because it was kept by previous logic as an exception). + // this turn should be kept (because it was kept by DiscardTurnByIngoingAndOutgoingEdges as an exception). // Note 3. Keeping a turn at this point means that the decision to keep this turn or not // will be made after. if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn) && nodes.candidates.size() != 1) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || - DiscardTurnByNoAlignedAlternatives(nodes, turnInfo, numMwmIds)) + DiscardTurnByNoAlignedAlternatives(candidates_without_uturn, turnInfo, numMwmIds)) { turn.m_turn = CarDirection::None; return; @@ -1219,47 +1207,54 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex vehicleSettings.m_notSoCloseMaxDistMeters, false /* forward */); // Removing a slight turn if there's only one way to leave the turn and there's no ingoing edges. - if (!KeepTurnByIngoingEdges(junctionPoint, notSoCloseToTheTurnPoint, outgoingPoint, hasMultiTurns, - nodes.candidates.size() + ingoingCount)) + if (!KeepTurnByIngoingEdges(junctionPoint, notSoCloseToTheTurnPoint, outgoingPoint, hasMultiTurns, ingoingCount)) { turn.m_turn = CarDirection::None; return; } // Removing a slight turn if ingoing and outgoing edges are not links and all other - // possible ways out are links. + // possible ways out (except of uturn) are links. if (!turnInfo.m_ingoing.m_isLink && !turnInfo.m_outgoing.m_isLink && turnInfo.m_ingoing.m_highwayClass == turnInfo.m_outgoing.m_highwayClass && - GetLinkCount(nodes.candidates) + 1 == nodes.candidates.size()) + GetLinkCount(candidates_without_uturn) + 1 == candidates_without_uturn.size()) { turn.m_turn = CarDirection::None; return; } } - if (turn.m_turn == CarDirection::GoStraight) + if (turn.m_turn == CarDirection::GoStraight && !hasMultiTurns) { - if (!hasMultiTurns) turn.m_turn = CarDirection::None; - - // Note. If the turn direction is |CarDirection::GoStraight| and there's only one extra way out - // from the junction the direction may be corrected in some cases. So two turn candidates - // (one for the route and an extra one) require special processing. - if (nodes.candidates.size() != 2) return; + } - if (nodes.candidates.front().m_segment == firstOutgoingSeg) + if (candidates_without_uturn.size() >= 2) + { + // If the route goes along the rightmost or the leftmost way among available ones: + // 1. RightmostDirection or LeftmostDirection is selected + // 2. If the turn direction is |CarDirection::GoStraight| + // and there's another GoStraight or SlightTurn turn + // GoStraight is corrected to TurnSlightRight/TurnSlightLeft + // to avoid ambiguity: 2 or more almost straight turns and GoStraight direction. + + if (candidates_without_uturn.front().m_segment == firstOutgoingSeg) { - // The route goes along the leftmost candidate. - GoStraightCorrection(nodes.candidates.back(), CarDirection::TurnSlightLeft, turn); + // The route goes along the leftmost candidate. + turn.m_turn = LeftmostDirection(turnAngle); + // Compare with the closest left candidate. + GoStraightCorrection(candidates_without_uturn[1], CarDirection::TurnSlightLeft, turn); } - else if (nodes.candidates.back().m_segment == firstOutgoingSeg) + else if (candidates_without_uturn.back().m_segment == firstOutgoingSeg) { - // The route goes along the rightmost candidate. - GoStraightCorrection(nodes.candidates.front(), CarDirection::TurnSlightRight, turn); + // The route goes along the rightmost candidate. + turn.m_turn = RightmostDirection(turnAngle); + //Compare with the closest right candidate. + GoStraightCorrection(candidates_without_uturn[candidates_without_uturn.size() - 2], CarDirection::TurnSlightRight, turn); } - // Note. It's possible that |firstOutgoingSeg| is not contained in |nodes.candidates|. - // It may happened if |firstOutgoingSeg| and candidates in |nodes.candidates| are + // Note. It's possible that |firstOutgoingSeg| is not contained in |candidates_without_uturn|. + // It may happened if |firstOutgoingSeg| and candidates in |candidates_without_uturn| are // from different mwms. } } -- 2.45.3 From ddb2bbfd363f543d7bc77865083ba9f40e710808 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Wed, 20 Apr 2022 18:44:53 +0300 Subject: [PATCH 2/9] [Routing] coding standard Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 58 ++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index bc5fda7978..47b3ef0ed6 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -125,7 +125,7 @@ bool GetTurnHighwayClasses(TurnCandidates const & possibleTurns, TurnInfo const { // Let's consider all outgoing segments except for // (1) route outgoing segment - // (2) a U-turn segment (inversedLastIngoingSegment) + // (2) a U-turn segment (inversedLastIngoingSegment). if (t.m_segment == firstOutgoingSegment || t.m_segment == inversedLastIngoingSegment) continue; ftypes::HighwayClass const highwayClass = t.m_highwayClass; @@ -180,7 +180,7 @@ bool DiscardTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo co int constexpr kMaxHighwayClassDiffForService = 1; TurnHighwayClasses turnHighwayClasses; - // Looks like a bug. If no info received turn should not be discarded + // Looks like a bug. If no info received turn should not be discarded. if (!GetTurnHighwayClasses(possibleTurns, turnInfo, numMwmIds, turnHighwayClasses)) return true; @@ -201,10 +201,10 @@ bool DiscardTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo co /// * - or any alternative turn to a similar road if the turn's angle is less than kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn) /// * - or any alternative turn to a smaller road if it's GoStraight or SlightTurn. /// * Returns true otherwise. -/// \param candidates_without_uturn is all possible ways out from a junction except uturn. +/// \param candidatesWithoutUturn is all possible ways out from a junction except uturn. /// \param turnInfo is information about ingoing and outgoing segments of the route. /// it is supposed that current turn is GoStraight or SlightTurn -bool DiscardTurnByNoAlignedAlternatives(std::vector const & candidates_without_uturn, TurnInfo const & turnInfo, +bool DiscardTurnByNoAlignedAlternatives(std::vector const & candidatesWithoutUturn, TurnInfo const & turnInfo, NumMwmIds const & numMwmIds) { double constexpr kMaxAbsAngleSameRoadClass = 70.0; @@ -217,10 +217,9 @@ bool DiscardTurnByNoAlignedAlternatives(std::vector const & candi if (!turnInfo.m_outgoing.m_segmentRange.GetFirstSegment(numMwmIds, firstOutgoingSegment)) return false; - for (auto const & t : candidates_without_uturn) + for (auto const & t : candidatesWithoutUturn) { - // Let's consider all outgoing segments except for - // route outgoing segment + // Let's consider all outgoing segments except for route outgoing segment. if (t.m_segment == firstOutgoingSegment) continue; ftypes::HighwayClass const highwayClass = t.m_highwayClass; @@ -1147,13 +1146,13 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex bool const hasMultiTurns = HasMultiTurns(numMwmIds, nodes, turnInfo); - // Check for enter or exit to/from roundabout + // Check for enter or exit to/from roundabout. if (turnInfo.m_ingoing.m_onRoundabout || turnInfo.m_outgoing.m_onRoundabout) { - bool const keepTurnByHighwayClass = - KeepRoundaboutTurnByHighwayClass(nodes, turnInfo, numMwmIds); + bool const keepTurnByHighwayClass = KeepRoundaboutTurnByHighwayClass(nodes, turnInfo, numMwmIds); turn.m_turn = GetRoundaboutDirection(turnInfo.m_ingoing.m_onRoundabout, - turnInfo.m_outgoing.m_onRoundabout, hasMultiTurns, + turnInfo.m_outgoing.m_onRoundabout, + hasMultiTurns, keepTurnByHighwayClass); return; } @@ -1171,17 +1170,18 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex if (DiscardTurnByIngoingAndOutgoingEdges(intermediateDirection, hasMultiTurns, turnInfo, turn, nodes)) return; - // Collect in candidates_without_uturn all turn candidates except uturn (if any). - auto candidates_without_uturn = nodes.candidates; + // Collect in candidatesWithoutUturn all turn candidates except uturn (if any). + auto candidatesWithoutUturn = nodes.candidates; Segment lastIngoingSegment; - if (nodes.candidates.size() > 0 && turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, lastIngoingSegment)) + if (turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, lastIngoingSegment)) { - if (candidates_without_uturn.front().m_segment.IsInverse(lastIngoingSegment)) - candidates_without_uturn.assign(candidates_without_uturn.cbegin() + 1, candidates_without_uturn.cend()); - else if (candidates_without_uturn.back().m_segment.IsInverse(lastIngoingSegment)) - candidates_without_uturn.assign(candidates_without_uturn.cbegin(), candidates_without_uturn.cend() - 1); + if (candidatesWithoutUturn.front().m_segment.IsInverse(lastIngoingSegment)) + candidatesWithoutUturn.assign(candidatesWithoutUturn.cbegin() + 1, candidatesWithoutUturn.cend()); + else if (candidatesWithoutUturn.back().m_segment.IsInverse(lastIngoingSegment)) + candidatesWithoutUturn.assign(candidatesWithoutUturn.cbegin(), candidatesWithoutUturn.cend() - 1); } - ASSERT(hasMultiTurns == (candidates_without_uturn.size() >= 2), ()); + ASSERT(hasMultiTurns == (candidatesWithoutUturn.size() >= 2), + ("hasMultiTurns is true if there are two or more possible ways which don't go along an ingoing segment and false otherwise")); turn.m_turn = intermediateDirection; @@ -1193,7 +1193,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn) && nodes.candidates.size() != 1) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || - DiscardTurnByNoAlignedAlternatives(candidates_without_uturn, turnInfo, numMwmIds)) + DiscardTurnByNoAlignedAlternatives(candidatesWithoutUturn, turnInfo, numMwmIds)) { turn.m_turn = CarDirection::None; return; @@ -1217,7 +1217,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // possible ways out (except of uturn) are links. if (!turnInfo.m_ingoing.m_isLink && !turnInfo.m_outgoing.m_isLink && turnInfo.m_ingoing.m_highwayClass == turnInfo.m_outgoing.m_highwayClass && - GetLinkCount(candidates_without_uturn) + 1 == candidates_without_uturn.size()) + GetLinkCount(candidatesWithoutUturn) + 1 == candidatesWithoutUturn.size()) { turn.m_turn = CarDirection::None; return; @@ -1230,7 +1230,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex return; } - if (candidates_without_uturn.size() >= 2) + if (candidatesWithoutUturn.size() >= 2) { // If the route goes along the rightmost or the leftmost way among available ones: // 1. RightmostDirection or LeftmostDirection is selected @@ -1239,22 +1239,22 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // GoStraight is corrected to TurnSlightRight/TurnSlightLeft // to avoid ambiguity: 2 or more almost straight turns and GoStraight direction. - if (candidates_without_uturn.front().m_segment == firstOutgoingSeg) + if (candidatesWithoutUturn.front().m_segment == firstOutgoingSeg) { // The route goes along the leftmost candidate. turn.m_turn = LeftmostDirection(turnAngle); // Compare with the closest left candidate. - GoStraightCorrection(candidates_without_uturn[1], CarDirection::TurnSlightLeft, turn); + GoStraightCorrection(candidatesWithoutUturn[1], CarDirection::TurnSlightLeft, turn); } - else if (candidates_without_uturn.back().m_segment == firstOutgoingSeg) + else if (candidatesWithoutUturn.back().m_segment == firstOutgoingSeg) { // The route goes along the rightmost candidate. turn.m_turn = RightmostDirection(turnAngle); - //Compare with the closest right candidate. - GoStraightCorrection(candidates_without_uturn[candidates_without_uturn.size() - 2], CarDirection::TurnSlightRight, turn); + // Compare with the closest right candidate. + GoStraightCorrection(candidatesWithoutUturn[candidatesWithoutUturn.size() - 2], CarDirection::TurnSlightRight, turn); } - // Note. It's possible that |firstOutgoingSeg| is not contained in |candidates_without_uturn|. - // It may happened if |firstOutgoingSeg| and candidates in |candidates_without_uturn| are + // Note. It's possible that |firstOutgoingSeg| is not contained in |candidatesWithoutUturn|. + // It may happened if |firstOutgoingSeg| and candidates in |candidatesWithoutUturn| are // from different mwms. } } -- 2.45.3 From f30dfd471e255a726322367a4297a06069506561 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 12:26:00 +0300 Subject: [PATCH 3/9] [Routing] Refactoring It was checked trough debugging that there are no significant changes. Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 47b3ef0ed6..243a1a5a00 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -281,13 +281,6 @@ bool DiscardTurnByIngoingAndOutgoingEdges(CarDirection intermediateDirection, bo TurnInfo const & turnInfo, TurnItem const & turn, TurnCandidates const & turnCandidates) { - if (turn.m_keepAnyway || turnInfo.m_ingoing.m_onRoundabout || - turnInfo.m_outgoing.m_onRoundabout || - turnInfo.m_ingoing.m_highwayClass != turnInfo.m_outgoing.m_highwayClass) - { - return false; - } - // @TODO(bykoianko) If all turn candidates go almost straight and there are several ways // from the junction (|hasMultiTurns| == true) the turn will be discarded. // If all turn candidates go almost straight and there is only one way @@ -1167,8 +1160,9 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex return; } - if (DiscardTurnByIngoingAndOutgoingEdges(intermediateDirection, hasMultiTurns, turnInfo, turn, nodes)) - return; + if (!turn.m_keepAnyway) + if (DiscardTurnByIngoingAndOutgoingEdges(intermediateDirection, hasMultiTurns, turnInfo, turn, nodes)) + return; // Collect in candidatesWithoutUturn all turn candidates except uturn (if any). auto candidatesWithoutUturn = nodes.candidates; @@ -1186,11 +1180,9 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex turn.m_turn = intermediateDirection; // Note 1. If the road significantly changes its direction this turn shall be kept here. - // Note 2. If there's only one exit from this junction (nodes.candidates.size() == 1) - // this turn should be kept (because it was kept by DiscardTurnByIngoingAndOutgoingEdges as an exception). - // Note 3. Keeping a turn at this point means that the decision to keep this turn or not + // Note 2. Keeping a turn at this point means that the decision to keep this turn or not // will be made after. - if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn) && nodes.candidates.size() != 1) + if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn))) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || DiscardTurnByNoAlignedAlternatives(candidatesWithoutUturn, turnInfo, numMwmIds)) -- 2.45.3 From 2c085b2c1a43c2918c604fce16ae564618787b9c Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 12:46:08 +0300 Subject: [PATCH 4/9] [Routing] Refactoring DiscardTurnByIngoingAndOutgoingEdges() removed as absolete. Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 38 +------------------------------------ 1 file changed, 1 insertion(+), 37 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 243a1a5a00..7cd05c591d 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -266,38 +266,6 @@ bool KeepRoundaboutTurnByHighwayClass(TurnCandidates const & possibleTurns, return false; } -bool DoAllTurnCandidatesGoAlmostStraight(vector const & candidates) -{ - return all_of(candidates.cbegin(), candidates.cend(), [](TurnCandidate const & c) { - return IsGoStraightOrSlightTurn(IntermediateDirection(c.m_angle)); - }); -} - -/// \brief Analyzes its args and makes a decision if it's possible to have a turn at this junction -/// or not. -/// \returns true if based on this analysis there's no turn at this junction and -/// false if the junction should be considered as possible turn. -bool DiscardTurnByIngoingAndOutgoingEdges(CarDirection intermediateDirection, bool hasMultiTurns, - TurnInfo const & turnInfo, TurnItem const & turn, - TurnCandidates const & turnCandidates) -{ - // @TODO(bykoianko) If all turn candidates go almost straight and there are several ways - // from the junction (|hasMultiTurns| == true) the turn will be discarded. - // If all turn candidates go almost straight and there is only one way - // from the junction (|hasMultiTurns| == false) the turn will not be discarded in this method, - // and then may be kept. It means that in some cases if there are two or more possible - // ways from a junction the turn may be discarded and if there is only one way out - // the turn may be kept. This code should be redesigned. - if (turnCandidates.isCandidatesAngleValid && - DoAllTurnCandidatesGoAlmostStraight(turnCandidates.candidates)) - { - return !hasMultiTurns; - } - - return ((!hasMultiTurns && IsGoStraightOrSlightTurn(intermediateDirection)) || - (hasMultiTurns && intermediateDirection == CarDirection::GoStraight)); -} - bool KeepTurnByIngoingEdges(m2::PointD const & junctionPoint, m2::PointD const & ingoingPointOneSegment, m2::PointD const & outgoingPoint, bool hasMultiTurns, @@ -1160,10 +1128,6 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex return; } - if (!turn.m_keepAnyway) - if (DiscardTurnByIngoingAndOutgoingEdges(intermediateDirection, hasMultiTurns, turnInfo, turn, nodes)) - return; - // Collect in candidatesWithoutUturn all turn candidates except uturn (if any). auto candidatesWithoutUturn = nodes.candidates; Segment lastIngoingSegment; @@ -1182,7 +1146,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // Note 1. If the road significantly changes its direction this turn shall be kept here. // Note 2. Keeping a turn at this point means that the decision to keep this turn or not // will be made after. - if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn))) + if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn)) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || DiscardTurnByNoAlignedAlternatives(candidatesWithoutUturn, turnInfo, numMwmIds)) -- 2.45.3 From d4337fd060636eb2c398d2460a4da813ea29f69d Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 14:06:21 +0300 Subject: [PATCH 5/9] [Routing] Refactoring Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 59 ++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index 7cd05c591d..fe8f87aeeb 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -201,10 +201,10 @@ bool DiscardTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo co /// * - or any alternative turn to a similar road if the turn's angle is less than kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn) /// * - or any alternative turn to a smaller road if it's GoStraight or SlightTurn. /// * Returns true otherwise. -/// \param candidatesWithoutUturn is all possible ways out from a junction except uturn. +/// \param turnCandidates is all possible ways out from a junction except uturn. /// \param turnInfo is information about ingoing and outgoing segments of the route. /// it is supposed that current turn is GoStraight or SlightTurn -bool DiscardTurnByNoAlignedAlternatives(std::vector const & candidatesWithoutUturn, TurnInfo const & turnInfo, +bool DiscardTurnByNoAlignedAlternatives(std::vector const & turnCandidates, TurnInfo const & turnInfo, NumMwmIds const & numMwmIds) { double constexpr kMaxAbsAngleSameRoadClass = 70.0; @@ -217,7 +217,7 @@ bool DiscardTurnByNoAlignedAlternatives(std::vector const & candi if (!turnInfo.m_outgoing.m_segmentRange.GetFirstSegment(numMwmIds, firstOutgoingSegment)) return false; - for (auto const & t : candidatesWithoutUturn) + for (auto const & t : turnCandidates) { // Let's consider all outgoing segments except for route outgoing segment. if (t.m_segment == firstOutgoingSegment) @@ -1008,6 +1008,18 @@ bool HasMultiTurns(NumMwmIds const & numMwmIds, TurnCandidates const & turnCandi return !OneOfTurnCandidatesGoesAlongIngoingSegment(numMwmIds, turnCandidates, turnInfo); } +void RemoveUTurnCandidate(TurnInfo const & turnInfo, NumMwmIds const & numMwmIds, std::vector & turnCandidates) +{ + Segment lastIngoingSegment; + if (turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, lastIngoingSegment)) + { + if (turnCandidates.front().m_segment.IsInverse(lastIngoingSegment)) + turnCandidates.erase(turnCandidates.begin()); + else if (turnCandidates.back().m_segment.IsInverse(lastIngoingSegment)) + turnCandidates.pop_back(); + } +} + /// \returns true if there is exactly 1 turn in |turnCandidates| with angle less then /// |kMaxForwardAngleCandidates|. bool HasSingleForwardTurn(TurnCandidates const & turnCandidates) @@ -1106,6 +1118,10 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex return; bool const hasMultiTurns = HasMultiTurns(numMwmIds, nodes, turnInfo); + RemoveUTurnCandidate(turnInfo, numMwmIds, nodes.candidates); + auto const & turnCandidates = nodes.candidates; + ASSERT(hasMultiTurns == (turnCandidates.size() >= 2), + ("hasMultiTurns is true if there are two or more possible ways which don't go along an ingoing segment and false otherwise")); // Check for enter or exit to/from roundabout. if (turnInfo.m_ingoing.m_onRoundabout || turnInfo.m_outgoing.m_onRoundabout) @@ -1128,19 +1144,6 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex return; } - // Collect in candidatesWithoutUturn all turn candidates except uturn (if any). - auto candidatesWithoutUturn = nodes.candidates; - Segment lastIngoingSegment; - if (turnInfo.m_ingoing.m_segmentRange.GetLastSegment(numMwmIds, lastIngoingSegment)) - { - if (candidatesWithoutUturn.front().m_segment.IsInverse(lastIngoingSegment)) - candidatesWithoutUturn.assign(candidatesWithoutUturn.cbegin() + 1, candidatesWithoutUturn.cend()); - else if (candidatesWithoutUturn.back().m_segment.IsInverse(lastIngoingSegment)) - candidatesWithoutUturn.assign(candidatesWithoutUturn.cbegin(), candidatesWithoutUturn.cend() - 1); - } - ASSERT(hasMultiTurns == (candidatesWithoutUturn.size() >= 2), - ("hasMultiTurns is true if there are two or more possible ways which don't go along an ingoing segment and false otherwise")); - turn.m_turn = intermediateDirection; // Note 1. If the road significantly changes its direction this turn shall be kept here. @@ -1149,7 +1152,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn)) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || - DiscardTurnByNoAlignedAlternatives(candidatesWithoutUturn, turnInfo, numMwmIds)) + DiscardTurnByNoAlignedAlternatives(turnCandidates, turnInfo, numMwmIds)) { turn.m_turn = CarDirection::None; return; @@ -1173,20 +1176,14 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // possible ways out (except of uturn) are links. if (!turnInfo.m_ingoing.m_isLink && !turnInfo.m_outgoing.m_isLink && turnInfo.m_ingoing.m_highwayClass == turnInfo.m_outgoing.m_highwayClass && - GetLinkCount(candidatesWithoutUturn) + 1 == candidatesWithoutUturn.size()) + GetLinkCount(turnCandidates) + 1 == turnCandidates.size()) { turn.m_turn = CarDirection::None; return; } } - if (turn.m_turn == CarDirection::GoStraight && !hasMultiTurns) - { - turn.m_turn = CarDirection::None; - return; - } - - if (candidatesWithoutUturn.size() >= 2) + if (turnCandidates.size() >= 2) { // If the route goes along the rightmost or the leftmost way among available ones: // 1. RightmostDirection or LeftmostDirection is selected @@ -1195,22 +1192,22 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // GoStraight is corrected to TurnSlightRight/TurnSlightLeft // to avoid ambiguity: 2 or more almost straight turns and GoStraight direction. - if (candidatesWithoutUturn.front().m_segment == firstOutgoingSeg) + if (turnCandidates.front().m_segment == firstOutgoingSeg) { // The route goes along the leftmost candidate. turn.m_turn = LeftmostDirection(turnAngle); // Compare with the closest left candidate. - GoStraightCorrection(candidatesWithoutUturn[1], CarDirection::TurnSlightLeft, turn); + GoStraightCorrection(turnCandidates[1], CarDirection::TurnSlightLeft, turn); } - else if (candidatesWithoutUturn.back().m_segment == firstOutgoingSeg) + else if (turnCandidates.back().m_segment == firstOutgoingSeg) { // The route goes along the rightmost candidate. turn.m_turn = RightmostDirection(turnAngle); // Compare with the closest right candidate. - GoStraightCorrection(candidatesWithoutUturn[candidatesWithoutUturn.size() - 2], CarDirection::TurnSlightRight, turn); + GoStraightCorrection(turnCandidates[turnCandidates.size() - 2], CarDirection::TurnSlightRight, turn); } - // Note. It's possible that |firstOutgoingSeg| is not contained in |candidatesWithoutUturn|. - // It may happened if |firstOutgoingSeg| and candidates in |candidatesWithoutUturn| are + // Note. It's possible that |firstOutgoingSeg| is not contained in |turnCandidates|. + // It may happened if |firstOutgoingSeg| and candidates in |turnCandidates| are // from different mwms. } } -- 2.45.3 From d6f42ac795cbd0e0ab77124d96e1f941ee23fc99 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 16:22:00 +0300 Subject: [PATCH 6/9] [Routing] Refactoring Added logic for routes which go straight and don't go to a smaller road. Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 52 ++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 18 deletions(-) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index fe8f87aeeb..d8658ca74c 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -197,19 +197,26 @@ bool DiscardTurnByHighwayClass(TurnCandidates const & possibleTurns, TurnInfo co } /// * \returns false when +/// * for routes which go straight and don't go to a smaller road: +/// * - any alternative GoStraight or SlightTurn turn +/// * for other routes: /// * - any alternative turn to a bigger road /// * - or any alternative turn to a similar road if the turn's angle is less than kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn) /// * - or any alternative turn to a smaller road if it's GoStraight or SlightTurn. /// * Returns true otherwise. +/// \param turnRoute is current route turn /// \param turnCandidates is all possible ways out from a junction except uturn. /// \param turnInfo is information about ingoing and outgoing segments of the route. /// it is supposed that current turn is GoStraight or SlightTurn -bool DiscardTurnByNoAlignedAlternatives(std::vector const & turnCandidates, TurnInfo const & turnInfo, +bool DiscardTurnByNoAlignedAlternatives(TurnItem const & turnRoute, + std::vector const & turnCandidates, + TurnInfo const & turnInfo, NumMwmIds const & numMwmIds) { double constexpr kMaxAbsAngleSameRoadClass = 70.0; ftypes::HighwayClass outgoingRouteRoadClass = turnInfo.m_outgoing.m_highwayClass; + ftypes::HighwayClass ingoingRouteRoadClass = turnInfo.m_ingoing.m_highwayClass; // The turn should be kept if there's no any information about feature id of outgoing segment // just to be on the safe side. It may happen in case of outgoing segment is a finish segment. @@ -222,26 +229,35 @@ bool DiscardTurnByNoAlignedAlternatives(std::vector const & turnC // Let's consider all outgoing segments except for route outgoing segment. if (t.m_segment == firstOutgoingSegment) continue; - ftypes::HighwayClass const highwayClass = t.m_highwayClass; - // @note The bigger is the road, the lesser is HighwayClass value. - if (static_cast(highwayClass) < static_cast(outgoingRouteRoadClass)) + + if (turnRoute.m_turn == CarDirection::GoStraight && static_cast(outgoingRouteRoadClass) >= static_cast(ingoingRouteRoadClass)) { - // Any alternative turn to a bigger road keeps the turn direction. - return false; - } - else if (highwayClass == outgoingRouteRoadClass) - { - // Any alternative turn to a similar road keeps the turn direction if the turn's angle is less than - // kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn). - if (fabs(t.m_angle) < kMaxAbsAngleSameRoadClass) - return false; - } - else // static_cast(highwayClass) > static_cast(outgoingRouteRoadClass) - { - // Any alternative turn to a smaller road keeps the turn direction if it's GoStraight or SlightTurn. if (IsGoStraightOrSlightTurn(IntermediateDirection(t.m_angle))) return false; } + else + { + ftypes::HighwayClass const highwayClass = t.m_highwayClass; + // @note The bigger is the road, the lesser is HighwayClass value. + if (static_cast(highwayClass) < static_cast(outgoingRouteRoadClass)) + { + // Any alternative turn to a bigger road keeps the turn direction. + return false; + } + else if (highwayClass == outgoingRouteRoadClass) + { + // Any alternative turn to a similar road keeps the turn direction if the turn's angle is less than + // kMaxAbsAngleSameRoadClass degrees (wider than SlightTurn). + if (fabs(t.m_angle) < kMaxAbsAngleSameRoadClass) + return false; + } + else // static_cast(highwayClass) > static_cast(outgoingRouteRoadClass) + { + // Any alternative turn to a smaller road keeps the turn direction if it's GoStraight or SlightTurn. + if (IsGoStraightOrSlightTurn(IntermediateDirection(t.m_angle))) + return false; + } + } } return true; } @@ -1152,7 +1168,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex if (!turn.m_keepAnyway && IsGoStraightOrSlightTurn(turn.m_turn)) { if (DiscardTurnByHighwayClass(nodes, turnInfo, numMwmIds, turn.m_turn) || - DiscardTurnByNoAlignedAlternatives(turnCandidates, turnInfo, numMwmIds)) + DiscardTurnByNoAlignedAlternatives(turn, turnCandidates, turnInfo, numMwmIds)) { turn.m_turn = CarDirection::None; return; -- 2.45.3 From 58e233bc5db61ed7eb4122ff943d286593b7f651 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 16:38:11 +0300 Subject: [PATCH 7/9] [Routing] Refactoring Tests update Signed-off-by: Anton Makouski --- routing/routing_tests/turns_generator_test.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/routing/routing_tests/turns_generator_test.cpp b/routing/routing_tests/turns_generator_test.cpp index 98659d8936..73d7a96bb0 100644 --- a/routing/routing_tests/turns_generator_test.cpp +++ b/routing/routing_tests/turns_generator_test.cpp @@ -328,9 +328,9 @@ UNIT_TEST(TestRightmostDirection) TEST_EQUAL(RightmostDirection(90.), CarDirection::TurnRight, ()); TEST_EQUAL(RightmostDirection(45.), CarDirection::TurnSlightRight, ()); TEST_EQUAL(RightmostDirection(0.), CarDirection::GoStraight, ()); - TEST_EQUAL(RightmostDirection(-20.), CarDirection::TurnSlightLeft, ()); - TEST_EQUAL(RightmostDirection(-90.), CarDirection::TurnLeft, ()); - TEST_EQUAL(RightmostDirection(-170.), CarDirection::TurnSharpLeft, ()); + TEST_EQUAL(RightmostDirection(-20.), CarDirection::GoStraight, ()); + TEST_EQUAL(RightmostDirection(-90.), CarDirection::GoStraight, ()); + TEST_EQUAL(RightmostDirection(-170.), CarDirection::GoStraight, ()); } UNIT_TEST(TestLeftmostDirection) -- 2.45.3 From 6f529ad728bfe1c6964764c1f58258da6e5e98e7 Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 18:55:39 +0300 Subject: [PATCH 8/9] [Routing] Refactoring Tests update Signed-off-by: Anton Makouski --- routing/routing_tests/turns_generator_test.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/routing/routing_tests/turns_generator_test.cpp b/routing/routing_tests/turns_generator_test.cpp index 73d7a96bb0..a4e551195b 100644 --- a/routing/routing_tests/turns_generator_test.cpp +++ b/routing/routing_tests/turns_generator_test.cpp @@ -335,10 +335,10 @@ UNIT_TEST(TestRightmostDirection) UNIT_TEST(TestLeftmostDirection) { - TEST_EQUAL(LeftmostDirection(180.), CarDirection::TurnSharpRight, ()); - TEST_EQUAL(LeftmostDirection(170.), CarDirection::TurnSharpRight, ()); - TEST_EQUAL(LeftmostDirection(90.), CarDirection::TurnRight, ()); - TEST_EQUAL(LeftmostDirection(45.), CarDirection::TurnSlightRight, ()); + TEST_EQUAL(LeftmostDirection(180.), CarDirection::GoStraight, ()); + TEST_EQUAL(LeftmostDirection(170.), CarDirection::GoStraight, ()); + TEST_EQUAL(LeftmostDirection(90.), CarDirection::GoStraight, ()); + TEST_EQUAL(LeftmostDirection(45.), CarDirection::GoStraight, ()); TEST_EQUAL(LeftmostDirection(0.), CarDirection::GoStraight, ()); TEST_EQUAL(LeftmostDirection(-20.), CarDirection::TurnSlightLeft, ()); TEST_EQUAL(LeftmostDirection(-90.), CarDirection::TurnLeft, ()); -- 2.45.3 From c2bbe51418666aa0b377dcba8443bf72149defff Mon Sep 17 00:00:00 2001 From: Anton Makouski Date: Thu, 21 Apr 2022 22:18:31 +0300 Subject: [PATCH 9/9] [Routing] Refactoring Comments Signed-off-by: Anton Makouski --- routing/turns_generator.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/routing/turns_generator.cpp b/routing/turns_generator.cpp index d8658ca74c..6a1c0246d7 100644 --- a/routing/turns_generator.cpp +++ b/routing/turns_generator.cpp @@ -1208,6 +1208,7 @@ void GetTurnDirection(IRoutingResult const & result, size_t outgoingSegmentIndex // GoStraight is corrected to TurnSlightRight/TurnSlightLeft // to avoid ambiguity: 2 or more almost straight turns and GoStraight direction. + // turnCandidates are sorted by angle from leftmost to rightmost. if (turnCandidates.front().m_segment == firstOutgoingSeg) { // The route goes along the leftmost candidate. -- 2.45.3