From 8d060411fdb45ff45c9fa87993ee925f91606671 Mon Sep 17 00:00:00 2001 From: Daria Volvenkova Date: Fri, 24 Jan 2020 15:39:18 +0300 Subject: [PATCH] [topography_generator] Review fixes. --- generator/srtm_parser.cpp | 8 ++-- generator/srtm_parser.hpp | 2 +- topography_generator/filters_impl.hpp | 3 ++ topography_generator/generator.cpp | 7 +++- topography_generator/generator.hpp | 2 +- topography_generator/main.cpp | 37 +++++++++++++------ .../marching_squares/contours_builder.cpp | 5 +-- topography_generator/utils/contours.hpp | 2 +- .../utils/contours_serdes.hpp | 34 ++++++++--------- 9 files changed, 59 insertions(+), 41 deletions(-) diff --git a/generator/srtm_parser.cpp b/generator/srtm_parser.cpp index 9eb148fe5a..781bb43f25 100644 --- a/generator/srtm_parser.cpp +++ b/generator/srtm_parser.cpp @@ -160,8 +160,8 @@ void SrtmTile::Invalidate() SrtmTileManager::SrtmTileManager(std::string const & dir) : m_dir(dir) {} geometry::Altitude SrtmTileManager::GetHeight(ms::LatLon const & coord) { - LatLonKey const key = {static_cast(floor(coord.m_lat)), - static_cast(floor(coord.m_lon))}; + LatLonKey const key = {static_cast(floor(coord.m_lat)), + static_cast(floor(coord.m_lon))}; auto it = m_tiles.find(key); if (it == m_tiles.end()) @@ -186,8 +186,8 @@ geometry::Altitude SrtmTileManager::GetHeight(ms::LatLon const & coord) bool SrtmTileManager::HasValidTile(ms::LatLon const & coord) const { - LatLonKey const key = {static_cast(floor(coord.m_lat)), - static_cast(floor(coord.m_lon))}; + LatLonKey const key = {static_cast(floor(coord.m_lat)), + static_cast(floor(coord.m_lon))}; auto it = m_tiles.find(key); if (it != m_tiles.end()) return it->second.IsValid(); diff --git a/generator/srtm_parser.hpp b/generator/srtm_parser.hpp index 3c3c738b6e..b0e062d214 100644 --- a/generator/srtm_parser.hpp +++ b/generator/srtm_parser.hpp @@ -55,7 +55,7 @@ public: private: std::string m_dir; - using LatLonKey = std::pair; + using LatLonKey = std::pair; struct Hash { size_t operator()(LatLonKey const & key) const diff --git a/topography_generator/filters_impl.hpp b/topography_generator/filters_impl.hpp index 41726ac752..7daeadf522 100644 --- a/topography_generator/filters_impl.hpp +++ b/topography_generator/filters_impl.hpp @@ -57,6 +57,7 @@ void ProcessWithLinearKernel(std::vector const & kernel, size_t tileSize auto const kernelRadius = kernel.size() / 2; CHECK_LESS_OR_EQUAL(kernelRadius, tileOffset, ()); CHECK_GREATER(tileSize, tileOffset * 2, ()); + CHECK_EQUAL(dstValues.size(), tileSize * tileSize, ()); std::vector tempValues(tileSize, 0); @@ -105,6 +106,7 @@ void ProcessWithSquareKernel(std::vector const & kernel, size_t kernelSi size_t const kernelRadius = kernelSize / 2; CHECK_LESS_OR_EQUAL(kernelRadius, tileOffset, ()); CHECK_GREATER(tileSize, tileOffset * 2, ()); + CHECK_EQUAL(dstValues.size(), tileSize * tileSize, ()); for (size_t i = tileOffset; i < tileSize - tileOffset; ++i) { @@ -131,6 +133,7 @@ void ProcessMedian(size_t kernelRadius, size_t tileSize, size_t tileOffset, { CHECK_LESS_OR_EQUAL(kernelRadius, tileOffset, ()); CHECK_GREATER(tileSize, tileOffset * 2, ()); + CHECK_EQUAL(dstValues.size(), tileSize * tileSize, ()); size_t const kernelSize = kernelRadius * 2 + 1; std::vector kernel(kernelSize * kernelSize); diff --git a/topography_generator/generator.cpp b/topography_generator/generator.cpp index 134c161f4a..803609fb4e 100644 --- a/topography_generator/generator.cpp +++ b/topography_generator/generator.cpp @@ -176,7 +176,12 @@ public: , m_strmDir(srtmDir) , m_srtmProvider(srtmDir) , m_params(params) - {} + { + CHECK(right >= -180 && right <= 179, ()); + CHECK(left >= -180 && left <= 179, ()); + CHECK(top >= -90 && top <= 89, ()); + CHECK(bottom >= -90 && bottom <= 89, ()); + } void Do() override { diff --git a/topography_generator/generator.hpp b/topography_generator/generator.hpp index 7578949908..394ef110c5 100644 --- a/topography_generator/generator.hpp +++ b/topography_generator/generator.hpp @@ -11,9 +11,9 @@ #include "base/thread_pool.hpp" #include -#include #include #include +#include namespace topography_generator { diff --git a/topography_generator/main.cpp b/topography_generator/main.cpp index dd4c2e241f..bccf66c8c0 100644 --- a/topography_generator/main.cpp +++ b/topography_generator/main.cpp @@ -19,10 +19,10 @@ DEFINE_uint64(alt_step_factor, 1, "Isolines packing mode. Altitude step factor." DEFINE_string(srtm_path, "", "Isolines generating mode. Path to srtm directory."); -DEFINE_int32(left, 0, "Isolines generating mode. Left longitude of tiles rect [-180, 180]."); -DEFINE_int32(right, 0, "Isolines generating mode. Right longitude of tiles rect [-180, 180]."); -DEFINE_int32(bottom, 0, "Isolines generating mode. Bottom latitude of tiles rect [-90, 90]."); -DEFINE_int32(top, 0, "Isolines generating mode. Top latitude of tiles rect [-90, 90]."); +DEFINE_int32(left, 0, "Isolines generating mode. Left longitude of tiles rect [-180, 179]."); +DEFINE_int32(right, 0, "Isolines generating mode. Right longitude of tiles rect [-179, 180]."); +DEFINE_int32(bottom, 0, "Isolines generating mode. Bottom latitude of tiles rect [-90, 89]."); +DEFINE_int32(top, 0, "Isolines generating mode. Top latitude of tiles rect [-89, 90]."); DEFINE_uint64(isolines_step, 10, "Isolines generating mode. Isolines step in meters."); DEFINE_uint64(latlon_step_factor, 2, "Isolines generating mode. Lat/lon step factor."); DEFINE_double(gaussian_st_dev, 2.0, "Isolines generating mode. Gaussian filter standard deviation."); @@ -42,9 +42,29 @@ int main(int argc, char ** argv) return EXIT_FAILURE; } + auto const validTilesRect = FLAGS_right > FLAGS_left && FLAGS_top > FLAGS_bottom && + FLAGS_right <= 180 && FLAGS_left >= -180 && + FLAGS_top <= 90 && FLAGS_bottom >= -90; + + auto const isGeneratingMode = validTilesRect; + auto const isPackingMode = !FLAGS_countryId.empty(); + + if (isGeneratingMode && isPackingMode) + { + LOG(LERROR, ("Both tiles rect and country id are set. Сhoose one operation: " + "generation of tiles rect or packing tiles for the country")); + return EXIT_FAILURE; + } + + if (!isGeneratingMode && !isPackingMode) + { + LOG(LERROR, ("Valid tiles rect or country id must be set.")); + return EXIT_FAILURE; + } + topography_generator::Generator generator(FLAGS_srtm_path, FLAGS_threads, FLAGS_tiles_per_thread); - if (!FLAGS_countryId.empty()) + if (isPackingMode) { if (FLAGS_isolines_path.empty()) { @@ -67,12 +87,7 @@ int main(int argc, char ** argv) return EXIT_FAILURE; } - if (FLAGS_right <= FLAGS_left || FLAGS_top <= FLAGS_bottom || - FLAGS_right > 180 || FLAGS_left < -180 || FLAGS_top > 90 || FLAGS_bottom < -90) - { - LOG(LERROR, ("Invalid tiles rect.")); - return EXIT_FAILURE; - } + CHECK(!validTilesRect, ()); topography_generator::TileIsolinesParams params; if (FLAGS_median_r > 0) diff --git a/topography_generator/marching_squares/contours_builder.cpp b/topography_generator/marching_squares/contours_builder.cpp index 6142252933..701be024e1 100644 --- a/topography_generator/marching_squares/contours_builder.cpp +++ b/topography_generator/marching_squares/contours_builder.cpp @@ -26,9 +26,8 @@ void ContoursBuilder::AddSegment(size_t levelInd, ms::LatLon const & beginPos, m if (connectStart && connectEnd && contourItBefore != contourItAfter) { - contourItBefore->m_countour.insert(contourItBefore->m_countour.end(), - contourItAfter->m_countour.begin(), - contourItAfter->m_countour.end()); + std::move(contourItAfter->m_countour.begin(), contourItAfter->m_countour.end(), + std::back_inserter(contourItBefore->m_countour)); contourItBefore->m_active = true; m_activeContours[levelInd].erase(contourItAfter); } diff --git a/topography_generator/utils/contours.hpp b/topography_generator/utils/contours.hpp index db3bcdab10..13b471a999 100644 --- a/topography_generator/utils/contours.hpp +++ b/topography_generator/utils/contours.hpp @@ -65,7 +65,7 @@ void CropContours(m2::RectD & rect, std::vector & regions, size_t m levelCroppedContours.emplace_back(std::move(cropped)); } } - it->second.swap(levelCroppedContours); + it->second = std::move(levelCroppedContours); if (!it->second.empty()) { diff --git a/topography_generator/utils/contours_serdes.hpp b/topography_generator/utils/contours_serdes.hpp index 3391da02ac..3be14a349f 100644 --- a/topography_generator/utils/contours_serdes.hpp +++ b/topography_generator/utils/contours_serdes.hpp @@ -42,9 +42,8 @@ private: template void SerializeContour(Sink & sink, topography_generator::Contour const & contour) { - serial::GeometryCodingParams codingParams; - serial::SavePoint(sink, contour[0], codingParams); - codingParams.SetBasePoint(contour[0]); + serial::GeometryCodingParams codingParams(kPointCoordBits, contour[0]); + codingParams.Save(sink); serial::SaveOuterPath(contour, codingParams, sink); } @@ -75,7 +74,7 @@ public: ValueType levelValue; std::vector levelContours; DeserializeContours(source, levelValue, levelContours); - contours.m_contours[levelValue].swap(levelContours); + contours.m_contours[levelValue] = std::move(levelContours); } } @@ -94,11 +93,10 @@ private: topography_generator::Contour & contour) { serial::GeometryCodingParams codingParams; - auto const pt = serial::LoadPoint(source, codingParams); - codingParams.SetBasePoint(pt); + codingParams.Load(source); std::vector points; serial::LoadOuterPath(source, codingParams, points); - contour.swap(points); + contour = std::move(points); } }; @@ -123,19 +121,17 @@ bool SaveContrours(std::string const & filePath, template bool LoadContours(std::string const & filePath, Contours & contours) { + try { - try - { - FileReader file(filePath); - DeserializerContours des; - des.Deserialize(file, contours); - } - catch (FileReader::Exception const & ex) - { - LOG(LWARNING, ("File writer exception raised:", ex.what(), ", file", filePath)); - return false; - } - return true; + FileReader file(filePath); + DeserializerContours des; + des.Deserialize(file, contours); } + catch (FileReader::Exception const & ex) + { + LOG(LWARNING, ("File reader exception raised:", ex.what(), ", file", filePath)); + return false; + } + return true; } } // namespace topography_generator