diff --git a/search/geocoder.cpp b/search/geocoder.cpp index 0d93b8fda8..acb6f05854 100644 --- a/search/geocoder.cpp +++ b/search/geocoder.cpp @@ -281,41 +281,6 @@ struct KeyedMwmInfo double m_distance; }; -// Reorders maps in a way that prefix consists of maps intersecting -// with pivot, suffix consists of all other maps ordered by minimum -// distance from pivot. Returns number of maps in prefix. -// In viewport search mode, prefers mwms that contain the user's position. -size_t OrderCountries(boost::optional const & position, m2::RectD const & pivot, - bool inViewport, vector> & infos) -{ - // TODO (@y): remove this if crashes in this function - // disappear. Otherwise, remove null infos and re-check MwmSet - // again. - for (auto const & info : infos) - { - CHECK(info.get(), - ("MwmSet invariant violated. Please, contact @y if you know how to reproduce this.")); - } - - vector keyedInfos; - keyedInfos.reserve(infos.size()); - for (auto const & info : infos) - keyedInfos.emplace_back(info, position, pivot, inViewport); - sort(keyedInfos.begin(), keyedInfos.end()); - - infos.clear(); - for (auto const & info : keyedInfos) - infos.emplace_back(info.m_info); - - auto intersects = [&](shared_ptr const & info) -> bool { - if (!inViewport && position && info->m_bordersRect.IsPointInside(*position)) - return true; - return pivot.IsIntersect(info->m_bordersRect); - }; - auto const sep = stable_partition(infos.begin(), infos.end(), intersects); - return distance(infos.begin(), sep); -} - unique_ptr GetWorldContext(DataSource const & dataSource) { vector> infos; @@ -470,6 +435,51 @@ void Geocoder::SetParamsForCategorialSearch(Params const & params) LOG(LDEBUG, (static_cast(m_params))); } +size_t Geocoder::OrderCountries(bool inViewport, vector> & infos) +{ + // TODO (@y): remove this if crashes in this function + // disappear. Otherwise, remove null infos and re-check MwmSet + // again. + for (auto const & info : infos) + { + CHECK(info.get(), + ("MwmSet invariant violated. Please, contact @y if you know how to reproduce this.")); + } + + vector keyedInfos; + keyedInfos.reserve(infos.size()); + for (auto const & info : infos) + keyedInfos.emplace_back(info, m_params.m_position, m_params.m_pivot, inViewport); + sort(keyedInfos.begin(), keyedInfos.end()); + + infos.clear(); + for (auto const & info : keyedInfos) + infos.emplace_back(info.m_info); + + set mwmsWithCities; + if (!inViewport) + { + for (auto const & p : m_cities) + { + for (auto const & city : p.second) + mwmsWithCities.insert(m_infoGetter.GetRegionCountryId(city.m_rect.Center())); + } + } + + auto const firstBatch = [&](shared_ptr const & info) { + if (!inViewport) + { + if (m_params.m_position && info->m_bordersRect.IsPointInside(*m_params.m_position)) + return true; + if (mwmsWithCities.count(info->GetCountryName()) != 0) + return true; + } + return m_params.m_pivot.IsIntersect(info->m_bordersRect); + }; + auto const sep = stable_partition(infos.begin(), infos.end(), firstBatch); + return distance(infos.begin(), sep); +} + void Geocoder::GoImpl(vector> & infos, bool inViewport) { // base::PProf pprof("/tmp/geocoder.prof"); @@ -511,8 +521,7 @@ void Geocoder::GoImpl(vector> & infos, bool inViewport) // maps are ordered by distance from pivot, and we stop to call // MatchAroundPivot() on them as soon as at least one feature is // found. - size_t const numIntersectingMaps = - OrderCountries(m_params.m_position, m_params.m_pivot, inViewport, infos); + size_t const numIntersectingMaps = OrderCountries(inViewport, infos); // MatchAroundPivot() should always be matched in mwms // intersecting with position and viewport. diff --git a/search/geocoder.hpp b/search/geocoder.hpp index d07cd75ee1..c73c5cd3e6 100644 --- a/search/geocoder.hpp +++ b/search/geocoder.hpp @@ -262,6 +262,14 @@ private: WARN_UNUSED_RESULT bool GetTypeInGeocoding(BaseContext const & ctx, uint32_t featureId, Model::Type & type); + // Reorders maps in a way that prefix consists of "best" maps to search and suffix consists of all + // other maps ordered by minimum distance from pivot. Returns number of maps in prefix. + // For viewport mode prefix consists of maps intersecting with pivot ordered by distance from pivot + // center. + // For non-viewport search mode prefix consists of maps intersecting with pivot, map with user location + // and maps with cities matched to the query, sorting prefers mwms that contain the user's position. + size_t OrderCountries(bool inViewport, std::vector> & infos); + DataSource const & m_dataSource; storage::CountryInfoGetter const & m_infoGetter; CategoriesHolder const & m_categories; diff --git a/search/search_integration_tests/processor_test.cpp b/search/search_integration_tests/processor_test.cpp index 5fe628ba0d..faadef90a7 100644 --- a/search/search_integration_tests/processor_test.cpp +++ b/search/search_integration_tests/processor_test.cpp @@ -2390,5 +2390,65 @@ UNIT_CLASS_TEST(ProcessorTest, Postbox) } } +UNIT_CLASS_TEST(ProcessorTest, OrderCountries) +{ + string const cafeLandName = "CafeLand"; + string const UkCountryName = "UK"; + TestCity london(m2::PointD(1.0, 1.0), "London", "en", 100 /* rank */); + TestStreet piccadilly(vector{m2::PointD(0.5, 0.5), m2::PointD(1.5, 1.5)}, + "Piccadilly Circus", "en"); + TestVillage cambridge(m2::PointD(3.0, 3.0), "Cambridge", "en", 5 /* rank */); + TestStreet wheeling(vector{m2::PointD(2.5, 2.5), m2::PointD(3.5, 3.5)}, + "Wheeling Avenue", "en"); + + TestPOI dummyPoi(m2::PointD(0.0, 4.0), "dummy", "en"); + TestCafe londonCafe(m2::PointD(-10.01, 14.0), "London Piccadilly cafe", "en"); + TestCafe cambridgeCafe(m2::PointD(-10.02, 14.01), "Cambridge Wheeling cafe", "en"); + + auto worldId = BuildWorld([&](TestMwmBuilder & builder) { builder.Add(london); }); + auto UkId = BuildCountry(UkCountryName, [&](TestMwmBuilder & builder) { + builder.Add(london); + builder.Add(piccadilly); + builder.Add(cambridge); + builder.Add(wheeling); + }); + auto const UkCountryRect = m2::RectD(m2::PointD(0.5, 0.5), m2::PointD(3.5, 3.5)); + RegisterCountry(UkCountryName, UkCountryRect); + + auto cafeLandId = BuildCountry(cafeLandName, [&](TestMwmBuilder & builder) { + builder.Add(dummyPoi); + builder.Add(londonCafe); + builder.Add(cambridgeCafe); + }); + + auto const cafeLandRect = m2::RectD(m2::PointD(-10.5, 4.0), m2::PointD(0.0, 14.5)); + RegisterCountry(cafeLandName, cafeLandRect); + + auto const viewportRect = m2::RectD(m2::PointD(-1.0, 5.0), m2::PointD(0.0, 4.0)); + SetViewport(viewportRect); + CHECK(!viewportRect.IsIntersect(UkCountryRect), ()); + CHECK(viewportRect.IsIntersect(cafeLandRect), ()); + { + auto request = MakeRequest("London Piccadilly"); + auto const & results = request->Results(); + + TEST_EQUAL(results.size(), 2, ("Unexpected number of results.")); + // (UkId, piccadilly) should be ranked first, it means it was in the first batch. + TEST(ResultsMatch({results[0]}, {ExactMatch(UkId, piccadilly)}), ()); + TEST(ResultsMatch({results[1]}, {ExactMatch(cafeLandId, londonCafe)}), ()); + } + { + auto request = MakeRequest("Cambridge Wheeling"); + auto const & results = request->Results(); + + TEST_EQUAL(results.size(), 2, ("Unexpected number of results.")); + TEST(ResultsMatch({results[0]}, {ExactMatch(cafeLandId, cambridgeCafe)}), ()); + TEST(ResultsMatch({results[1]}, {ExactMatch(UkId, wheeling)}), ()); + // (UkId, wheeling) has higher rank but it's second in results because it was not in the first + // batch. + TEST_GREATER(results[1].GetRankingInfo().GetLinearModelRank(), + results[0].GetRankingInfo().GetLinearModelRank(), ()); + } +} } // namespace } // namespace search