diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index 45dd0da2c4..e6b76a88f2 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -58,7 +58,7 @@ void LoadRestrictions(MwmValue const & mwmValue, RestrictionVec & restrictions) header.Deserialize(src); RestrictionVec restrictionsOnly; - RestrictionSerializer::Deserialize(header, restrictions /* restriction no */, restrictionsOnly, src); + RestrictionSerializer::Deserialize(header, restrictions, restrictionsOnly, src); restrictions.insert(restrictions.end(), restrictionsOnly.cbegin(), restrictionsOnly.cend()); } catch (Reader::OpenException const & e) diff --git a/routing/geometry.hpp b/routing/geometry.hpp index 1c9e7b8821..c172f4cf88 100644 --- a/routing/geometry.hpp +++ b/routing/geometry.hpp @@ -74,8 +74,6 @@ public: return GetRoad(rp.GetFeatureId()).GetPoint(rp.GetPointId()); } - bool IsValid() const { return m_loader.operator bool(); } - private: // Feature id to RoadGeometry map. unordered_map m_roads; diff --git a/routing/index_graph.cpp b/routing/index_graph.cpp index 2a2a51aeed..7677aa4cf3 100644 --- a/routing/index_graph.cpp +++ b/routing/index_graph.cpp @@ -3,6 +3,7 @@ #include "routing/restrictions_serialization.hpp" #include "base/assert.hpp" +#include "base/checked_cast.hpp" #include "base/exception.hpp" #include @@ -10,6 +11,7 @@ namespace { +using namespace base; using namespace routing; using namespace std; @@ -19,21 +21,30 @@ bool IsRestricted(RestrictionVec const & restrictions, Segment const & u, Segmen uint32_t const featureIdTo = isOutgoing ? v.GetFeatureId() : u.GetFeatureId(); // Looking for at least one restriction of type No from |featureIdFrom| to |featureIdTo|. - if (binary_search(restrictions.cbegin(), restrictions.cend(), - Restriction(Restriction::Type::No, {featureIdFrom, featureIdTo}))) + if (!binary_search(restrictions.cbegin(), restrictions.cend(), + Restriction(Restriction::Type::No, {featureIdFrom, featureIdTo}))) { - if (featureIdFrom != featureIdTo) - return true; - - // @TODO(bykoianko) According to current code if a feature id is marked as a feature with - // restrictricted U-turn it's restricted to make a U-turn on the both ends of the feature. - // Generally speaking it's wrong. In osm there's information about the end of the feature - // where the U-turn is restricted. It's necessary to pass the data to mwm and to use it here. - // Please see test LineGraph_RestrictionF1F1No for details. - return u.GetSegmentIdx() == v.GetSegmentIdx() && u.IsForward() != v.IsForward(); + return false; } - return false; + if (featureIdFrom != featureIdTo) + return true; + + // @TODO(bykoianko) According to current code if a feature id is marked as a feature with + // restrictricted U-turn it's restricted to make a U-turn on the both ends of the feature. + // Generally speaking it's wrong. In osm there's information about the end of the feature + // where the U-turn is restricted. It's necessary to pass the data to mwm and to use it here. + // Please see test LineGraph_RestrictionF1F1No for details. + // + // Another exapmle when it's necessary to be aware about feature end particepated in restriction is + // *---F1---* + // | | + // *--F3--A B--F4--* + // | | + // *---F2---* + // In case of restriction F1-A-F2 or F1-B-F2 of any type (No, Only) the important information + // is lost. + return u.GetSegmentIdx() == v.GetSegmentIdx() && u.IsForward() != v.IsForward(); } bool IsUTurn(Segment const & u, Segment const & v) @@ -57,24 +68,15 @@ void IndexGraph::GetEdgeList(Segment const & segment, bool isOutgoing, vector rawEdges; if (jointId != Joint::kInvalidId) { m_jointIndex.ForEachPoint(jointId, [&](RoadPoint const & rp) { - GetNeighboringEdges(segment, rp, isOutgoing, rawEdges); + GetNeighboringEdges(segment, rp, isOutgoing, edges); }); } else { - GetNeighboringEdges(segment, roadPoint, isOutgoing, rawEdges); - } - - // Removing some edges according to restriction rules. - edges.reserve(rawEdges.size()); - for (SegmentEdge const & e : rawEdges) - { - if (!IsRestricted(m_restrictions, segment, e.GetTarget(), isOutgoing)) - edges.push_back(e); + GetNeighboringEdges(segment, roadPoint, isOutgoing, edges); } } @@ -84,7 +86,7 @@ void IndexGraph::Import(vector const & joints) { m_roadIndex.Import(joints); CHECK_LESS_OR_EQUAL(joints.size(), numeric_limits::max(), ()); - Build(static_cast(joints.size())); + Build(checked_cast(joints.size())); } void IndexGraph::SetRestrictions(RestrictionVec && restrictions) @@ -124,6 +126,7 @@ void IndexGraph::GetNeighboringEdges(Segment const & from, RoadPoint const & rp, void IndexGraph::GetNeighboringEdge(Segment const & from, Segment const & to, bool isOutgoing, vector & edges) { + // Blocking U-turns on internal feature points. RoadPoint const rp = from.GetRoadPoint(isOutgoing); if (IsUTurn(from, to) && m_roadIndex.GetJointId(rp) == Joint::kInvalidId @@ -133,6 +136,10 @@ void IndexGraph::GetNeighboringEdge(Segment const & from, Segment const & to, bo return; } + // Blocking restriction edges. + if (IsRestricted(m_restrictions, from, to, isOutgoing)) + return; + double const weight = CalcSegmentWeight(isOutgoing ? to : from) + GetPenalties(isOutgoing ? from : to, isOutgoing ? to : from); edges.emplace_back(to, weight); diff --git a/routing/index_graph.hpp b/routing/index_graph.hpp index b0e55d76c5..6c218b789e 100644 --- a/routing/index_graph.hpp +++ b/routing/index_graph.hpp @@ -49,11 +49,6 @@ public: m_roadIndex.PushFromSerializer(jointId, rp); } - bool IsValid() const - { - return m_geometry.IsValid() && m_estimator && m_roadIndex.GetSize() != 0 && m_jointIndex.GetNumPoints() != 0; - } - template void ForEachRoad(F && f) const { diff --git a/routing/restriction_loader.cpp b/routing/restriction_loader.cpp index 481fa5fb1d..f559566178 100644 --- a/routing/restriction_loader.cpp +++ b/routing/restriction_loader.cpp @@ -15,23 +15,23 @@ using namespace routing; /// \note In general case ends of features don't have to be joints. For example all /// loose feature ends aren't joints. But if ends of r1 and r2 are connected at this /// point there has to be a joint. So the method is valid. -Joint::Id GetCommonEndJoint(routing::RoadJointIds const & r1, routing::RoadJointIds const & r2) +Joint::Id GetCommonEndJoint(RoadJointIds const & r1, RoadJointIds const & r2) { auto const IsEqual = [](Joint::Id j1, Joint::Id j2) { return j1 != Joint::kInvalidId && j1 == j2; }; - if (IsEqual(r1.GetJointId(0 /* point id */), r2.GetLastJointId())) + + if (IsEqual(r1.GetJointId(0 /* point id */), r2.GetEndingJointId())) return r1.GetJointId(0); - if (IsEqual(r1.GetJointId(0 /* point id */), r2.GetJointId(0))) + if (IsEqual(r1.GetJointId(0 /* point id */), r2.GetJointId(0 /* point id */))) return r1.GetJointId(0); - if (IsEqual(r2.GetJointId(0 /* point id */), r1.GetLastJointId())) - return r2.GetJointId(0); + if (IsEqual(r1.GetEndingJointId(), r2.GetJointId(0 /* point id */))) + return r1.GetEndingJointId(); - Joint::Id const r1Last = r1.GetLastJointId(); - if (IsEqual(r1Last, r2.GetLastJointId())) - return r1Last; + if (IsEqual(r1.GetEndingJointId(), r2.GetEndingJointId())) + return r1.GetEndingJointId(); return Joint::kInvalidId; } @@ -53,7 +53,7 @@ RestrictionLoader::RestrictionLoader(MwmValue const & mwmValue, IndexGraph const RestrictionVec restrictionsOnly; RestrictionSerializer::Deserialize(m_header, m_restrictions /* restriction no */, restrictionsOnly, src); - ConvertRestrictionOnlyToNo(graph, restrictionsOnly, m_restrictions); + ConvertRestrictionsOnlyToNoAndSort(graph, restrictionsOnly, m_restrictions); } catch (Reader::OpenException const & e) { @@ -63,15 +63,9 @@ RestrictionLoader::RestrictionLoader(MwmValue const & mwmValue, IndexGraph const } } -void ConvertRestrictionOnlyToNo(IndexGraph const & graph, RestrictionVec const & restrictionsOnly, - RestrictionVec & restrictionsNo) +void ConvertRestrictionsOnlyToNoAndSort(IndexGraph const & graph, RestrictionVec const & restrictionsOnly, + RestrictionVec & restrictionsNo) { - if (!graph.IsValid()) - { - LOG(LWARNING, ("Index graph is not valid. All the restrictions of type Only aren't used.")); - return; - } - for (Restriction const & o : restrictionsOnly) { CHECK_EQUAL(o.m_featureIds.size(), 2, ("Only two link restrictions are support.")); diff --git a/routing/restriction_loader.hpp b/routing/restriction_loader.hpp index 5074a743cb..e41999424c 100644 --- a/routing/restriction_loader.hpp +++ b/routing/restriction_loader.hpp @@ -27,6 +27,6 @@ private: string const m_countryFileName; }; -void ConvertRestrictionOnlyToNo(IndexGraph const & graph, RestrictionVec const & restrictionsOnly, - RestrictionVec & restrictionsNo); +void ConvertRestrictionsOnlyToNoAndSort(IndexGraph const & graph, RestrictionVec const & restrictionsOnly, + RestrictionVec & restrictionsNo); } // namespace routing diff --git a/routing/road_index.hpp b/routing/road_index.hpp index e614fac36b..2f13c83330 100644 --- a/routing/road_index.hpp +++ b/routing/road_index.hpp @@ -29,15 +29,13 @@ public: return Joint::kInvalidId; } - Joint::Id GetLastJointId() const + Joint::Id GetEndingJointId() const { - Joint::Id lastJointId = Joint::kInvalidId; - for (Joint::Id const jointId : m_jointIds) - { - if (jointId != Joint::kInvalidId) - lastJointId = jointId; - } - return lastJointId; + if (m_jointIds.empty()) + return Joint::kInvalidId; + + ASSERT_NOT_EQUAL(m_jointIds.back(), Joint::kInvalidId, ()); + return m_jointIds.back(); } void AddJoint(uint32_t pointId, Joint::Id jointId) @@ -129,7 +127,7 @@ public: RoadJointIds const & GetRoad(uint32_t featureId) const { auto const & it = m_roads.find(featureId); - CHECK(it != m_roads.cend(), ("Feture id:", featureId)); + CHECK(it != m_roads.cend(), ("Feature id:", featureId)); return it->second; } diff --git a/routing/routing_tests/restriction_test.cpp b/routing/routing_tests/restriction_test.cpp index f29089e7ba..72519c6ef8 100644 --- a/routing/routing_tests/restriction_test.cpp +++ b/routing/routing_tests/restriction_test.cpp @@ -191,7 +191,7 @@ UNIT_CLASS_TEST(RestrictionTest, TriangularGraph_RestrictionOnlyF5F3) RestrictionVec restrictionsOnly = { {Restriction::Type::Only, {5 /* feature from */, 3 /* feature to */}}}; RestrictionVec restrictionsNo; - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {3 /* x */, 0 /* y */}, {2, 0}, {1, 0}, {0, 0}, {0, 2}, {0, 3}}; @@ -292,7 +292,7 @@ UNIT_CLASS_TEST(RestrictionTest, TwowayCornerGraph_RestrictionF3F1Only) RestrictionVec restrictionsOnly = { {Restriction::Type::Only, {3 /* feature from */, 1 /* feature to */}}}; RestrictionVec restrictionsNo; - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {3 /* x */, 0 /* y */}, {2, 0}, {1, 0}, {0, 0}, {0, 1}, {0, 2}, {0, 3}}; @@ -385,7 +385,7 @@ UNIT_CLASS_TEST(RestrictionTest, TwoSquaresGraph_RestrictionF10F3Only) RestrictionVec restrictionsOnly = { {Restriction::Type::Only, {10 /* feature from */, 3 /* feature to */}}}; RestrictionVec restrictionsNo; - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {3 /* x */, 0 /* y */}, {2, 0}, {1, 0}, {1, 1}, {0, 2}, {0, 3}}; @@ -403,7 +403,7 @@ UNIT_CLASS_TEST(RestrictionTest, TwoSquaresGraph_RestrictionF10F3OnlyF3F4Only) {Restriction::Type::Only, {3 /* feature from */, 4 /* feature to */}}, {Restriction::Type::Only, {10 /* feature from */, 3 /* feature to */}}}; RestrictionVec restrictionsNo; - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {3 /* x */, 0 /* y */}, {2, 0}, {1, 0}, {0, 0}, {0, 2}, {0, 3}}; @@ -421,7 +421,7 @@ UNIT_CLASS_TEST(RestrictionTest, TwoSquaresGraph_RestrictionF2F8NoRestrictionF9F {Restriction::Type::No, {2 /* feature from */, 8 /* feature to */}}}; // Invalid restriction. RestrictionVec const restrictionsOnly = { {Restriction::Type::Only, {9 /* feature from */, 1 /* feature to */}}}; // Invalid restriction. - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {3 /* x */, 0 /* y */}, {2, 0}, {1, 1}, {0, 2}, {0, 3}}; @@ -614,7 +614,7 @@ UNIT_CLASS_TEST(RestrictionTest, PosterGraph_RestrictionF0F1Only) RestrictionVec restrictionsOnly = { {Restriction::Type::Only, {0 /* feature from */, 1 /* feature to */}}}; RestrictionVec restrictionsNo; - ConvertRestrictionOnlyToNo(*m_graph, restrictionsOnly, restrictionsNo); + ConvertRestrictionsOnlyToNoAndSort(*m_graph, restrictionsOnly, restrictionsNo); vector const expectedGeom = { {2 /* x */, 0 /* y */}, {1, 0}, {0, 0}, {0, 1}, {0.5, 1}, {1, 1}, {2, 1}};