From b7bd733b679260bbeff0198d06d03de11a9d301d Mon Sep 17 00:00:00 2001 From: Vladimir Byko-Ianko Date: Wed, 22 Nov 2017 17:10:28 +0300 Subject: [PATCH] Review fixes. --- defines.hpp | 2 +- generator/generator_tool/generator_tool.cpp | 3 ++- generator/routing_index_generator.cpp | 21 ++++++++++----- routing/cross_mwm_connector.hpp | 19 ++++++------- routing/cross_mwm_connector_serialization.hpp | 27 ++++++++++--------- routing_common/transit_types.cpp | 2 +- 6 files changed, 42 insertions(+), 32 deletions(-) diff --git a/defines.hpp b/defines.hpp index 358cf9136d..70de66e7f6 100644 --- a/defines.hpp +++ b/defines.hpp @@ -41,8 +41,8 @@ // Temporary addresses section that is used in search index generation. #define SEARCH_TOKENS_FILE_TAG "addrtags" #define TRAFFIC_KEYS_FILE_TAG "traffic" -#define TRANSIT_FILE_TAG "transit" #define TRANSIT_CROSS_MWM_FILE_TAG "transit_cross_mwm" +#define TRANSIT_FILE_TAG "transit" #define UGC_FILE_TAG "ugc" #define ROUTING_MATRIX_FILE_TAG "mercedes" diff --git a/generator/generator_tool/generator_tool.cpp b/generator/generator_tool/generator_tool.cpp index 1e2bcd0aed..15b4719e86 100644 --- a/generator/generator_tool/generator_tool.cpp +++ b/generator/generator_tool/generator_tool.cpp @@ -21,6 +21,8 @@ #include "generator/ugc_section_builder.hpp" #include "generator/unpack_mwm.hpp" +#include "routing/cross_mwm_ids.hpp" + #include "indexer/classificator.hpp" #include "indexer/classificator_loader.hpp" #include "indexer/data_header.hpp" @@ -43,7 +45,6 @@ #include #include -#include #include "defines.hpp" diff --git a/generator/routing_index_generator.cpp b/generator/routing_index_generator.cpp index b16c0ee8d6..e3d4575a14 100644 --- a/generator/routing_index_generator.cpp +++ b/generator/routing_index_generator.cpp @@ -209,8 +209,9 @@ double CalcDistanceAlongTheBorders(vector const & borders, return distance; } -/// \brief Fills |transitions| for osm id case. That means for VehicleType::Pedestrian, -/// VehicleType::Bicycle and VehicleType::Car. +/// \brief Fills |transitions| for osm id case. That means |Transition::m_roadMask| for items in +/// |transitions| will be combination of |VehicleType::Pedestrian|, |VehicleType::Bicycle| +/// and |VehicleType::Car|. void CalcCrossMwmTransitions( string const & mwmFile, string const & mappingFile, vector const & borders, string const & country, CountryParentNameGetterFn const & countryParentNameGetterFn, @@ -254,7 +255,8 @@ void CalcCrossMwmTransitions( }); } -/// \brief Fills |transitions| for transit id case. That means for VehicleType::Transit. +/// \brief Fills |transitions| for transit case. That means Transition::m_roadMask for items in +/// |transitions| will be equal to VehicleType::Transit after call of this method. void CalcCrossMwmTransitions(string const & mwmFile, string const & mappingFile, vector const & borders, string const & country, CountryParentNameGetterFn const & countryParentNameGetterFn, @@ -265,7 +267,7 @@ void CalcCrossMwmTransitions(string const & mwmFile, string const & mappingFile, // @todo(bykoianko) Filling |transitions| based on transit section should be implemented. } -/// \brief Fills |transitions| and |connectors| fields. +/// \brief Fills |transitions| and |connectors| params. /// \note This method fills only |connections| which are applicable for |CrossMwmId|. /// For example |VehicleType::Pedestrian|, |VehicleType::Bicycle| and |VehicleType::Car| /// are applicable for |connector::OsmId|. @@ -319,7 +321,7 @@ void CalcCrossMwmConnectors( for (size_t i = 0; i < connectors.size(); ++i) { auto const vehicleType = static_cast(i); - CrossMwmConnector const & connector = connectors[i]; + auto const & connector = connectors[i]; LOG(LINFO, (vehicleType, "model: enters:", connector.GetEnters().size(), ", exits:", connector.GetExits().size())); } @@ -431,7 +433,7 @@ bool BuildRoutingIndex(string const & filename, string const & country, /// \brief Serializes all the cross mwm information to |sectionName| of |mwmFile| including: /// * header /// * transitions -/// * weight buffers if there are +/// * weight buffers if any template void SerializeCrossMwm(string const & mwmFile, string const & sectionName, CrossMwmConnectorPerVehicleType const & connectors, @@ -439,7 +441,7 @@ void SerializeCrossMwm(string const & mwmFile, string const & sectionName, { serial::CodingParams const codingParams = LoadCodingParams(mwmFile); FilesContainerW cont(mwmFile, FileWriter::OP_WRITE_EXISTING); - FileWriter writer = cont.GetWriter(sectionName); + auto writer = cont.GetWriter(sectionName); auto const startPos = writer.Pos(); CrossMwmConnectorSerializer::Serialize(transitions, connectors, codingParams, writer); auto const sectionSize = writer.Pos() - startPos; @@ -465,6 +467,7 @@ void BuildRoutingCrossMwmSection(string const & path, string const & mwmFile, FillWeights(path, mwmFile, country, countryParentNameGetterFn, disableCrossMwmProgress, connectors[static_cast(VehicleType::Car)]); + CHECK(connectors[static_cast(VehicleType::Transit)].IsEmpty(), ()); SerializeCrossMwm(mwmFile, CROSS_MWM_FILE_TAG, connectors, transitions); } @@ -480,6 +483,10 @@ void BuildTransitCrossMwmSection(string const & path, string const & mwmFile, CalcCrossMwmConnectors(path, mwmFile, country, countryParentNameGetterFn, "" /* mapping file */, transitions, connectors); + + CHECK(connectors[static_cast(VehicleType::Pedestrian)].IsEmpty(), ()); + CHECK(connectors[static_cast(VehicleType::Bicycle)].IsEmpty(), ()); + CHECK(connectors[static_cast(VehicleType::Car)].IsEmpty(), ()); SerializeCrossMwm(mwmFile, TRANSIT_CROSS_MWM_FILE_TAG, connectors, transitions); } } // namespace routing diff --git a/routing/cross_mwm_connector.hpp b/routing/cross_mwm_connector.hpp index cc9629112b..5862462bfe 100644 --- a/routing/cross_mwm_connector.hpp +++ b/routing/cross_mwm_connector.hpp @@ -19,7 +19,7 @@ namespace routing { namespace connector { -uint32_t constexpr kFakeId = std::numeric_limits::max(); +uint32_t constexpr kFakeIndex = std::numeric_limits::max(); double constexpr kNoRoute = 0.0; using Weight = uint32_t; @@ -46,8 +46,8 @@ public: bool forwardIsEnter, m2::PointD const & backPoint, m2::PointD const & frontPoint) { - Transition transition(connector::kFakeId, connector::kFakeId, crossMwmId, oneWay, - forwardIsEnter, backPoint, frontPoint); + Transition transition(connector::kFakeIndex, connector::kFakeIndex, crossMwmId, + oneWay, forwardIsEnter, backPoint, frontPoint); if (forwardIsEnter) { @@ -103,15 +103,15 @@ public: } // returns nullptr if there is no transition for such osm id. - Segment const * GetTransition(CrossMwmId crossMwmId, uint32_t segmentIdx, bool isEnter) const + Segment const * GetTransition(CrossMwmId const & crossMwmId, uint32_t segmentIdx, bool isEnter) const { - auto fIt = m_crossMwmIdToFeatureId.find(crossMwmId); + auto const fIt = m_crossMwmIdToFeatureId.find(crossMwmId); if (fIt == m_crossMwmIdToFeatureId.cend()) return nullptr; uint32_t const featureId = fIt->second; - auto tIt = m_transitions.find(Key(featureId, segmentIdx)); + auto const tIt = m_transitions.find(Key(featureId, segmentIdx)); if (tIt == m_transitions.cend()) return nullptr; @@ -142,7 +142,7 @@ public: auto const & transition = GetTransition(segment); if (isOutgoing) { - ASSERT_NOT_EQUAL(transition.m_enterIdx, connector::kFakeId, ()); + ASSERT_NOT_EQUAL(transition.m_enterIdx, connector::kFakeIndex, ()); for (size_t exitIdx = 0; exitIdx < m_exits.size(); ++exitIdx) { auto const weight = GetWeight(base::asserted_cast(transition.m_enterIdx), exitIdx); @@ -151,7 +151,7 @@ public: } else { - ASSERT_NOT_EQUAL(transition.m_exitIdx, connector::kFakeId, ()); + ASSERT_NOT_EQUAL(transition.m_exitIdx, connector::kFakeIndex, ()); for (size_t enterIdx = 0; enterIdx < m_enters.size(); ++enterIdx) { auto const weight = GetWeight(enterIdx, base::asserted_cast(transition.m_exitIdx)); @@ -176,6 +176,7 @@ public: } bool HasWeights() const { return !m_weights.empty(); } + bool IsEmpty() const { return m_enters.empty() && m_exits.empty(); } bool WeightsWereLoaded() const { @@ -252,7 +253,7 @@ private: uint32_t m_enterIdx = 0; uint32_t m_exitIdx = 0; - CrossMwmIdInner m_crossMwmId = CrossMwmIdInner(); + CrossMwmIdInner m_crossMwmId = {}; // Endpoints of transition segment. // m_backPoint = points[segmentIdx] // m_frontPoint = points[segmentIdx + 1] diff --git a/routing/cross_mwm_connector_serialization.hpp b/routing/cross_mwm_connector_serialization.hpp index 362b6f86ad..a62f7c12e2 100644 --- a/routing/cross_mwm_connector_serialization.hpp +++ b/routing/cross_mwm_connector_serialization.hpp @@ -19,6 +19,7 @@ #include #include +#include #include namespace routing @@ -30,8 +31,8 @@ using CrossMwmConnectorPerVehicleType = class CrossMwmConnectorSerializer final { public: - static uint8_t constexpr stopIdBits = 64; - static uint8_t constexpr lineIdBits = 32; + static uint8_t constexpr kStopIdBits = 64; + static uint8_t constexpr kLineIdBits = 32; template class Transition final @@ -54,17 +55,17 @@ public: } template - void WriteCrossMwmId(connector::OsmId const & id, uint8_t n, BitWriter & w) const + void WriteCrossMwmId(connector::OsmId const & id, uint8_t bits, BitWriter & w) const { - w.WriteAtMost64Bits(id.Get(), n); + w.WriteAtMost64Bits(id.Get(), bits); } template void WriteCrossMwmId(connector::TransitId const & id, uint8_t /* n */, BitWriter & w) const { - w.WriteAtMost64Bits(id.m_stop1Id, stopIdBits); - w.WriteAtMost64Bits(id.m_stop2Id, stopIdBits); - w.WriteAtMost32Bits(id.m_lineId, lineIdBits); + w.WriteAtMost64Bits(id.m_stop1Id, kStopIdBits); + w.WriteAtMost64Bits(id.m_stop2Id, kStopIdBits); + w.WriteAtMost32Bits(id.m_lineId, kLineIdBits); } template @@ -86,9 +87,9 @@ public: template void ReadCrossMwmId(uint8_t /* n */, BitReader & reader, connector::TransitId & readed) { - readed.m_stop1Id = reader.ReadAtMost64Bits(stopIdBits); - readed.m_stop2Id = reader.ReadAtMost64Bits(stopIdBits); - readed.m_lineId = reader.ReadAtMost32Bits(lineIdBits); + readed.m_stop1Id = reader.ReadAtMost64Bits(kStopIdBits); + readed.m_stop2Id = reader.ReadAtMost64Bits(kStopIdBits); + readed.m_lineId = reader.ReadAtMost32Bits(kLineIdBits); }; template @@ -152,7 +153,7 @@ public: for (size_t i = 0; i < connectors.size(); ++i) { - CrossMwmConnector const & connector = connectors[i]; + auto const & connector = connectors[i]; if (connector.m_weights.empty()) continue; @@ -454,6 +455,6 @@ private: static void WriteWeights(std::vector const & weights, std::vector & buffer); }; -static_assert(sizeof(transit::StopId) * 8 == 64, "Wrong transit::StopId size"); -static_assert(sizeof(transit::LineId) * 8 == 32, "Wrong transit::LineId size"); +static_assert(CrossMwmConnectorSerializer::kStopIdBits <= 64, "Wrong kStopIdBits."); +static_assert(CrossMwmConnectorSerializer::kLineIdBits <= 32, "Wrong kLineIdBitsю"); } // namespace routing diff --git a/routing_common/transit_types.cpp b/routing_common/transit_types.cpp index 7172fa1f6f..dade26a2a8 100644 --- a/routing_common/transit_types.cpp +++ b/routing_common/transit_types.cpp @@ -344,7 +344,7 @@ bool Line::IsEqualForTesting(Line const & line) const bool Line::IsValid() const { return m_id != kInvalidLineId && m_color != kInvalidColor && m_networkId != kInvalidNetworkId && - m_stopIds.IsValid(), m_interval != kInvalidWeight; + m_stopIds.IsValid() && m_interval != kInvalidWeight; } // Shape ------------------------------------------------------------------------------------------