[routing] Section format was changed. Using delta coding for storing feature id in bidirectional case. Review fixes.

generator/camera_node_processor.hpp

@@ -67,7 +67,7 @@ private:
   /// \brief Gets text with speed, returns formatted speed string in km per hour.
   /// \param maxSpeedString - text with speed. Possible format:
   ///                         "130" - means 130 km per hour.
-  ///                         "130 mph" - means 130 mile per hour.
+  ///                         "130 mph" - means 130 miles per hour.
   ///                         "130 kmh" - means 130 km per hour.
   /// See https://wiki.openstreetmap.org/wiki/Key:maxspeed
   /// for more details about input string.

generator/generator_tests/maxspeed_tests.cpp

@@ -80,7 +80,7 @@ void TestMaxspeedSection(Features const & roads, string const & maxspeedCsvConte
 
   // Creating maxspeed section in test.mwm.
   string const testMwmFullPath = base::JoinPath(testDirFullPath, testMwm);
-  BuildMaxspeed(testMwmFullPath, featureIdToOsmId, base::JoinPath(testDirFullPath, kCsv));
+  BuildMaxspeedSection(testMwmFullPath, featureIdToOsmId, base::JoinPath(testDirFullPath, kCsv));
 
   // Loading maxspeed section.
   FrozenDataSource dataSource;
@@ -137,7 +137,6 @@ UNIT_TEST(ParseMaxspeeds_Smoke)
 
 UNIT_TEST(ParseMaxspeeds1)
 {
-//  string const maxspeedCsvContent = "10,Metric,60\n11,Metric,90\n";
   string const maxspeedCsvContent = R"(10,Metric,60
                                        11,Metric,90)";
   OsmIdToMaxspeed const expectedMapping = {
@@ -150,7 +149,6 @@ UNIT_TEST(ParseMaxspeeds1)
 
 UNIT_TEST(ParseMaxspeeds2)
 {
-//  string const maxspeedCsvContent = "10,Metric,60,80\n11,Metric,120\n";
   string const maxspeedCsvContent = R"(10,Metric,60,80
                                        11,Metric,120)";
   OsmIdToMaxspeed const expectedMapping = {
@@ -163,7 +161,6 @@ UNIT_TEST(ParseMaxspeeds2)
 
 UNIT_TEST(ParseMaxspeeds3)
 {
-//  string const maxspeedCsvContent = "184467440737095516,Imperial,60,80\n184467440737095517,Metric,120\n";
   string const maxspeedCsvContent = R"(184467440737095516,Imperial,60,80
                                        184467440737095517,Metric,120)";
 

generator/maxspeed_builder.cpp

@@ -171,8 +171,9 @@ void SerializeMaxspeeds(string const & dataPath, vector<FeatureMaxspeed> && spee
   LOG(LINFO, ("SerializeMaxspeeds(", dataPath, ", ...) serialized:", speeds.size(), "maxspeed tags."));
 }
 
-void BuildMaxspeed(string const & dataPath, map<uint32_t, base::GeoObjectId> const & featureIdToOsmId,
-                   string const & maxspeedFilename)
+void BuildMaxspeedSection(string const & dataPath,
+                          map<uint32_t, base::GeoObjectId> const & featureIdToOsmId,
+                          string const & maxspeedFilename)
 {
   MaxspeedMwmCollector collector(dataPath, featureIdToOsmId, maxspeedFilename);
   SerializeMaxspeeds(dataPath, collector.StealMaxspeeds());
@@ -185,6 +186,6 @@ void BuildMaxspeedSection(string const & dataPath, string const & osmToFeaturePa
 
   map<uint32_t, base::GeoObjectId> featureIdToOsmId;
   CHECK(ParseFeatureIdToOsmIdMapping(osmToFeaturePath, featureIdToOsmId), ());
-  BuildMaxspeed(dataPath, featureIdToOsmId, maxspeedFilename);
+  BuildMaxspeedSection(dataPath, featureIdToOsmId, maxspeedFilename);
 }
 }  // namespace routing

generator/maxspeed_builder.hpp

@@ -22,12 +22,12 @@ bool ParseMaxspeeds(std::string const & maxspeedFilename, OsmIdToMaxspeed & osmI
 /// \brief Writes |speeds| to maxspeed section to mwm with |dataPath|.
 void SerializeMaxspeeds(std::string const & dataPath, std::vector<FeatureMaxspeed> && speeds);
 
-void BuildMaxspeed(std::string const & dataPath,
-                   std::map<uint32_t, base::GeoObjectId> const & featureIdToOsmId,
-                   std::string const & maxspeedFilename);
+void BuildMaxspeedSection(std::string const & dataPath,
+                          std::map<uint32_t, base::GeoObjectId> const & featureIdToOsmId,
+                          std::string const & maxspeedFilename);
 
 /// \brief Builds maxspeed section in mwm with |dataPath|. This section contains max speed limits
-/// if it's available.
+/// if they are available in file |maxspeedFilename|.
 /// \param maxspeedFilename file name to csv file with maxspeed tag values.
 /// \note To start building the section, the following data must be ready:
 /// 1. GenerateIntermediateData(). Saves to a file data about maxspeed tags value of road features

generator/maxspeed_parser.hpp

@@ -30,6 +30,6 @@ bool ParseMaxspeedTag(std::string const & maxspeedValue, routing::SpeedInUnits &
 std::string UnitsToString(measurement_utils::Units units);
 
 /// \brief Converts string to measurement_utils::Units.
-/// \note |units| should be equale to "Metric" or "Imperial".
+/// \note |units| should be "Metric" or "Imperial".
 measurement_utils::Units StringToUnits(std::string const & units);
 }  // namespace generator

routing/maxspeeds_serialization.hpp

@@ -96,6 +96,7 @@ public:
     header.m_bidirectionalMaxspeedOffset = static_cast<uint32_t>(sink.Pos() - startOffset);
 
     // Keeping bidirectional maxspeeds.
+    uint32_t prevFeatureId = 0;
     for (auto const & s : speeds)
     {
       if (!s.IsBidirectional())
@@ -107,7 +108,15 @@ public:
         continue; // No appropriate speed for |s| in SpeedMacro enum.
 
       ++header.m_bidirectionalMaxspeedNumber;
-      WriteToSink(sink, s.GetFeatureId());
+      // Note. |speeds| sorted by feature ids and they are unique. So the first item in |speed|
+      // has feature id 0 or greater. And the second item in |speed| has feature id grater than 0.
+      // This guarantees that for only first saved |s| the if branch below is called.
+      if (prevFeatureId != 0)
+        CHECK_GREATER(s.GetFeatureId(), prevFeatureId, ());
+      uint32_t delta = (prevFeatureId == 0 ? s.GetFeatureId() : s.GetFeatureId() - prevFeatureId);
+      prevFeatureId = s.GetFeatureId();
+
+      WriteToSink(sink, delta);
       WriteToSink(sink, static_cast<uint8_t>(forwardMacro));
       WriteToSink(sink, static_cast<uint8_t>(backwardMacro));
     }
@@ -120,13 +129,23 @@ public:
     LOG(LINFO, ("Serialized", serializedForwardNumber, "forward maxspeeds and",
                 header.m_bidirectionalMaxspeedNumber,
                 "bidirectional maxspeeds. Section size:", endOffset - startOffset, "bytes."));
+
+    size_t constexpr kBidirectionalMaxspeedLimit = 2000;
+    if (header.m_bidirectionalMaxspeedNumber > kBidirectionalMaxspeedLimit)
+    {
+      LOG(LWARNING,
+          ("There's to many bidirectional maxpeed tags:", header.m_bidirectionalMaxspeedNumber,
+           ". All maxpeed tags are serialized correctly but they may take too large space in mwm. "
+           "Consider about changing section format to keep bidirectional maxspeeds in another "
+           "way."));
+    }
   }
 
   template <class Source>
   static void Deserialize(Source & src, Maxspeeds & maxspeeds)
   {
     // Note. Now it's assumed that only little-endian architectures are supported.
-    // (See a check at the beginning of main method in generator_tools.cpp) If it's necessary
+    // (See a check at the beginning of main method in generator_tool.cpp) If it's necessary
     // to support big-endian architectures code below should be modified.
     CHECK(maxspeeds.IsEmpty(), ());
 
@@ -151,9 +170,10 @@ public:
     }
 
     maxspeeds.m_bidirectionalMaxspeeds.reserve(header.m_bidirectionalMaxspeedNumber);
+    uint32_t prevFeatureId = 0;
     for (size_t i = 0; i < header.m_bidirectionalMaxspeedNumber; ++i)
     {
-      auto const fid = ReadPrimitiveFromSource<uint32_t>(src);
+      auto const delta = ReadPrimitiveFromSource<uint32_t>(src);
       auto const forward = ReadPrimitiveFromSource<uint8_t>(src);
       auto const backward = ReadPrimitiveFromSource<uint8_t>(src);
       CHECK(GetMaxspeedConverter().IsValidMacro(forward), (i));
@@ -173,6 +193,8 @@ public:
       // Note. If neither |forwardSpeed| nor |backwardSpeed| are numeric it means
       // both of them have value "walk" or "none". So the units are not relevant for this case.
 
+      prevFeatureId += delta;
+      uint32_t fid = (i == 0 ? delta : prevFeatureId);
       maxspeeds.m_bidirectionalMaxspeeds.emplace_back(fid, units, forwardSpeed.GetSpeed(),
                                                       backwardSpeed.GetSpeed());
     }

routing_common/car_model.cpp

@@ -240,8 +240,8 @@ SpeedKMpH CarModel::GetSpeed(FeatureType & f, SpeedParams const & speedParams) c
   // Note. It's the first rough attempt using maxspeed tag value for speed calculation.
   // It's used as a feature speed if it's valid and less then some value.
   // @TODO maxspeed tag value should be used more sophisticated.
-  uint16_t const maxspeedBasedspeedKmPH = speedParams.m_maxspeed.GetSpeedKmPH(speedParams.m_forward);
-  auto const speedKmPH = min(static_cast<double>(maxspeedBasedspeedKmPH), GetMaxWeightSpeed());
+  uint16_t const maxspeedBasedSpeedKmPH = speedParams.m_maxspeed.GetSpeedKmPH(speedParams.m_forward);
+  auto const speedKmPH = min(static_cast<double>(maxspeedBasedSpeedKmPH), GetMaxWeightSpeed());
   return {speedKmPH /* weight */, speedKmPH /* eta */};
 }
 

routing_common/maxspeed_conversion.hpp

@@ -10,7 +10,7 @@
 
 namespace routing
 {
-/// \brief This enum class contains most popular maxspeed value according to
+/// \brief This enum class contains the most popular maxspeed values according to
 /// https://taginfo.openstreetmap.org/keys/maxspeed#values.
 /// \note Value of this enum is saved to mwm. So they should not be changed because of backward
 /// compatibility. But it's possible to add some new values to this enum.