Review fixes.

6 files changed, 68 insertions, 57 deletions

diff --git a/search/intermediate_result.cpp b/search/intermediate_result.cpp
index 0cdedb1ff1..d22792b51b 100644
--- a/search/intermediate_result.cpp
+++ b/search/intermediate_result.cpp
@@ -82,23 +82,22 @@ bool PreResult1::LessPriority(PreResult1 const & r1, PreResult1 const & r2)
   return r1.m_info.m_rank > r2.m_info.m_rank;
 }
 
-PreResult2::PreResult2(FeatureType const & f, PreResult1 const * p, m2::PointD const & pivot,
-                       string const & displayName, string const & fileName)
-  : m_id(f.GetID()),
-    m_types(f),
-    m_str(displayName),
-    m_resultType(ftypes::IsBuildingChecker::Instance()(m_types) ? RESULT_BUILDING : RESULT_FEATURE),
-    m_geomType(f.GetFeatureType())
+PreResult2::PreResult2(FeatureType const & f, PreResult1 const * p, m2::PointD const & center,
+                       m2::PointD const & pivot, string const & displayName,
+                       string const & fileName)
+  : m_id(f.GetID())
+  , m_types(f)
+  , m_str(displayName)
+  , m_resultType(ftypes::IsBuildingChecker::Instance()(m_types) ? RESULT_BUILDING : RESULT_FEATURE)
+  , m_geomType(f.GetFeatureType())
 {
   ASSERT(m_id.IsValid(), ());
   ASSERT(!m_types.Empty(), ());
 
   m_types.SortBySpec();
 
-  auto const center = feature::GetCenter(f);
-
   m_region.SetParams(fileName, center);
-  m_distance = PointDistance(GetCenter(), pivot);
+  m_distance = PointDistance(center, pivot);
 
   ProcessMetadata(f, m_metadata);
 }
diff --git a/search/intermediate_result.hpp b/search/intermediate_result.hpp
index 6a213acb1f..8322eb7b41 100644
--- a/search/intermediate_result.hpp
+++ b/search/intermediate_result.hpp
@@ -64,8 +64,8 @@ public:
   };
 
   /// For RESULT_FEATURE and RESULT_BUILDING.
-  PreResult2(FeatureType const & f, PreResult1 const * p, m2::PointD const & pivot,
-             string const & displayName, string const & fileName);
+  PreResult2(FeatureType const & f, PreResult1 const * p, m2::PointD const & center,
+             m2::PointD const & pivot, string const & displayName, string const & fileName);
 
   /// For RESULT_LATLON.
   PreResult2(double lat, double lon);
diff --git a/search/search_query.cpp b/search/search_query.cpp
index 8ac47833d1..c14fb066f7 100644
--- a/search/search_query.cpp
+++ b/search/search_query.cpp
@@ -558,7 +558,8 @@ class PreResult2Maker
   unique_ptr<Index::FeaturesLoaderGuard> m_pFV;
 
   // For the best performance, incoming id's should be sorted by id.first (mwm file id).
-  void LoadFeature(FeatureID const & id, FeatureType & f, string & name, string & country)
+  void LoadFeature(FeatureID const & id, FeatureType & f, m2::PointD & center, string & name,
+                   string & country)
   {
     if (m_pFV.get() == 0 || m_pFV->GetId() != id.m_mwmId)
       m_pFV.reset(new Index::FeaturesLoaderGuard(m_query.m_index, id.m_mwmId));
@@ -566,6 +567,8 @@ class PreResult2Maker
     m_pFV->GetFeatureByIndex(id.m_index, f);
     f.SetID(id);
 
+    center = feature::GetCenter(f);
+
     m_query.GetBestMatchName(f, name);
 
     // country (region) name is a file name if feature isn't from World.mwm
@@ -575,14 +578,14 @@ class PreResult2Maker
       country = m_pFV->GetCountryFileName();
   }
 
