Merge pull request #3466 from bykoianko/master-no-bicycle-and-surface-type-support

[bicycle routing] Using attr bicycle=yes or no and oneway:bicycle for bicycle routing.

data/classificator.txt

@@ -363,7 +363,7 @@ world  +
         wayside_shrine  -
         {}
     hwtag  +
-        bicycle_bidir  -
+        bidir_bicycle  -
         lit  -
         nobicycle  -
         nofoot  -

data/mapcss-mapping.csv

@@ -1113,7 +1113,7 @@ sponsored;[sponsored];;name;int_name;1112;
 sponsored|booking;1113;
 hwtag|nobicycle;1114;
 hwtag|yesbicycle;1115;
-hwtag|bicycle_bidir;1116;
+hwtag|bidir_bicycle;1116;
 psurface|paved_good;1117;
 psurface|paved_bad;1118;
 psurface|unpaved_good;1119;

data/types.txt

@@ -1113,7 +1113,7 @@ sponsored
 sponsored|booking
 hwtag|nobicycle
 hwtag|yesbicycle
-hwtag|bicycle_bidir
+hwtag|bidir_bicycle
 psurface|paved_good
 psurface|paved_bad
 psurface|unpaved_good

data/visibility.txt

@@ -363,7 +363,7 @@ world  00000000000000000000  +
         wayside_shrine  00000000000000000111  -
         {}
     hwtag  00000000000000000000  +
-        bicycle_bidir  00000000000000000000  -
+        bidir_bicycle  00000000000000000000  -
         lit  00000000000000000000  -
         nobicycle  00000000000000000000  -
         nofoot  00000000000000000000  -

generator/feature_builder.cpp

@@ -232,8 +232,8 @@ bool FeatureBuilder1::IsRoad() const
 {
   static routing::PedestrianModel const pedModel;
   static routing::BicycleModel const bicModel;
-  return routing::CarModel::Instance().IsRoad(m_params.m_Types) || pedModel.IsRoad(m_params.m_Types)
-      || bicModel.IsRoad(m_params.m_Types);
+  return routing::CarModel::Instance().HasRoadType(m_params.m_Types) ||
+         pedModel.HasRoadType(m_params.m_Types) || bicModel.HasRoadType(m_params.m_Types);
 }
 
 bool FeatureBuilder1::PreSerialize()

generator/generator_tests/osm_type_test.cpp

