Merge pull request #3898 from bykoianko/master-correct-working-on-maps-without-altitude-section

Bugfix. Building pedestrian and bicycle routes with maps without altitude section.

3 files changed, 82 insertions, 39 deletions

diff --git a/generator/generator_tests/altitude_test.cpp b/generator/generator_tests/altitude_test.cpp
index af95ff2481..0138edea7f 100644
--- a/generator/generator_tests/altitude_test.cpp
+++ b/generator/generator_tests/altitude_test.cpp
@@ -69,7 +69,24 @@ class MockAltitudeGetter : public AltitudeGetter
 public:
   using TMockAltitudes = map<m2::PointI, TAltitude>;
 
-  MockAltitudeGetter(TMockAltitudes && altitudes) : m_altitudes(altitudes) {}
+  MockAltitudeGetter(vector<TPoint3DList> const & roads)
+  {
+    for (TPoint3DList const & geom3D : roads)
+    {
+      for (auto const & g : geom3D)
+      {
+        auto const it = m_altitudes.find(g.m_point);
+        if (it != m_altitudes.cend())
+        {
+          CHECK_EQUAL(it->second, g.m_altitude,
+                      ("Point", it->first, "is set with two different altitudes."));
+          continue;
+        }
+        m_altitudes[g.m_point] = g.m_altitude;
+      }
+    }
+  }
+
   // AltitudeGetter overrides:
   TAltitude GetAltitude(m2::PointD const & p) override
   {
@@ -82,7 +99,17 @@ public:
   }
 
 private:
-  TMockAltitudes const & m_altitudes;
+
+  TMockAltitudes m_altitudes;
+};
+
+class MockNoAltitudeGetter : public AltitudeGetter
+{
+public:
+  TAltitude GetAltitude(m2::PointD const &) override
+  {
+    return kInvalidAltitude;
+  }
 };
 
 vector<m2::PointD> ExtractPoints(TPoint3DList const & geom3D)
@@ -93,25 +120,6 @@ vector<m2::PointD> ExtractPoints(TPoint3DList const & geom3D)
   return result;
 }
 