-  void InitRankingInfo(FeatureType const & ft, impl::PreResult1 const & res,
-                       search::v2::RankingInfo & info)
+  void InitRankingInfo(FeatureType const & ft, m2::PointD const & center,
+                       impl::PreResult1 const & res, search::v2::RankingInfo & info)
   {
     auto const & preInfo = res.GetInfo();
 
-    auto const & pivot = m_params.m_pivotCenter;
+    auto const & pivot = m_params.m_accuratePivotCenter;
 
-    info.m_distanceToPivot = MercatorBounds::DistanceOnEarth(feature::GetCenter(ft), pivot);
+    info.m_distanceToPivot = MercatorBounds::DistanceOnEarth(center, pivot);
     info.m_rank = preInfo.m_rank;
     info.m_searchType = preInfo.m_searchType;
 
@@ -612,13 +615,37 @@ class PreResult2Maker
 
     if (info.m_searchType == v2::SearchModel::SEARCH_TYPE_BUILDING)
     {
-      string houseNumber = ft.GetHouseNumber();
+      string const houseNumber = ft.GetHouseNumber();
       auto score = GetNameScore(houseNumber, m_params, preInfo.m_startToken, preInfo.m_endToken);
       if (score > info.m_nameScore)
         info.m_nameScore = score;
     }
   }
 
+  uint8_t NormalizeRank(uint8_t rank, v2::SearchModel::SearchType type, m2::PointD const & center,
+                        string const & country)
+  {
+    switch (type)
+    {
+    case v2::SearchModel::SEARCH_TYPE_VILLAGE: return rank /= 1.5;
+    case v2::SearchModel::SEARCH_TYPE_CITY:
+    {
+      if (m_query.GetViewport(Query::CURRENT_V).IsPointInside(center))
+        return rank * 2;
+
+      storage::CountryInfo info;
+      if (country.empty())
+        m_query.m_infoGetter.GetRegionInfo(center, info);
+      else
+        m_query.m_infoGetter.GetRegionInfo(country, info);
+      if (info.IsNotEmpty() && info.m_name == m_query.GetPivotRegion())
+        return rank *= 1.7;
+    }
+    case v2::SearchModel::SEARCH_TYPE_COUNTRY: return rank /= 1.5;
+    default: return rank;
+    }
+  }
+
 public:
   explicit PreResult2Maker(Query & q, v2::Geocoder::Params const & params)
     : m_query(q), m_params(params)
@@ -628,41 +655,20 @@ public:
   unique_ptr<impl::PreResult2> operator()(impl::PreResult1 const & res1)
   {
     FeatureType ft;
-    string name, country;
-    LoadFeature(res1.GetID(), ft, name, country);
+    m2::PointD center;
+    string name;
+    string country;
+
+    LoadFeature(res1.GetID(), ft, center, name, country);
+
+    auto res2 = make_unique<impl::PreResult2>(ft, &res1, center, m_query.GetPosition() /* pivot */,
+                                              name, country);
 
-    auto res2 = make_unique<impl::PreResult2>(ft, &res1, m_query.GetPosition(), name, country);
     search::v2::RankingInfo info;
-    InitRankingInfo(ft, res1, info);
+    InitRankingInfo(ft, center, res1, info);
+    info.m_rank = NormalizeRank(info.m_rank, info.m_searchType, center, country);
     res2->SetRankingInfo(move(info));
 
-    /// @todo: add exluding of states (without USA states), continents
-    using namespace ftypes;
-    Type const tp = IsLocalityChecker::Instance().GetType(res2->m_types);
-    switch (tp)
-    {
-      case COUNTRY:
-        res2->m_info.m_rank /= 1.5;
-        break;
-      case CITY:
-      case TOWN:
-        if (m_query.GetViewport(Query::CURRENT_V).IsPointInside(res2->GetCenter()))
-          res2->m_info.m_rank *= 2;
-        else
-        {
-          storage::CountryInfo ci;
-          res2->m_region.GetRegion(m_query.m_infoGetter, ci);
-          if (ci.IsNotEmpty() && ci.m_name == m_query.GetPivotRegion())
-            res2->m_info.m_rank *= 1.7;
-        }
-        break;
-      case VILLAGE:
-        res2->m_info.m_rank /= 1.5;
-        break;
-      default:
-        break;
-    }
-
     return res2;
   }
 };
