diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 00e71766d5..3333ac0b92 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -28,19 +28,18 @@ string const kRestrictionTestDir = "test-restrictions"; UNIT_TEST(RestrictionTest_ValidCase) { RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); - // Adding restrictions and feature ids to restrictionCollector in mixed order. - restrictionCollector.AddRestriction(Restriction::Type::No, {1, 2} /* osmIds */); + // Adding feature ids. restrictionCollector.AddFeatureId(30 /* featureId */, {3} /* osmIds */); - restrictionCollector.AddRestriction(Restriction::Type::No, {2, 3} /* osmIds */); restrictionCollector.AddFeatureId(10 /* featureId */, {1} /* osmIds */); restrictionCollector.AddFeatureId(50 /* featureId */, {5} /* osmIds */); - restrictionCollector.AddRestriction(Restriction::Type::Only, {5, 7} /* osmIds */); restrictionCollector.AddFeatureId(70 /* featureId */, {7} /* osmIds */); restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */); - // Composing restriction in feature id terms. - restrictionCollector.ComposeRestrictions(); - restrictionCollector.RemoveInvalidRestrictions(); + // Adding restrictions. + TEST(restrictionCollector.AddRestriction(Restriction::Type::No, {1, 2} /* osmIds */), ()); + TEST(restrictionCollector.AddRestriction(Restriction::Type::No, {2, 3} /* osmIds */), ()); + TEST(restrictionCollector.AddRestriction(Restriction::Type::Only, {5, 7} /* osmIds */), ()); + my::SortUnique(restrictionCollector.m_restrictions); // Checking the result. TEST(restrictionCollector.IsValid(), ()); @@ -55,18 +54,10 @@ UNIT_TEST(RestrictionTest_InvalidCase) { RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); restrictionCollector.AddFeatureId(0 /* featureId */, {0} /* osmIds */); - restrictionCollector.AddRestriction(Restriction::Type::No, {0, 1} /* osmIds */); restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */); - restrictionCollector.ComposeRestrictions(); + TEST(!restrictionCollector.AddRestriction(Restriction::Type::No, {0, 1} /* osmIds */), ()); - TEST(!restrictionCollector.IsValid(), ()); - - RestrictionVec const expectedRestrictions = { - {Restriction::Type::No, {0, Restriction::kInvalidFeatureId}}}; - TEST_EQUAL(restrictionCollector.m_restrictions, expectedRestrictions, ()); - - restrictionCollector.RemoveInvalidRestrictions(); TEST(!restrictionCollector.HasRestrictions(), ()); TEST(restrictionCollector.IsValid(), ()); } @@ -91,17 +82,7 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) TEST(restrictionCollector.ParseRestrictions( my::JoinFoldersToPath(platform.WritableDir(), kRestrictionPath)), ()); - RestrictionVec expectedRestrictions = {{Restriction::Type::No, 2 /* linkNumber */}, - {Restriction::Type::Only, 2 /* linkNumber */}, - {Restriction::Type::Only, 2 /* linkNumber */}, - {Restriction::Type::No, 2 /* linkNumber */}, - {Restriction::Type::No, 2 /* linkNumber */}}; - TEST_EQUAL(restrictionCollector.m_restrictions, expectedRestrictions, ()); - - vector> const expectedRestrictionIndex = { - {1, {0, 0}}, {1, {0, 1}}, {0, {1, 0}}, {2, {1, 1}}, {2, {2, 0}}, - {3, {2, 1}}, {38028428, {3, 0}}, {38028428, {3, 1}}, {4, {4, 0}}, {5, {4, 1}}}; - TEST_EQUAL(restrictionCollector.m_restrictionIndex, expectedRestrictionIndex, ()); + TEST(!restrictionCollector.HasRestrictions(), ()); } UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) @@ -126,8 +107,8 @@ UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) vector> const expectedOsmIds2FeatureId = { {10, 1}, {20, 2}, {30, 3}, {5423239545, 779703}}; vector> osmIds2FeatureId( - restrictionCollector.m_osmIds2FeatureId.cbegin(), - restrictionCollector.m_osmIds2FeatureId.cend()); + restrictionCollector.m_osmId2FeatureId.cbegin(), + restrictionCollector.m_osmId2FeatureId.cend()); sort(osmIds2FeatureId.begin(), osmIds2FeatureId.end(), my::LessBy(&pair::first)); TEST_EQUAL(osmIds2FeatureId, expectedOsmIds2FeatureId, ()); diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index 6128b4838f..f60696935a 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -40,9 +40,11 @@ RestrictionCollector::RestrictionCollector(string const & restrictionPath, m_restrictions.clear(); return; } - ComposeRestrictions(); - RemoveInvalidRestrictions(); - LOG(LINFO, ("Number of restrictions: =", m_restrictions.size())); + my::SortUnique(m_restrictions); + + if (!IsValid()) + LOG(LERROR, ("Some restrictions are not valid.")); + LOG(LDEBUG, ("Number of loaded restrictions:", m_restrictions.size())); } bool RestrictionCollector::IsValid() const @@ -102,53 +104,25 @@ bool RestrictionCollector::ParseRestrictions(string const & path) return true; } -void RestrictionCollector::ComposeRestrictions() +bool RestrictionCollector::AddRestriction(Restriction::Type type, vector const & osmIds) { - // Going through all osm ids saved in |m_restrictionIndex| (mentioned in restrictions). - size_t const numRestrictions = m_restrictions.size(); - for (auto const & osmIdAndIndex : m_restrictionIndex) + vector featureIds(osmIds.size()); + for (size_t i = 0; i < osmIds.size(); ++i) { - LinkIndex const & index = osmIdAndIndex.second; - CHECK_LESS(index.m_restrictionNumber, numRestrictions, ()); - Restriction & restriction = m_restrictions[index.m_restrictionNumber]; - CHECK_LESS(index.m_linkNumber, restriction.m_featureIds.size(), ()); - - uint64_t const & osmId = osmIdAndIndex.first; - // Checking if there's an osm id belongs to a restriction is saved only once as feature id. - auto const rangeId = m_osmIds2FeatureId.equal_range(osmId); - if (rangeId.first == rangeId.second) + auto const result = m_osmId2FeatureId.find(osmIds[i]); + if (result == m_osmId2FeatureId.cend()) { - // There's no |osmId| in |m_osmIds2FeatureId| which was mentioned in restrictions. // It could happend near mwm border when one of a restriction lines is not included in mwm // but the restriction is included. - continue; + return false; } - if (distance(rangeId.first, rangeId.second) != 1) - continue; // |osmId| mentioned in restrictions was included in more than one feature. - uint32_t const & featureId = rangeId.first->second; - // Adding feature id to restriction coresponded to the osm id. - restriction.m_featureIds[index.m_linkNumber] = featureId; + // Only one feature id is found for |osmIds[i]|. + featureIds[i] = result->second; } - my::SortUnique(m_restrictions); - // After sorting m_restrictions |m_restrictionIndex| is invalid. - m_restrictionIndex.clear(); -} - -void RestrictionCollector::RemoveInvalidRestrictions() -{ - m_restrictions.erase(remove_if(m_restrictions.begin(), m_restrictions.end(), - [](Restriction const & r) { return !r.IsValid(); }), - m_restrictions.end()); -} - -void RestrictionCollector::AddRestriction(Restriction::Type type, vector const & osmIds) -{ - size_t const numRestrictions = m_restrictions.size(); - m_restrictions.emplace_back(type, osmIds.size()); - for (size_t i = 0; i < osmIds.size(); ++i) - m_restrictionIndex.emplace_back(osmIds[i], LinkIndex({numRestrictions, i})); + m_restrictions.emplace_back(type, featureIds); + return true; } void RestrictionCollector::AddFeatureId(uint32_t featureId, vector const & osmIds) @@ -156,7 +130,14 @@ void RestrictionCollector::AddFeatureId(uint32_t featureId, vector con // Note. One |featureId| could correspond to several osm ids. // but for road feature |featureId| corresponds exactly one osm id. for (uint64_t const & osmId : osmIds) - m_osmIds2FeatureId.insert(make_pair(osmId, featureId)); + { + auto const result = m_osmId2FeatureId.insert(make_pair(osmId, featureId)); + if (result.second == false) + { + LOG(LERROR, ("Osm id", osmId, "is included in two feature ids: ", featureId, + m_osmId2FeatureId.find(osmId)->second)); + } + } } bool FromString(string str, Restriction::Type & type) @@ -174,12 +155,4 @@ bool FromString(string str, Restriction::Type & type) return false; } - -string DebugPrint(RestrictionCollector::LinkIndex const & index) -{ - ostringstream out; - out << "m_restrictionNumber:" << index.m_restrictionNumber - << " m_linkNumber:" << index.m_linkNumber << " "; - return out.str(); -} } // namespace routing diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp index fcf401e3ed..bbc8745fb1 100644 --- a/generator/restriction_collector.hpp +++ b/generator/restriction_collector.hpp @@ -4,8 +4,8 @@ #include "std/functional.hpp" #include "std/limits.hpp" +#include "std/map.hpp" #include "std/string.hpp" -#include "std/unordered_map.hpp" #include "std/utility.hpp" #include "std/vector.hpp" @@ -16,25 +16,6 @@ namespace routing class RestrictionCollector { public: - /// \brief Addresses a link in vector. - struct LinkIndex - { - LinkIndex(size_t restrictionNumber, size_t linkNumber) - : m_restrictionNumber(restrictionNumber), m_linkNumber(linkNumber) - { - } - - bool operator==(LinkIndex const & index) const - { - return m_restrictionNumber == index.m_restrictionNumber && m_linkNumber == index.m_linkNumber; - } - - // Restriction number in restriction vector. - size_t const m_restrictionNumber; - // Link number for a restriction. It's equal to zero or one for most cases. - size_t const m_linkNumber; - }; - /// \param restrictionPath full path to file with road restrictions in osm id terms. /// \param featureId2OsmIdsPath full path to file with mapping from feature id to osm id. RestrictionCollector(string const & restrictionPath, string const & featureId2OsmIdsPath); @@ -76,13 +57,6 @@ private: /// \param featureId2OsmIdsPath path to the text file. bool ParseRestrictions(string const & path); - /// \brief Sets feature id for all restrictions in |m_restrictions|. - void ComposeRestrictions(); - - /// \brief removes all restriction with incorrect feature id. - /// \note The method should be called after ComposeRestrictions(). - void RemoveInvalidRestrictions(); - /// \brief Adds feature id and corresponding vector of |osmIds| to |m_osmId2FeatureId|. /// \note One feature id (|featureId|) may correspond to several osm ids (|osmIds|). void AddFeatureId(uint32_t featureId, vector const & osmIds); @@ -92,14 +66,12 @@ private: /// \param osmIds is osm ids of restriction links /// \note This method should be called to add a restriction when feature ids of the restriction /// are unknown. The feature ids should be set later with a call of |SetFeatureId(...)| method. - void AddRestriction(Restriction::Type type, vector const & osmIds); + /// \returns true if restriction is add and false otherwise. + bool AddRestriction(Restriction::Type type, vector const & osmIds); RestrictionVec m_restrictions; - vector> m_restrictionIndex; - - unordered_multimap m_osmIds2FeatureId; + map m_osmId2FeatureId; }; bool FromString(string str, Restriction::Type & type); -string DebugPrint(RestrictionCollector::LinkIndex const & index); } // namespace routing diff --git a/generator/restriction_generator.cpp b/generator/restriction_generator.cpp index 69cd8a5d94..7c0d1d02db 100644 --- a/generator/restriction_generator.cpp +++ b/generator/restriction_generator.cpp @@ -31,8 +31,8 @@ bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPat RestrictionVec const & restrictions = restrictionCollector.GetRestrictions(); auto const firstOnlyIt = - upper_bound(restrictions.cbegin(), restrictions.cend(), Restriction(Restriction::Type::No, 0), - my::LessBy(&Restriction::m_type)); + upper_bound(restrictions.cbegin(), restrictions.cend(), + Restriction(Restriction::Type::No, {} /* links */), my::LessBy(&Restriction::m_type)); RoutingHeader header; header.m_noRestrictionCount = distance(restrictions.cbegin(), firstOnlyIt); header.m_onlyRestrictionCount = restrictions.size() - header.m_noRestrictionCount; diff --git a/indexer/routing.hpp b/indexer/routing.hpp index 0a7c1ef6e4..77d7922ef3 100644 --- a/indexer/routing.hpp +++ b/indexer/routing.hpp @@ -34,7 +34,6 @@ struct Restriction Only, // Only going according such restriction is permitted }; - Restriction(Type type, size_t linkNumber) : m_featureIds(linkNumber, kInvalidFeatureId), m_type(type) {} Restriction(Type type, vector const & links) : m_featureIds(links), m_type(type) {} bool IsValid() const; @@ -136,7 +135,7 @@ private: routing::Restriction::Type const type = begin->m_type; - routing::Restriction prevRestriction(type, 0 /* linkNumber */); + routing::Restriction prevRestriction(type, {} /* links */); prevRestriction.m_featureIds.resize(kSupportedLinkNumber, kDefaultFeatureId); for (auto it = begin; it != end; ++it) { @@ -162,12 +161,13 @@ private: static bool DeserializeSingleType(routing::Restriction::Type type, uint32_t count, routing::RestrictionVec & restrictions, Source & src) { - routing::Restriction prevRestriction(type, 0 /* linkNumber */); + routing::Restriction prevRestriction(type, {} /* links */); prevRestriction.m_featureIds.resize(kSupportedLinkNumber, kDefaultFeatureId); for (size_t i = 0; i < count; ++i) { BitReader bits(src); - routing::Restriction restriction(type, kSupportedLinkNumber); + routing::Restriction restriction(type, {} /* links */); + restriction.m_featureIds.resize(kSupportedLinkNumber); for (size_t i = 0; i < kSupportedLinkNumber; ++i) { uint32_t const biasedDelta = coding::DeltaCoder::Decode(bits);