Fixed altitude tiles cache bug.

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

generator/altitude_generator.cpp

@@ -9,25 +9,20 @@
 #include "indexer/feature_processor.hpp"
 
 #include "coding/files_container.hpp"
-#include "coding/read_write_utils.hpp"
 #include "coding/reader.hpp"
 #include "coding/succinct_mapper.hpp"
-#include "coding/varint.hpp"
 
 #include "geometry/latlon.hpp"
 
 #include "base/assert.hpp"
 #include "base/checked_cast.hpp"
-#include "base/file_name_utils.hpp"
 #include "base/logging.hpp"
 #include "base/scope_guard.hpp"
 #include "base/stl_helpers.hpp"
-#include "base/string_utils.hpp"
 
 #include "defines.hpp"
 
 #include "3party/succinct/elias_fano.hpp"
-#include "3party/succinct/mapper.hpp"
 #include "3party/succinct/rs_bit_vector.hpp"
 
 namespace routing
@@ -57,7 +52,6 @@ class Processor
 public:
   struct FeatureAltitude
   {
-    FeatureAltitude() : m_featureId(0) {}
     FeatureAltitude(uint32_t featureId, geometry::Altitudes && altitudes)
       : m_featureId(featureId), m_altitudes(std::move(altitudes))
     {
@@ -72,13 +66,10 @@ public:
   {
   }
 
-  void operator()(FeatureType & f, uint32_t const & id)
+  void operator()(FeatureType & f, uint32_t id)
   {
-    if (id != m_altitudeAvailabilityBuilder.size())
+    CHECK_EQUAL(f.GetID().m_index, id, ());
-    {
+    CHECK_EQUAL(id, m_altitudeAvailabilityBuilder.size(), ());
-      LOG(LERROR, ("There's a gap in feature id order."));
-      return;
-    }
 
     bool hasAltitude = false;
     SCOPE_GUARD(altitudeAvailabilityBuilding,
@@ -96,9 +87,15 @@ public:
     Altitude minFeatureAltitude = geometry::kInvalidAltitude;
     for (size_t i = 0; i < pointsCount; ++i)
     {
-      Altitude const a = m_altitudeGetter.GetAltitude(f.GetPoint(i));
+      auto const & pt = f.GetPoint(i);
+      Altitude const a = m_altitudeGetter.GetAltitude(pt);
       if (a == geometry::kInvalidAltitude)
       {
+        // Print warning for missing altitude point (if not a ferry or so).
+        auto const type = CarModel::AllLimitsInstance().GetHighwayType(feature::TypesHolder(f));
+        if (type && *type != HighwayType::RouteFerry && *type != HighwayType::RouteShuttleTrain)
+          LOG(LWARNING, ("Invalid altitude at:", mercator::ToLatLon(pt)));
+
         // One invalid point invalidates the whole feature.
         return;
       }
@@ -122,12 +119,6 @@ public:
 
   bool HasAltitudeInfo() const { return !m_featureAltitudes.empty(); }
 
-  bool IsFeatureAltitudesSorted()
-  {
-    return base::IsSortedAndUnique(m_featureAltitudes.begin(), m_featureAltitudes.end(),
-                                   base::LessBy(&Processor::FeatureAltitude::m_featureId));
-  }
-
 public:
   std::vector<FeatureAltitude> m_featureAltitudes;
   succinct::bit_vector_builder m_altitudeAvailabilityBuilder;
@@ -148,12 +139,11 @@ void BuildRoadAltitudes(std::string const & mwmPath, AltitudeGetter & altitudeGe
 
     if (!processor.HasAltitudeInfo())
     {
-      LOG(LINFO, ("No altitude information for road features of mwm:", mwmPath));
+      // Possible for small islands like Bouvet or Willis.
+      LOG(LWARNING, ("No altitude information for road features of mwm:", mwmPath));
       return;
     }
 
-    CHECK(processor.IsFeatureAltitudesSorted(), ());
-
     FilesContainerW cont(mwmPath, FileWriter::OP_WRITE_EXISTING);
     auto w = cont.GetWriter(ALTITUDES_FILE_TAG);
 

generator/generator_tests/srtm_parser_test.cpp

@@ -2,6 +2,8 @@
 
 #include "generator/srtm_parser.hpp"
 
+#include "coding/endianness.hpp"
+
 #include <iostream>
 
 namespace srtm_parser_test
@@ -14,6 +16,11 @@ inline std::string GetBase(ms::LatLon const & coord)
   return SrtmTile::GetBase(coord);
 }
 
+inline SrtmTile::LatLonKey GetKey(ms::LatLon const & coord)
+{
+  return SrtmTile::GetKey(coord);
+}
+
 UNIT_TEST(SRTM_FilenameTest)
 {
   auto name = GetBase({56.4566, 37.3467});
@@ -22,18 +29,34 @@ UNIT_TEST(SRTM_FilenameTest)
   name = GetBase({34.077433, -118.304569});
   TEST_EQUAL(name, "N34W119", ());
 
+  name = GetBase({1.0, 1.0});
+  TEST_EQUAL(name, "N01E001", ());
+
   name = GetBase({0.1, 0.1});
   TEST_EQUAL(name, "N00E000", ());
 
+  TEST_NOT_EQUAL(GetKey({0.1, 0.1}), GetKey({1.0, 1.0}), ());
+
+  name = GetBase({-0.1, -0.1});
+  TEST_EQUAL(name, "S01W001", ());
+
+  TEST_NOT_EQUAL(GetKey({0.1, 0.1}), GetKey({-0.1, -0.1}), ());
+
   name = GetBase({-0.9, -0.9});
   TEST_EQUAL(name, "S01W001", ());
 
+  TEST_EQUAL(GetKey({-0.9, -0.9}), GetKey({-0.1, -0.1}), ());
+
   name = GetBase({-1.0, -1.0});
   TEST_EQUAL(name, "S01W001", ());
 
+  TEST_EQUAL(GetKey({-0.9, -0.9}), GetKey({-1.0, -1.0}), ());
+
   name = GetBase({-1.9, -1.1});
   TEST_EQUAL(name, "S02W002", ());
 
+  TEST_NOT_EQUAL(GetKey({-1.1, -1.1}), GetKey({-1.0, -1.0}), ());
+
   name = GetBase({-35.35, -12.1});
   TEST_EQUAL(name, "S36W013", ());
 

generator/srtm_parser.cpp

@@ -16,7 +16,7 @@ namespace generator
 {
 namespace
 {
-size_t constexpr kArcSecondsInDegree = 60 * 60;
+int constexpr kArcSecondsInDegree = 60 * 60;
 size_t constexpr kSrtmTileSize = (kArcSecondsInDegree + 1) * (kArcSecondsInDegree + 1) * 2;
 
 struct UnzipMemDelegate : public ZipFileReader::Delegate
@@ -191,39 +191,41 @@ std::string SrtmTile::GetPath(std::string const & dir, std::string const & base)
 }
 
 // static
-ms::LatLon SrtmTile::GetCenter(ms::LatLon const & coord)
+SrtmTile::LatLonKey SrtmTile::GetKey(ms::LatLon const & coord)
 {
-  return {floor(coord.m_lat) + 0.5, floor(coord.m_lon) + 0.5};
+  ms::LatLon center{floor(coord.m_lat) + 0.5, floor(coord.m_lon) + 0.5};
+  if (coord.m_lat < 0)
+    center.m_lat -= 1.0;
+  if (coord.m_lon < 0)
+    center.m_lon -= 1.0;
+
+  return {static_cast<int32_t>(center.m_lat), static_cast<int32_t>(center.m_lon)};
 }
 
 // static
 std::string SrtmTile::GetBase(ms::LatLon const & coord)
 {
-  auto center = GetCenter(coord);
+  auto key = GetKey(coord);
   std::ostringstream ss;
-  if (center.m_lat < 0)
+  if (coord.m_lat < 0)
   {
     ss << "S";
-    center.m_lat *= -1;
+    key.first = -key.first;
-    center.m_lat += 1;
   }
   else
-  {
     ss << "N";
-  }
-  ss << std::setw(2) << std::setfill('0') << static_cast<int>(center.m_lat);
 
-  if (center.m_lon < 0)
+  ss << std::setw(2) << std::setfill('0') << key.first;
+
+  if (coord.m_lon < 0)
   {
     ss << "W";
-    center.m_lon *= -1;
+    key.second = -key.second;
-    center.m_lon += 1;
   }
   else
-  {
     ss << "E";
-  }
+
-  ss << std::setw(3) << static_cast<int>(center.m_lon);
+  ss << std::setw(3) << key.second;
   return ss.str();
 }
 
@@ -245,7 +247,7 @@ void SrtmTile::Invalidate()
 // SrtmTileManager ---------------------------------------------------------------------------------
 SrtmTile const & SrtmTileManager::GetTile(ms::LatLon const & coord)
 {
-  auto res = m_tiles.emplace(GetKey(coord), SrtmTile());
+  auto res = m_tiles.emplace(SrtmTile::GetKey(coord), SrtmTile());
   if (res.second)
   {
     try
@@ -261,13 +263,6 @@ SrtmTile const & SrtmTileManager::GetTile(ms::LatLon const & coord)
   return res.first->second;
 }
 
-// static
-SrtmTileManager::LatLonKey SrtmTileManager::GetKey(ms::LatLon const & coord)
-{
-  auto const tileCenter = SrtmTile::GetCenter(coord);
-  return {static_cast<int32_t>(tileCenter.m_lat), static_cast<int32_t>(tileCenter.m_lon)};
-}
-
 void SrtmTileManager::Purge()
 {
   MapT().swap(m_tiles);

generator/srtm_parser.hpp

@@ -2,15 +2,18 @@
 
 #include "geometry/latlon.hpp"
 
-#include "indexer/feature_altitude.hpp"
-
 #include "geometry/point_with_altitude.hpp"
 
 #include "base/macros.hpp"
 
+#include <boost/container_hash/hash.hpp>
+
 #include <cstdint>
 #include <string>
 #include <unordered_map>
+#include <utility>
+#include <vector>
+
 
 namespace generator
 {
@@ -31,8 +34,10 @@ public:
   geometry::Altitude GetTriangleHeight(ms::LatLon const & coord) const;
   /// @}
 
+  using LatLonKey = std::pair<int32_t, int32_t>;
+  static LatLonKey GetKey(ms::LatLon const & coord);
+
   static std::string GetBase(ms::LatLon const & coord);
-  static ms::LatLon GetCenter(ms::LatLon const & coord);
   static std::string GetPath(std::string const & dir, std::string const & base);
 
   /// Used in unit tests only to prepare mock tile.
@@ -76,20 +81,20 @@ public:
   void Purge();
 
 private:
-  using LatLonKey = std::pair<int32_t, int32_t>;
-  static LatLonKey GetKey(ms::LatLon const & coord);
-
   std::string m_dir;
 
   struct Hash
   {
-    size_t operator()(LatLonKey const & key) const
+    size_t operator()(SrtmTile::LatLonKey const & key) const
     {
-      return (static_cast<size_t>(key.first) << 32u) | static_cast<size_t>(key.second);
+      size_t seed = 0;
+      boost::hash_combine(seed, key.first);
+      boost::hash_combine(seed, key.second);
+      return seed;
     }
   };
 
-  using MapT = std::unordered_map<LatLonKey, SrtmTile, Hash>;
+  using MapT = std::unordered_map<SrtmTile::LatLonKey, SrtmTile, Hash>;
   MapT m_tiles;
 
   DISALLOW_COPY(SrtmTileManager);