From 3d682e80a0d6d934a2f37d4dd0c7a1b38eb3cafc Mon Sep 17 00:00:00 2001 From: zyphlar Date: Thu, 11 Jul 2024 19:24:53 -0700 Subject: [PATCH 1/4] Create logic for different search result address ordering (TODO: place page order) --- search/intermediate_result.cpp | 4 +- search/intermediate_result.hpp | 31 +++++----- search/ranker.cpp | 100 +++++++++++++++++++++------------ search/result.cpp | 1 + search/result.hpp | 1 + search/reverse_geocoder.cpp | 1 + 6 files changed, 85 insertions(+), 53 deletions(-) diff --git a/search/intermediate_result.cpp b/search/intermediate_result.cpp index 2c9a7a8219..a2308e390f 100644 --- a/search/intermediate_result.cpp +++ b/search/intermediate_result.cpp @@ -229,8 +229,8 @@ uint32_t RankerResult::GetBestType(vector const * preferredTypes /* = return m_types.GetBestType(); } -// RankerResult::RegionInfo ------------------------------------------------------------------------ -bool RankerResult::RegionInfo::GetCountryId(storage::CountryInfoGetter const & infoGetter, +// RegionInfo ------------------------------------------------------------------------ +bool RegionInfo::GetCountryId(storage::CountryInfoGetter const & infoGetter, storage::CountryId & countryId) const { if (!m_countryId.empty()) diff --git a/search/intermediate_result.hpp b/search/intermediate_result.hpp index d618699933..8439c6f50f 100644 --- a/search/intermediate_result.hpp +++ b/search/intermediate_result.hpp @@ -95,6 +95,21 @@ private: #endif }; +struct RegionInfo +{ + storage::CountryId m_countryId; + m2::PointD m_point; + + void SetParams(storage::CountryId const & countryId, m2::PointD const & point) + { + m_countryId = countryId; + m_point = point; + } + + bool GetCountryId(storage::CountryInfoGetter const & infoGetter, + storage::CountryId & countryId) const; +}; + // Second result class. Objects are created during reading of features. // Read and fill needed info for ranking and getting final results. class RankerResult @@ -141,6 +156,7 @@ public: bool GetCountryId(storage::CountryInfoGetter const & infoGetter, uint32_t ftype, storage::CountryId & countryId) const; + RegionInfo const & GetRegion() const { return m_region; } bool IsEqualBasic(RankerResult const & r) const; bool IsEqualCommon(RankerResult const & r) const; @@ -157,21 +173,6 @@ private: friend class RankerResultMaker; friend class Ranker; - struct RegionInfo - { - storage::CountryId m_countryId; - m2::PointD m_point; - - void SetParams(storage::CountryId const & countryId, m2::PointD const & point) - { - m_countryId = countryId; - m_point = point; - } - - bool GetCountryId(storage::CountryInfoGetter const & infoGetter, - storage::CountryId & countryId) const; - }; - RegionInfo m_region; feature::TypesHolder m_types; std::string m_str; diff --git a/search/ranker.cpp b/search/ranker.cpp index 3d155a87ba..6fdc7df6af 100644 --- a/search/ranker.cpp +++ b/search/ranker.cpp @@ -299,23 +299,6 @@ ftypes::LocalityType GetLocalityIndex(feature::TypesHolder const & types) UNREACHABLE(); } -// TODO: Format street and house number according to local country's rules. -string FormatStreetAndHouse(ReverseGeocoder::Address const & addr) -{ - return addr.GetStreetName() + ", " + addr.GetHouseNumber(); -} - -// TODO: Share common formatting code for search results and place page. -string FormatFullAddress(ReverseGeocoder::Address const & addr, string const & region) -{ - /// @todo Print "near" for not exact addresses. - /// Add some threshold for addr:interpolation or refactor ReverseGeocoder? - if (addr.GetDistance() != 0) - return region; - - return FormatStreetAndHouse(addr) + (region.empty() ? "" : ", ") + region; -} - } // namespace class RankerResultMaker @@ -494,7 +477,8 @@ private: { string streetName; m_ranker.GetBestMatchName(*streetFeature, streetName); - name = streetName + ", " + addr.GetHouseNumber(); + LOG(LDEBUG, ("WAB LoadFeature", streetName, addr.GetHouseNumber())); + name = streetName + ", " + addr.GetHouseNumber(); /// @todo consider using partialAddress logic below } } } @@ -719,25 +703,79 @@ void Ranker::Finish(bool cancelled) m_emitter.Finish(cancelled); } +/// This is one of the final steps in finishing a result and displaying an address in the GUI +/// So it returns a full string like "City, Street Name, 123, Province, Country" +/// @todo Share common formatting code for search results and place page. Result Ranker::MakeResult(RankerResult const & rankerResult, bool needAddress, bool needHighlighting) const { + // If we don't want to bother with addresses at all, just use the ranker result which is "Main St, 123" Result res(rankerResult.GetCenter(), rankerResult.m_str); if (needAddress) { - string address = GetLocalizedRegionInfoForResult(rankerResult); + string partialAddress = ""; + string fullAddress = ""; + + ReverseGeocoder::Address addr; + string_view city; + string region = GetLocalizedRegionInfoForResult(rankerResult); // Format full address only for suitable results. - if (ftypes::IsAddressObjectChecker::Instance()(rankerResult.GetTypes())) - { - ReverseGeocoder::Address addr; + if (ftypes::IsAddressObjectChecker::Instance()(rankerResult.GetTypes())) { if (!(rankerResult.GetID().IsValid() && m_reverseGeocoder.GetExactAddress(rankerResult.GetID(), addr))) m_reverseGeocoder.GetNearbyAddress(rankerResult.GetCenter(), addr); - - address = FormatFullAddress(addr, address); } - res.SetAddress(std::move(address)); + // Get city when possible + if (ftypes::IsLocalityChecker::Instance().GetType(rankerResult.GetTypes()) == ftypes::LocalityType::None) + { + m_localities.GetLocality(res.GetFeatureCenter(), [&](LocalityItem const & item) + { + item.GetReadableName(city); + }); + } + + /// @todo Print "near" for not exact addresses. + /// Add some threshold for addr:interpolation or refactor ReverseGeocoder? + if (rankerResult.GetRegion().m_countryId.starts_with("US_")) { + // US partial is 123 Main St + if (addr.IsValid() && addr.GetDistance() == 0) partialAddress += addr.GetHouseNumber() + " " + addr.GetStreetName(); + + // US full is 123 Main St, Chicago, Illinois, USA + if (addr.IsValid() && addr.GetDistance() == 0) fullAddress += addr.GetHouseNumber() + " " + addr.GetStreetName(); + if (!city.empty()) { + if(!fullAddress.empty()) fullAddress += ", "; + fullAddress.append(city); + } + if (!region.empty()) + { + if(!fullAddress.empty()) fullAddress += ", "; + fullAddress += region; + } + } + else + { + // OM default partial is Main St, 123 + if (addr.IsValid() && addr.GetDistance() == 0) partialAddress += addr.GetStreetName() + ", " + addr.GetHouseNumber(); + + // OM default full is Chicago, Main St, 123, Illinois, USA + if (!city.empty()) fullAddress.append(city); + + if (addr.IsValid() && addr.GetDistance() == 0) + { + if(!fullAddress.empty()) fullAddress += ", "; + fullAddress += addr.GetStreetName() + ", " + addr.GetHouseNumber(); + } + + if (!region.empty()) + { + if(!fullAddress.empty()) fullAddress += ", "; + fullAddress += region; + } + } + + if(!partialAddress.empty()) res.SetString(std::move(partialAddress)); + res.SetAddress(std::move(fullAddress)); } switch (rankerResult.GetResultType()) @@ -750,16 +788,6 @@ Result Ranker::MakeResult(RankerResult const & rankerResult, bool needAddress, b case RankerResult::Type::Postcode: res.SetType(Result::Type::Postcode); break; } - if (needAddress && ftypes::IsLocalityChecker::Instance().GetType(rankerResult.GetTypes()) == ftypes::LocalityType::None) - { - m_localities.GetLocality(res.GetFeatureCenter(), [&](LocalityItem const & item) - { - string_view city; - if (item.GetReadableName(city)) - res.PrependCity(city); - }); - } - if (needHighlighting) HighlightResult(m_params.m_query.m_tokens, m_params.m_query.m_prefix, res); @@ -768,7 +796,7 @@ Result Ranker::MakeResult(RankerResult const & rankerResult, bool needAddress, b #ifdef SEARCH_USE_PROVENANCE res.SetProvenance(std::move(rankerResult.m_provenance)); #endif - + LOG(LDEBUG, ("WAB MakeResult", res)); return res; } diff --git a/search/result.cpp b/search/result.cpp index 80d8a115b8..9542a2ad62 100644 --- a/search/result.cpp +++ b/search/result.cpp @@ -237,6 +237,7 @@ string DebugPrint(Result const & result) ostringstream os; os << "Result ["; os << "name: " << result.GetString(); + os << ", addr: " << result.GetAddress(); os << ", type: " << readableType; os << ", info: " << DebugPrint(result.GetRankingInfo()); diff --git a/search/result.hpp b/search/result.hpp index 9b2a56b5ac..cae47edfc1 100644 --- a/search/result.hpp +++ b/search/result.hpp @@ -61,6 +61,7 @@ public: Result(m2::PointD const & pt, std::string const & name) : m_center(pt), m_str(name) {} void FromFeature(FeatureID const & id, uint32_t mainType, uint32_t matchedType, Details const & details); + void SetString(std::string && str) { m_str = std::move(str); } void SetAddress(std::string && address) { m_address = std::move(address); } void SetType(Result::Type type) { m_resultType = type; } diff --git a/search/reverse_geocoder.cpp b/search/reverse_geocoder.cpp index f26e886e93..356967ec10 100644 --- a/search/reverse_geocoder.cpp +++ b/search/reverse_geocoder.cpp @@ -331,6 +331,7 @@ string ReverseGeocoder::GetLocalizedRegionAddress(RegionAddress const & addr, addrStr = nameGetter.GetLocalizedFullName(addr.m_countryId); } + LOG(LWARNING, ("WAB LocalizedRegionAddress", addrStr)); return addrStr; } -- 2.45.3 From 14db5277d1ca759d645759303a1601418a05046b Mon Sep 17 00:00:00 2001 From: zyphlar Date: Thu, 11 Jul 2024 23:22:22 -0700 Subject: [PATCH 2/4] Fix place page as well --- map/framework.cpp | 20 ++++++++++---------- map/place_page_info.hpp | 1 + search/reverse_geocoder.cpp | 7 +++++-- search/reverse_geocoder.hpp | 3 ++- 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 80311c6fec..18e5885a1a 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -674,9 +674,6 @@ void Framework::FillInfoFromFeatureType(FeatureType & ft, place_page::Info & inf info.SetFeatureStatus(featureStatus); info.SetLocalizedWifiString(m_stringsBundle.GetString("wifi")); - if (ftypes::IsAddressObjectChecker::Instance()(ft)) - info.SetAddress(GetAddressAtPoint(feature::GetCenter(ft)).FormatAddress()); - info.SetFromFeatureType(ft); FillDescription(ft, info); @@ -691,18 +688,21 @@ void Framework::FillInfoFromFeatureType(FeatureType & ft, place_page::Info & inf // Fill countryId for place page info auto const & types = info.GetTypes(); bool const isState = ftypes::IsStateChecker::Instance()(types); + size_t const level = isState ? 1 : 0; + CountriesVec countries; + CountryId countryId = m_infoGetter->GetRegionCountryId(info.GetMercator()); + GetStorage().GetTopmostNodesFor(countryId, countries, level); + if (countries.size() == 1) + countryId = countries.front(); + if (isState || ftypes::IsCountryChecker::Instance()(types)) { - size_t const level = isState ? 1 : 0; - CountriesVec countries; - CountryId countryId = m_infoGetter->GetRegionCountryId(info.GetMercator()); - GetStorage().GetTopmostNodesFor(countryId, countries, level); - if (countries.size() == 1) - countryId = countries.front(); - info.SetCountryId(countryId); info.SetTopmostCountryIds(std::move(countries)); } + if (ftypes::IsAddressObjectChecker::Instance()(ft)) { + info.SetTitleAndAddress(GetAddressAtPoint(feature::GetCenter(ft)).FormatAddress(countryId)); + } } void Framework::FillApiMarkInfo(ApiMarkPoint const & api, place_page::Info & info) const diff --git a/map/place_page_info.hpp b/map/place_page_info.hpp index 28ce82dd93..d5c1ba1460 100644 --- a/map/place_page_info.hpp +++ b/map/place_page_info.hpp @@ -138,6 +138,7 @@ public: /// UI setters void SetCustomName(std::string const & name); void SetTitlesForBookmark(); + void SetTitleAndAddress(std::string && title) { m_uiTitle = std::move(title); m_address = m_uiTitle; }; void SetCustomNames(std::string const & title, std::string const & subtitle); void SetCustomNameWithCoordinates(m2::PointD const & mercator, std::string const & name); void SetAddress(std::string && address) { m_address = std::move(address); } diff --git a/search/reverse_geocoder.cpp b/search/reverse_geocoder.cpp index 356967ec10..b328b5d0ba 100644 --- a/search/reverse_geocoder.cpp +++ b/search/reverse_geocoder.cpp @@ -371,7 +371,7 @@ std::optional ReverseGeocoder::HouseTable::Get(Featu return res; } -string ReverseGeocoder::Address::FormatAddress() const +string ReverseGeocoder::Address::FormatAddress(storage::CountryId countryId) const { // Check whether we can format address according to the query type // and actual address distance. @@ -381,7 +381,10 @@ string ReverseGeocoder::Address::FormatAddress() const if (m_building.m_distanceMeters > 200.0) return {}; - return Join(m_street.m_name, m_building.m_name); + if (countryId == "United States of America") + return m_building.m_name + " " + m_street.m_name; // WB + else + return Join(m_street.m_name, m_building.m_name); // WB } bool ReverseGeocoder::RegionAddress::IsValid() const diff --git a/search/reverse_geocoder.hpp b/search/reverse_geocoder.hpp index 0fde082dc8..44722075a0 100644 --- a/search/reverse_geocoder.hpp +++ b/search/reverse_geocoder.hpp @@ -92,7 +92,8 @@ public: bool IsValid() const { return m_building.IsValid() && m_street.IsValid(); } // 7 vulica Frunze - std::string FormatAddress() const; + std::string FormatAddress() const { return FormatAddress(storage::CountryId()); }; + std::string FormatAddress(storage::CountryId countryId) const; }; struct RegionAddress -- 2.45.3 From 5be700c9165e75ccbb4aed3dd1461748e425f0ea Mon Sep 17 00:00:00 2001 From: zyphlar Date: Thu, 8 Aug 2024 02:40:39 -0700 Subject: [PATCH 3/4] Run info set and FillDescription again after getting address --- map/framework.cpp | 4 +++- map/place_page_info.hpp | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/map/framework.cpp b/map/framework.cpp index 18e5885a1a..2483fde953 100644 --- a/map/framework.cpp +++ b/map/framework.cpp @@ -701,8 +701,10 @@ void Framework::FillInfoFromFeatureType(FeatureType & ft, place_page::Info & inf info.SetTopmostCountryIds(std::move(countries)); } if (ftypes::IsAddressObjectChecker::Instance()(ft)) { - info.SetTitleAndAddress(GetAddressAtPoint(feature::GetCenter(ft)).FormatAddress(countryId)); + info.SetAddress(GetAddressAtPoint(feature::GetCenter(ft)).FormatAddress(countryId)); // SetTitleAndAddress } + info.SetFromFeatureType(ft); + FillDescription(ft, info); } void Framework::FillApiMarkInfo(ApiMarkPoint const & api, place_page::Info & info) const diff --git a/map/place_page_info.hpp b/map/place_page_info.hpp index d5c1ba1460..8b87151b20 100644 --- a/map/place_page_info.hpp +++ b/map/place_page_info.hpp @@ -138,7 +138,7 @@ public: /// UI setters void SetCustomName(std::string const & name); void SetTitlesForBookmark(); - void SetTitleAndAddress(std::string && title) { m_uiTitle = std::move(title); m_address = m_uiTitle; }; + // void SetTitleAndAddress(std::string && title) { m_uiTitle = std::move(title); m_address = m_uiTitle; }; void SetCustomNames(std::string const & title, std::string const & subtitle); void SetCustomNameWithCoordinates(m2::PointD const & mercator, std::string const & name); void SetAddress(std::string && address) { m_address = std::move(address); } -- 2.45.3 From 32b248b88ef60ee00e561ee2562c946a649b0df7 Mon Sep 17 00:00:00 2001 From: zyphlar Date: Thu, 8 Aug 2024 11:16:34 -0700 Subject: [PATCH 4/4] Move GetCountryId back inside RankerResult --- search/intermediate_result.cpp | 4 ++-- search/intermediate_result.hpp | 30 +++++++++++++++--------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/search/intermediate_result.cpp b/search/intermediate_result.cpp index a2308e390f..2c9a7a8219 100644 --- a/search/intermediate_result.cpp +++ b/search/intermediate_result.cpp @@ -229,8 +229,8 @@ uint32_t RankerResult::GetBestType(vector const * preferredTypes /* = return m_types.GetBestType(); } -// RegionInfo ------------------------------------------------------------------------ -bool RegionInfo::GetCountryId(storage::CountryInfoGetter const & infoGetter, +// RankerResult::RegionInfo ------------------------------------------------------------------------ +bool RankerResult::RegionInfo::GetCountryId(storage::CountryInfoGetter const & infoGetter, storage::CountryId & countryId) const { if (!m_countryId.empty()) diff --git a/search/intermediate_result.hpp b/search/intermediate_result.hpp index 8439c6f50f..383b41aff6 100644 --- a/search/intermediate_result.hpp +++ b/search/intermediate_result.hpp @@ -95,21 +95,6 @@ private: #endif }; -struct RegionInfo -{ - storage::CountryId m_countryId; - m2::PointD m_point; - - void SetParams(storage::CountryId const & countryId, m2::PointD const & point) - { - m_countryId = countryId; - m_point = point; - } - - bool GetCountryId(storage::CountryInfoGetter const & infoGetter, - storage::CountryId & countryId) const; -}; - // Second result class. Objects are created during reading of features. // Read and fill needed info for ranking and getting final results. class RankerResult @@ -123,6 +108,21 @@ public: Postcode }; + struct RegionInfo + { + storage::CountryId m_countryId; + m2::PointD m_point; + + void SetParams(storage::CountryId const & countryId, m2::PointD const & point) + { + m_countryId = countryId; + m_point = point; + } + + bool GetCountryId(storage::CountryInfoGetter const & infoGetter, + storage::CountryId & countryId) const; + }; + /// For Type::Feature and Type::Building. RankerResult(FeatureType & ft, m2::PointD const & center, std::string displayName, std::string const & fileName); -- 2.45.3