@@ -507,7 +507,7 @@ UNIT_TEST(OsmType_Hwtag)
 {
   char const * tags[][2] = {
       {"hwtag", "oneway"}, {"hwtag", "private"}, {"hwtag", "lit"}, {"hwtag", "nofoot"}, {"hwtag", "yesfoot"},
-      {"hwtag", "yesbicycle"}, {"hwtag", "bicycle_bidir"}
+      {"hwtag", "yesbicycle"}, {"hwtag", "bidir_bicycle"}
   };
 
   {
@@ -612,15 +612,15 @@ UNIT_TEST(OsmType_Ferry)
 
   uint32_t type = GetType({"highway", "primary", "bridge"});
   TEST(params.IsTypeExist(type), ());
-  TEST(carModel.IsRoad(type), ());
+  TEST(carModel.IsRoadType(type), ());
 
   type = GetType({"route", "ferry", "motorcar"});
   TEST(params.IsTypeExist(type), ());
-  TEST(carModel.IsRoad(type), ());
+  TEST(carModel.IsRoadType(type), ());
 
   type = GetType({"route", "ferry"});
   TEST(!params.IsTypeExist(type), ());
-  TEST(!carModel.IsRoad(type), ());
+  TEST(!carModel.IsRoadType(type), ());
 }
 
 UNIT_TEST(OsmType_Boundary)

generator/osm2type.cpp

@@ -228,7 +228,7 @@ namespace ftype
       {
         {"building", "address"}, {"hwtag", "oneway"}, {"hwtag", "private"},
         {"hwtag", "lit"}, {"hwtag", "nofoot"}, {"hwtag", "yesfoot"},
-        {"hwtag", "nobicycle"}, {"hwtag", "yesbicycle"}, {"hwtag", "bicycle_bidir"},
+        {"hwtag", "nobicycle"}, {"hwtag", "yesbicycle"}, {"hwtag", "bidir_bicycle"},
         {"psurface", "paved_good"}, {"psurface", "paved_bad"},
         {"psurface", "unpaved_good"}, {"psurface", "unpaved_bad"},
       };

indexer/index.hpp

@@ -58,7 +58,7 @@ public:
 class Index : public MwmSet
 {
 protected:
-  /// @name MwmSet overrides.
+  /// MwmSet overrides:
   //@{
   unique_ptr<MwmInfo> CreateInfo(platform::LocalCountryFile const & localFile) const override;
 

routing/bicycle_model.cpp

@@ -601,40 +601,41 @@ BicycleModel::BicycleModel(VehicleModel::InitListT const & speedLimits)
 
 void BicycleModel::Init()
 {
-  // @TODO(bykoianko) Uncomment line below what tags hwtag=nobicycle and hwtag=yesbicycle
-  // will be added to classificator.txt. (https://jira.mail.ru/browse/MAPSME-858)
-//  m_noBicycleType = classif().GetTypeByPath({ "hwtag", "nobicycle" });
-//  m_yesBicycleType = classif().GetTypeByPath({ "hwtag", "yesbicycle" });
+  initializer_list<char const *> hwtagYesBicycle = {"hwtag", "yesbicycle"};
 
-  initializer_list<char const *> arr[] =
-  {
-    { "route", "ferry" },
-    { "man_made", "pier" },
+  m_yesBicycleType = classif().GetTypeByPath(hwtagYesBicycle);
+  m_noBicycleType = classif().GetTypeByPath({"hwtag", "nobicycle"});
+  m_bidirBicycleType = classif().GetTypeByPath({"hwtag", "bidir_bicycle"});
+
+  initializer_list<char const *> arr[] = {
+      hwtagYesBicycle, {"route", "ferry"}, {"man_made", "pier"},
   };
 
   SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr));
 }
 
-bool BicycleModel::IsNoBicycle(feature::TypesHolder const & types) const
+IVehicleModel::RoadAvailability BicycleModel::GetRoadAvailability(feature::TypesHolder const & types) const
 {
-  return find(types.begin(), types.end(), m_noBicycleType) != types.end();
+  if (types.Has(m_yesBicycleType))
+    return RoadAvailability::Available;
+  if (types.Has(m_noBicycleType))
+    return RoadAvailability::NotAvailable;
+  return RoadAvailability::Unknown;
 }
 
-bool BicycleModel::IsYesBicycle(feature::TypesHolder const & types) const
+bool BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const
 {
-  return find(types.begin(), types.end(), m_yesBicycleType) != types.end();
+  return types.Has(m_bidirBicycleType);
 }
 
-double BicycleModel::GetSpeed(FeatureType const & f) const
+bool BicycleModel::IsOneWay(FeatureType const & f) const
 {
-  feature::TypesHolder types(f);
+  feature::TypesHolder const types(f);
 
-  if (IsYesBicycle(types))
-    return VehicleModel::GetMaxSpeed();
-  if (!IsNoBicycle(types) && IsRoad(types))
-    return VehicleModel::GetSpeed(types);
+  if (IsBicycleBidir(types))
+    return false;
 
-  return 0.0;
+  return VehicleModel::IsOneWay(f);
 }
 
 BicycleModelFactory::BicycleModelFactory()

routing/bicycle_model.hpp

@@ -14,24 +14,21 @@ public:
   BicycleModel();
   BicycleModel(VehicleModel::InitListT const & speedLimits);
 
-  /// @name Overrides from VehicleModel.
-  //@{
-  double GetSpeed(FeatureType const & f) const override;
-  //@}
+  /// VehicleModel overrides:
+  bool IsOneWay(FeatureType const & f) const override;
+
+protected:
+  RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const override;
 
 private:
   void Init();
 
-  /// @return true if road is prohibited for bicycle,
-  /// but if function returns false, real prohibition is unknown.
-  bool IsNoBicycle(feature::TypesHolder const & types) const;
-
-  /// @return true if road is allowed for bicycle,
-  /// but if function returns false, real allowance is unknown.
-  bool IsYesBicycle(feature::TypesHolder const & types) const;
+  /// @return true if it is allowed to ride a bicycle in both directions.
+  bool IsBicycleBidir(feature::TypesHolder const & types) const;
 
   uint32_t m_noBicycleType = 0;
   uint32_t m_yesBicycleType = 0;
+  uint32_t m_bidirBicycleType = 0;
 };
 
 class BicycleModelFactory : public IVehicleModelFactory

