Fixed hgt tile's grid traversal.

Signed-off-by: Viktor Govako <viktor.govako@gmail.com>

coding/coding_tests/file_data_test.cpp

@@ -1,14 +1,11 @@
 #include "testing/testing.hpp"
 
 #include "coding/internal/file_data.hpp"
-#include "coding/writer.hpp"
-
-#include "base/logging.hpp"
 
 #include <cstring>  // strlen
 #include <fstream>
 #include <string>
-#include <vector>
+
 
 namespace file_data_test
 {
@@ -220,6 +217,35 @@ UNIT_TEST(EmptyFile)
   TEST(DeleteFileX(copy), ());
 }
 
+UNIT_TEST(RenameOnExistingFile)
+{
+  using namespace base;
+
+  std::string const name = "test.empty";
+  std::string const copy = "test.empty.copy";
+  {
+    FileData f(name, FileData::Op::WRITE_TRUNCATE);
+    uint8_t const x = 1;
+    f.Write(&x, 1);
+  }
+  {
+    FileData f(copy, FileData::Op::WRITE_TRUNCATE);
+    uint8_t const x = 2;
+    f.Write(&x, 1);
+  }
+
+  TEST(RenameFileX(name, copy), ());
+
+  {
+    FileData f(copy, FileData::Op::READ);
+    uint8_t x;
+    f.Read(0, &x, 1);
+    TEST_EQUAL(x, 1, ());
+  }
+
+  TEST(DeleteFileX(copy), ());
+}
+
 // Made this 'obvious' test for getline. I had (or not?) behaviour when 'while (getline)' loop
 // didn't get last string in file without trailing '\n'.
 UNIT_TEST(File_StdGetLine)

topography_generator/generator.cpp

@@ -9,8 +9,6 @@
 
 #include "geometry/mercator.hpp"
 
-#include "base/scope_guard.hpp"
-#include "base/string_utils.hpp"
 #include "base/thread_pool_computational.hpp"
 
 #include <algorithm>
@@ -124,13 +122,11 @@ private:
         else if (lonDist >= 1.0)
           pos.m_lon -= kPointEqualityEps;
 
-        /// @todo Can't call GetTriangleHeight here and below because it breaks
+        return m_preferredTile->GetAltitude(pos);
-        /// ContoursBuilder::AddSegment level constraint. Should investigate deeper.
-        return m_preferredTile->GetHeight(pos);
       }
     }
 
-    return m_srtmManager.GetTile(pos).GetHeight(pos);
+    return m_srtmManager.GetAltitude(pos);
   }
 
   Altitude GetMedianValue(ms::LatLon const & pos)
@@ -185,7 +181,7 @@ public:
   {}
 
   /// @todo Should we use the same approach as in SrtmTile::GetTriangleHeight/GetBilinearHeight?
-  /// This function is used in ASTER fiter only.
+  /// This function is used in ASTER filter only.
   Altitude GetValue(ms::LatLon const & pos) override
   {
     double ln = pos.m_lon - m_leftLon;
@@ -304,8 +300,8 @@ private:
       }
     }
     auto const & pl = GetPlatform();
