From 4750d84a608bf898b0b5a5559fa2f1bf605f6aa1 Mon Sep 17 00:00:00 2001 From: Ilya Zverev Date: Wed, 7 Jun 2017 18:53:17 +0300 Subject: [PATCH] [generator] Metalines review fixes --- generator/CMakeLists.txt | 4 +- generator/generator.pro | 4 +- generator/generator_tool/generator_tool.cpp | 4 +- ...line_builder.cpp => metalines_builder.cpp} | 87 ++++++++++++------- ...line_builder.hpp => metalines_builder.hpp} | 8 +- generator/osm_translator.hpp | 2 +- 6 files changed, 69 insertions(+), 40 deletions(-) rename generator/{metaline_builder.cpp => metalines_builder.cpp} (64%) rename generator/{metaline_builder.hpp => metalines_builder.hpp} (73%) diff --git a/generator/CMakeLists.txt b/generator/CMakeLists.txt index 784d862fd6..f833bd2695 100644 --- a/generator/CMakeLists.txt +++ b/generator/CMakeLists.txt @@ -37,8 +37,8 @@ set(SRC generate_info.hpp intermediate_data.hpp intermediate_elements.hpp - metaline_builder.cpp - metaline_builder.hpp + metalines_builder.cpp + metalines_builder.hpp opentable_dataset.cpp opentable_dataset.hpp opentable_scoring.cpp diff --git a/generator/generator.pro b/generator/generator.pro index 9d8a3c452b..7131cd6fbd 100644 --- a/generator/generator.pro +++ b/generator/generator.pro @@ -28,7 +28,7 @@ SOURCES += \ feature_generator.cpp \ feature_merger.cpp \ feature_sorter.cpp \ - metaline_builder.cpp \ + metalines_builder.cpp \ opentable_dataset.cpp \ opentable_scoring.cpp \ osm2meta.cpp \ @@ -71,7 +71,7 @@ HEADERS += \ generate_info.hpp \ intermediate_data.hpp\ intermediate_elements.hpp\ - metaline_builder.hpp \ + metalines_builder.hpp \ opentable_dataset.hpp \ osm2meta.hpp \ osm2type.hpp \ diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 1cdbadc704..b18a2e3c6c 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -7,7 +7,7 @@ #include "generator/feature_generator.hpp" #include "generator/feature_sorter.hpp" #include "generator/generate_info.hpp" -#include "generator/metaline_builder.hpp" +#include "generator/metalines_builder.hpp" #include "generator/osm_source.hpp" #include "generator/restriction_generator.hpp" #include "generator/road_access_generator.hpp" @@ -232,7 +232,7 @@ int main(int argc, char ** argv) LOG(LINFO, ("Processing metalines from", metalinesFilename)); if (!feature::WriteMetalinesSection(datFile, metalinesFilename, osmToFeatureFilename)) - LOG(LWARNING, ("Error generating metalines section.")); + LOG(LCRITICAL, ("Error generating metalines section.")); } if (FLAGS_generate_index) diff --git a/generator/metaline_builder.cpp b/generator/metalines_builder.cpp similarity index 64% rename from generator/metaline_builder.cpp rename to generator/metalines_builder.cpp index 31b36083cc..951c934ff2 100644 --- a/generator/metaline_builder.cpp +++ b/generator/metalines_builder.cpp @@ -1,4 +1,4 @@ -#include "generator/metaline_builder.hpp" +#include "generator/metalines_builder.hpp" #include "generator/routing_helpers.hpp" #include "indexer/classificator.hpp" @@ -17,13 +17,16 @@ namespace { -using Ways = std::vector; +uint8_t const kMetaLinesSectionVersion = 1; +using Ways = std::vector; + +/// A string of connected ways. class LineString { Ways m_ways; - std::uint64_t m_start; - std::uint64_t m_end; + uint64_t m_start; + uint64_t m_end; bool m_oneway; public: @@ -31,8 +34,9 @@ public: { std::string const oneway = way.GetTag("oneway"); m_oneway = !oneway.empty() && oneway != "no"; - std::int32_t const wayId = base::checked_cast(way.id); + int32_t const wayId = base::checked_cast(way.id); m_ways.push_back(oneway == "-1" ? -wayId : wayId); + CHECK_GREATER_OR_EQUAL(way.Nodes().size(), 2, ()); m_start = way.Nodes().front(); m_end = way.Nodes().back(); } @@ -82,12 +86,13 @@ public: namespace feature { +/// A list of segments, that is, LineStrings, sharing the same attributes. class Segments { std::list m_parts; public: - explicit Segments(OsmElement const & way) { m_parts.push_back(LineString(way)); } + explicit Segments(OsmElement const & way) { m_parts.emplace_back(way); } void Add(OsmElement const & way) { @@ -101,11 +106,13 @@ public: break; } } + // If no LineString accepted the way in its Add method, create a new LineString with it. if (found == m_parts.cend()) { m_parts.push_back(line); return; } + // Otherwise check if the extended LineString can be merged with some other LineString. for (LineString & part : m_parts) { if (part.Add(*found)) @@ -120,25 +127,32 @@ public: { std::vector result; for (LineString const & line : m_parts) + { if (line.Ways().size() > 1) + { result.push_back(line.Ways()); + } + } return result; } }; +// MetalinesBuilder -------------------------------------------------------------------------------- void MetalinesBuilder::operator()(OsmElement const & el, FeatureParams const & params) { static uint32_t const highwayType = classif().GetTypeByPath({"highway"}); if (params.FindType(highwayType, 1) == ftype::GetEmptyValue() || el.Nodes().front() == el.Nodes().back()) + { return; + } std::string name; - params.name.GetString(0, name); + params.name.GetString(StringUtf8Multilang::kDefaultCode, name); if (name.empty() || params.ref.empty()) return; - std::size_t const key = std::hash{}(name + '\0' + params.ref); + size_t const key = std::hash{}(name + '\0' + params.ref); auto segment = m_data.find(key); if (segment == m_data.cend()) m_data.emplace(key, std::make_shared(el)); @@ -148,23 +162,31 @@ void MetalinesBuilder::operator()(OsmElement const & el, FeatureParams const & p void MetalinesBuilder::Flush() { - uint32_t count = 0; - FileWriter writer(m_filePath); - for (auto const & seg : m_data) + try { - auto const & longWays = seg.second->GetLongWays(); - for (Ways const & ways : longWays) + uint32_t count = 0; + FileWriter writer(m_filePath); + for (auto const & seg : m_data) { - uint16_t size = ways.size(); - WriteToSink(writer, size); - for (std::int32_t const way : ways) - WriteToSink(writer, way); - ++count; + auto const & longWays = seg.second->GetLongWays(); + for (Ways const & ways : longWays) + { + uint16_t size = base::checked_cast(ways.size()); + WriteToSink(writer, size); + for (int32_t const way : ways) + WriteToSink(writer, way); + ++count; + } } + LOG_SHORT(LINFO, ("Wrote", count, "metalines with OSM IDs for the entire planet to", m_filePath)); + } + catch (RootException const & e) + { + LOG(LERROR, ("An exception happened while saving metalines to", m_filePath, ":", e.what())); } - LOG_SHORT(LINFO, ("Wrote", count, "metalines with OSM IDs for the entire planet to", m_filePath)); } +// Functions -------------------------------------------------------------------------------- bool WriteMetalinesSection(std::string const & mwmPath, std::string const & metalinesPath, std::string const & osmIdsToFeatureIdsPath) { @@ -174,23 +196,27 @@ bool WriteMetalinesSection(std::string const & mwmPath, std::string const & meta FileReader reader(metalinesPath); ReaderSource src(reader); - std::vector buffer; - MemWriter> memWriter(buffer); - std::uint32_t count = 0; + std::vector buffer; + MemWriter> memWriter(buffer); + WriteToSink(memWriter, kMetaLinesSectionVersion); + uint32_t count = 0; - while (src.Size()) + while (src.Size() > 0) { - std::vector featureIds; - std::uint16_t size = ReadPrimitiveFromSource(src); - std::vector ways(size); - src.Read(&ways[0], size * sizeof(std::int32_t)); + std::vector featureIds; + uint32_t size = ReadPrimitiveFromSource(src); + std::vector ways(size); + src.Read(ways.data(), size * sizeof(int32_t)); for (auto const wayId : ways) { + // We get a negative wayId when a feature direction should be reversed. auto fid = osmIdToFeatureId.find(osm::Id::Way(std::abs(wayId))); if (fid == osmIdToFeatureId.cend()) break; - featureIds.push_back(wayId > 0 ? fid->second : -fid->second); + // Keeping the sign for the feature direction. + int32_t const featureId = static_cast(fid->second); + featureIds.push_back(wayId > 0 ? featureId : -featureId); } if (featureIds.size() > 1) @@ -203,13 +229,10 @@ bool WriteMetalinesSection(std::string const & mwmPath, std::string const & meta } } - // Terminate the metalines stream with a zero-length list. - uint8_t const zero = 0; - WriteToSink(memWriter, zero); - // Write buffer to section. FilesContainerW cont(mwmPath, FileWriter::OP_WRITE_EXISTING); FileWriter writer = cont.GetWriter(METALINES_FILE_TAG); + WriteVarUint(writer, count); writer.Write(buffer.data(), buffer.size()); LOG(LINFO, ("Finished writing metalines section, found", count, "metalines.")); return true; diff --git a/generator/metaline_builder.hpp b/generator/metalines_builder.hpp similarity index 73% rename from generator/metaline_builder.hpp rename to generator/metalines_builder.hpp index a138d86ee2..bf7e160d30 100644 --- a/generator/metaline_builder.hpp +++ b/generator/metalines_builder.hpp @@ -17,15 +17,21 @@ class MetalinesBuilder { public: explicit MetalinesBuilder(std::string const & filePath) : m_filePath(filePath) {} + ~MetalinesBuilder() { Flush(); } + + /// Add a highway segment to the collection of metalines. void operator()(OsmElement const & el, FeatureParams const & params); + + /// Write all metalines to the intermediate file. void Flush(); private: - std::unordered_map> m_data; + std::unordered_map> m_data; std::string m_filePath; }; +/// Read an intermediate file from MetalinesBuilder and convert it to an mwm section. bool WriteMetalinesSection(std::string const & mwmPath, std::string const & metalinesPath, std::string const & osmIdsToFeatureIdsPath); } diff --git a/generator/osm_translator.hpp b/generator/osm_translator.hpp index 87bbb33fa1..7262ae9bc9 100644 --- a/generator/osm_translator.hpp +++ b/generator/osm_translator.hpp @@ -1,7 +1,7 @@ #pragma once #include "generator/feature_builder.hpp" -#include "generator/metaline_builder.hpp" +#include "generator/metalines_builder.hpp" #include "generator/osm2type.hpp" #include "generator/osm_element.hpp" #include "generator/restriction_writer.hpp"