From d73fc8b5e49538db4b6f054c19f0979161cbda59 Mon Sep 17 00:00:00 2001 From: vng Date: Thu, 28 Jan 2016 16:39:40 +0300 Subject: [PATCH] [search] Limited ReverseGeocoder with single MWM while matching streets. --- generator/search_index_builder.cpp | 6 ++- search/reverse_geocoder.cpp | 56 +++++++++++++++------------- search/reverse_geocoder.hpp | 20 ++++++---- search/v2/features_layer_matcher.cpp | 24 +++++++++--- search/v2/features_layer_matcher.hpp | 51 ++++++++++--------------- search/v2/geocoder.cpp | 20 +++++----- search/v2/house_to_street_table.hpp | 2 +- search/v2/mwm_context.cpp | 6 +-- search/v2/mwm_context.hpp | 35 +++++++++++++++-- search/v2/street_vicinity_loader.cpp | 2 +- search/v2/street_vicinity_loader.hpp | 5 ++- 11 files changed, 135 insertions(+), 92 deletions(-) diff --git a/generator/search_index_builder.cpp b/generator/search_index_builder.cpp index 5556d8d586..46c7c54dbb 100644 --- a/generator/search_index_builder.cpp +++ b/generator/search_index_builder.cpp @@ -318,7 +318,8 @@ void BuildAddressTable(FilesContainerR & container, Writer & writer) Index mwmIndex; /// @ todo Make some better solution, or legalize MakeTemporary. - mwmIndex.RegisterMap(platform::LocalCountryFile::MakeTemporary(container.GetFileName())); + auto const mwmId = mwmIndex.RegisterMap(platform::LocalCountryFile::MakeTemporary(container.GetFileName())); + ASSERT_EQUAL(mwmId.second, MwmSet::RegResult::Success, ()); search::ReverseGeocoder rgc(mwmIndex); { @@ -338,10 +339,11 @@ void BuildAddressTable(FilesContainerR & container, Writer & writer) { FeatureType ft; features.GetVector().GetByIndex(index, ft); + ft.SetID(FeatureID(mwmId.first, index)); using TStreet = search::ReverseGeocoder::Street; vector streets; - rgc.GetNearbyStreets(feature::GetCenter(ft), streets); + rgc.GetNearbyStreets(ft, streets); streetIndex = rgc.GetMatchedStreetIndex(street, streets); if (streetIndex < streets.size()) diff --git a/search/reverse_geocoder.cpp b/search/reverse_geocoder.cpp index c15ebb11ff..1076667e6a 100644 --- a/search/reverse_geocoder.cpp +++ b/search/reverse_geocoder.cpp @@ -2,6 +2,7 @@ #include "search_string_utils.hpp" #include "search/v2/house_to_street_table.hpp" +#include "search/v2/mwm_context.hpp" #include "indexer/feature.hpp" #include "indexer/feature_algo.hpp" @@ -26,17 +27,18 @@ double const ReverseGeocoder::kLookupRadiusM = 500.0; ReverseGeocoder::ReverseGeocoder(Index const & index) : m_index(index) {} -void ReverseGeocoder::GetNearbyStreets(m2::PointD const & center, vector & streets) const +void ReverseGeocoder::GetNearbyStreets(MwmSet::MwmId id, m2::PointD const & center, + vector & streets) const { m2::RectD const rect = GetLookupRect(center, kLookupRadiusM); - auto const addStreet = [&](FeatureType const & ft) + auto const addStreet = [&](FeatureType & ft) { - if (ft.GetFeatureType() != feature::GEOM_LINE) - return; - - if (!ftypes::IsStreetChecker::Instance()(ft)) + if (ft.GetFeatureType() != feature::GEOM_LINE || + !ftypes::IsStreetChecker::Instance()(ft)) + { return; + } string name; static int8_t const lang = StringUtf8Multilang::GetLangIndex("default"); @@ -48,8 +50,18 @@ void ReverseGeocoder::GetNearbyStreets(m2::PointD const & center, vector streets.push_back({ft.GetID(), feature::GetMinDistanceMeters(ft, center), name}); }; - m_index.ForEachInRect(addStreet, rect, kQueryScale); - sort(streets.begin(), streets.end(), my::CompareBy(&Street::m_distanceMeters)); + MwmSet::MwmHandle mwmHandle = m_index.GetMwmHandleById(id); + if (mwmHandle.IsAlive()) + { + search::v2::MwmContext(move(mwmHandle)).ForEachFeature(rect, addStreet); + sort(streets.begin(), streets.end(), my::CompareBy(&Street::m_distanceMeters)); + } +} + +void ReverseGeocoder::GetNearbyStreets(FeatureType & ft, vector & streets) const +{ + ASSERT(ft.GetID().IsValid(), ()); + GetNearbyStreets(ft.GetID().m_mwmId, feature::GetCenter(ft), streets); } // static @@ -108,7 +120,7 @@ void ReverseGeocoder::GetNearbyAddress(m2::PointD const & center, Address & addr table = search::v2::HouseToStreetTable::Load(*mwmHandle.GetValue()); } - GetNearbyStreets(b.m_center, streets); + GetNearbyStreets(b.m_id.m_mwmId, b.m_center, streets); uint32_t ind; if (table->Get(b.m_id.m_index, ind) && ind < streets.size()) @@ -120,8 +132,8 @@ void ReverseGeocoder::GetNearbyAddress(m2::PointD const & center, Address & addr } } -pair, uint32_t> ReverseGeocoder::GetNearbyFeatureStreets( - FeatureType const & feature) const +pair, uint32_t> +ReverseGeocoder::GetNearbyFeatureStreets(FeatureType const & feature) const { pair, uint32_t> result; auto & streetIndex = result.second; @@ -131,33 +143,24 @@ pair, uint32_t> ReverseGeocoder::GetNearbyFeatur MwmSet::MwmHandle const mwmHandle = m_index.GetMwmHandleById(fid.m_mwmId); if (!mwmHandle.IsAlive()) { - LOG(LERROR, ("Feature handle is not alive", feature)); + LOG(LWARNING, ("MWM for", feature, "is dead")); return result; } auto & streets = result.first; - GetNearbyStreets(feature::GetCenter(feature), streets); + GetNearbyStreets(const_cast(feature), streets); unique_ptr const table = search::v2::HouseToStreetTable::Load(*mwmHandle.GetValue()); if (table->Get(fid.m_index, streetIndex) && streetIndex >= streets.size()) - LOG(LERROR, ("Critical reverse geocoder error, returned", streetIndex, "for", feature)); + LOG(LWARNING, ("Out of bound index", streetIndex, "for", feature)); return result; } void ReverseGeocoder::GetNearbyBuildings(m2::PointD const & center, vector & buildings) const { - GetNearbyBuildings(center, kLookupRadiusM, buildings); -} - -void ReverseGeocoder::GetNearbyBuildings(m2::PointD const & center, double radiusM, - vector & buildings) const -{ - // Seems like a copy-paste here of the GetNearbyStreets function. - // Trying to factor out common logic will cause many variables logic. - - m2::RectD const rect = GetLookupRect(center, radiusM); + m2::RectD const rect = GetLookupRect(center, kLookupRadiusM); auto const addBuilding = [&](FeatureType const & ft) { @@ -169,8 +172,8 @@ void ReverseGeocoder::GetNearbyBuildings(m2::PointD const & center, double radiu if (number.empty()) return; - buildings.push_back( - {ft.GetID(), feature::GetMinDistanceMeters(ft, center), number, feature::GetCenter(ft)}); + buildings.push_back({ft.GetID(), feature::GetMinDistanceMeters(ft, center), + number, feature::GetCenter(ft)}); }; m_index.ForEachInRect(addBuilding, rect, kQueryScale); @@ -182,4 +185,5 @@ m2::RectD ReverseGeocoder::GetLookupRect(m2::PointD const & center, double radiu { return MercatorBounds::RectByCenterXYAndSizeInMeters(center, radiusM); } + } // namespace search diff --git a/search/reverse_geocoder.hpp b/search/reverse_geocoder.hpp index 656620ba39..18229f62c9 100644 --- a/search/reverse_geocoder.hpp +++ b/search/reverse_geocoder.hpp @@ -33,6 +33,7 @@ class ReverseGeocoder }; public: + /// All "Nearby" functions work in this lookup radius. static double const kLookupRadiusM; explicit ReverseGeocoder(Index const & index); @@ -65,17 +66,22 @@ public: double GetDistance() const { return m_building.m_distanceMeters; } }; - void GetNearbyStreets(m2::PointD const & center, vector & streets) const; + /// @return Sorted by distance streets vector for the specified MwmId. + //@{ + void GetNearbyStreets(MwmSet::MwmId id, m2::PointD const & center, + vector & streets) const; + void GetNearbyStreets(FeatureType & ft, vector & streets) const; + //@} - void GetNearbyAddress(m2::PointD const & center, Address & addr) const; - - /// @returns street segments (can be duplicate names) sorted by distance to feature's center. - /// uint32_t, if less than vector.size(), contains index of exact feature's street specified in OSM data. + /// @todo Leave const reference for now to support client's legacy code. + /// It's better to use honest non-const reference when feature can be modified in any way. pair, uint32_t> GetNearbyFeatureStreets(FeatureType const & feature) const; - void GetNearbyBuildings(m2::PointD const & center, vector & buildings) const; + /// @return The nearest exact address where building has house number and valid street match. + void GetNearbyAddress(m2::PointD const & center, Address & addr) const; - void GetNearbyBuildings(m2::PointD const & center, double radiusM, vector & buildings) const; + /// @return Sorted by distance houses vector with valid house number. + void GetNearbyBuildings(m2::PointD const & center, vector & buildings) const; private: static m2::RectD GetLookupRect(m2::PointD const & center, double radiusM); diff --git a/search/v2/features_layer_matcher.cpp b/search/v2/features_layer_matcher.cpp index 25102c56b1..9355431b35 100644 --- a/search/v2/features_layer_matcher.cpp +++ b/search/v2/features_layer_matcher.cpp @@ -65,7 +65,8 @@ uint32_t FeaturesLayerMatcher::GetMatchingStreet(uint32_t houseId, FeatureType & return entry.first; } -vector const & FeaturesLayerMatcher::GetNearbyStreets(uint32_t featureId) +vector const & +FeaturesLayerMatcher::GetNearbyStreets(uint32_t featureId) { auto entry = m_nearbyStreetsCache.Get(featureId); if (!entry.second) @@ -74,21 +75,34 @@ vector const & FeaturesLayerMatcher::GetNearbyStreets(u FeatureType feature; GetByIndex(featureId, feature); - m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), entry.first); + GetNearbyStreetsImpl(feature, entry.first); return entry.first; } -vector const & FeaturesLayerMatcher::GetNearbyStreets( - uint32_t featureId, FeatureType & feature) +vector const & +FeaturesLayerMatcher::GetNearbyStreets(uint32_t featureId, FeatureType & feature) { auto entry = m_nearbyStreetsCache.Get(featureId); if (!entry.second) return entry.first; - m_reverseGeocoder.GetNearbyStreets(feature::GetCenter(feature), entry.first); + GetNearbyStreetsImpl(feature, entry.first); return entry.first; } +void FeaturesLayerMatcher::GetNearbyStreetsImpl(FeatureType & feature, vector & streets) +{ + m_reverseGeocoder.GetNearbyStreets(feature, streets); + for (size_t i = 0; i < streets.size(); ++i) + { + if (streets[i].m_distanceMeters > ReverseGeocoder::kLookupRadiusM) + { + streets.resize(i); + return; + } + } +} + uint32_t FeaturesLayerMatcher::GetMatchingStreetImpl(uint32_t houseId, FeatureType & houseFeature) { auto const & streets = GetNearbyStreets(houseId, houseFeature); diff --git a/search/v2/features_layer_matcher.hpp b/search/v2/features_layer_matcher.hpp index c43379a1fc..2ea2bf50ec 100644 --- a/search/v2/features_layer_matcher.hpp +++ b/search/v2/features_layer_matcher.hpp @@ -158,21 +158,19 @@ private: if (queryTokens.empty()) return; - vector nearbyBuildings; for (size_t i = 0; i < pois.size(); ++i) { - // TODO (@y, @m, @vng): implement a faster version - // ReverseGeocoder::GetNearbyBuildings() for only one (current) - // map. - nearbyBuildings.clear(); - m_reverseGeocoder.GetNearbyBuildings(poiCenters[i], kBuildingRadiusMeters, nearbyBuildings); - for (auto const & building : nearbyBuildings) - { - if (building.m_id.m_mwmId != m_context->m_id || building.m_distanceMeters > kBuildingRadiusMeters) - continue; - if (HouseNumbersMatch(strings::MakeUniString(building.m_name), queryTokens)) - fn(pois[i], building.m_id.m_index); - } + m_context->ForEachFeature( + MercatorBounds::RectByCenterXYAndSizeInMeters(poiCenters[i], kBuildingRadiusMeters), + [&](FeatureType & ft) + { + if (HouseNumbersMatch(strings::MakeUniString(ft.GetHouseNumber()), queryTokens)) + { + double const distanceM = MercatorBounds::DistanceOnEarth(feature::GetCenter(ft), poiCenters[i]); + if (distanceM < kBuildingRadiusMeters) + fn(pois[i], ft.GetID().m_index); + } + }); } } @@ -191,17 +189,8 @@ private: { for (uint32_t poiId : pois) { - // TODO (@y, @m, @vng): implement a faster version - // ReverseGeocoder::GetNearbyStreets() for only one (current) - // map. for (auto const & street : GetNearbyStreets(poiId)) { - if (street.m_id.m_mwmId != m_context->m_id || - street.m_distanceMeters > ReverseGeocoder::kLookupRadiusM) - { - continue; - } - uint32_t const streetId = street.m_id.m_index; if (binary_search(streets.begin(), streets.end(), streetId)) fn(poiId, streetId); @@ -331,20 +320,20 @@ private: // Returns id of a street feature corresponding to a |houseId|, or // kInvalidId if there're not such street. uint32_t GetMatchingStreet(uint32_t houseId); - uint32_t GetMatchingStreet(uint32_t houseId, FeatureType & houseFeature); - - vector const & GetNearbyStreets(uint32_t featureId); - - vector const & GetNearbyStreets(uint32_t featureId, - FeatureType & feature); - uint32_t GetMatchingStreetImpl(uint32_t houseId, FeatureType & houseFeature); + using TStreet = ReverseGeocoder::Street; + + vector const & GetNearbyStreets(uint32_t featureId); + vector const & GetNearbyStreets(uint32_t featureId, + FeatureType & feature); + void GetNearbyStreetsImpl(FeatureType & feature, vector & streets); + inline void GetByIndex(uint32_t id, FeatureType & ft) const { /// @todo Add Cache for feature id -> (point, name / house number). - m_context->m_vector.GetByIndex(id, ft); + m_context->GetFeature(id, ft); } MwmContext * m_context; @@ -353,7 +342,7 @@ private: // Cache of streets in a feature's vicinity. All lists in the cache // are ordered by distance from the corresponding feature. - Cache> m_nearbyStreetsCache; + Cache> m_nearbyStreetsCache; // Cache of correct streets for buildings. Current search algorithm // supports only one street for a building, whereas buildings can be diff --git a/search/v2/geocoder.cpp b/search/v2/geocoder.cpp index 499c3c4b3e..c5a2c35c3b 100644 --- a/search/v2/geocoder.cpp +++ b/search/v2/geocoder.cpp @@ -527,10 +527,10 @@ void Geocoder::GoImpl(vector> & infos, bool inViewport) m_villages = nullptr; }); - auto it = m_matchersCache.find(m_context->m_id); + auto it = m_matchersCache.find(m_context->GetId()); if (it == m_matchersCache.end()) { - it = m_matchersCache.insert(make_pair(m_context->m_id, make_unique( + it = m_matchersCache.insert(make_pair(m_context->GetId(), make_unique( m_index, cancellable))).first; } m_matcher = it->second.get(); @@ -540,7 +540,7 @@ void Geocoder::GoImpl(vector> & infos, bool inViewport) if (inViewport) { viewportCBV = - Retrieval::RetrieveGeometryFeatures(m_context->m_id, m_context->m_value, cancellable, + Retrieval::RetrieveGeometryFeatures(m_context->GetId(), m_context->m_value, cancellable, m_params.m_viewport, m_params.m_scale); } @@ -616,7 +616,7 @@ void Geocoder::PrepareAddressFeatures() { PrepareRetrievalParams(i, i + 1); m_addressFeatures[i] = Retrieval::RetrieveAddressFeatures( - m_context->m_id, m_context->m_value, static_cast(*this), + m_context->GetId(), m_context->m_value, static_cast(*this), m_retrievalParams); ASSERT(m_addressFeatures[i], ()); } @@ -644,7 +644,7 @@ void Geocoder::FillLocalityCandidates(coding::CompressedBitVector const * filter [&](uint32_t featureId) { Locality l; - l.m_countryId = m_context->m_id; + l.m_countryId = m_context->GetId(); l.m_featureId = featureId; l.m_startToken = startToken; l.m_endToken = endToken; @@ -675,7 +675,7 @@ void Geocoder::FillLocalitiesTable() for (auto & l : preLocalities) { FeatureType ft; - m_context->m_vector.GetByIndex(l.m_featureId, ft); + m_context->GetFeature(l.m_featureId, ft); switch (m_model.GetSearchType(ft)) { @@ -1220,7 +1220,7 @@ void Geocoder::FindPaths() ASSERT(result.IsValid(), ()); // TODO(@y, @m, @vng): use rest fields of IntersectionResult for // better scoring. - m_results->emplace_back(m_context->m_id, result.InnermostResult()); + m_results->emplace_back(m_context->GetId(), result.InnermostResult()); }); } @@ -1277,7 +1277,7 @@ unique_ptr Geocoder::LoadCategories( { m_retrievalParams.m_tokens[0][0] = category; auto cbv = Retrieval::RetrieveAddressFeatures( - context.m_id, context.m_value, static_cast(*this), + context.GetId(), context.m_value, static_cast(*this), m_retrievalParams); if (!coding::CompressedBitVector::IsEmpty(cbv)) cbvs.push_back(move(cbv)); @@ -1323,14 +1323,14 @@ coding::CompressedBitVector const * Geocoder::RetrieveGeometryFeatures(MwmContex /// - Implement more smart strategy according to id. /// - Move all rect limits here - auto & features = m_geometryFeatures[context.m_id]; + auto & features = m_geometryFeatures[context.GetId()]; for (auto const & v : features) { if (v.m_rect.IsRectInside(rect)) return v.m_cbv.get(); } - auto cbv = Retrieval::RetrieveGeometryFeatures(context.m_id, context.m_value, + auto cbv = Retrieval::RetrieveGeometryFeatures(context.GetId(), context.m_value, static_cast(*this), rect, m_params.m_scale); diff --git a/search/v2/house_to_street_table.hpp b/search/v2/house_to_street_table.hpp index a92a3fa4d4..4d991c70f1 100644 --- a/search/v2/house_to_street_table.hpp +++ b/search/v2/house_to_street_table.hpp @@ -14,7 +14,7 @@ class HouseToStreetTable public: virtual ~HouseToStreetTable() = default; - /// @todo Actually, value may be nullptr in the very common case . + /// @todo Actually, value may be nullptr in the very common case. /// It's better to construct a table from MwmHandle. static unique_ptr Load(MwmValue & value); diff --git a/search/v2/mwm_context.cpp b/search/v2/mwm_context.cpp index c574fe0d32..b6b6a053c6 100644 --- a/search/v2/mwm_context.cpp +++ b/search/v2/mwm_context.cpp @@ -1,6 +1,5 @@ #include "search/v2/mwm_context.hpp" -#include "indexer/index.hpp" namespace search { @@ -9,14 +8,11 @@ namespace v2 MwmContext::MwmContext(MwmSet::MwmHandle handle) : m_handle(move(handle)) , m_value(*m_handle.GetValue()) - , m_id(m_handle.GetId()) , m_vector(m_value.m_cont, m_value.GetHeader(), m_value.m_table) , m_index(m_value.m_cont.GetReader(INDEX_FILE_TAG), m_value.m_factory) { } -string const & MwmContext::GetName() const { return m_id.GetInfo()->GetCountryName(); } - -shared_ptr const & MwmContext::GetInfo() const { return m_id.GetInfo(); } +shared_ptr const & MwmContext::GetInfo() const { return GetId().GetInfo(); } } // namespace v2 } // namespace search diff --git a/search/v2/mwm_context.hpp b/search/v2/mwm_context.hpp index 2285400671..d06128a67f 100644 --- a/search/v2/mwm_context.hpp +++ b/search/v2/mwm_context.hpp @@ -1,7 +1,7 @@ #pragma once #include "indexer/features_vector.hpp" -#include "indexer/mwm_set.hpp" +#include "indexer/index.hpp" #include "indexer/scale_index.hpp" #include "base/macros.hpp" @@ -18,13 +18,42 @@ struct MwmContext MwmSet::MwmHandle m_handle; MwmValue & m_value; - MwmSet::MwmId const & m_id; FeaturesVector m_vector; ScaleIndex m_index; - string const & GetName() const; + inline MwmSet::MwmId const & GetId() const { return m_handle.GetId(); } + inline string const & GetName() const { return GetInfo()->GetCountryName(); } shared_ptr const & GetInfo() const; + template void ForEachFeature(m2::RectD const & rect, TFn && fn) + { + feature::DataHeader const & header = m_value.GetHeader(); + covering::CoveringGetter covering(rect, covering::ViewportWithLowLevels); + + uint32_t const scale = header.GetLastScale(); + covering::IntervalsT const & interval = covering.Get(scale); + + CheckUniqueIndexes checkUnique(header.GetFormat() >= version::Format::v5); + for (auto const & i : interval) + { + m_index.ForEachInIntervalAndScale([&](uint32_t index) + { + if (checkUnique(index)) + { + FeatureType ft; + GetFeature(index, ft); + fn(ft); + } + }, i.first, i.second, scale); + } + } + + inline void GetFeature(uint32_t index, FeatureType & ft) const + { + m_vector.GetByIndex(index, ft); + ft.SetID(FeatureID(GetId(), index)); + } + DISALLOW_COPY_AND_MOVE(MwmContext); }; } // namespace v2 diff --git a/search/v2/street_vicinity_loader.cpp b/search/v2/street_vicinity_loader.cpp index b89aa6b0e5..331b8eddcf 100644 --- a/search/v2/street_vicinity_loader.cpp +++ b/search/v2/street_vicinity_loader.cpp @@ -49,7 +49,7 @@ StreetVicinityLoader::Street const & StreetVicinityLoader::GetStreet(uint32_t fe void StreetVicinityLoader::LoadStreet(uint32_t featureId, Street & street) { FeatureType feature; - m_context->m_vector.GetByIndex(featureId, feature); + m_context->GetFeature(featureId, feature); if (feature.GetFeatureType() != feature::GEOM_LINE) return; diff --git a/search/v2/street_vicinity_loader.hpp b/search/v2/street_vicinity_loader.hpp index f59698babd..64e57de815 100644 --- a/search/v2/street_vicinity_loader.hpp +++ b/search/v2/street_vicinity_loader.hpp @@ -37,6 +37,9 @@ public: m2::RectD m_rect; unique_ptr m_calculator; + /// @todo Cache GetProjection results for features here, because + /// feature::GetCenter and ProjectionOnStreetCalculator::GetProjection are not so fast. + DISALLOW_COPY(Street); }; @@ -61,7 +64,7 @@ public: continue; FeatureType ft; - m_context->m_vector.GetByIndex(id, ft); + m_context->GetFeature(id, ft); if (!calculator.GetProjection(feature::GetCenter(ft, FeatureType::WORST_GEOMETRY), proj)) continue;