-void FillAltitudes(vector<TPoint3DList> const & roads,
-                   MockAltitudeGetter::TMockAltitudes & altitudes)
-{
-  for (TPoint3DList const & geom3D : roads)
-  {
-    for (size_t i = 0; i < geom3D.size(); ++i)
-    {
-      auto it = altitudes.find(geom3D[i].m_point);
-      if (it != altitudes.end())
-      {
-        CHECK_EQUAL(it->second, geom3D[i].m_altitude,
-                    ("Point", it->first, "is set with two different altitudes."));
-        continue;
-      }
-      altitudes[geom3D[i].m_point] = geom3D[i].m_altitude;
-    }
-  }
-}
-
 void BuildMwmWithoutAltitudes(vector<TPoint3DList> const & roads, LocalCountryFile & country)
 {
   generator::tests_support::TestMwmBuilder builder(country, feature::DataHeader::country);
@@ -121,11 +129,13 @@ void BuildMwmWithoutAltitudes(vector<TPoint3DList> const & roads, LocalCountryFi
 }
 
 void TestAltitudes(MwmValue const & mwmValue, string const & mwmPath,
-                   MockAltitudeGetter & altitudeGetter)
+                   bool hasAltitudeExpected, AltitudeGetter & expectedAltitudes)
 {
   AltitudeLoader loader(mwmValue);
+  TEST_EQUAL(loader.HasAltitudes(), hasAltitudeExpected, ());
 
-  auto processor = [&altitudeGetter, &loader](FeatureType const & f, uint32_t const & id) {
+  auto processor = [&expectedAltitudes, &loader](FeatureType const & f, uint32_t const & id)
+  {
     f.ParseGeometry(FeatureType::BEST_GEOMETRY);
     size_t const pointsCount = f.GetPointsCount();
     TAltitudes const altitudes = loader.GetAltitudes(id, pointsCount);
@@ -139,12 +149,17 @@ void TestAltitudes(MwmValue const & mwmValue, string const & mwmPath,
     TEST_EQUAL(altitudes.size(), pointsCount, ());
 
     for (size_t i = 0; i < pointsCount; ++i)
-      TEST_EQUAL(altitudeGetter.GetAltitude(f.GetPoint(i)), altitudes[i], ("A wrong altitude"));
+    {
+      TAltitude const fromGetter = expectedAltitudes.GetAltitude(f.GetPoint(i));
+      TAltitude const expected = (fromGetter == kInvalidAltitude ? kDefaultAltitudeMeters : fromGetter);
+      TEST_EQUAL(expected, altitudes[i], ("A wrong altitude"));
+    }
   };
   feature::ForEachFromDat(mwmPath, processor);
 }
 
-void TestAltitudesBuilding(vector<TPoint3DList> const & roads)
+void TestAltitudesBuilding(vector<TPoint3DList> const & roads, bool hasAltitudeExpected,
+                           AltitudeGetter & altitudeGetter)
 {
   classificator::Load();
   Platform & platform = GetPlatform();
@@ -156,11 +171,6 @@ void TestAltitudesBuilding(vector<TPoint3DList> const & roads)
   platform::tests_support::ScopedFile testScopedMwm(country.GetPath(MapOptions::Map));
   BuildMwmWithoutAltitudes(roads, country);
 
-  // Creating MockAltitudeGetter.
-  MockAltitudeGetter::TMockAltitudes altitudes;
-  FillAltitudes(roads, altitudes);
-  MockAltitudeGetter altitudeGetter(move(altitudes));
-
   // Adding altitude section to mwm.
   string const mwmPath = my::JoinFoldersToPath(testDirFullPath, kTestMwm + DATA_FILE_EXTENSION);
   BuildRoadAltitudes(mwmPath, altitudeGetter);
@@ -173,42 +183,72 @@ void TestAltitudesBuilding(vector<TPoint3DList> const & roads)
   MwmSet::MwmHandle mwmHandle = index.GetMwmHandleById(regResult.first);
   TEST(mwmHandle.IsAlive(), ());
 
-  TestAltitudes(*mwmHandle.GetValue<MwmValue>(), mwmPath, altitudeGetter);
+  TestAltitudes(*mwmHandle.GetValue<MwmValue>(), mwmPath, hasAltitudeExpected, altitudeGetter);
+}
+
+void TestBuildingAllFeaturesHaveAltitude(vector<TPoint3DList> const & roads, bool hasAltitudeExpected)
+{
+  MockAltitudeGetter altitudeGetter(roads);
+  TestAltitudesBuilding(roads, hasAltitudeExpected, altitudeGetter);
+}
+
+void TestBuildingNoFeatureHasAltitude(vector<TPoint3DList> const & roads, bool hasAltitudeExpected)
+{
+  MockNoAltitudeGetter altitudeGetter;
+  TestAltitudesBuilding(roads, hasAltitudeExpected, altitudeGetter);
 }
 
 UNIT_TEST(AltitudeGenerationTest_ZeroFeatures)
 {
   vector<TPoint3DList> const roads = {};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, false /* hasAltitudeExpected */);
 }
 
 UNIT_TEST(AltitudeGenerationTest_OneRoad)
 {
   vector<TPoint3DList> const roads = {kRoad1};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, true /* hasAltitudeExpected */);
 }
 
 UNIT_TEST(AltitudeGenerationTest_TwoConnectedRoads)
 {
   vector<TPoint3DList> const roads = {kRoad1, kRoad2};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, true /* hasAltitudeExpected */);
 }
 
 UNIT_TEST(AltitudeGenerationTest_TwoDisconnectedRoads)
 {
   vector<TPoint3DList> const roads = {kRoad1, kRoad3};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, true /* hasAltitudeExpected */);
 }
 
 UNIT_TEST(AltitudeGenerationTest_ThreeRoads)
 {
   vector<TPoint3DList> const roads = {kRoad1, kRoad2, kRoad3};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, true /* hasAltitudeExpected */);
 }
 
 UNIT_TEST(AltitudeGenerationTest_FourRoads)
 {
   vector<TPoint3DList> const roads = {kRoad1, kRoad2, kRoad3, kRoad4};
-  TestAltitudesBuilding(roads);
+  TestBuildingAllFeaturesHaveAltitude(roads, true /* hasAltitudeExpected */);
+}
+
+UNIT_TEST(AltitudeGenerationTest_ZeroFeaturesWithoutAltitude)
+{
+  vector<TPoint3DList> const roads = {};
+  TestBuildingNoFeatureHasAltitude(roads, false /* hasAltitudeExpected */);
+}
+
+UNIT_TEST(AltitudeGenerationTest_OneRoadWithoutAltitude)
+{
+  vector<TPoint3DList> const roads = {kRoad1};
+  TestBuildingNoFeatureHasAltitude(roads, false /* hasAltitudeExpected */);
+}
+
+UNIT_TEST(AltitudeGenerationTest_FourRoadsWithoutAltitude)
+{
+  vector<TPoint3DList> const roads = {kRoad1, kRoad2, kRoad3, kRoad4};
+  TestBuildingNoFeatureHasAltitude(roads, false /* hasAltitudeExpected */);
 }
 }  // namespace
diff --git a/indexer/altitude_loader.cpp b/indexer/altitude_loader.cpp
index f253056543..a794c96537 100644
--- a/indexer/altitude_loader.cpp
+++ b/indexer/altitude_loader.cpp
@@ -54,7 +54,10 @@ AltitudeLoader::AltitudeLoader(MwmValue const & mwmValue)
   }
 }
 
-bool AltitudeLoader::HasAltitudes() const { return m_header.m_minAltitude != kInvalidAltitude; }
+bool AltitudeLoader::HasAltitudes() const
+{
+  return m_reader != nullptr && m_header.m_minAltitude != kInvalidAltitude;
+}
 
 TAltitudes const & AltitudeLoader::GetAltitudes(uint32_t featureId, size_t pointCount)
 {
diff --git a/indexer/altitude_loader.hpp b/indexer/altitude_loader.hpp
index bcca0c893e..ff76fd9ba9 100644
--- a/indexer/altitude_loader.hpp
+++ b/indexer/altitude_loader.hpp
@@ -20,9 +20,9 @@ public:
   /// or the returned vector is empty.
   TAltitudes const & GetAltitudes(uint32_t featureId, size_t pointCount);
 
-private:
   bool HasAltitudes() const;
 
+private:
   unique_ptr<CopiedMemoryRegion> m_altitudeAvailabilityRegion;
   unique_ptr<CopiedMemoryRegion> m_featureTableRegion;