@@ -699,9 +705,9 @@ void Query::MakePreResult2(v2::Geocoder::Params const & params, vector<T> & cont
       ;
 
     // The main reason of shuffling here is to select a random subset
-    // from the low-priority results. We're using a linear congruental
-    // method with default seed because it is fast, simple and doesn't
-    // need an external entropy source.
+    // from the low-priority results. We're using a linear
+    // congruential method with default seed because it is fast,
+    // simple and doesn't need an external entropy source.
     //
     // TODO (@y, @m, @vng): consider to take some kind of hash from
     // features and then select a subset with smallest values of this
diff --git a/search/v2/geocoder.cpp b/search/v2/geocoder.cpp
index b96beace65..d9d9dd7963 100644
--- a/search/v2/geocoder.cpp
+++ b/search/v2/geocoder.cpp
@@ -399,7 +399,10 @@ bool SameMwm(Geocoder::TResult const & lhs, Geocoder::TResult const & rhs)
 }  // namespace
 
 // Geocoder::Params --------------------------------------------------------------------------------
-Geocoder::Params::Params() : m_mode(Mode::Everywhere), m_pivotCenter(0, 0), m_maxNumResults(0) {}
+Geocoder::Params::Params()
+  : m_mode(Mode::Everywhere), m_accuratePivotCenter(0, 0), m_maxNumResults(0)
+{
+}
 
 // Geocoder::Geocoder ------------------------------------------------------------------------------
 Geocoder::Geocoder(Index & index, storage::CountryInfoGetter const & infoGetter)
@@ -1312,7 +1315,7 @@ void Geocoder::FillMissingFieldsInResults()
 
   if (m_results->size() > m_params.m_maxNumResults)
   {
-    m_pivotFeatures.SetPosition(m_params.m_pivotCenter, m_params.m_scale);
+    m_pivotFeatures.SetPosition(m_params.m_accuratePivotCenter, m_params.m_scale);
     for (auto & result : *m_results)
     {
       auto const & id = result.first;
diff --git a/search/v2/geocoder.hpp b/search/v2/geocoder.hpp
index 78b074c272..309fbde96c 100644
--- a/search/v2/geocoder.hpp
+++ b/search/v2/geocoder.hpp
@@ -80,9 +80,12 @@ public:
     // We need to pass both pivot and pivot center because pivot is
     // usually a rectangle created by radius and center, and due to
     // precision loss, |m_pivot|.Center() may differ from
-    // |m_pivotCenter|.
+    // |m_accuratePivotCenter|. Therefore |m_pivot| should be used for
+    // fast filtering of features outside of the rectangle, while
+    // |m_accuratePivotCenter| should be used when it's needed to
+    // compute a distance from a feature to the pivot.
     m2::RectD m_pivot;
-    m2::PointD m_pivotCenter;
+    m2::PointD m_accuratePivotCenter;
     size_t m_maxNumResults;
   };
 
diff --git a/search/v2/search_query_v2.cpp b/search/v2/search_query_v2.cpp
index 33b4fa0549..d623711c2b 100644
--- a/search/v2/search_query_v2.cpp
+++ b/search/v2/search_query_v2.cpp
@@ -43,7 +43,7 @@ void SearchQueryV2::Search(Results & res, size_t resCount)
   params.m_mode = m_mode;
 
   params.m_pivot = GetPivotRect();
-  params.m_pivotCenter = GetPivotPoint();
+  params.m_accuratePivotCenter = GetPivotPoint();
   params.m_maxNumResults = max(resCount, kPreResultsCount);
   m_geocoder.SetParams(params);