routing/features_road_graph.cpp

@@ -60,6 +60,11 @@ bool FeaturesRoadGraph::CrossCountryVehicleModel::IsOneWay(FeatureType const & f
   return GetVehicleModel(f.GetID())->IsOneWay(f);
 }
 
+bool FeaturesRoadGraph::CrossCountryVehicleModel::IsRoad(FeatureType const & f) const
+{
+  return GetVehicleModel(f.GetID())->IsRoad(f);
+}
+
 IVehicleModel * FeaturesRoadGraph::CrossCountryVehicleModel::GetVehicleModel(FeatureID const & featureId) const
 {
   auto itr = m_cache.find(featureId.m_mwmId);
@@ -114,7 +119,7 @@ public:
 
   void operator()(FeatureType & ft)
   {
-    if (ft.GetFeatureType() != feature::GEOM_LINE)
+    if (!m_graph.IsRoad(ft))
       return;
 
     double const speedKMPH = m_graph.GetSpeedKMPHFromFt(ft);
@@ -167,7 +172,7 @@ void FeaturesRoadGraph::FindClosestEdges(m2::PointD const & point, uint32_t coun
 
   auto const f = [&finder, this](FeatureType & ft)
   {
-    if (ft.GetFeatureType() != feature::GEOM_LINE)
+    if (!m_vehicleModel.IsRoad(ft))
       return;
 
     double const speedKMPH = m_vehicleModel.GetSpeed(ft);
@@ -236,6 +241,8 @@ void FeaturesRoadGraph::ClearState()
   m_mwmLocks.clear();
 }
 
+bool FeaturesRoadGraph::IsRoad(FeatureType const & ft) const { return m_vehicleModel.IsRoad(ft); }
+
 bool FeaturesRoadGraph::IsOneWay(FeatureType const & ft) const
 {
   return m_vehicleModel.IsOneWay(ft);

routing/features_road_graph.hpp

@@ -31,6 +31,7 @@ private:
     double GetSpeed(FeatureType const & f) const override;
     double GetMaxSpeed() const override;
     bool IsOneWay(FeatureType const & f) const override;
+    bool IsRoad(FeatureType const & f) const override;
 
     void Clear();
 
@@ -77,6 +78,7 @@ public:
 private:
   friend class CrossFeaturesLoader;
 
+  bool IsRoad(FeatureType const & ft) const;
   bool IsOneWay(FeatureType const & ft) const;
   double GetSpeedKMPHFromFt(FeatureType const & ft) const;
 

routing/pedestrian_model.cpp

@@ -621,41 +621,27 @@ PedestrianModel::PedestrianModel(VehicleModel::InitListT const & speedLimits)
 
 void PedestrianModel::Init()
 {
-  m_noFootType = classif().GetTypeByPath({ "hwtag", "nofoot" });
-  m_yesFootType = classif().GetTypeByPath({ "hwtag", "yesfoot" });
+  initializer_list<char const *> hwtagYesFoot = {"hwtag", "yesfoot"};
 
-  initializer_list<char const *> arr[] =
-  {
-    { "route", "ferry" },
-    { "man_made", "pier" },
+  m_noFootType = classif().GetTypeByPath({ "hwtag", "nofoot" });
+  m_yesFootType = classif().GetTypeByPath(hwtagYesFoot);
+
+  initializer_list<char const *> arr[] = {
+      hwtagYesFoot, {"route", "ferry"}, {"man_made", "pier"},
   };
 
   SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr));
 }
 
-bool PedestrianModel::IsNoFoot(feature::TypesHolder const & types) const
+IVehicleModel::RoadAvailability PedestrianModel::GetRoadAvailability(feature::TypesHolder const & types) const
 {
-  return find(types.begin(), types.end(), m_noFootType) != types.end();
+  if (types.Has(m_yesFootType))
+    return RoadAvailability::Available;
+  if (types.Has(m_noFootType))
+    return RoadAvailability::NotAvailable;
+  return RoadAvailability::Unknown;
 }
 
-bool PedestrianModel::IsYesFoot(feature::TypesHolder const & types) const
-{
-  return find(types.begin(), types.end(), m_yesFootType) != types.end();
-}
-
-double PedestrianModel::GetSpeed(FeatureType const & f) const
-{
-  feature::TypesHolder types(f);
-
-  if (IsYesFoot(types))
-    return VehicleModel::GetMaxSpeed();
-  if (!IsNoFoot(types) && IsRoad(types))
-    return VehicleModel::GetSpeed(types);
-
-  return 0.0;
-}
-
-
 PedestrianModelFactory::PedestrianModelFactory()
 {
   m_models[string()] = make_shared<PedestrianModel>(g_pedestrianLimitsDefault);
@@ -700,5 +686,4 @@ shared_ptr<IVehicleModel> PedestrianModelFactory::GetVehicleModelForCountry(stri
   LOG(LDEBUG, ("Pedestrian model wasn't found, default model is used instead:", country));
   return PedestrianModelFactory::GetVehicleModel();
 }
-
 }  // routing

routing/pedestrian_model.hpp

@@ -14,23 +14,15 @@ public:
   PedestrianModel();
   PedestrianModel(VehicleModel::InitListT const & speedLimits);
 
-  /// @name Overrides from VehicleModel.
-  //@{
-  double GetSpeed(FeatureType const & f) const override;
+  /// VehicleModel overrides:
   bool IsOneWay(FeatureType const &) const override { return false; }
-  //@}
+
+protected:
+  RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const override;
 
 private:
   void Init();
 
-  /// @return True if road is prohibited for pedestrian,
-  /// but if function returns False, real prohibition is unknown.
-  bool IsNoFoot(feature::TypesHolder const & types) const;
-
-  /// @return True if road is allowed for pedestrian,
-  /// but if function returns False, real allowance is unknown.
-  bool IsYesFoot(feature::TypesHolder const & types) const;
-
   uint32_t m_noFootType = 0;
   uint32_t m_yesFootType = 0;
 };
@@ -49,5 +41,4 @@ public:
 private:
   unordered_map<string, shared_ptr<IVehicleModel>> m_models;
 };
-
 }  // namespace routing

