From 1363334ec91a1ca0f0795d192097a1765208613f Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 10 Nov 2016 11:55:00 +0300 Subject: [PATCH] Review fixes. --- generator/feature_generator.hpp | 2 +- generator/generate_info.hpp | 4 +- generator/generator.pro | 2 +- .../restriction_collector_test.cpp | 12 ++--- .../generator_tests/restriction_test.cpp | 2 +- generator/generator_tool/generator_tool.cpp | 12 ++--- generator/osm_source.cpp | 29 ++++++----- generator/osm_source.hpp | 1 + generator/osm_translator.hpp | 26 +++++----- generator/polygonizer.hpp | 33 ++++++------ generator/restriction_collector.cpp | 38 +++++++++----- generator/restriction_collector.hpp | 17 +++--- generator/restriction_generator.cpp | 21 ++++---- generator/restriction_generator.hpp | 4 +- generator/restriction_writer.cpp | 3 +- generator/restriction_writer.hpp | 3 +- generator/sync_ofsteam.cpp | 3 ++ generator/sync_ofsteam.hpp | 10 ++-- generator/world_map_generator.hpp | 8 +-- indexer/routing.cpp | 7 --- indexer/routing.hpp | 52 +++++++++++-------- 21 files changed, 158 insertions(+), 131 deletions(-) diff --git a/generator/feature_generator.hpp b/generator/feature_generator.hpp index 4122d5d134..059470b6e2 100644 --- a/generator/feature_generator.hpp +++ b/generator/feature_generator.hpp @@ -44,7 +44,7 @@ public: string const & GetFilePath() const { return m_datFile.GetName(); } /// \brief Serializes |f|. /// \returns feature id of serialized feature if |f| is serialized after the call - /// and numeric_limits::max() if not. + /// and |kInvalidFeatureId| if not. /// \note See implementation operator() in derived class for cases when |f| cannot be /// serialized. virtual uint32_t operator()(FeatureBuilder1 const & f); diff --git a/generator/generate_info.hpp b/generator/generate_info.hpp index 092143d39f..ccfe28dd09 100644 --- a/generator/generate_info.hpp +++ b/generator/generate_info.hpp @@ -38,8 +38,8 @@ struct GenerateInfo // File name for a file with restrictions in osm id terms. string m_restrictions; - // File name for a file with mapping from features id to osm ids. - // Note. One feature id maps to one or several osm ids. + // File name for a file with mapping from feature ids to osm ids. + // Note. One feature id maps to one or more osm ids. string m_featureIdToOsmIds; NodeStorageType m_nodeStorageType; diff --git a/generator/generator.pro b/generator/generator.pro index 9630f0b0c2..2fd4d2a676 100644 --- a/generator/generator.pro +++ b/generator/generator.pro @@ -87,7 +87,7 @@ HEADERS += \ sponsored_scoring.hpp \ srtm_parser.hpp \ statistics.hpp \ - sync_ofsteam.cpp \ + sync_ofsteam.hpp \ tag_admixer.hpp \ tesselator.hpp \ towns_dumper.hpp \ diff --git a/generator/generator_tests/restriction_collector_test.cpp b/generator/generator_tests/restriction_collector_test.cpp index 3333ac0b92..efa668c09b 100644 --- a/generator/generator_tests/restriction_collector_test.cpp +++ b/generator/generator_tests/restriction_collector_test.cpp @@ -27,7 +27,7 @@ string const kRestrictionTestDir = "test-restrictions"; UNIT_TEST(RestrictionTest_ValidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); // Adding feature ids. restrictionCollector.AddFeatureId(30 /* featureId */, {3} /* osmIds */); restrictionCollector.AddFeatureId(10 /* featureId */, {1} /* osmIds */); @@ -52,7 +52,7 @@ UNIT_TEST(RestrictionTest_ValidCase) UNIT_TEST(RestrictionTest_InvalidCase) { - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); restrictionCollector.AddFeatureId(0 /* featureId */, {0} /* osmIds */); restrictionCollector.AddFeatureId(20 /* featureId */, {2} /* osmIds */); @@ -75,7 +75,7 @@ UNIT_TEST(RestrictionTest_ParseRestrictions) ScopedDir const scopedDir(kRestrictionTestDir); ScopedFile const scopedFile(kRestrictionPath, kRestrictionContent); - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); Platform const & platform = Platform(); @@ -98,7 +98,7 @@ UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) ScopedDir const scopedDir(kRestrictionTestDir); ScopedFile const scopedFile(kFeatureIdToOsmIdsPath, kFeatureIdToOsmIdsContent); - RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureId2OsmIdsPath */); + RestrictionCollector restrictionCollector("" /* restrictionPath */, "" /* featureIdToOsmIdsPath */); Platform const & platform = Platform(); restrictionCollector.ParseFeatureId2OsmIdsMapping( @@ -107,8 +107,8 @@ UNIT_TEST(RestrictionTest_ParseFeatureId2OsmIdsMapping) vector> const expectedOsmIds2FeatureId = { {10, 1}, {20, 2}, {30, 3}, {5423239545, 779703}}; vector> osmIds2FeatureId( - restrictionCollector.m_osmId2FeatureId.cbegin(), - restrictionCollector.m_osmId2FeatureId.cend()); + restrictionCollector.m_osmIdToFeatureId.cbegin(), + restrictionCollector.m_osmIdToFeatureId.cend()); sort(osmIds2FeatureId.begin(), osmIds2FeatureId.end(), my::LessBy(&pair::first)); TEST_EQUAL(osmIds2FeatureId, expectedOsmIds2FeatureId, ()); diff --git a/generator/generator_tests/restriction_test.cpp b/generator/generator_tests/restriction_test.cpp index 27e88f25d3..711065b0a2 100644 --- a/generator/generator_tests/restriction_test.cpp +++ b/generator/generator_tests/restriction_test.cpp @@ -46,7 +46,7 @@ void BuildEmptyMwm(LocalCountryFile & country) /// \brief Generates a restriction section, adds it to an empty mwm, /// loads the restriction section and test loaded restrictions. /// \param restrictionContent comma separated text with restrictions in osm id terms. -/// \param mappingContent comma separated text with with mapping from feature id to osm ids. +/// \param mappingContent comma separated text with with mapping from feature ids to osm ids. void TestRestrictionBuilding(string const & restrictionContent, string const & mappingContent) { Platform & platform = GetPlatform(); diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 5c1609b14f..7e7394cc24 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -83,10 +83,10 @@ DEFINE_uint64(planet_version, my::SecondsSinceEpoch(), DEFINE_string(srtm_path, "", "Path to srtm directory. If it is set, generates section with altitude information " "about roads."); -DEFINE_string(restriction_name, "", "Name of file with relation restriction in osm id term."); -DEFINE_string(feature_id_to_osm_ids_name, "", - "Name of to file with mapping from feature id to osm ids."); -DEFINE_bool(generate_routing, false, "Generates section with routing information"); +DEFINE_string(restriction_name, "", "Name of file with relation restriction in osm id terms."); +DEFINE_string(feature_ids_to_osm_ids_name, "", + "Name of file with mapping from feature ids to osm ids."); +DEFINE_bool(generate_routing, false, "Generate section with routing information."); int main(int argc, char ** argv) { @@ -118,7 +118,7 @@ int main(int argc, char ** argv) genInfo.m_osmFileName = FLAGS_osm_file_name; genInfo.m_restrictions = FLAGS_restriction_name; - genInfo.m_featureIdToOsmIds = FLAGS_feature_id_to_osm_ids_name; + genInfo.m_featureIdToOsmIds = FLAGS_feature_ids_to_osm_ids_name; genInfo.m_failOnCoasts = FLAGS_fail_on_coasts; genInfo.m_preloadCache = FLAGS_preload_cache; genInfo.m_bookingDatafileName = FLAGS_booking_data; @@ -163,7 +163,7 @@ int main(int argc, char ** argv) LOG(LINFO, ("Generating final data ...")); genInfo.m_restrictions = FLAGS_restriction_name; - genInfo.m_featureIdToOsmIds = FLAGS_feature_id_to_osm_ids_name; + genInfo.m_featureIdToOsmIds = FLAGS_feature_ids_to_osm_ids_name; genInfo.m_splitByPolygons = FLAGS_split_by_polygons; genInfo.m_createWorld = FLAGS_generate_world; genInfo.m_makeCoasts = FLAGS_make_coasts; diff --git a/generator/osm_source.cpp b/generator/osm_source.cpp index 52163bbaef..40d3d7fd1f 100644 --- a/generator/osm_source.cpp +++ b/generator/osm_source.cpp @@ -2,13 +2,13 @@ #include "generator/feature_generator.hpp" #include "generator/intermediate_data.hpp" #include "generator/intermediate_elements.hpp" -#include "generator/sync_ofsteam.hpp" #include "generator/osm_element.hpp" #include "generator/osm_o5m_source.hpp" #include "generator/osm_source.hpp" #include "generator/osm_translator.hpp" #include "generator/osm_xml_source.hpp" #include "generator/polygonizer.hpp" +#include "generator/sync_ofsteam.hpp" #include "generator/tag_admixer.hpp" #include "generator/towns_dumper.hpp" #include "generator/world_map_generator.hpp" @@ -268,6 +268,8 @@ class MainFeaturesEmitter : public EmitterBase { using TWorldGenerator = WorldMapGenerator; using TCountriesGenerator = CountryMapGenerator>; + + generator::SyncOfstream m_featureIdToOsmIds; unique_ptr m_countries; unique_ptr m_world; @@ -299,7 +301,6 @@ class MainFeaturesEmitter : public EmitterBase uint32_t m_types[TYPES_COUNT]; inline uint32_t Type(TypeIndex i) const { return m_types[i]; } - SyncOfstream m_featureId2osmIds; public: MainFeaturesEmitter(feature::GenerateInfo const & info) @@ -336,20 +337,20 @@ public: m_coastsHolder.reset( new feature::FeaturesCollector(info.GetTmpFileName(WORLD_COASTS_FILE_NAME))); + string const featureId2OsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, ""); + LOG(LINFO, ("Saving mapping from feature ids to osm ids to", featureId2OsmIdsFile)); + m_featureIdToOsmIds.Open(featureId2OsmIdsFile); + if (!m_featureIdToOsmIds.IsOpened()) + { + LOG(LWARNING, + ("Cannot open", featureId2OsmIdsFile, ". Feature ids to osm ids to map won't be saved.")); + } + if (info.m_splitByPolygons || !info.m_fileName.empty()) - m_countries.reset(new TCountriesGenerator(info)); + m_countries = make_unique(info, m_featureIdToOsmIds); if (info.m_createWorld) m_world.reset(new TWorldGenerator(info)); - - string const featureId2OsmIdsFile = info.GetIntermediateFileName(info.m_featureIdToOsmIds, ""); - LOG(LINFO, ("Saving mapping from feature id osm ids to file", featureId2OsmIdsFile)); - m_featureId2osmIds.Open(featureId2OsmIdsFile); - if (!m_featureId2osmIds.IsOpened()) - { - LOG(LWARNING, - ("Cannot open", featureId2OsmIdsFile, ". Feature id to osm ids to map won't be saved.")); - } } void operator()(FeatureBuilder1 & fb) override @@ -450,7 +451,7 @@ public: auto & emitter = m_countries->Parent(); emitter.Start(); - (*m_countries)(fb, m_featureId2osmIds); + (*m_countries)(fb); emitter.Finish(); if (m_coastsHolder) @@ -507,7 +508,7 @@ private: (*m_world)(fb); if (m_countries) - (*m_countries)(fb, m_featureId2osmIds); + (*m_countries)(fb); } void DumpSkippedElements() diff --git a/generator/osm_source.hpp b/generator/osm_source.hpp index cdc23e2d88..44022809fe 100644 --- a/generator/osm_source.hpp +++ b/generator/osm_source.hpp @@ -5,6 +5,7 @@ #include "std/function.hpp" #include "std/iostream.hpp" +#include "std/string.hpp" #include "std/unique_ptr.hpp" #include "std/vector.hpp" diff --git a/generator/osm_translator.hpp b/generator/osm_translator.hpp index 419e998264..f5b37bf5d1 100644 --- a/generator/osm_translator.hpp +++ b/generator/osm_translator.hpp @@ -29,8 +29,8 @@ namespace class RelationTagsBase { public: - RelationTagsBase(routing::RestrictionWriter & restrictionDumper) - : m_restrictionDumper(restrictionDumper), m_cache(14) + RelationTagsBase(routing::RestrictionWriter & restrictionWriter) + : m_restrictionWriter(restrictionWriter), m_cache(14) { } @@ -77,7 +77,7 @@ protected: protected: uint64_t m_featureID; OsmElement * m_current; - routing::RestrictionWriter & m_restrictionDumper; + routing::RestrictionWriter & m_restrictionWriter; private: my::Cache m_cache; @@ -88,8 +88,8 @@ class RelationTagsNode : public RelationTagsBase using TBase = RelationTagsBase; public: - RelationTagsNode(routing::RestrictionWriter & restrictionDumper) - : RelationTagsBase(restrictionDumper) + RelationTagsNode(routing::RestrictionWriter & restrictionWriter) + : RelationTagsBase(restrictionWriter) { } @@ -102,7 +102,7 @@ protected: if (type == "restriction") { - m_restrictionDumper.Write(e); + m_restrictionWriter.Write(e); return; } @@ -131,8 +131,8 @@ protected: class RelationTagsWay : public RelationTagsBase { public: - RelationTagsWay(routing::RestrictionWriter & restrictionDumper) - : RelationTagsBase(restrictionDumper) + RelationTagsWay(routing::RestrictionWriter & restrictionWriter) + : RelationTagsBase(restrictionWriter) { } @@ -161,7 +161,7 @@ protected: if (type == "restriction") { - m_restrictionDumper.Write(e); + m_restrictionWriter.Write(e); return; } @@ -213,7 +213,7 @@ class OsmToFeatureTranslator uint32_t m_coastType; unique_ptr m_addrWriter; - routing::RestrictionWriter m_restrictionDumper; + routing::RestrictionWriter m_restrictionWriter; RelationTagsNode m_nodeRelations; RelationTagsWay m_wayRelations; @@ -521,13 +521,13 @@ public: : m_emitter(emitter) , m_holder(holder) , m_coastType(coastType) - , m_nodeRelations(m_restrictionDumper) - , m_wayRelations(m_restrictionDumper) + , m_nodeRelations(m_restrictionWriter) + , m_wayRelations(m_restrictionWriter) { if (!addrFilePath.empty()) m_addrWriter.reset(new FileWriter(addrFilePath)); if (!restrictionsFilePath.empty()) - m_restrictionDumper.Open(restrictionsFilePath); + m_restrictionWriter.Open(restrictionsFilePath); } }; diff --git a/generator/polygonizer.hpp b/generator/polygonizer.hpp index 48a4f1ab35..5b6f0ec5ba 100644 --- a/generator/polygonizer.hpp +++ b/generator/polygonizer.hpp @@ -4,6 +4,7 @@ #include "generator/feature_builder.hpp" #include "generator/generate_info.hpp" #include "generator/osm_source.hpp" +#include "generator/sync_ofsteam.hpp" #include "indexer/feature_visibility.hpp" #include "indexer/cell_id.hpp" @@ -18,7 +19,6 @@ #include "std/string.hpp" - #ifndef PARALLEL_POLYGONIZER #define PARALLEL_POLYGONIZER 1 #endif @@ -42,6 +42,8 @@ namespace feature vector m_Names; borders::CountriesContainerT m_countries; + generator::SyncOfstream & m_featureIdToOsmIds; + #if PARALLEL_POLYGONIZER QThreadPool m_ThreadPool; QSemaphore m_ThreadPoolSemaphore; @@ -49,7 +51,8 @@ namespace feature #endif public: - explicit Polygonizer(feature::GenerateInfo const & info) : m_info(info) + Polygonizer(feature::GenerateInfo const & info, generator::SyncOfstream & featureIdToOsmIds) + : m_info(info), m_featureIdToOsmIds(featureIdToOsmIds) #if PARALLEL_POLYGONIZER , m_ThreadPoolSemaphore(m_ThreadPool.maxThreadCount() * 8) #endif @@ -110,7 +113,7 @@ namespace feature } }; - void operator()(FeatureBuilder1 & fb, SyncOfstream & featureId2osmIds) + void operator()(FeatureBuilder1 & fb) { buffer_vector vec; m_countries.ForEachInRect(fb.GetLimitRect(), InsertCountriesPtr(vec)); @@ -119,14 +122,14 @@ namespace feature { case 0: break; - case 1: EmitFeature(vec[0], fb, featureId2osmIds); break; + case 1: EmitFeature(vec[0], fb, m_featureIdToOsmIds); break; default: { #if PARALLEL_POLYGONIZER m_ThreadPoolSemaphore.acquire(); - m_ThreadPool.start(new PolygonizerTask(this, vec, fb, featureId2osmIds)); + m_ThreadPool.start(new PolygonizerTask(this, vec, fb, m_featureIdToOsmIds)); #else - PolygonizerTask task(this, vec, fb, featureId2osmIds); + PolygonizerTask task(this, vec, fb, m_featureIdToOsmIds); task.RunBase(); #endif } @@ -148,7 +151,7 @@ namespace feature } void EmitFeature(borders::CountryPolygons const * country, FeatureBuilder1 const & fb, - SyncOfstream & featureId2osmIds) + generator::SyncOfstream & featureIdToOsmIds) { #if PARALLEL_POLYGONIZER QMutexLocker mutexLocker(&m_EmitFeatureMutex); @@ -169,7 +172,7 @@ namespace feature uint32_t const featureId = bucket(fb); if (fb.IsLine()) - featureId2osmIds.Write(featureId /* feature id of |fb| */, fb.GetOsmIds()); + featureIdToOsmIds.Write(featureId /* feature id of |fb| */, fb.GetOsmIds()); } vector const & Names() const @@ -188,11 +191,11 @@ namespace feature public: PolygonizerTask(Polygonizer * pPolygonizer, buffer_vector const & countries, - FeatureBuilder1 const & fb, SyncOfstream & featureId2osmIds) + FeatureBuilder1 const & fb, generator::SyncOfstream & featureIdToOsmIds) : m_pPolygonizer(pPolygonizer) , m_Countries(countries) - , m_FB(fb) - , m_featureId2osmIds(featureId2osmIds) + , m_fb(fb) + , m_featureIdToOsmIds(featureIdToOsmIds) { } @@ -201,10 +204,10 @@ namespace feature for (size_t i = 0; i < m_Countries.size(); ++i) { PointChecker doCheck(m_Countries[i]->m_regions); - m_FB.ForEachGeometryPoint(doCheck); + m_fb.ForEachGeometryPoint(doCheck); if (doCheck.m_belongs) - m_pPolygonizer->EmitFeature(m_Countries[i], m_FB, m_featureId2osmIds); + m_pPolygonizer->EmitFeature(m_Countries[i], m_fb, m_featureIdToOsmIds); } } @@ -220,8 +223,8 @@ namespace feature private: Polygonizer * m_pPolygonizer; buffer_vector m_Countries; - FeatureBuilder1 m_FB; - SyncOfstream & m_featureId2osmIds; + FeatureBuilder1 m_fb; + generator::SyncOfstream & m_featureIdToOsmIds; }; }; } diff --git a/generator/restriction_collector.cpp b/generator/restriction_collector.cpp index f60696935a..33bc95dc9e 100644 --- a/generator/restriction_collector.cpp +++ b/generator/restriction_collector.cpp @@ -2,6 +2,7 @@ #include "base/assert.hpp" #include "base/logging.hpp" +#include "base/scope_guard.hpp" #include "base/stl_helpers.hpp" #include "base/string_utils.hpp" @@ -30,16 +31,21 @@ bool ParseLineOfNumbers(strings::SimpleTokenizer & iter, vector & numb namespace routing { RestrictionCollector::RestrictionCollector(string const & restrictionPath, - string const & featureId2OsmIdsPath) + string const & featureIdToOsmIdsPath) { - if (!ParseFeatureId2OsmIdsMapping(featureId2OsmIdsPath)) - return; - - if (!ParseRestrictions(restrictionPath)) - { + MY_SCOPE_GUARD(parseErrorGuard, [this]{ + m_osmIdToFeatureId.clear(); m_restrictions.clear(); + }); + + if (!ParseFeatureId2OsmIdsMapping(featureIdToOsmIdsPath) + || !ParseRestrictions(restrictionPath)) + { + LOG(LWARNING, ("An error happened while parsing", featureIdToOsmIdsPath, "or", restrictionPath)); return; } + parseErrorGuard.release(); + my::SortUnique(m_restrictions); if (!IsValid()) @@ -53,9 +59,9 @@ bool RestrictionCollector::IsValid() const [](Restriction const & r) { return !r.IsValid(); }) == end(m_restrictions); } -bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureId2OsmIdsPath) +bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureIdToOsmIdsPath) { - ifstream featureId2OsmIdsStream(featureId2OsmIdsPath); + ifstream featureId2OsmIdsStream(featureIdToOsmIdsPath); if (featureId2OsmIdsStream.fail()) return false; @@ -68,7 +74,7 @@ bool RestrictionCollector::ParseFeatureId2OsmIdsMapping(string const & featureId return false; if (osmIds.size() < 2) - return false; // Every line should contain at least feature id and osm id. + return false; // Every line should contain feature id and at least one osm id. uint32_t const featureId = static_cast(osmIds.front()); osmIds.erase(osmIds.begin()); @@ -92,12 +98,18 @@ bool RestrictionCollector::ParseRestrictions(string const & path) Restriction::Type type; if (!FromString(*iter, type)) + { + LOG(LWARNING, ("Cannot parse a restriction type", *iter, "form", path)); return false; + } ++iter; vector osmIds; if (!ParseLineOfNumbers(iter, osmIds)) + { + LOG(LWARNING, ("Cannot parse osm ids from", path)); return false; + } AddRestriction(type, osmIds); } @@ -109,8 +121,8 @@ bool RestrictionCollector::AddRestriction(Restriction::Type type, vector featureIds(osmIds.size()); for (size_t i = 0; i < osmIds.size(); ++i) { - auto const result = m_osmId2FeatureId.find(osmIds[i]); - if (result == m_osmId2FeatureId.cend()) + auto const result = m_osmIdToFeatureId.find(osmIds[i]); + if (result == m_osmIdToFeatureId.cend()) { // It could happend near mwm border when one of a restriction lines is not included in mwm // but the restriction is included. @@ -131,11 +143,11 @@ void RestrictionCollector::AddFeatureId(uint32_t featureId, vector con // but for road feature |featureId| corresponds exactly one osm id. for (uint64_t const & osmId : osmIds) { - auto const result = m_osmId2FeatureId.insert(make_pair(osmId, featureId)); + auto const result = m_osmIdToFeatureId.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)); + m_osmIdToFeatureId.find(osmId)->second)); } } } diff --git a/generator/restriction_collector.hpp b/generator/restriction_collector.hpp index bbc8745fb1..19444a0c52 100644 --- a/generator/restriction_collector.hpp +++ b/generator/restriction_collector.hpp @@ -17,14 +17,13 @@ class RestrictionCollector { public: /// \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); + /// \param featureIdToOsmIdsPath full path to file with mapping from feature id to osm id. + RestrictionCollector(string const & restrictionPath, string const & featureIdToOsmIdsPath); bool HasRestrictions() const { return !m_restrictions.empty(); } /// \returns true if all restrictions in |m_restrictions| are valid and false otherwise. - /// \note Empty |m_restrictions| is considered as an invalid restriction. - /// \note Complexity of the method is up to linear in the size of |m_restrictions|. + /// \note Complexity of the method is linear in the size of |m_restrictions|. bool IsValid() const; /// \returns Sorted vector of restrictions. @@ -43,10 +42,10 @@ private: /// 137999, 5170186, /// 138000, 5170209, 5143342, /// 138001, 5170228, - /// \param featureId2OsmIdsPath path to the text file. + /// \param featureIdToOsmIdsPath path to the text file. /// \note Most restrictions consist of type and two linear(road) features. /// \note For the time being only line-point-line restrictions are supported. - bool ParseFeatureId2OsmIdsMapping(string const & featureId2OsmIdsPath); + bool ParseFeatureId2OsmIdsMapping(string const & featureIdToOsmIdsPath); /// \brief Parses comma separated text file with line in following format: /// , , , and so on @@ -54,10 +53,10 @@ private: /// Only, 335049632, 49356687, /// No, 157616940, 157616940, /// No, 157616940, 157617107, - /// \param featureId2OsmIdsPath path to the text file. + /// \param featureIdToOsmIdsPath path to the text file. bool ParseRestrictions(string const & path); - /// \brief Adds feature id and corresponding vector of |osmIds| to |m_osmId2FeatureId|. + /// \brief Adds feature id and corresponding vector of |osmIds| to |m_osmIdToFeatureId|. /// \note One feature id (|featureId|) may correspond to several osm ids (|osmIds|). void AddFeatureId(uint32_t featureId, vector const & osmIds); @@ -70,7 +69,7 @@ private: bool AddRestriction(Restriction::Type type, vector const & osmIds); RestrictionVec m_restrictions; - map m_osmId2FeatureId; + map m_osmIdToFeatureId; }; bool FromString(string str, Restriction::Type & type); diff --git a/generator/restriction_generator.cpp b/generator/restriction_generator.cpp index 7c0d1d02db..3483b176fb 100644 --- a/generator/restriction_generator.cpp +++ b/generator/restriction_generator.cpp @@ -1,4 +1,5 @@ #include "generator/restriction_generator.hpp" + #include "generator/restriction_collector.hpp" #include "coding/file_container.hpp" @@ -16,34 +17,34 @@ using namespace feature; namespace routing { bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPath, - string const & featureId2OsmIdsPath) + string const & featureIdToOsmIdsPath) { LOG(LINFO, - ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", featureId2OsmIdsPath, ");")); - RestrictionCollector restrictionCollector(restrictionPath, featureId2OsmIdsPath); + ("BuildRoadRestrictions(", mwmPath, ", ", restrictionPath, ", ", featureIdToOsmIdsPath, ");")); + RestrictionCollector restrictionCollector(restrictionPath, featureIdToOsmIdsPath); if (!restrictionCollector.HasRestrictions() || !restrictionCollector.IsValid()) { - LOG(LWARNING, ("No valid restrictions for", mwmPath, "It's necessary to check if", - restrictionPath, "and", featureId2OsmIdsPath, "are available.")); + LOG(LWARNING, ("No valid restrictions for", mwmPath, "It's necessary to check that", + restrictionPath, "and", featureIdToOsmIdsPath, "are available.")); return false; } RestrictionVec const & restrictions = restrictionCollector.GetRestrictions(); auto const firstOnlyIt = - upper_bound(restrictions.cbegin(), restrictions.cend(), - Restriction(Restriction::Type::No, {} /* links */), my::LessBy(&Restriction::m_type)); + lower_bound(restrictions.cbegin(), restrictions.cend(), + Restriction(Restriction::Type::Only, {} /* links */), my::LessBy(&Restriction::m_type)); RoutingHeader header; header.m_noRestrictionCount = distance(restrictions.cbegin(), firstOnlyIt); header.m_onlyRestrictionCount = restrictions.size() - header.m_noRestrictionCount; - LOG(LINFO, ("Header info. There are", header.m_noRestrictionCount, "no restrictions and", - header.m_onlyRestrictionCount, "only restrictions")); + LOG(LINFO, ("Header info. There are", header.m_noRestrictionCount, "of type No restrictions and", + header.m_onlyRestrictionCount, "of type Only restrictions")); FilesContainerW cont(mwmPath, FileWriter::OP_WRITE_EXISTING); FileWriter w = cont.GetWriter(ROUTING_FILE_TAG); header.Serialize(w); - RestrictionSerializer::Serialize(restrictions.cbegin(), firstOnlyIt, restrictions.cend(), w); + RestrictionSerializer::Serialize(header, restrictions.cbegin(), restrictions.cend(), w); return true; } diff --git a/generator/restriction_generator.hpp b/generator/restriction_generator.hpp index 9c6f41f5a1..17636f6fc8 100644 --- a/generator/restriction_generator.hpp +++ b/generator/restriction_generator.hpp @@ -12,7 +12,7 @@ namespace routing /// For example: /// Only, 335049632, 49356687, /// No, 157616940, 157616940, -/// \param featureId2OsmIdsPath comma separated (csv like) file with mapping from feature id to osm +/// \param featureIdToOsmIdsPath comma separated (csv like) file with mapping from feature id to osm /// ids /// in following format: /// , , , and so @@ -21,5 +21,5 @@ namespace routing /// 137999, 5170186, /// 138000, 5170209, bool BuildRoadRestrictions(string const & mwmPath, string const & restrictionPath, - string const & featureId2OsmIdsPath); + string const & featureIdToOsmIdsPath); } // namespace routing diff --git a/generator/restriction_writer.cpp b/generator/restriction_writer.cpp index c10b7aea6f..636d0e3ce7 100644 --- a/generator/restriction_writer.cpp +++ b/generator/restriction_writer.cpp @@ -1,4 +1,5 @@ #include "generator/restriction_writer.hpp" + #include "generator/intermediate_elements.hpp" #include "generator/osm_id.hpp" #include "generator/restriction_collector.hpp" @@ -23,7 +24,7 @@ vector const kRestrictionTypesOnly = {"only_right_turn", "only_left_turn "only_straight_on"}; /// \brief Converts restriction type form string to RestrictionCollector::Type. -/// \returns Fisrt true if convertion was successful and false otherwise. +/// \returns true if conversion was successful and false otherwise. bool TagToType(string const & tag, Restriction::Type & type) { if (find(kRestrictionTypesNo.cbegin(), kRestrictionTypesNo.cend(), tag) != diff --git a/generator/restriction_writer.hpp b/generator/restriction_writer.hpp index a89aa99e01..99ff70581b 100644 --- a/generator/restriction_writer.hpp +++ b/generator/restriction_writer.hpp @@ -15,8 +15,7 @@ public: /// \brief Writes |relationElement| to |m_stream| if |relationElement| is a supported restriction. /// \note For the time being only line-point-line restrictions are processed. The other - /// restrictions - /// are ignored. + /// restrictions are ignored. // @TODO(bykoianko) It's necessary to process all kind of restrictions. void Write(RelationElement const & relationElement); diff --git a/generator/sync_ofsteam.cpp b/generator/sync_ofsteam.cpp index 92b29aae5a..885a737428 100644 --- a/generator/sync_ofsteam.cpp +++ b/generator/sync_ofsteam.cpp @@ -2,6 +2,8 @@ #include "std/iostream.hpp" +namespace generator +{ void SyncOfstream::Open(string const & fullPath) { lock_guard guard(m_mutex); @@ -25,3 +27,4 @@ void SyncOfstream::Write(uint32_t featureId, vector const & osmIds) m_stream << "," << osmId.OsmId(); m_stream << endl; } +} // namespace generator diff --git a/generator/sync_ofsteam.hpp b/generator/sync_ofsteam.hpp index 9890eff341..856cdac6c1 100644 --- a/generator/sync_ofsteam.hpp +++ b/generator/sync_ofsteam.hpp @@ -7,13 +7,17 @@ #include "std/string.hpp" #include "std/vector.hpp" +namespace generator +{ class SyncOfstream { - ofstream m_stream; - mutex m_mutex; - public: void Open(string const & fullPath); bool IsOpened(); void Write(uint32_t featureId, vector const & osmIds); + +private: + ofstream m_stream; + mutex m_mutex; }; +} // namespace generator diff --git a/generator/world_map_generator.hpp b/generator/world_map_generator.hpp index 593fa074cf..5cc9322ab6 100644 --- a/generator/world_map_generator.hpp +++ b/generator/world_map_generator.hpp @@ -312,11 +312,13 @@ class CountryMapGenerator FeatureOutT m_bucket; public: - explicit CountryMapGenerator(feature::GenerateInfo const & info) : m_bucket(info) {} - void operator()(FeatureBuilder1 fb, SyncOfstream & featureId2osmIds) + CountryMapGenerator(feature::GenerateInfo const & info, generator::SyncOfstream & featureIdToOsmIds) + : m_bucket(info, featureIdToOsmIds) {} + + void operator()(FeatureBuilder1 fb) { if (feature::PreprocessForCountryMap(fb)) - m_bucket(fb, featureId2osmIds); + m_bucket(fb); } inline FeatureOutT & Parent() { return m_bucket; } diff --git a/indexer/routing.cpp b/indexer/routing.cpp index 985c5db8f7..283a238702 100644 --- a/indexer/routing.cpp +++ b/indexer/routing.cpp @@ -49,10 +49,3 @@ string DebugPrint(Restriction const & restriction) return out.str(); } } // namespace routing - -namespace feature -{ -// static -uint32_t const RestrictionSerializer::kDefaultFeatureId = 0; -} // feature - diff --git a/indexer/routing.hpp b/indexer/routing.hpp index 3c18a38d01..ae269a4149 100644 --- a/indexer/routing.hpp +++ b/indexer/routing.hpp @@ -97,10 +97,11 @@ class RestrictionSerializer { public: template - static void Serialize(routing::RestrictionVec::const_iterator begin, - routing::RestrictionVec::const_iterator firstOnlyIt, + static void Serialize(RoutingHeader const & header, + routing::RestrictionVec::const_iterator begin, routing::RestrictionVec::const_iterator end, Sink & sink) { + auto const firstOnlyIt = begin + header.m_noRestrictionCount; SerializeSingleType(begin, firstOnlyIt, sink); SerializeSingleType(firstOnlyIt, end, sink); } @@ -116,7 +117,7 @@ public: } private: - static uint32_t const kDefaultFeatureId; + static uint32_t const kDefaultFeatureId = 0; /// \brief Serializes a range of restrictions form |begin| to |end| to |sink|. /// \param begin is an iterator to the first item to serialize. @@ -132,26 +133,28 @@ private: CHECK(is_sorted(begin, end), ()); routing::Restriction::Type const type = begin->m_type; - uint32_t prevFisrtLinkFeatureId = 0; - for (auto it = begin; it != end; ++it) + uint32_t prevFirstLinkFeatureId = 0; + for (; begin != end; ++begin) { - CHECK_EQUAL(type, it->m_type, ()); + CHECK_EQUAL(type, begin->m_type, ()); - routing::Restriction const & restriction = *it; + routing::Restriction const & restriction = *begin; CHECK(restriction.IsValid(), ()); - CHECK_LESS(1, restriction.m_featureIds.size(), ("No meaning in zero or one link restrictions.")); + CHECK_LESS(1, restriction.m_featureIds.size(), ("No sense in zero or one link restrictions.")); BitWriter bits(sink); - coding::DeltaCoder::Encode(bits, restriction.m_featureIds.size() - 1 /* link number is two or more */); - uint32_t prevLinkFeatureId = prevFisrtLinkFeatureId; - for (size_t i = 0; i < restriction.m_featureIds.size(); ++i) + coding::DeltaCoder::Encode(bits, restriction.m_featureIds.size() - 1 /* number of link is two or more */); + + CHECK_LESS_OR_EQUAL(prevFirstLinkFeatureId, restriction.m_featureIds[0], ()); + coding::DeltaCoder::Encode(bits, + restriction.m_featureIds[0] - prevFirstLinkFeatureId + 1 /* making it greater than zero */); + for (size_t i = 1; i < restriction.m_featureIds.size(); ++i) { uint32_t const delta = bits::ZigZagEncode(static_cast(restriction.m_featureIds[i]) - - static_cast(prevLinkFeatureId)); + static_cast(restriction.m_featureIds[i - 1])); coding::DeltaCoder::Encode(bits, delta + 1 /* making it greater than zero */); - prevLinkFeatureId = restriction.m_featureIds[i]; } - prevFisrtLinkFeatureId = restriction.m_featureIds[0]; + prevFirstLinkFeatureId = restriction.m_featureIds[0]; } } @@ -159,7 +162,7 @@ private: static bool DeserializeSingleType(routing::Restriction::Type type, uint32_t count, routing::RestrictionVec & restrictions, Source & src) { - uint32_t prevFisrtLinkFeatureId = 0; + uint32_t prevFirstLinkFeatureId = 0; for (size_t i = 0; i < count; ++i) { BitReader bits(src); @@ -169,12 +172,18 @@ private: LOG(LERROR, ("Decoded link restriction number is zero.")); return false; } - size_t const linkNumber = biasedLinkNumber + 1 /* link number is two or more */; + size_t const numLinks = biasedLinkNumber + 1 /* number of link is two or more */; routing::Restriction restriction(type, {} /* links */); - restriction.m_featureIds.resize(linkNumber); - uint32_t prevLinkFeatureId = prevFisrtLinkFeatureId; - for (size_t i = 0; i < linkNumber; ++i) + restriction.m_featureIds.resize(numLinks); + uint32_t const biasedFirstFeatureId = coding::DeltaCoder::Decode(bits); + if (biasedFirstFeatureId == 0) + { + LOG(LERROR, ("Decoded first link restriction feature id delta is zero..")); + return false; + } + restriction.m_featureIds[0] = prevFirstLinkFeatureId + biasedFirstFeatureId - 1; + for (size_t i = 1; i < numLinks; ++i) { uint32_t const biasedDelta = coding::DeltaCoder::Decode(bits); if (biasedDelta == 0) @@ -184,11 +193,10 @@ private: } uint32_t const delta = biasedDelta - 1; restriction.m_featureIds[i] = static_cast( - bits::ZigZagDecode(delta) + prevLinkFeatureId); - prevLinkFeatureId = restriction.m_featureIds[i]; + bits::ZigZagDecode(delta) + restriction.m_featureIds[i - 1]); } - prevFisrtLinkFeatureId = restriction.m_featureIds[0]; + prevFirstLinkFeatureId = restriction.m_featureIds[0]; restrictions.push_back(restriction); } return true;