From 91b59dfdf57672500cda7b26141acd74a3e1f90c Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Thu, 1 Nov 2018 15:46:22 +0300 Subject: [PATCH] [routing] Maxspeed section format changed. Some optimization based on single feature may have maxspeed in the same units. --- generator/generator_tests/maxspeed_tests.cpp | 11 ++-- generator/maxspeed_builder.cpp | 13 +++-- routing/maxspeed_conversion.cpp | 16 +++--- routing/maxspeed_conversion.hpp | 31 +++++----- routing/maxspeed_serialization.hpp | 20 +++---- routing/routing_tests/maxspeed_tests.cpp | 59 +++++++++----------- 6 files changed, 70 insertions(+), 80 deletions(-) diff --git a/generator/generator_tests/maxspeed_tests.cpp b/generator/generator_tests/maxspeed_tests.cpp index 05905c9ef8..123ed76202 100644 --- a/generator/generator_tests/maxspeed_tests.cpp +++ b/generator/generator_tests/maxspeed_tests.cpp @@ -131,8 +131,8 @@ void TestMaxspeedSection(FeatureVector const & roads, string const & maxspeedCsv // Looking for maxspeed form mwm. // Note. FeatureMaxspeed::operator<() is base only on FeatureMaxspeed::m_featureId. - auto const it = - equal_range(maxspeeds.cbegin(), maxspeeds.cend(), FeatureMaxspeed(id, SpeedInUnits())); + auto const it = equal_range(maxspeeds.cbegin(), maxspeeds.cend(), + FeatureMaxspeed(id, Units::Metric, kInvalidSpeed)); TEST_EQUAL(it.second - it.first, 1, ("If there's no maxspeed in mwm for", id, @@ -141,10 +141,9 @@ void TestMaxspeedSection(FeatureVector const & roads, string const & maxspeedCsv // Comparing maxspeed from csv and maxspeed from mwm section. TEST_EQUAL(id, maxspeedMwm.GetFeatureId(), ()); - TEST_EQUAL(maxspeedCsv.m_units, maxspeedMwm.GetForward().m_units, ()); - TEST_EQUAL(maxspeedCsv.m_units, maxspeedMwm.GetBackward().m_units, ()); - TEST_EQUAL(maxspeedCsv.m_forward, maxspeedMwm.GetForward().m_speed, ()); - TEST_EQUAL(maxspeedCsv.m_backward, maxspeedMwm.GetBackward().m_speed, ()); + TEST_EQUAL(maxspeedCsv.m_units, maxspeedMwm.GetUnits(), ()); + TEST_EQUAL(maxspeedCsv.m_forward, maxspeedMwm.GetForward(), ()); + TEST_EQUAL(maxspeedCsv.m_backward, maxspeedMwm.GetBackward(), ()); }; feature::ForEachFromDat(testMwmFullPath, processor); } diff --git a/generator/maxspeed_builder.cpp b/generator/maxspeed_builder.cpp index 5c43400f9c..a7afe66bc6 100644 --- a/generator/maxspeed_builder.cpp +++ b/generator/maxspeed_builder.cpp @@ -53,6 +53,12 @@ bool ParseOneSpeedValue(strings::SimpleTokenizer & iter, uint16_t & value) return true; } +FeatureMaxspeed ToFeatureMaxspeed(uint32_t featureId, ParsedMaxspeed const & parsedMaxspeed) +{ + return FeatureMaxspeed(featureId, parsedMaxspeed.m_units, parsedMaxspeed.m_forward, + parsedMaxspeed.m_backward); +} + /// \brief Collects all maxspeed tag value of specified mwm based on maxspeed.csv file. class MaxspeedMwmCollector { @@ -88,12 +94,7 @@ MaxspeedMwmCollector::MaxspeedMwmCollector( return; auto const & parsedMaxspeed = maxspeedIt->second; - // Note. It's wrong that by default if there's no maxspeed backward SpeedInUnits::m_units - // is Metric and according to this code it's be saved as Imperial for country - // with imperial metrics. Anyway this code should be rewritten before merge. - m_maxspeeds.push_back( - FeatureMaxspeed(fid, SpeedInUnits(parsedMaxspeed.m_forward, parsedMaxspeed.m_units), - SpeedInUnits(parsedMaxspeed.m_backward, parsedMaxspeed.m_units))); + m_maxspeeds.push_back(ToFeatureMaxspeed(fid, parsedMaxspeed)); }); } } // namespace diff --git a/routing/maxspeed_conversion.cpp b/routing/maxspeed_conversion.cpp index fea176b855..5265f704dc 100644 --- a/routing/maxspeed_conversion.cpp +++ b/routing/maxspeed_conversion.cpp @@ -26,15 +26,16 @@ bool SpeedInUnits::IsNumeric() const } // FeatureMaxspeed --------------------------------------------------------------------------------- -FeatureMaxspeed::FeatureMaxspeed(uint32_t fid, SpeedInUnits const & forward, - SpeedInUnits const & backward /* = SpeedInUnits() */) - : m_featureId(fid), m_forward(forward), m_backward(backward) +FeatureMaxspeed::FeatureMaxspeed(uint32_t fid, measurement_utils::Units units, uint16_t forward, + uint16_t backward /* = kInvalidSpeed */) noexcept + : m_featureId(fid), m_units(units), m_forward(forward), m_backward(backward) { } bool FeatureMaxspeed::operator==(FeatureMaxspeed const & rhs) const { - return m_featureId == rhs.m_featureId && m_forward == rhs.m_forward && m_backward == rhs.m_backward; + return m_featureId == rhs.m_featureId && m_units == rhs.m_units && + m_forward == rhs.m_forward && m_backward == rhs.m_backward; } // MaxspeedConverter ------------------------------------------------------------------------------- @@ -228,7 +229,7 @@ std::string DebugPrint(SpeedInUnits const & speed) { std::ostringstream oss; oss << "SpeedInUnits [ m_speed == " << speed.m_speed - << ", m_units == " << DebugPrint(speed.m_units) << " ]"; + << ", m_units:" << DebugPrint(speed.m_units) << " ]"; return oss.str(); } @@ -236,8 +237,9 @@ std::string DebugPrint(FeatureMaxspeed const & featureMaxspeed) { std::ostringstream oss; oss << "FeatureMaxspeed [ m_featureId:" << featureMaxspeed.GetFeatureId() - << " m_forward:" << DebugPrint(featureMaxspeed.GetForward()) - << " m_backward:" << DebugPrint(featureMaxspeed.GetBackward()) << " ]"; + << " m_units:" << DebugPrint(featureMaxspeed.GetUnits()) + << " m_forward:" << featureMaxspeed.GetForward() + << " m_backward:" << featureMaxspeed.GetBackward() << " ]"; return oss.str(); } } // namespace routing diff --git a/routing/maxspeed_conversion.hpp b/routing/maxspeed_conversion.hpp index d59714db42..d2a0fb74a6 100644 --- a/routing/maxspeed_conversion.hpp +++ b/routing/maxspeed_conversion.hpp @@ -11,8 +11,8 @@ namespace routing { /// \brief This enum class contains most popular maxspeed value according to /// https://taginfo.openstreetmap.org/keys/maxspeed#values. -/// \note Value of this enum is saved to mwm. So they should not be change because of backward -/// compatibility. But it's possible to add some value to this enum. +/// \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. enum class Maxspeed : uint8_t { // Special values. @@ -175,31 +175,34 @@ struct SpeedInUnits measurement_utils::Units m_units = measurement_utils::Units::Metric; }; -/// \brief Maxspeed tag value for feature id. |m_forward| and |m_backward| fields reflect the fact -/// that a feature may have different maxspeed tag value for different directions. -/// If |m_backward| is invalid it means that |m_forward| tag contains maxspeed for the both -/// directions. +/// \brief Maxspeed tag value for feature id. |m_forward| and |m_backward| fields reflect +/// the fact that a feature may have different maxspeed tag value for different directions. +/// If |m_backward| is invalid it means that |m_forward| tag contains maxspeed for +/// the both directions. If a feature has maxspeed forward and maxspeed backward in different units +/// it's considered as an invalid one and it's not saved into mwm. class FeatureMaxspeed { public: - FeatureMaxspeed(uint32_t fid, SpeedInUnits const & forward, - SpeedInUnits const & backward = SpeedInUnits()); + FeatureMaxspeed(uint32_t fid, measurement_utils::Units units, uint16_t forward, + uint16_t backward = kInvalidSpeed) noexcept; /// \note operator==() and operator<() do not correspond to each other. bool operator==(FeatureMaxspeed const & rhs) const; bool operator<(FeatureMaxspeed const & rhs) const { return m_featureId < rhs.m_featureId; } - bool IsValid() const { return m_forward.IsValid(); } - bool IsBidirectional() const { return IsValid() && m_backward.IsValid(); } + bool IsValid() const { return m_forward != kInvalidSpeed; } + measurement_utils::Units GetUnits() const { return m_units; } + bool IsBidirectional() const { return IsValid() && m_backward != kInvalidSpeed; } uint32_t GetFeatureId() const { return m_featureId; } - SpeedInUnits const & GetForward() const { return m_forward; } - SpeedInUnits const & GetBackward() const { return m_backward; } + uint16_t const & GetForward() const { return m_forward; } + uint16_t const & GetBackward() const { return m_backward; } private: uint32_t m_featureId = 0; - SpeedInUnits m_forward; - SpeedInUnits m_backward; + measurement_utils::Units m_units = measurement_utils::Units::Metric; + uint16_t m_forward = kInvalidSpeed; // Speed in km per hour or mile per hour depends on |m_units|. + uint16_t m_backward = kInvalidSpeed; // Speed in km per hour or mile per hour depends on |m_units|. }; class MaxspeedConverter diff --git a/routing/maxspeed_serialization.hpp b/routing/maxspeed_serialization.hpp index 6d745885b2..b557050e99 100644 --- a/routing/maxspeed_serialization.hpp +++ b/routing/maxspeed_serialization.hpp @@ -31,10 +31,9 @@ public: for (auto const & s : speeds) { WriteToSink(sink, s.GetFeatureId()); - WriteToSink(sink, s.GetForward().m_speed); - WriteToSink(sink, static_cast(s.GetForward().m_units)); - WriteToSink(sink, s.GetBackward().m_speed); - WriteToSink(sink, static_cast(s.GetBackward().m_units)); + WriteToSink(sink, static_cast(s.GetUnits())); + WriteToSink(sink, s.GetForward()); + WriteToSink(sink, s.GetBackward()); } } @@ -48,16 +47,11 @@ public: for (size_t i = 0; i < header.GetSize(); ++i) { auto const fid = ReadPrimitiveFromSource(src); + auto const units = static_cast(ReadPrimitiveFromSource(src)); + auto const forwardSpeed = ReadPrimitiveFromSource(src); + auto const backwardSpeed = ReadPrimitiveFromSource(src); - SpeedInUnits forward; - forward.m_speed = ReadPrimitiveFromSource(src); - forward.m_units = static_cast(ReadPrimitiveFromSource(src)); - - SpeedInUnits backward; - backward.m_speed = ReadPrimitiveFromSource(src); - backward.m_units = static_cast(ReadPrimitiveFromSource(src)); - - speeds.emplace_back(fid, forward, backward); + speeds.emplace_back(fid, units, forwardSpeed, backwardSpeed); } } diff --git a/routing/routing_tests/maxspeed_tests.cpp b/routing/routing_tests/maxspeed_tests.cpp index c6a86b1981..c92f1307cf 100644 --- a/routing/routing_tests/maxspeed_tests.cpp +++ b/routing/routing_tests/maxspeed_tests.cpp @@ -83,58 +83,52 @@ UNIT_TEST(MaxspeedSerializer_Smoke) UNIT_TEST(MaxspeedSerializer_OneForwardMetric) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(20 /* speed */, Units::Metric))}); + {FeatureMaxspeed(0 /* feature id */, Units::Metric, 20 /* speed */)}); } UNIT_TEST(MaxspeedSerializer_OneNone) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(kNoneMaxSpeed, Units::Metric))}); + {FeatureMaxspeed(0 /* feature id */, Units::Metric, kNoneMaxSpeed)}); } UNIT_TEST(MaxspeedSerializer_OneWalk) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(kWalkMaxSpeed, Units::Metric))}); + {FeatureMaxspeed(0 /* feature id */, Units::Metric, kWalkMaxSpeed)}); } UNIT_TEST(MaxspeedSerializer_OneBidirectionalMetric_1) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(20 /* speed */, Units::Metric), - SpeedInUnits(40 /* speed */, Units::Metric))}); + {FeatureMaxspeed(0 /* feature id */, Units::Metric, 20 /* speed */, 40 /* speed */)}); } UNIT_TEST(MaxspeedSerializer_OneBidirectionalMetric_2) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(10 /* speed */, Units::Metric), - SpeedInUnits(kWalkMaxSpeed, Units::Metric))}); + {FeatureMaxspeed(0 /* feature id */, Units::Metric, 10 /* speed */, kWalkMaxSpeed)}); } UNIT_TEST(MaxspeedSerializer_OneBidirectionalImperial) { TestMaxspeedSerialization( - {FeatureMaxspeed(0 /* feature id */, SpeedInUnits(30 /* speed */, Units::Imperial), - SpeedInUnits(50 /* speed */, Units::Imperial))}); + {FeatureMaxspeed(0 /* feature id */, Units::Imperial, 30 /* speed */, 50 /* speed */)}); } UNIT_TEST(MaxspeedSerializer_BigMetric) { std::vector const maxspeeds = { - FeatureMaxspeed(0 /* feature id */, SpeedInUnits(20 /* speed */, Units::Metric)), - FeatureMaxspeed(1 /* feature id */, SpeedInUnits(60 /* speed */, Units::Metric)), - FeatureMaxspeed(4 /* feature id */, SpeedInUnits(90 /* speed */, Units::Metric)), - FeatureMaxspeed(5 /* feature id */, SpeedInUnits(5 /* speed */, Units::Metric)), - FeatureMaxspeed(7 /* feature id */, SpeedInUnits(70 /* speed */, Units::Metric), - SpeedInUnits(90 /* speed */, Units::Metric)), - FeatureMaxspeed(8 /* feature id */, SpeedInUnits(100 /* speed */, Units::Metric)), - FeatureMaxspeed(9 /* feature id */, SpeedInUnits(60 /* speed */, Units::Metric)), - FeatureMaxspeed(10 /* feature id */, SpeedInUnits(kNoneMaxSpeed, Units::Metric)), - FeatureMaxspeed(11 /* feature id */, SpeedInUnits(40 /* speed */, Units::Metric), - SpeedInUnits(50 /* speed */, Units::Metric)), - FeatureMaxspeed(12 /* feature id */, SpeedInUnits(40 /* speed */, Units::Metric), - SpeedInUnits(60 /* speed */, Units::Metric)), + {0 /* feature id */, Units::Metric, 20 /* speed */}, + {1 /* feature id */, Units::Metric, 60 /* speed */}, + {4 /* feature id */, Units::Metric, 90 /* speed */}, + {5 /* feature id */, Units::Metric, 5 /* speed */}, + {7 /* feature id */, Units::Metric, 70 /* speed */, 90 /* speed */}, + {8 /* feature id */, Units::Metric, 100 /* speed */}, + {9 /* feature id */, Units::Metric, 60 /* speed */}, + {10 /* feature id */, Units::Metric, kNoneMaxSpeed}, + {11 /* feature id */, Units::Metric, 40 /* speed */, 50 /* speed */}, + {12 /* feature id */, Units::Metric, 40 /* speed */, 60 /* speed */}, }; TestMaxspeedSerialization(maxspeeds); } @@ -142,18 +136,15 @@ UNIT_TEST(MaxspeedSerializer_BigMetric) UNIT_TEST(MaxspeedSerializer_BigImperial) { std::vector const maxspeeds = { - FeatureMaxspeed(0 /* feature id */, SpeedInUnits(30 /* speed */, Units::Imperial)), - FeatureMaxspeed(1 /* feature id */, SpeedInUnits(5 /* speed */, Units::Imperial)), - FeatureMaxspeed(4 /* feature id */, SpeedInUnits(1 /* speed */, Units::Imperial)), - FeatureMaxspeed(5 /* feature id */, SpeedInUnits(5 /* speed */, Units::Imperial)), - FeatureMaxspeed(7 /* feature id */, SpeedInUnits(30 /* speed */, Units::Imperial), - SpeedInUnits(50 /* speed */, Units::Imperial)), - FeatureMaxspeed(8 /* feature id */, SpeedInUnits(70 /* speed */, Units::Imperial)), - FeatureMaxspeed(9 /* feature id */, SpeedInUnits(50 /* speed */, Units::Imperial)), - FeatureMaxspeed(10 /* feature id */, SpeedInUnits(40 /* speed */, Units::Imperial), - SpeedInUnits(50 /* speed */, Units::Metric)), - FeatureMaxspeed(11 /* feature id */, SpeedInUnits(30 /* speed */, Units::Imperial), - SpeedInUnits(50 /* speed */, Units::Metric)), + {0 /* feature id */, Units::Imperial, 30 /* speed */}, + {1 /* feature id */, Units::Imperial, 5 /* speed */}, + {4 /* feature id */, Units::Imperial, 1 /* speed */}, + {5 /* feature id */, Units::Imperial, 5 /* speed */}, + {7 /* feature id */, Units::Imperial, 30 /* speed */, 50 /* speed */}, + {8 /* feature id */, Units::Imperial, 70 /* speed */}, + {9 /* feature id */, Units::Imperial, 50 /* speed */}, + {10 /* feature id */, Units::Imperial, 40 /* speed */, 50 /* speed */}, + {11 /* feature id */, Units::Imperial, 30 /* speed */, 50 /* speed */}, }; TestMaxspeedSerialization(maxspeeds); }