routing/routing_integration_tests/bicycle_route_test.cpp

@@ -34,3 +34,32 @@ UNIT_TEST(SwedenStockholmCyclewayPriority)
       integration::GetBicycleComponents(), MercatorBounds::FromLatLon(59.33151, 18.09347), {0., 0.},
       MercatorBounds::FromLatLon(59.33052, 18.09391), 113.0);
 }
+
+UNIT_TEST(NetherlandsAmsterdamBicycleNo)
+{
+  TRouteResult const routeResult = integration::CalculateRoute(
+      integration::GetBicycleComponents(), MercatorBounds::FromLatLon(52.32716, 5.05932),
+      {0.0, 0.0}, MercatorBounds::FromLatLon(52.32587, 5.06121));
+
+  IRouter::ResultCode const result = routeResult.second;
+  TEST_EQUAL(result, IRouter::RouteNotFound, ());
+}
+
+UNIT_TEST(NetherlandsAmsterdamBicycleYes)
+{
+  TRouteResult const routeResult = integration::CalculateRoute(
+      integration::GetBicycleComponents(), MercatorBounds::FromLatLon(52.32872, 5.07527),
+      {0.0, 0.0}, MercatorBounds::FromLatLon(52.33853, 5.08941));
+
+  Route const & route = *routeResult.first;
+  IRouter::ResultCode const result = routeResult.second;
+  TEST_EQUAL(result, IRouter::NoError, ());
+  TEST_EQUAL(route.GetTotalTimeSec(), 356, ());
+}
+
+UNIT_TEST(NetherlandsAmsterdamSingelStOnewayBicycleNo)
+{
+  integration::CalculateRouteAndTestRouteLength(
+      integration::GetBicycleComponents(), MercatorBounds::FromLatLon(52.3785, 4.89407), {0., 0.},
+      MercatorBounds::FromLatLon(52.37462, 4.88983), 519.0);
+}

