From afea24bc7f403e40cea7f3f067b6cced9dbfd7f9 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Fri, 3 Jun 2016 19:17:14 +0300 Subject: [PATCH 1/8] [bicycle routing] Using attr bicycle=yes or no and oneway:bicycle for bicycle routing. --- generator/feature_builder.cpp | 4 +- generator/generator_tests/osm_type_test.cpp | 6 +-- routing/bicycle_model.cpp | 41 ++++++++++++++--- routing/bicycle_model.hpp | 11 +++-- routing/features_road_graph.cpp | 14 +++++- routing/features_road_graph.hpp | 2 + routing/pedestrian_model.cpp | 20 +++++++-- routing/pedestrian_model.hpp | 6 +-- routing/routing_tests/vehicle_model_test.cpp | 7 +-- routing/vehicle_model.cpp | 12 ++--- routing/vehicle_model.hpp | 47 ++++++++++++-------- 11 files changed, 119 insertions(+), 51 deletions(-) diff --git a/generator/feature_builder.cpp b/generator/feature_builder.cpp index b3f2faf0ca..e9814c380f 100644 --- a/generator/feature_builder.cpp +++ b/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() diff --git a/generator/generator_tests/osm_type_test.cpp b/generator/generator_tests/osm_type_test.cpp index 4a156d6730..9c3d7925a1 100644 --- a/generator/generator_tests/osm_type_test.cpp +++ b/generator/generator_tests/osm_type_test.cpp @@ -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) diff --git a/routing/bicycle_model.cpp b/routing/bicycle_model.cpp index 77cae657e1..962939377c 100644 --- a/routing/bicycle_model.cpp +++ b/routing/bicycle_model.cpp @@ -601,13 +601,15 @@ 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 hwtagYesbicycle = { "hwtag", "yesbicycle" }; + + m_yesBicycleType = classif().GetTypeByPath(hwtagYesbicycle); + m_noBicycleType = classif().GetTypeByPath({ "hwtag", "nobicycle" }); + m_bicycleBidirType = classif().GetTypeByPath({ "hwtag", "bicycle_bidir" }); initializer_list arr[] = { + hwtagYesbicycle, { "route", "ferry" }, { "man_made", "pier" }, }; @@ -625,18 +627,45 @@ bool BicycleModel::IsYesBicycle(feature::TypesHolder const & types) const return find(types.begin(), types.end(), m_yesBicycleType) != types.end(); } +bool BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const +{ + return find(types.begin(), types.end(), m_bicycleBidirType) != types.end(); +} + double BicycleModel::GetSpeed(FeatureType const & f) const { feature::TypesHolder types(f); if (IsYesBicycle(types)) return VehicleModel::GetMaxSpeed(); - if (!IsNoBicycle(types) && IsRoad(types)) - return VehicleModel::GetSpeed(types); + if (!IsNoBicycle(types) && HasRoadType(types)) + return VehicleModel::GetMinTypeSpeed(types); return 0.0; } +bool BicycleModel::IsOneWay(FeatureType const & f) const +{ + feature::TypesHolder const types(f); + + if (IsBicycleBidir(types)) + return false; + + return VehicleModel::IsOneWay(f); +} + +bool BicycleModel::IsRoad(FeatureType const & f) const +{ + if (f.GetFeatureType() != feature::GEOM_LINE) + return false; + + feature::TypesHolder types(f); + + if (IsNoBicycle(types)) + return false; + return VehicleModel::HasRoadType(types); +} + BicycleModelFactory::BicycleModelFactory() { m_models[string()] = make_shared(g_bicycleLimitsDefault); diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 2a7fdaa4fa..83875e6cbb 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -14,10 +14,10 @@ public: BicycleModel(); BicycleModel(VehicleModel::InitListT const & speedLimits); - /// @name Overrides from VehicleModel. - //@{ + /// VehicleModel overrides. double GetSpeed(FeatureType const & f) const override; - //@} + bool IsOneWay(FeatureType const & f) const override; + bool IsRoad(FeatureType const & f) const override; private: void Init(); @@ -30,8 +30,13 @@ private: /// but if function returns false, real allowance is unknown. bool IsYesBicycle(feature::TypesHolder const & types) const; + /// @return true if it is allowed to ride bicycle in two directions, + /// but if function returns false, real allowance is unknown. + bool IsBicycleBidir(feature::TypesHolder const & types) const; + uint32_t m_noBicycleType = 0; uint32_t m_yesBicycleType = 0; + uint32_t m_bicycleBidirType = 0; }; class BicycleModelFactory : public IVehicleModelFactory diff --git a/routing/features_road_graph.cpp b/routing/features_road_graph.cpp index f32794f4a6..6d8711a006 100644 --- a/routing/features_road_graph.cpp +++ b/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,11 @@ 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); diff --git a/routing/features_road_graph.hpp b/routing/features_road_graph.hpp index af61afd11f..df3d7ec47a 100644 --- a/routing/features_road_graph.hpp +++ b/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; diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index af22fa59f3..e2a305a147 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -621,11 +621,14 @@ PedestrianModel::PedestrianModel(VehicleModel::InitListT const & speedLimits) void PedestrianModel::Init() { + initializer_list hwtagYesfoot = { "hwtag", "yesfoot" }; + m_noFootType = classif().GetTypeByPath({ "hwtag", "nofoot" }); - m_yesFootType = classif().GetTypeByPath({ "hwtag", "yesfoot" }); + m_yesFootType = classif().GetTypeByPath(hwtagYesfoot); initializer_list arr[] = { + hwtagYesfoot, { "route", "ferry" }, { "man_made", "pier" }, }; @@ -649,12 +652,23 @@ double PedestrianModel::GetSpeed(FeatureType const & f) const if (IsYesFoot(types)) return VehicleModel::GetMaxSpeed(); - if (!IsNoFoot(types) && IsRoad(types)) - return VehicleModel::GetSpeed(types); + if (!IsNoFoot(types) && HasRoadType(types)) + return VehicleModel::GetMinTypeSpeed(types); return 0.0; } +bool PedestrianModel::IsRoad(FeatureType const & f) const +{ + if (f.GetFeatureType() != feature::GEOM_LINE) + return false; + + feature::TypesHolder types(f); + + if (IsNoFoot(types)) + return false; + return VehicleModel::HasRoadType(types); +} PedestrianModelFactory::PedestrianModelFactory() { diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index b32db45e27..3369ed46be 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -14,11 +14,10 @@ public: PedestrianModel(); PedestrianModel(VehicleModel::InitListT const & speedLimits); - /// @name Overrides from VehicleModel. - //@{ + /// VehicleModel overrides. double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const &) const override { return false; } - //@} + bool IsRoad(FeatureType const & f) const override; private: void Init(); @@ -49,5 +48,4 @@ public: private: unordered_map> m_models; }; - } // namespace routing diff --git a/routing/routing_tests/vehicle_model_test.cpp b/routing/routing_tests/vehicle_model_test.cpp index 7f4bffe8fc..1215dbc5f6 100644 --- a/routing/routing_tests/vehicle_model_test.cpp +++ b/routing/routing_tests/vehicle_model_test.cpp @@ -21,6 +21,8 @@ routing::VehicleModel::InitListT const s_testLimits = { class TestVehicleModel : public routing::VehicleModel { + friend void CheckOneWay(initializer_list const & types, bool expectedValue); + friend void CheckSpeed(initializer_list const & types, double expectedSpeed); public: TestVehicleModel() : VehicleModel(classif(), s_testLimits) {} }; @@ -44,7 +46,7 @@ void CheckSpeed(initializer_list 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 const & types, bool expectedValue) @@ -54,9 +56,8 @@ void CheckOneWay(initializer_list 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) diff --git a/routing/vehicle_model.cpp b/routing/vehicle_model.cpp index 9f2cfe4412..d08faaef9b 100644 --- a/routing/vehicle_model.cpp +++ b/routing/vehicle_model.cpp @@ -32,10 +32,10 @@ void VehicleModel::SetAdditionalRoadTypes(Classificator const & c, double VehicleModel::GetSpeed(FeatureType const & f) const { - return GetSpeed(feature::TypesHolder(f)); + return GetMinTypeSpeed(feature::TypesHolder(f)); } -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,20 +53,20 @@ 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)); + return (f.GetFeatureType() == feature::GEOM_LINE) && HasRoadType(feature::TypesHolder(f)); } -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(); diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index dbd1ef5205..f01427838b 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -29,6 +29,10 @@ public: virtual double GetMaxSpeed() const = 0; virtual bool IsOneWay(FeatureType const & f) const = 0; + + /// @returns true if feature |f| can be used for routing with corresponding vehicle model + /// and returns false otherwise. + virtual bool IsRoad(FeatureType const & f) const = 0; }; class IVehicleModelFactory @@ -56,36 +60,41 @@ public: 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; - //@} + /// @note If VehicleModel::IsRoad() returns true for a feature its implementation in + /// inherited class may return true or false. + /// If VehicleModel::IsRoad() returns false for a feature its implementation in + /// inherited class must return false as well. + bool IsRoad(FeatureType const & f) const override; - double GetSpeed(feature::TypesHolder const & types) const; + /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise. + /// @note The set of |types| IsRoadType method returns true for should contain any set of feature types + /// IsRoad method (and its implementation in inherited classes) returns true for. + bool IsRoadType(uint32_t type) const; + template bool HasRoadType(TList const & types) const + { + for (uint32_t t : types) + if (IsRoadType(t)) + return true; + return false; + } + +protected: + /// Used in derived class constructors only. Not for public use. + void SetAdditionalRoadTypes(Classificator const & c, + initializer_list 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 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; + bool HasOneWayType(feature::TypesHolder const & types) const; - bool IsRoad(FeatureType const & f) const; - template bool IsRoad(TList const & types) const - { - for (uint32_t t : types) - if (IsRoad(t)) - return true; - return false; - } - bool IsRoad(uint32_t type) const; - -protected: - /// Used in derived class constructors only. Not for public use. - void SetAdditionalRoadTypes(Classificator const & c, - initializer_list const * arr, size_t sz); + double GetMinTypeSpeed(feature::TypesHolder const & types) const; double m_maxSpeedKMpH; From f61de40e04d2a75d85c98a5b8743b3c0e0a6d308 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Mon, 6 Jun 2016 17:57:48 +0300 Subject: [PATCH 2/8] [bicycle routing] Tests on using bicycle=yes or no and oneway:bicycle for bicycle routing. --- .../bicycle_route_test.cpp | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/routing/routing_integration_tests/bicycle_route_test.cpp b/routing/routing_integration_tests/bicycle_route_test.cpp index a6b82fe039..2929dfa1b7 100644 --- a/routing/routing_integration_tests/bicycle_route_test.cpp +++ b/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); +} From 4cac8206b33bf59fb44e039580fe312c31d7a5dd Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Tue, 7 Jun 2016 13:22:44 +0300 Subject: [PATCH 3/8] Review fixes. --- indexer/index.hpp | 2 +- routing/bicycle_model.cpp | 24 ++++++++++++------------ routing/bicycle_model.hpp | 22 ++++++++++++---------- routing/pedestrian_model.cpp | 18 +++++++++--------- routing/pedestrian_model.hpp | 17 ++++++++++------- routing/vehicle_model.hpp | 16 +++++++++------- 6 files changed, 53 insertions(+), 46 deletions(-) diff --git a/indexer/index.hpp b/indexer/index.hpp index f57c62adc3..8c8467e49e 100644 --- a/indexer/index.hpp +++ b/indexer/index.hpp @@ -58,7 +58,7 @@ public: class Index : public MwmSet { protected: - /// @name MwmSet overrides. + /// MwmSet overrides: //@{ unique_ptr CreateInfo(platform::LocalCountryFile const & localFile) const override; diff --git a/routing/bicycle_model.cpp b/routing/bicycle_model.cpp index 962939377c..c5e4798a81 100644 --- a/routing/bicycle_model.cpp +++ b/routing/bicycle_model.cpp @@ -617,28 +617,28 @@ void BicycleModel::Init() SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -bool BicycleModel::IsNoBicycle(feature::TypesHolder const & types) const +VehicleModel::Restriction BicycleModel::IsNoBicycle(feature::TypesHolder const & types) const { - return find(types.begin(), types.end(), m_noBicycleType) != types.end(); + return types.Has(m_noBicycleType) ? Restriction::Yes : Restriction::Unknown; } -bool BicycleModel::IsYesBicycle(feature::TypesHolder const & types) const +VehicleModel::Restriction BicycleModel::IsYesBicycle(feature::TypesHolder const & types) const { - return find(types.begin(), types.end(), m_yesBicycleType) != types.end(); + return types.Has(m_yesBicycleType) ? Restriction::Yes : Restriction::Unknown; } -bool BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const +VehicleModel::Restriction BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const { - return find(types.begin(), types.end(), m_bicycleBidirType) != types.end(); + return types.Has(m_bicycleBidirType) ? Restriction::Yes : Restriction::Unknown; } double BicycleModel::GetSpeed(FeatureType const & f) const { - feature::TypesHolder types(f); + feature::TypesHolder const types(f); - if (IsYesBicycle(types)) + if (IsYesBicycle(types) == Restriction::Yes) return VehicleModel::GetMaxSpeed(); - if (!IsNoBicycle(types) && HasRoadType(types)) + if (IsNoBicycle(types) == Restriction::Unknown && HasRoadType(types)) return VehicleModel::GetMinTypeSpeed(types); return 0.0; @@ -648,7 +648,7 @@ bool BicycleModel::IsOneWay(FeatureType const & f) const { feature::TypesHolder const types(f); - if (IsBicycleBidir(types)) + if (IsBicycleBidir(types) == Restriction::Yes) return false; return VehicleModel::IsOneWay(f); @@ -659,9 +659,9 @@ bool BicycleModel::IsRoad(FeatureType const & f) const if (f.GetFeatureType() != feature::GEOM_LINE) return false; - feature::TypesHolder types(f); + feature::TypesHolder const types(f); - if (IsNoBicycle(types)) + if (IsNoBicycle(types) == Restriction::Yes) return false; return VehicleModel::HasRoadType(types); } diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 83875e6cbb..64e1ded4d0 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -14,25 +14,27 @@ public: BicycleModel(); BicycleModel(VehicleModel::InitListT const & speedLimits); - /// VehicleModel overrides. + /// VehicleModel overrides: double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const & f) const override; + /// @returns true if |f| could be considered as a road. + /// @note If BicycleModel::IsRoad(f) returns false for a feature f and for an instance + /// of |BicycleModel| created by default constructor + /// BicycleModel::IsRoad(f) for the same feature f and for any instance + /// of |BicycleModel| created by |BicycleModel(VehicleModel::InitListT const &)| must return false. bool IsRoad(FeatureType const & f) 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 Restriction::Yes if road is prohibited for bicycle. + Restriction 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 Restriction::Yes if road is allowed for bicycle. + Restriction IsYesBicycle(feature::TypesHolder const & types) const; - /// @return true if it is allowed to ride bicycle in two directions, - /// but if function returns false, real allowance is unknown. - bool IsBicycleBidir(feature::TypesHolder const & types) const; + /// @return Restriction::Yes if it is allowed to ride bicycle in two directions. + Restriction IsBicycleBidir(feature::TypesHolder const & types) const; uint32_t m_noBicycleType = 0; uint32_t m_yesBicycleType = 0; diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index e2a305a147..7820f390a3 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -636,23 +636,23 @@ void PedestrianModel::Init() SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -bool PedestrianModel::IsNoFoot(feature::TypesHolder const & types) const +VehicleModel::Restriction PedestrianModel::IsNoFoot(feature::TypesHolder const & types) const { - return find(types.begin(), types.end(), m_noFootType) != types.end(); + return types.Has(m_noFootType) ? Restriction::Yes : Restriction::Unknown; } -bool PedestrianModel::IsYesFoot(feature::TypesHolder const & types) const +VehicleModel::Restriction PedestrianModel::IsYesFoot(feature::TypesHolder const & types) const { - return find(types.begin(), types.end(), m_yesFootType) != types.end(); + return types.Has(m_yesFootType) ? Restriction::Yes : Restriction::Unknown; } double PedestrianModel::GetSpeed(FeatureType const & f) const { - feature::TypesHolder types(f); + feature::TypesHolder const types(f); - if (IsYesFoot(types)) + if (IsYesFoot(types) == Restriction::Yes) return VehicleModel::GetMaxSpeed(); - if (!IsNoFoot(types) && HasRoadType(types)) + if (IsNoFoot(types) == Restriction::Unknown && HasRoadType(types)) return VehicleModel::GetMinTypeSpeed(types); return 0.0; @@ -663,9 +663,9 @@ bool PedestrianModel::IsRoad(FeatureType const & f) const if (f.GetFeatureType() != feature::GEOM_LINE) return false; - feature::TypesHolder types(f); + feature::TypesHolder const types(f); - if (IsNoFoot(types)) + if (IsNoFoot(types) == Restriction::Yes) return false; return VehicleModel::HasRoadType(types); } diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index 3369ed46be..7b27868452 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -14,21 +14,24 @@ public: PedestrianModel(); PedestrianModel(VehicleModel::InitListT const & speedLimits); - /// VehicleModel overrides. + /// VehicleModel overrides: double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const &) const override { return false; } + /// @returns true if |f| could be considered as a road for pedestrian routing. + /// @note If PedestrianModel::IsRoad(f) returns false for a feature f and for an instance + /// of |PedestrianModel| created by default constructor + /// PedestrianModel::IsRoad(f) for the same feature f and for any instance + /// of |PedestrianModel| created by |PedestrianModel(VehicleModel::InitListT const &)| must return false. bool IsRoad(FeatureType const & f) 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 Restriction::Yes if road is prohibited for pedestrian. + Restriction 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; + /// @return Restriction::Yes if road is allowed for pedestrian. + Restriction IsYesFoot(feature::TypesHolder const & types) const; uint32_t m_noFootType = 0; uint32_t m_yesFootType = 0; diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index f01427838b..ad8d927e7f 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -60,29 +60,31 @@ public: VehicleModel(Classificator const & c, InitListT const & speedLimits); - /// IVehicleModel overrides. + /// IVehicleModel overrides: double GetSpeed(FeatureType const & f) const override; double GetMaxSpeed() const override { return m_maxSpeedKMpH; } bool IsOneWay(FeatureType const & f) const override; - /// @note If VehicleModel::IsRoad() returns true for a feature its implementation in - /// inherited class may return true or false. - /// If VehicleModel::IsRoad() returns false for a feature its implementation in - /// inherited class must return false as well. bool IsRoad(FeatureType const & f) const override; /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise. - /// @note The set of |types| IsRoadType method returns true for should contain any set of feature types - /// IsRoad method (and its implementation in inherited classes) returns true for. bool IsRoadType(uint32_t type) const; template bool HasRoadType(TList const & types) const { for (uint32_t t : types) + { if (IsRoadType(t)) return true; + } return false; } protected: + enum class Restriction + { + Unknown, + Yes, + }; + /// Used in derived class constructors only. Not for public use. void SetAdditionalRoadTypes(Classificator const & c, initializer_list const * arr, size_t sz); From 97a2bab208945d75de78fd07527084e7d34b3b8c Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 8 Jun 2016 10:38:41 +0300 Subject: [PATCH 4/8] git-clang-format --- generator/feature_builder.cpp | 4 ++-- routing/bicycle_model.cpp | 13 +++++-------- routing/bicycle_model.hpp | 5 ----- routing/features_road_graph.cpp | 5 +---- routing/pedestrian_model.cpp | 9 +++------ routing/pedestrian_model.hpp | 5 ----- routing/routing_tests/vehicle_model_test.cpp | 1 + routing/vehicle_model.hpp | 3 ++- 8 files changed, 14 insertions(+), 31 deletions(-) diff --git a/generator/feature_builder.cpp b/generator/feature_builder.cpp index e9814c380f..9d83b94bbf 100644 --- a/generator/feature_builder.cpp +++ b/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().HasRoadType(m_params.m_Types) || pedModel.HasRoadType(m_params.m_Types) - || bicModel.HasRoadType(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() diff --git a/routing/bicycle_model.cpp b/routing/bicycle_model.cpp index c5e4798a81..c662d38545 100644 --- a/routing/bicycle_model.cpp +++ b/routing/bicycle_model.cpp @@ -601,17 +601,14 @@ BicycleModel::BicycleModel(VehicleModel::InitListT const & speedLimits) void BicycleModel::Init() { - initializer_list hwtagYesbicycle = { "hwtag", "yesbicycle" }; + initializer_list hwtagYesbicycle = {"hwtag", "yesbicycle"}; m_yesBicycleType = classif().GetTypeByPath(hwtagYesbicycle); - m_noBicycleType = classif().GetTypeByPath({ "hwtag", "nobicycle" }); - m_bicycleBidirType = classif().GetTypeByPath({ "hwtag", "bicycle_bidir" }); + m_noBicycleType = classif().GetTypeByPath({"hwtag", "nobicycle"}); + m_bicycleBidirType = classif().GetTypeByPath({"hwtag", "bicycle_bidir"}); - initializer_list arr[] = - { - hwtagYesbicycle, - { "route", "ferry" }, - { "man_made", "pier" }, + initializer_list arr[] = { + hwtagYesbicycle, {"route", "ferry"}, {"man_made", "pier"}, }; SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 64e1ded4d0..bd4185a8c4 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -17,11 +17,6 @@ public: /// VehicleModel overrides: double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const & f) const override; - /// @returns true if |f| could be considered as a road. - /// @note If BicycleModel::IsRoad(f) returns false for a feature f and for an instance - /// of |BicycleModel| created by default constructor - /// BicycleModel::IsRoad(f) for the same feature f and for any instance - /// of |BicycleModel| created by |BicycleModel(VehicleModel::InitListT const &)| must return false. bool IsRoad(FeatureType const & f) const override; private: diff --git a/routing/features_road_graph.cpp b/routing/features_road_graph.cpp index 6d8711a006..62d8aa7829 100644 --- a/routing/features_road_graph.cpp +++ b/routing/features_road_graph.cpp @@ -241,10 +241,7 @@ void FeaturesRoadGraph::ClearState() m_mwmLocks.clear(); } -bool FeaturesRoadGraph::IsRoad(FeatureType const & ft) const -{ - return m_vehicleModel.IsRoad(ft); -} +bool FeaturesRoadGraph::IsRoad(FeatureType const & ft) const { return m_vehicleModel.IsRoad(ft); } bool FeaturesRoadGraph::IsOneWay(FeatureType const & ft) const { diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index 7820f390a3..26147ec7ba 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -621,16 +621,13 @@ PedestrianModel::PedestrianModel(VehicleModel::InitListT const & speedLimits) void PedestrianModel::Init() { - initializer_list hwtagYesfoot = { "hwtag", "yesfoot" }; + initializer_list hwtagYesfoot = {"hwtag", "yesfoot"}; m_noFootType = classif().GetTypeByPath({ "hwtag", "nofoot" }); m_yesFootType = classif().GetTypeByPath(hwtagYesfoot); - initializer_list arr[] = - { - hwtagYesfoot, - { "route", "ferry" }, - { "man_made", "pier" }, + initializer_list arr[] = { + hwtagYesfoot, {"route", "ferry"}, {"man_made", "pier"}, }; SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index 7b27868452..f513ec92c7 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -17,11 +17,6 @@ public: /// VehicleModel overrides: double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const &) const override { return false; } - /// @returns true if |f| could be considered as a road for pedestrian routing. - /// @note If PedestrianModel::IsRoad(f) returns false for a feature f and for an instance - /// of |PedestrianModel| created by default constructor - /// PedestrianModel::IsRoad(f) for the same feature f and for any instance - /// of |PedestrianModel| created by |PedestrianModel(VehicleModel::InitListT const &)| must return false. bool IsRoad(FeatureType const & f) const override; private: diff --git a/routing/routing_tests/vehicle_model_test.cpp b/routing/routing_tests/vehicle_model_test.cpp index 1215dbc5f6..4763ed78cd 100644 --- a/routing/routing_tests/vehicle_model_test.cpp +++ b/routing/routing_tests/vehicle_model_test.cpp @@ -23,6 +23,7 @@ class TestVehicleModel : public routing::VehicleModel { friend void CheckOneWay(initializer_list const & types, bool expectedValue); friend void CheckSpeed(initializer_list const & types, double expectedSpeed); + public: TestVehicleModel() : VehicleModel(classif(), s_testLimits) {} }; diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index ad8d927e7f..076819bdaa 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -68,7 +68,8 @@ public: /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise. bool IsRoadType(uint32_t type) const; - template bool HasRoadType(TList const & types) const + template + bool HasRoadType(TList const & types) const { for (uint32_t t : types) { From a35256f8e791dda8e5e343de8483efc75d83ef58 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 8 Jun 2016 16:40:35 +0300 Subject: [PATCH 5/8] Renaming tag bicycle_bidir to bidir_bicycle and other review fixes. --- data/classificator.txt | 2 +- data/types.txt | 2 +- generator/generator_tests/osm_type_test.cpp | 2 +- generator/osm2type.cpp | 2 +- routing/bicycle_model.cpp | 24 ++++++++++----------- routing/bicycle_model.hpp | 9 ++------ routing/pedestrian_model.cpp | 21 +++++++++--------- routing/pedestrian_model.hpp | 7 +----- routing/vehicle_model.hpp | 3 ++- 9 files changed, 32 insertions(+), 40 deletions(-) diff --git a/data/classificator.txt b/data/classificator.txt index 14e45b22bd..55e365c71e 100644 --- a/data/classificator.txt +++ b/data/classificator.txt @@ -363,7 +363,7 @@ world + wayside_shrine - {} hwtag + - bicycle_bidir - + bidir_bicycle - lit - nobicycle - nofoot - diff --git a/data/types.txt b/data/types.txt index 8cfa84722c..777238e35e 100644 --- a/data/types.txt +++ b/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 diff --git a/generator/generator_tests/osm_type_test.cpp b/generator/generator_tests/osm_type_test.cpp index 9c3d7925a1..2a858edf5c 100644 --- a/generator/generator_tests/osm_type_test.cpp +++ b/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"} }; { diff --git a/generator/osm2type.cpp b/generator/osm2type.cpp index dc1fa670c0..f8069dbe71 100644 --- a/generator/osm2type.cpp +++ b/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"}, }; diff --git a/routing/bicycle_model.cpp b/routing/bicycle_model.cpp index c662d38545..5b7f830ee6 100644 --- a/routing/bicycle_model.cpp +++ b/routing/bicycle_model.cpp @@ -605,7 +605,7 @@ void BicycleModel::Init() m_yesBicycleType = classif().GetTypeByPath(hwtagYesbicycle); m_noBicycleType = classif().GetTypeByPath({"hwtag", "nobicycle"}); - m_bicycleBidirType = classif().GetTypeByPath({"hwtag", "bicycle_bidir"}); + m_bidirBicycleType = classif().GetTypeByPath({"hwtag", "bidir_bicycle"}); initializer_list arr[] = { hwtagYesbicycle, {"route", "ferry"}, {"man_made", "pier"}, @@ -614,28 +614,28 @@ void BicycleModel::Init() SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -VehicleModel::Restriction BicycleModel::IsNoBicycle(feature::TypesHolder const & types) const +VehicleModel::Restriction BicycleModel::IsBicycleAllowed(feature::TypesHolder const & types) const { - return types.Has(m_noBicycleType) ? Restriction::Yes : Restriction::Unknown; -} - -VehicleModel::Restriction BicycleModel::IsYesBicycle(feature::TypesHolder const & types) const -{ - return types.Has(m_yesBicycleType) ? Restriction::Yes : Restriction::Unknown; + if (types.Has(m_yesBicycleType)) + return Restriction::Yes; + if (types.Has(m_noBicycleType)) + return Restriction::No; + return Restriction::Unknown; } VehicleModel::Restriction BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const { - return types.Has(m_bicycleBidirType) ? Restriction::Yes : Restriction::Unknown; + return types.Has(m_bidirBicycleType) ? Restriction::Yes : Restriction::Unknown; } double BicycleModel::GetSpeed(FeatureType const & f) const { feature::TypesHolder const types(f); - if (IsYesBicycle(types) == Restriction::Yes) + Restriction const restriction = IsBicycleAllowed(types); + if (restriction == Restriction::Yes) return VehicleModel::GetMaxSpeed(); - if (IsNoBicycle(types) == Restriction::Unknown && HasRoadType(types)) + if (restriction != Restriction::No && HasRoadType(types)) return VehicleModel::GetMinTypeSpeed(types); return 0.0; @@ -658,7 +658,7 @@ bool BicycleModel::IsRoad(FeatureType const & f) const feature::TypesHolder const types(f); - if (IsNoBicycle(types) == Restriction::Yes) + if (IsBicycleAllowed(types) == Restriction::No) return false; return VehicleModel::HasRoadType(types); } diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index bd4185a8c4..38961f3d6f 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -21,19 +21,14 @@ public: private: void Init(); - - /// @return Restriction::Yes if road is prohibited for bicycle. - Restriction IsNoBicycle(feature::TypesHolder const & types) const; - - /// @return Restriction::Yes if road is allowed for bicycle. - Restriction IsYesBicycle(feature::TypesHolder const & types) const; + Restriction IsBicycleAllowed(feature::TypesHolder const & types) const; /// @return Restriction::Yes if it is allowed to ride bicycle in two directions. Restriction IsBicycleBidir(feature::TypesHolder const & types) const; uint32_t m_noBicycleType = 0; uint32_t m_yesBicycleType = 0; - uint32_t m_bicycleBidirType = 0; + uint32_t m_bidirBicycleType = 0; }; class BicycleModelFactory : public IVehicleModelFactory diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index 26147ec7ba..9e30abd9ee 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -633,23 +633,23 @@ void PedestrianModel::Init() SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -VehicleModel::Restriction PedestrianModel::IsNoFoot(feature::TypesHolder const & types) const +VehicleModel::Restriction PedestrianModel::IsPedestrianAllowed(feature::TypesHolder const & types) const { - return types.Has(m_noFootType) ? Restriction::Yes : Restriction::Unknown; -} - -VehicleModel::Restriction PedestrianModel::IsYesFoot(feature::TypesHolder const & types) const -{ - return types.Has(m_yesFootType) ? Restriction::Yes : Restriction::Unknown; + if (types.Has(m_yesFootType)) + return Restriction::Yes; + if (types.Has(m_noFootType)) + return Restriction::No; + return Restriction::Unknown; } double PedestrianModel::GetSpeed(FeatureType const & f) const { feature::TypesHolder const types(f); - if (IsYesFoot(types) == Restriction::Yes) + Restriction const restriction = IsPedestrianAllowed(types); + if (restriction == Restriction::Yes) return VehicleModel::GetMaxSpeed(); - if (IsNoFoot(types) == Restriction::Unknown && HasRoadType(types)) + if (restriction != Restriction::No && HasRoadType(types)) return VehicleModel::GetMinTypeSpeed(types); return 0.0; @@ -662,8 +662,9 @@ bool PedestrianModel::IsRoad(FeatureType const & f) const feature::TypesHolder const types(f); - if (IsNoFoot(types) == Restriction::Yes) + if (IsPedestrianAllowed(types) == Restriction::No) return false; + return VehicleModel::HasRoadType(types); } diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index f513ec92c7..6cc5af66f5 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -21,12 +21,7 @@ public: private: void Init(); - - /// @return Restriction::Yes if road is prohibited for pedestrian. - Restriction IsNoFoot(feature::TypesHolder const & types) const; - - /// @return Restriction::Yes if road is allowed for pedestrian. - Restriction IsYesFoot(feature::TypesHolder const & types) const; + Restriction IsPedestrianAllowed(feature::TypesHolder const & types) const; uint32_t m_noFootType = 0; uint32_t m_yesFootType = 0; diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index 076819bdaa..c57af6e396 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -82,8 +82,9 @@ public: protected: enum class Restriction { - Unknown, + No, Yes, + Unknown, }; /// Used in derived class constructors only. Not for public use. From b0a831d8ebb0c88b3bfaee345ce31a255d30ef30 Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 8 Jun 2016 17:56:28 +0300 Subject: [PATCH 6/8] Code refactoring. Implementing some functionality to VehicleModel class and removing it form derived classes. --- routing/bicycle_model.cpp | 45 ++++++++---------------------------- routing/bicycle_model.hpp | 10 ++++---- routing/pedestrian_model.cpp | 35 ++++------------------------ routing/pedestrian_model.hpp | 6 ++--- routing/vehicle_model.cpp | 23 ++++++++++++++++-- routing/vehicle_model.hpp | 21 +++++++++++------ 6 files changed, 57 insertions(+), 83 deletions(-) diff --git a/routing/bicycle_model.cpp b/routing/bicycle_model.cpp index 5b7f830ee6..458bdced2b 100644 --- a/routing/bicycle_model.cpp +++ b/routing/bicycle_model.cpp @@ -601,68 +601,43 @@ BicycleModel::BicycleModel(VehicleModel::InitListT const & speedLimits) void BicycleModel::Init() { - initializer_list hwtagYesbicycle = {"hwtag", "yesbicycle"}; + initializer_list hwtagYesBicycle = {"hwtag", "yesbicycle"}; - m_yesBicycleType = classif().GetTypeByPath(hwtagYesbicycle); + m_yesBicycleType = classif().GetTypeByPath(hwtagYesBicycle); m_noBicycleType = classif().GetTypeByPath({"hwtag", "nobicycle"}); m_bidirBicycleType = classif().GetTypeByPath({"hwtag", "bidir_bicycle"}); initializer_list arr[] = { - hwtagYesbicycle, {"route", "ferry"}, {"man_made", "pier"}, + hwtagYesBicycle, {"route", "ferry"}, {"man_made", "pier"}, }; SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -VehicleModel::Restriction BicycleModel::IsBicycleAllowed(feature::TypesHolder const & types) const +IVehicleModel::RoadAvailability BicycleModel::GetRoadAvailability(feature::TypesHolder const & types) const { if (types.Has(m_yesBicycleType)) - return Restriction::Yes; + return RoadAvailability::Available; if (types.Has(m_noBicycleType)) - return Restriction::No; - return Restriction::Unknown; + return RoadAvailability::NotAvailable; + return RoadAvailability::Unknown; } -VehicleModel::Restriction BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const +bool BicycleModel::IsBicycleBidir(feature::TypesHolder const & types) const { - return types.Has(m_bidirBicycleType) ? Restriction::Yes : Restriction::Unknown; -} - -double BicycleModel::GetSpeed(FeatureType const & f) const -{ - feature::TypesHolder const types(f); - - Restriction const restriction = IsBicycleAllowed(types); - if (restriction == Restriction::Yes) - return VehicleModel::GetMaxSpeed(); - if (restriction != Restriction::No && HasRoadType(types)) - return VehicleModel::GetMinTypeSpeed(types); - - return 0.0; + return types.Has(m_bidirBicycleType); } bool BicycleModel::IsOneWay(FeatureType const & f) const { feature::TypesHolder const types(f); - if (IsBicycleBidir(types) == Restriction::Yes) + if (IsBicycleBidir(types)) return false; return VehicleModel::IsOneWay(f); } -bool BicycleModel::IsRoad(FeatureType const & f) const -{ - if (f.GetFeatureType() != feature::GEOM_LINE) - return false; - - feature::TypesHolder const types(f); - - if (IsBicycleAllowed(types) == Restriction::No) - return false; - return VehicleModel::HasRoadType(types); -} - BicycleModelFactory::BicycleModelFactory() { m_models[string()] = make_shared(g_bicycleLimitsDefault); diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 38961f3d6f..1008fe07bd 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -15,16 +15,16 @@ public: BicycleModel(VehicleModel::InitListT const & speedLimits); /// VehicleModel overrides: - double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const & f) const override; - bool IsRoad(FeatureType const & f) const override; + +protected: + RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const override; private: void Init(); - Restriction IsBicycleAllowed(feature::TypesHolder const & types) const; - /// @return Restriction::Yes if it is allowed to ride bicycle in two directions. - Restriction IsBicycleBidir(feature::TypesHolder const & types) const; + /// @return true if it is allowed to ride bicycle in two directions. + bool IsBicycleBidir(feature::TypesHolder const & types) const; uint32_t m_noBicycleType = 0; uint32_t m_yesBicycleType = 0; diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index 9e30abd9ee..510568432f 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -633,39 +633,13 @@ void PedestrianModel::Init() SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); } -VehicleModel::Restriction PedestrianModel::IsPedestrianAllowed(feature::TypesHolder const & types) const +IVehicleModel::RoadAvailability PedestrianModel::GetRoadAvailability(feature::TypesHolder const & types) const { if (types.Has(m_yesFootType)) - return Restriction::Yes; + return RoadAvailability::Available; if (types.Has(m_noFootType)) - return Restriction::No; - return Restriction::Unknown; -} - -double PedestrianModel::GetSpeed(FeatureType const & f) const -{ - feature::TypesHolder const types(f); - - Restriction const restriction = IsPedestrianAllowed(types); - if (restriction == Restriction::Yes) - return VehicleModel::GetMaxSpeed(); - if (restriction != Restriction::No && HasRoadType(types)) - return VehicleModel::GetMinTypeSpeed(types); - - return 0.0; -} - -bool PedestrianModel::IsRoad(FeatureType const & f) const -{ - if (f.GetFeatureType() != feature::GEOM_LINE) - return false; - - feature::TypesHolder const types(f); - - if (IsPedestrianAllowed(types) == Restriction::No) - return false; - - return VehicleModel::HasRoadType(types); + return RoadAvailability::NotAvailable; + return RoadAvailability::Unknown; } PedestrianModelFactory::PedestrianModelFactory() @@ -712,5 +686,4 @@ shared_ptr PedestrianModelFactory::GetVehicleModelForCountry(stri LOG(LDEBUG, ("Pedestrian model wasn't found, default model is used instead:", country)); return PedestrianModelFactory::GetVehicleModel(); } - } // routing diff --git a/routing/pedestrian_model.hpp b/routing/pedestrian_model.hpp index 6cc5af66f5..c00df62db8 100644 --- a/routing/pedestrian_model.hpp +++ b/routing/pedestrian_model.hpp @@ -15,13 +15,13 @@ public: PedestrianModel(VehicleModel::InitListT const & speedLimits); /// VehicleModel overrides: - double GetSpeed(FeatureType const & f) const override; bool IsOneWay(FeatureType const &) const override { return false; } - bool IsRoad(FeatureType const & f) const override; + +protected: + RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const override; private: void Init(); - Restriction IsPedestrianAllowed(feature::TypesHolder const & types) const; uint32_t m_noFootType = 0; uint32_t m_yesFootType = 0; diff --git a/routing/vehicle_model.cpp b/routing/vehicle_model.cpp index d08faaef9b..79a94d835e 100644 --- a/routing/vehicle_model.cpp +++ b/routing/vehicle_model.cpp @@ -32,7 +32,15 @@ void VehicleModel::SetAdditionalRoadTypes(Classificator const & c, double VehicleModel::GetSpeed(FeatureType const & f) const { - return GetMinTypeSpeed(feature::TypesHolder(f)); + feature::TypesHolder const types(f); + + RoadAvailability const restriction = GetRoadAvailability(types); + if (restriction == RoadAvailability::Available) + return VehicleModel::GetMaxSpeed(); + if (restriction != RoadAvailability::NotAvailable && HasRoadType(types)) + return VehicleModel::GetMinTypeSpeed(types); + + return 0.0; } double VehicleModel::GetMinTypeSpeed(feature::TypesHolder const & types) const @@ -63,7 +71,14 @@ bool VehicleModel::HasOneWayType(feature::TypesHolder const & types) const bool VehicleModel::IsRoad(FeatureType const & f) const { - return (f.GetFeatureType() == feature::GEOM_LINE) && HasRoadType(feature::TypesHolder(f)); + if (f.GetFeatureType() != feature::GEOM_LINE) + return false; + + feature::TypesHolder const types(f); + + if (GetRoadAvailability(types) == RoadAvailability::NotAvailable) + return false; + return VehicleModel::HasRoadType(types); } bool VehicleModel::IsRoadType(uint32_t type) const @@ -72,4 +87,8 @@ bool VehicleModel::IsRoadType(uint32_t type) const m_types.find(ftypes::BaseChecker::PrepareToMatch(type, 2)) != m_types.end(); } +IVehicleModel::RoadAvailability VehicleModel::GetRoadAvailability(feature::TypesHolder const & /* types */) const +{ + return RoadAvailability::Unknown; +} } // namespace routing diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index c57af6e396..a052c6e65f 100644 --- a/routing/vehicle_model.hpp +++ b/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. @@ -56,6 +63,7 @@ public: char const * m_types[2]; /// 2-arity road type double m_speedKMpH; /// max allowed speed on this road type }; + typedef initializer_list InitListT; VehicleModel(Classificator const & c, InitListT const & speedLimits); @@ -66,6 +74,12 @@ public: bool IsOneWay(FeatureType const & f) const override; bool IsRoad(FeatureType const & f) const override; +protected: + /// @returns a special restriction which is set to the feature. + virtual RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const; + +public: + /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise. bool IsRoadType(uint32_t type) const; template @@ -80,13 +94,6 @@ public: } protected: - enum class Restriction - { - No, - Yes, - Unknown, - }; - /// Used in derived class constructors only. Not for public use. void SetAdditionalRoadTypes(Classificator const & c, initializer_list const * arr, size_t sz); From b9bab751c13c4f46eb8eac0989f4bccf975866fc Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 16 Jun 2016 17:46:08 +0300 Subject: [PATCH 7/8] Renaming tag bicycle_bidir to bidir_bicycle in visibility.txt and in mapcss-mapping.csv --- data/mapcss-mapping.csv | 2 +- data/visibility.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/data/mapcss-mapping.csv b/data/mapcss-mapping.csv index 7c3e47ad6e..cd0d81c512 100644 --- a/data/mapcss-mapping.csv +++ b/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; diff --git a/data/visibility.txt b/data/visibility.txt index a46afd4b42..d4afcc7311 100644 --- a/data/visibility.txt +++ b/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 - From 0e113f4388eecdfb4ada253edfd0b8f4c21d4cbc Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 16 Jun 2016 17:46:41 +0300 Subject: [PATCH 8/8] Review fixes. --- routing/bicycle_model.hpp | 2 +- routing/pedestrian_model.cpp | 6 +++--- routing/vehicle_model.cpp | 22 ++++++++++++++++++---- routing/vehicle_model.hpp | 18 +++++++++--------- 4 files changed, 31 insertions(+), 17 deletions(-) diff --git a/routing/bicycle_model.hpp b/routing/bicycle_model.hpp index 1008fe07bd..85820d4344 100644 --- a/routing/bicycle_model.hpp +++ b/routing/bicycle_model.hpp @@ -23,7 +23,7 @@ protected: private: void Init(); - /// @return true if it is allowed to ride bicycle in two directions. + /// @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; diff --git a/routing/pedestrian_model.cpp b/routing/pedestrian_model.cpp index 510568432f..678a4064d9 100644 --- a/routing/pedestrian_model.cpp +++ b/routing/pedestrian_model.cpp @@ -621,13 +621,13 @@ PedestrianModel::PedestrianModel(VehicleModel::InitListT const & speedLimits) void PedestrianModel::Init() { - initializer_list hwtagYesfoot = {"hwtag", "yesfoot"}; + initializer_list hwtagYesFoot = {"hwtag", "yesfoot"}; m_noFootType = classif().GetTypeByPath({ "hwtag", "nofoot" }); - m_yesFootType = classif().GetTypeByPath(hwtagYesfoot); + m_yesFootType = classif().GetTypeByPath(hwtagYesFoot); initializer_list arr[] = { - hwtagYesfoot, {"route", "ferry"}, {"man_made", "pier"}, + hwtagYesFoot, {"route", "ferry"}, {"man_made", "pier"}, }; SetAdditionalRoadTypes(classif(), arr, ARRAY_SIZE(arr)); diff --git a/routing/vehicle_model.cpp b/routing/vehicle_model.cpp index 79a94d835e..3ff16732d8 100644 --- a/routing/vehicle_model.cpp +++ b/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" })) { @@ -36,9 +36,9 @@ double VehicleModel::GetSpeed(FeatureType const & f) const RoadAvailability const restriction = GetRoadAvailability(types); if (restriction == RoadAvailability::Available) - return VehicleModel::GetMaxSpeed(); + return GetMaxSpeed(); if (restriction != RoadAvailability::NotAvailable && HasRoadType(types)) - return VehicleModel::GetMinTypeSpeed(types); + return GetMinTypeSpeed(types); return 0.0; } @@ -78,7 +78,7 @@ bool VehicleModel::IsRoad(FeatureType const & f) const if (GetRoadAvailability(types) == RoadAvailability::NotAvailable) return false; - return VehicleModel::HasRoadType(types); + return HasRoadType(types); } bool VehicleModel::IsRoadType(uint32_t type) const @@ -91,4 +91,18 @@ IVehicleModel::RoadAvailability VehicleModel::GetRoadAvailability(feature::Types { 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(l) << ")"; + return out.str(); +} } // namespace routing diff --git a/routing/vehicle_model.hpp b/routing/vehicle_model.hpp index a052c6e65f..b16bf32edf 100644 --- a/routing/vehicle_model.hpp +++ b/routing/vehicle_model.hpp @@ -37,8 +37,7 @@ public: virtual bool IsOneWay(FeatureType const & f) const = 0; - /// @returns true if feature |f| can be used for routing with corresponding vehicle model - /// and returns false otherwise. + /// @returns true iff feature |f| can be used for routing with corresponding vehicle model. virtual bool IsRoad(FeatureType const & f) const = 0; }; @@ -74,14 +73,11 @@ public: bool IsOneWay(FeatureType const & f) const override; bool IsRoad(FeatureType const & f) const override; -protected: - /// @returns a special restriction which is set to the feature. - virtual RoadAvailability GetRoadAvailability(feature::TypesHolder const & types) const; - public: /// @returns true if |m_types| or |m_addRoadTypes| contains |type| and false otherwise. bool IsRoadType(uint32_t type) const; + template bool HasRoadType(TList const & types) const { @@ -94,15 +90,18 @@ public: } 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 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 while map generation the geometry of such features + /// \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 - /// could be considered as features with forward geometry. + /// may be considered as features with forward geometry. bool HasOneWayType(feature::TypesHolder const & types) const; double GetMinTypeSpeed(feature::TypesHolder const & types) const; @@ -116,4 +115,5 @@ private: uint32_t m_onewayType; }; +string DebugPrint(IVehicleModel::RoadAvailability const l); } // namespace routing