-    if (!pl.IsFileExistsByFullPath(base::JoinPath(m_srtmDir, tileName + ".hgt"))
+    if (!pl.IsFileExistsByFullPath(base::JoinPath(m_srtmDir, tileName + ".hgt")) &&
-        && !pl.IsFileExistsByFullPath(generator::SrtmTile::GetPath(m_srtmDir, tileName)))
+        !pl.IsFileExistsByFullPath(generator::SrtmTile::GetPath(m_srtmDir, tileName)))
     {
       LOG(LINFO, ("SRTM tile", tileName, "doesn't exist, skip processing."));
       return;
@@ -500,8 +496,9 @@ void Generator::GenerateIsolines(int left, int bottom, int right, int top,
 
 void Generator::GenerateIsolinesForCountries()
 {
-  if (!GetPlatform().IsFileExistsByFullPath(m_isolinesTilesOutDir) &&
+  auto const & pl = GetPlatform();
-      !GetPlatform().MkDirRecursively(m_isolinesTilesOutDir))
+  if (!pl.IsFileExistsByFullPath(m_isolinesTilesOutDir) &&
+      !pl.MkDirRecursively(m_isolinesTilesOutDir))
   {
     LOG(LERROR, ("Can't create directory", m_isolinesTilesOutDir));
     return;
@@ -515,8 +512,8 @@ void Generator::GenerateIsolinesForCountries()
       continue;
     checkedProfiles.insert(profileName);
     auto const profileTilesDir = GetTilesDir(m_isolinesTilesOutDir, profileName);
-    if (!GetPlatform().IsFileExistsByFullPath(profileTilesDir) &&
+    if (!pl.IsFileExistsByFullPath(profileTilesDir) &&
-        !GetPlatform().MkDirChecked(profileTilesDir))
+        !pl.MkDirChecked(profileTilesDir))
     {
       LOG(LERROR, ("Can't create directory", profileTilesDir));
       return;
@@ -526,7 +523,7 @@ void Generator::GenerateIsolinesForCountries()
   auto const tmpTileProfilesDir = GetTileProfilesDir(m_isolinesTilesOutDir);
 
   Platform::RmDirRecursively(tmpTileProfilesDir);
-  if (!GetPlatform().MkDirChecked(tmpTileProfilesDir))
+  if (!pl.MkDirChecked(tmpTileProfilesDir))
   {
     LOG(LERROR, ("Can't create directory", tmpTileProfilesDir));
     return;
@@ -540,7 +537,7 @@ void Generator::GenerateIsolinesForCountries()
 
     auto const countryFile = GetIsolinesFilePath(countryId, m_isolinesCountriesOutDir);
 
-    if (!m_forceRegenerate && GetPlatform().IsFileExistsByFullPath(countryFile))
+    if (!m_forceRegenerate && pl.IsFileExistsByFullPath(countryFile))
     {
       LOG(LINFO, ("Isolines for", countryId, "are ready, skip processing."));
       continue;

topography_generator/marching_squares/marching_squares.hpp

@@ -3,6 +3,7 @@
 #include "topography_generator/marching_squares/contours_builder.hpp"
 #include "topography_generator/marching_squares/square.hpp"
 #include "topography_generator/utils/contours.hpp"
+#include "topography_generator/utils/values_provider.hpp"
 
 #include "base/logging.hpp"
 
@@ -34,7 +35,9 @@ public:
 
   void GenerateContours(Contours<ValueType> & result)
   {
-    ScanValuesInRect(result.m_minValue, result.m_maxValue, result.m_invalidValuesCount);
+    std::vector<ValueType> grid((m_stepsCountLat + 1) * (m_stepsCountLon + 1));
+
+    ScanValuesInRect(result, grid);
     result.m_valueStep = m_valueStep;
 
     auto const levelsCount = static_cast<size_t>(result.m_maxValue - result.m_minValue) / m_valueStep;
@@ -45,60 +48,72 @@ public:
     }
 
     ContoursBuilder contoursBuilder(levelsCount, m_debugId);
+    Square<ValueType> square(result.m_minValue, m_valueStep, m_debugId);
 
     for (size_t i = 0; i < m_stepsCountLat; ++i)
     {
       contoursBuilder.BeginLine();
       for (size_t j = 0; j < m_stepsCountLon; ++j)
       {
-        auto const leftBottom = ms::LatLon(m_leftBottom.m_lat + m_step * i,
+        // This point should be calculated _exact_ the same way as in ScanValuesInRect.
-                                           m_leftBottom.m_lon + m_step * j);
+        // leftBottom + m_step doesn't work due to different floating results.
-        // Use std::min to prevent floating-point number precision error.
+
-        auto const rightTop = ms::LatLon(std::min(leftBottom.m_lat + m_step, m_rightTop.m_lat),
+        square.Init(
-                                         std::min(leftBottom.m_lon + m_step, m_rightTop.m_lon));
+            m_leftBottom.m_lon + m_step * j,        // Left
+            m_leftBottom.m_lat + m_step * i,        // Bottom
+            m_leftBottom.m_lon + m_step * (j + 1),  // Right
+            m_leftBottom.m_lat + m_step * (i + 1),  // Top
+
+            grid[Idx(i, j)],          // LB
+            grid[Idx(i, j + 1)],      // RB
+            grid[Idx(i + 1, j)],      // LT
+            grid[Idx(i + 1, j + 1)],  // RT
+
+            m_valuesProvider.GetInvalidValue());
 
-        Square<ValueType> square(leftBottom, rightTop, result.m_minValue, m_valueStep,
-                                 m_valuesProvider, m_debugId);
         square.GenerateSegments(contoursBuilder);
       }
-      auto const isLastLine = i == m_stepsCountLat - 1;
+
-      contoursBuilder.EndLine(isLastLine);
+      contoursBuilder.EndLine(i == m_stepsCountLat - 1 /* finalLine */);
     }
 
     contoursBuilder.GetContours(result.m_minValue, result.m_valueStep, result.m_contours);
   }
 
 private:
-  void ScanValuesInRect(ValueType & minValue, ValueType & maxValue, size_t & invalidValuesCount) const
+  size_t Idx(size_t iLat, size_t jLon) const { return iLat * (m_stepsCountLon + 1) + jLon; }
+
+  void ScanValuesInRect(Contours<ValueType> & res, std::vector<ValueType> & grid) const
   {
-    minValue = maxValue = m_valuesProvider.GetValue(m_leftBottom);
+    res.m_minValue = res.m_maxValue = m_valuesProvider.GetValue(m_leftBottom);
-    invalidValuesCount = 0;
+    res.m_invalidValuesCount = 0;
 
     for (size_t i = 0; i <= m_stepsCountLat; ++i)
     {
       for (size_t j = 0; j <= m_stepsCountLon; ++j)
       {
-        auto const pos = ms::LatLon(m_leftBottom.m_lat + m_step * i,
+        ms::LatLon const pos(m_leftBottom.m_lat + m_step * i, m_leftBottom.m_lon + m_step * j);
-                                    m_leftBottom.m_lon + m_step * j);
         auto const value = m_valuesProvider.GetValue(pos);
+        grid[Idx(i, j)] = value;
+
         if (value == m_valuesProvider.GetInvalidValue())
         {
-          ++invalidValuesCount;
+          ++res.m_invalidValuesCount;
           continue;
         }
-        if (value < minValue)
+        if (value < res.m_minValue)
-          minValue = value;
+          res.m_minValue = value;
-        if (value > maxValue)
+        if (value > res.m_maxValue)
-          maxValue = value;
+          res.m_maxValue = value;
       }
     }
 
-    if (invalidValuesCount > 0)
+    if (res.m_invalidValuesCount > 0)
-      LOG(LWARNING, ("Tile", m_debugId, "contains", invalidValuesCount, "invalid values."));
+      LOG(LWARNING, ("Tile", m_debugId, "contains", res.m_invalidValuesCount, "invalid values."));
 
-    Square<ValueType>::ToLevelsRange(m_valueStep, minValue, maxValue);
+    Square<ValueType>::ToLevelsRange(m_valueStep, res.m_minValue, res.m_maxValue);
 
-    CHECK_GREATER_OR_EQUAL(maxValue, minValue, (m_debugId));
+    CHECK_GREATER_OR_EQUAL(res.m_maxValue, res.m_minValue, (m_debugId));
   }
 
   ms::LatLon const m_leftBottom;

topography_generator/marching_squares/square.hpp

@@ -1,7 +1,7 @@
 #pragma once
 
 #include "topography_generator/marching_squares/contours_builder.hpp"
-#include "topography_generator/utils/values_provider.hpp"
+
 
 namespace topography_generator
 {
@@ -9,28 +9,31 @@ template <typename ValueType>
 class Square
 {
 public:
-  Square(ms::LatLon const & leftBottom,
+  Square(ValueType minValue, ValueType valueStep, std::string const & debugId)
-         ms::LatLon const & rightTop,
-         ValueType minValue, ValueType valueStep,
-         ValuesProvider<ValueType> & valuesProvider,
-         std::string const & debugId)
     : m_minValue(minValue)
     , m_valueStep(valueStep)
-    , m_left(leftBottom.m_lon)
-    , m_right(rightTop.m_lon)
-    , m_bottom(leftBottom.m_lat)
-    , m_top(rightTop.m_lat)
     , m_debugId(debugId)
   {
-    static_assert(std::is_integral<ValueType>::value, "Only integral types are supported.");
+    static_assert(std::is_integral<ValueType>::value && std::is_signed<ValueType>::value);
-
-    m_valueLB = GetValue(leftBottom, valuesProvider);
-    m_valueLT = GetValue(ms::LatLon(m_top, m_left), valuesProvider);
-    m_valueRT = GetValue(ms::LatLon(m_top, m_right), valuesProvider);
-    m_valueRB = GetValue(ms::LatLon(m_bottom, m_right), valuesProvider);
   }
 
-  void GenerateSegments(ContoursBuilder & builder)
+  void Init(double left, double bottom, double right, double top,
+            ValueType lb, ValueType rb, ValueType lt, ValueType rt, ValueType invalid)
+  {
+    m_isValid = true;
+
+    m_left = left;
+    m_bottom = bottom;
+    m_right = right;
+    m_top = top;
+
+    m_valueLB = GetValue(lb, invalid);
+    m_valueRB = GetValue(rb, invalid);
+    m_valueLT = GetValue(lt, invalid);
+    m_valueRT = GetValue(rt, invalid);
+  }
+
+  void GenerateSegments(ContoursBuilder & builder) const
   {
     if (!m_isValid)
       return;
@@ -70,14 +73,13 @@ private:
     Unclear,
   };
 
-  ValueType GetValue(ms::LatLon const & pos, ValuesProvider<ValueType> & valuesProvider)
+  ValueType GetValue(ValueType val, ValueType invalid)
   {
     // If a contour goes right through the corner of the square false segments can be generated.
     // Shift the value slightly from the corner.
-    ValueType val = valuesProvider.GetValue(pos);
+    if (val == invalid)
-    if (val == valuesProvider.GetInvalidValue())
     {
-      LOG(LWARNING, ("Invalid value at the position", pos, m_debugId));
+      //LOG(LWARNING, ("Invalid value at the position", pos, m_debugId));
       m_isValid = false;
       return val;
     }
@@ -87,7 +89,7 @@ private:
     return val;
   }
 
-  void AddSegments(ValueType val, uint16_t ind, ContoursBuilder & builder)
+  void AddSegments(ValueType val, uint16_t ind, ContoursBuilder & builder) const
   {
     // Segment is a vector directed so that higher values is on the right.
     static const std::pair<Rib, Rib> intersectedRibs[] =
@@ -159,7 +161,7 @@ private:
     }
   }
 
-  ms::LatLon InterpolatePoint(Square::Rib rib, ValueType val)
+  ms::LatLon InterpolatePoint(Square::Rib rib, ValueType val) const
   {
     double val1;
     double val2;
@@ -212,9 +214,6 @@ private:
     return {lat, lon};
   }
 
-  ValueType m_minValue;
-  ValueType m_valueStep;
-
   double m_left;
   double m_right;
   double m_bottom;
@@ -225,7 +224,10 @@ private:
   ValueType m_valueRT;
   ValueType m_valueRB;
 
-  bool m_isValid = true;
+  ValueType m_minValue;
+  ValueType m_valueStep;
   std::string m_debugId;
+
+  bool m_isValid;
 };
 }  // topography_generator

topography_generator/utils/contours_serdes.hpp

@@ -117,8 +117,8 @@ bool SaveContrours(std::string const & filePath,
     LOG(LWARNING, ("File writer exception raised:", ex.what(), ", file", tmpFilePath));
     return false;
   }
-  base::DeleteFileX(filePath);
+
-  VERIFY(base::RenameFileX(tmpFilePath, filePath), (tmpFilePath, filePath));
+  CHECK(base::RenameFileX(tmpFilePath, filePath), (tmpFilePath, filePath));
   return true;
 }