diff --git a/indexer/ftypes_matcher.cpp b/indexer/ftypes_matcher.cpp index 123375d17e..27e5fea5b3 100644 --- a/indexer/ftypes_matcher.cpp +++ b/indexer/ftypes_matcher.cpp @@ -702,11 +702,6 @@ IsWifiChecker::IsWifiChecker() m_types.push_back(classif().GetTypeByPath({"internet_access", "wlan"})); } -IsShopChecker::IsShopChecker() : BaseChecker(1) -{ - m_types.push_back(classif().GetTypeByPath({"shop"})); -} - IsEatChecker::IsEatChecker() { // The order should be the same as in "enum class Type" declaration. @@ -716,7 +711,9 @@ IsEatChecker::IsEatChecker() {"amenity", "restaurant"}, {"amenity", "bar"}, {"amenity", "pub"}, - {"amenity", "biergarten"}}; + {"amenity", "biergarten"}, + {"amenity", "food_court"}, + }; Classificator const & c = classif(); // size_t i = 0; diff --git a/indexer/ftypes_matcher.hpp b/indexer/ftypes_matcher.hpp index ceb3ffef54..7832f02bac 100644 --- a/indexer/ftypes_matcher.hpp +++ b/indexer/ftypes_matcher.hpp @@ -411,15 +411,6 @@ public: uint32_t GetType() const { return m_types[0]; } }; -class IsShopChecker : public BaseChecker -{ -public: - DECLARE_CHECKER_INSTANCE(IsShopChecker); - -private: - IsShopChecker(); -}; - class IsEatChecker : public BaseChecker { public: diff --git a/search/ranker.cpp b/search/ranker.cpp index 5137f6ccd9..63b3aea526 100644 --- a/search/ranker.cpp +++ b/search/ranker.cpp @@ -377,7 +377,7 @@ public: if (info.m_classifType.poi != PoiType::Eat && info.m_classifType.poi != PoiType::Hotel && - info.m_classifType.poi != PoiType::Shop) + info.m_classifType.poi != PoiType::ShopOrAmenity) { info.m_hasName = false; } diff --git a/search/ranking_info.cpp b/search/ranking_info.cpp index f86694eebc..b5e4f75ca7 100644 --- a/search/ranking_info.cpp +++ b/search/ranking_info.cpp @@ -9,7 +9,6 @@ #include "base/stl_helpers.hpp" #include -#include #include #include @@ -73,7 +72,7 @@ double constexpr kPoiType[] = { 0.003, // TransportLocal (0 < it < Residential st) 0.01, // Eat 0.01, // Hotel - 0.01, // Shop + 0.01, // Shop or Amenity 0.01, // Attraction -0.01, // Service 0, // General @@ -131,11 +130,20 @@ void PrintParse(ostringstream & oss, array const oss << "]"; } -class IsAttraction +class BaseTypesChecker { - std::vector m_sights; - uint32_t m_leisure; +protected: + std::vector m_types; +public: + bool operator() (feature::TypesHolder const & th) const + { + return base::AnyOf(m_types, [&th](uint32_t t) { return th.HasWithSubclass(t); }); + } +}; + +class IsAttraction : public BaseTypesChecker +{ public: IsAttraction() { @@ -143,20 +151,57 @@ public: // list in ftypes::AttractionsChecker. We have highway-pedestrian, place-square, historic-tomb, // landuse-cemetery, amenity-townhall etc in long list and logic of long list is "if this object // has high popularity and/or wiki description probably it is attraction". It's better to use - // short list here. And leisures too! + // short list here. + m_types = search::GetCategoryTypes("sights", "en", GetDefaultCategories()); - m_sights = search::GetCategoryTypes("sights", "en", GetDefaultCategories()); - m_leisure = classif().GetTypeByPath({"leisure"}); - } + // Add _attraction_ leisures too! + base::StringIL const types[] = { + {"leisure", "beach_resort"}, + {"leisure", "garden"}, + {"leisure", "landscape_reserve"}, + {"leisure", "marina"}, + {"leisure", "nature_reserve"}, + {"leisure", "park"}, + }; - bool operator() (feature::TypesHolder const & th) const - { - return th.HasWithSubclass(m_leisure) || - base::AnyOf(m_sights, [&th](uint32_t t) { return th.HasWithSubclass(t); }); + Classificator const & c = classif(); + for (auto const & e : types) + m_types.push_back(c.GetTypeByPath(e)); } }; -class IsServiceTypeChecker +class IsShopOrAmenity : public BaseTypesChecker +{ +public: + IsShopOrAmenity() + { + base::StringIL const types[] = { + {"shop"}, + + // Amenity types are very fragmented, so take only most _interesting_ here. + {"amenity", "bank"}, + {"amenity", "brothel"}, + {"amenity", "car_rental"}, + {"amenity", "casino"}, + {"amenity", "cinema"}, + {"amenity", "clinic"}, + {"amenity", "hospital"}, + {"amenity", "library"}, + {"amenity", "marketplace"}, + {"amenity", "nightclub"}, + {"amenity", "pharmacy"}, + {"amenity", "post_office"}, + {"amenity", "stripclub"}, + {"amenity", "theatre"}, + }; + + Classificator const & c = classif(); + for (auto const & e : types) + m_types.push_back(c.GetTypeByPath(e)); + } +}; + +class IsServiceTypeChecker : public BaseTypesChecker { public: IsServiceTypeChecker() @@ -165,14 +210,6 @@ public: for (char const * e : {"barrier", "power", "traffic_calming"}) m_types.push_back(c.GetTypeByPath({e})); } - - bool operator() (feature::TypesHolder const & th) const - { - return base::AnyOf(m_types, [&th](uint32_t t) { return th.HasWithSubclass(t); }); - } - -private: - vector m_types; }; } // namespace @@ -363,8 +400,6 @@ PoiType GetPoiType(feature::TypesHolder const & th) return PoiType::Eat; if (IsHotelChecker::Instance()(th)) return PoiType::Hotel; - if (IsShopChecker::Instance()(th)) - return PoiType::Shop; if (IsRailwayStationChecker::Instance()(th) || IsSubwayStationChecker::Instance()(th) || @@ -379,6 +414,10 @@ PoiType GetPoiType(feature::TypesHolder const & th) if (attractionCheck(th)) return PoiType::Attraction; + static IsShopOrAmenity const shopOrAmenityCheck; + if (shopOrAmenityCheck(th)) + return PoiType::ShopOrAmenity; + static IsServiceTypeChecker const serviceCheck; if (serviceCheck(th)) return PoiType::Service; @@ -394,7 +433,7 @@ string DebugPrint(PoiType type) case PoiType::TransportLocal: return "TransportLocal"; case PoiType::Eat: return "Eat"; case PoiType::Hotel: return "Hotel"; - case PoiType::Shop: return "Shop"; + case PoiType::ShopOrAmenity: return "ShopOrAmenity"; case PoiType::Attraction: return "Attraction"; case PoiType::Service: return "Service"; case PoiType::General: return "General"; diff --git a/search/ranking_info.hpp b/search/ranking_info.hpp index abfe11825d..7095752666 100644 --- a/search/ranking_info.hpp +++ b/search/ranking_info.hpp @@ -24,8 +24,8 @@ enum class PoiType : uint8_t Eat, // Hotels. Hotel, - // Shops. - Shop, + // Shop or Amenity. + ShopOrAmenity, // Attractions. Attraction, // Service types: power lines and substations, barrier-fence, etc. diff --git a/search/search_quality/search_quality_tests/real_mwm_tests.cpp b/search/search_quality/search_quality_tests/real_mwm_tests.cpp index 2b136316be..a3fdb066da 100644 --- a/search/search_quality/search_quality_tests/real_mwm_tests.cpp +++ b/search/search_quality/search_quality_tests/real_mwm_tests.cpp @@ -984,4 +984,49 @@ UNIT_CLASS_TEST(MwmTestsFixture, Streets_Rank) } } +// https://github.com/organicmaps/organicmaps/issues/5756 +UNIT_CLASS_TEST(MwmTestsFixture, Pois_Rank) +{ + { + // Buenos Aires (Palermo) + ms::LatLon const center(-34.58524, -58.42516); + SetViewportAndLoadMaps(center); + + // mix of amenity=pharmacy + shop=chemistry. + auto request = MakeRequest("farmacity"); + auto const & results = request->Results(); + TEST_GREATER(results.size(), kPopularPoiResultsCount, ()); + TEST_LESS(SortedByDistance(Range(results, 0, kPopularPoiResultsCount), center), 3000, ()); + } + + { + // Salem, Oregon + ms::LatLon const center(44.8856466, -123.0628986); + SetViewportAndLoadMaps(center); + + auto request = MakeRequest("depot"); + auto const & results = request->Results(); + + /// @todo Probably, we should decrease Names matching rank/penalty. + // - railway station "XXX Depot" + // - a bunch of streets and POIs "Depot XXX" (up to 100km) + // - nearest post office "Mail Depot" (500m) + size_t constexpr kResultsCount = 11; + TEST_GREATER(results.size(), kResultsCount, ()); + TEST_EQUAL(CountClassifType(Range(results, 0, kResultsCount), + classif().GetTypeByPath({"amenity", "post_office"})), 1, ()); + } + + { + // Istanbul + ms::LatLon const center(40.95058, 29.17255); + SetViewportAndLoadMaps(center); + + auto request = MakeRequest("Göztepe 60. Yıl Parkı"); + auto const & results = request->Results(); + TEST_GREATER(results.size(), kTopPoiResultsCount, ()); + EqualClassifType(Range(results, 0, 1), GetClassifTypes({{"leisure", "park"}})); + } +} + } // namespace real_mwm_tests