routing/routing_tests/vehicle_model_test.cpp

@@ -21,6 +21,9 @@ routing::VehicleModel::InitListT const s_testLimits = {
 
 class TestVehicleModel : public routing::VehicleModel
 {
+  friend void CheckOneWay(initializer_list<uint32_t> const & types, bool expectedValue);
+  friend void CheckSpeed(initializer_list<uint32_t> const & types, double expectedSpeed);
+
 public:
   TestVehicleModel() : VehicleModel(classif(), s_testLimits) {}
 };
@@ -44,7 +47,7 @@ void CheckSpeed(initializer_list<uint32_t> const & types, double expectedSpeed)
   for (uint32_t t : types)
     h(t);
 
-  TEST_EQUAL(vehicleModel.GetSpeed(h), expectedSpeed, ());
+  TEST_EQUAL(vehicleModel.GetMinTypeSpeed(h), expectedSpeed, ());
 }
 
 void CheckOneWay(initializer_list<uint32_t> const & types, bool expectedValue)
@@ -54,9 +57,8 @@ void CheckOneWay(initializer_list<uint32_t> const & types, bool expectedValue)
   for (uint32_t t : types)
     h(t);
 
-  TEST_EQUAL(vehicleModel.IsOneWay(h), expectedValue, ());
+  TEST_EQUAL(vehicleModel.HasOneWayType(h), expectedValue, ());
 }
-
 }  // namespace
 
 UNIT_TEST(VehicleModel_MaxSpeed)

routing/vehicle_model.cpp

@@ -12,7 +12,7 @@
 namespace routing
 {
 
-VehicleModel::VehicleModel(Classificator const & c, VehicleModel::InitListT const & speedLimits)
+VehicleModel::VehicleModel(Classificator const & c, InitListT const & speedLimits)
   : m_maxSpeedKMpH(0),
     m_onewayType(c.GetTypeByPath({ "hwtag", "oneway" }))
 {
@@ -32,10 +32,18 @@ void VehicleModel::SetAdditionalRoadTypes(Classificator const & c,
 
 double VehicleModel::GetSpeed(FeatureType const & f) const
 {
-  return GetSpeed(feature::TypesHolder(f));
+  feature::TypesHolder const types(f);
+
+  RoadAvailability const restriction = GetRoadAvailability(types);
+  if (restriction == RoadAvailability::Available)
+    return GetMaxSpeed();
+  if (restriction != RoadAvailability::NotAvailable && HasRoadType(types))
+    return GetMinTypeSpeed(types);
+
+  return 0.0;
 }
 
-double VehicleModel::GetSpeed(feature::TypesHolder const & types) const
+double VehicleModel::GetMinTypeSpeed(feature::TypesHolder const & types) const
 {
   double speed = m_maxSpeedKMpH * 2;
   for (uint32_t t : types)
@@ -53,23 +61,48 @@ double VehicleModel::GetSpeed(feature::TypesHolder const & types) const
 
 bool VehicleModel::IsOneWay(FeatureType const & f) const
 {
-  return IsOneWay(feature::TypesHolder(f));
+  return HasOneWayType(feature::TypesHolder(f));
 }
 
-bool VehicleModel::IsOneWay(feature::TypesHolder const & types) const
+bool VehicleModel::HasOneWayType(feature::TypesHolder const & types) const
 {
   return types.Has(m_onewayType);
 }
 
 bool VehicleModel::IsRoad(FeatureType const & f) const
 {
-  return (f.GetFeatureType() == feature::GEOM_LINE) && IsRoad(feature::TypesHolder(f));
+  if (f.GetFeatureType() != feature::GEOM_LINE)
+    return false;
+
+  feature::TypesHolder const types(f);
+
+  if (GetRoadAvailability(types) == RoadAvailability::NotAvailable)
+    return false;
+  return HasRoadType(types);
 }
 
-bool VehicleModel::IsRoad(uint32_t type) const
+bool VehicleModel::IsRoadType(uint32_t type) const
 {
   return find(m_addRoadTypes.begin(), m_addRoadTypes.end(), type) != m_addRoadTypes.end() ||
          m_types.find(ftypes::BaseChecker::PrepareToMatch(type, 2)) != m_types.end();
 }
 
+IVehicleModel::RoadAvailability VehicleModel::GetRoadAvailability(feature::TypesHolder const & /* types */) const
+{
+  return RoadAvailability::Unknown;
+}
+
+string DebugPrint(IVehicleModel::RoadAvailability const l)
+{
+  switch (l)
+  {
+  case IVehicleModel::RoadAvailability::Available: return "Available";
+  case IVehicleModel::RoadAvailability::NotAvailable: return "NotAvailable";
+  case IVehicleModel::RoadAvailability::Unknown: return "Unknown";
+  }
+
+  stringstream out;
+  out << "Unknown IVehicleModel::RoadAvailability (" << static_cast<int>(l) << ")";
+  return out.str();
+}
 }  // namespace routing

routing/vehicle_model.hpp

@@ -19,6 +19,13 @@ namespace routing
 class IVehicleModel
 {
 public:
+  enum class RoadAvailability
+  {
+    NotAvailable,
+    Available,
+    Unknown,
+  };
+
   virtual ~IVehicleModel() {}
 
   /// @return Allowed speed in KMpH.
@@ -29,6 +36,9 @@ public:
   virtual double GetMaxSpeed() const = 0;
 
   virtual bool IsOneWay(FeatureType const & f) const = 0;
+
+  /// @returns true iff feature |f| can be used for routing with corresponding vehicle model.
+  virtual bool IsRoad(FeatureType const & f) const = 0;
 };
 
 class IVehicleModelFactory
@@ -52,41 +62,50 @@ public:
     char const * m_types[2];  /// 2-arity road type
     double m_speedKMpH;       /// max allowed speed on this road type
   };
+
   typedef initializer_list<SpeedForType> InitListT;
 
   VehicleModel(Classificator const & c, InitListT const & speedLimits);
 
-  /// @name Overrides from IVehicleModel.
-  //@{
+  /// IVehicleModel overrides:
   double GetSpeed(FeatureType const & f) const override;
   double GetMaxSpeed() const override { return m_maxSpeedKMpH; }
   bool IsOneWay(FeatureType const & f) const override;
-  //@}
+  bool IsRoad(FeatureType const & f) const override;
 
-  double GetSpeed(feature::TypesHolder const & types) const;
+public:
 
-  /// \returns true if |types| is a oneway feature.
-  /// \note According to OSM tag "oneway" could have value "-1". That means it's a oneway feature
-  /// with reversed geometry. In that case while map generation the geometry of such features
-  /// is reversed (the order of points is changed) so in vehicle model all oneway feature
-  /// could be considered as features with forward geometry.
-  bool IsOneWay(feature::TypesHolder const & types) const;
+  /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise.
+  bool IsRoadType(uint32_t type) const;
 
-  bool IsRoad(FeatureType const & f) const;
-  template <class TList> bool IsRoad(TList const & types) const
+  template <class TList>
+  bool HasRoadType(TList const & types) const
   {
     for (uint32_t t : types)
-      if (IsRoad(t))
+    {
+      if (IsRoadType(t))
         return true;
+    }
     return false;
   }
-  bool IsRoad(uint32_t type) const;
 
 protected:
+  /// @returns a special restriction which is set to the feature.
+  virtual RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const;
+
   /// Used in derived class constructors only. Not for public use.
   void SetAdditionalRoadTypes(Classificator const & c,
                               initializer_list<char const *> const * arr, size_t sz);
 
+  /// \returns true if |types| is a oneway feature.
+  /// \note According to OSM, tag "oneway" could have value "-1". That means it's a oneway feature
+  /// with reversed geometry. In that case at map generation the geometry of such features
+  /// is reversed (the order of points is changed) so in vehicle model all oneway feature
+  /// may be considered as features with forward geometry.
+  bool HasOneWayType(feature::TypesHolder const & types) const;
+
+  double GetMinTypeSpeed(feature::TypesHolder const & types) const;
+
   double m_maxSpeedKMpH;
 
 private:
@@ -96,4 +115,5 @@ private:
   uint32_t m_onewayType;
 };
 
+string DebugPrint(IVehicleModel::RoadAvailability const l);
 }  // namespace routing